Update volume mount funcs and delete funcs

Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com>

pkg/devfile/parser/data/interface.go

@@ -46,10 +46,10 @@ type DevfileData interface {
 	UpdateCommand(command v1.Command)
 	DeleteCommand(id string) error
 
-	// volume related methods
-	AddVolume(volume v1.Component, path string) error
-	DeleteVolume(name string) error
-	GetVolumeMountPath(name string) (string, error)
+	// volume mount related methods
+	AddVolumeMount(componentName, name, path string) error
+	DeleteVolumeMount(name string) error
+	GetVolumeMountPath(mountName, componentName string) (string, error)
 
 	// workspace related methods
 	GetDevfileWorkspace() *v1.DevWorkspaceTemplateSpecContent

pkg/devfile/parser/data/v2/commands.go

@@ -59,15 +59,7 @@ func (d *DevfileV2) DeleteCommand(id string) error {
 
 	found := false
 	for i := len(d.Commands) - 1; i >= 0; i-- {
-		if d.Commands[i].Composite != nil && d.Commands[i].Id != id {
-			var subCmd []string
-			for _, command := range d.Commands[i].Composite.Commands {
-				if command != id {
-					subCmd = append(subCmd, command)
-				}
-			}
-			d.Commands[i].Composite.Commands = subCmd
-		} else if d.Commands[i].Id == id {
+		if d.Commands[i].Id == id {
 			found = true
 			d.Commands = append(d.Commands[:i], d.Commands[i+1:]...)
 		}

pkg/devfile/parser/data/v2/commands_test.go

@@ -347,52 +347,7 @@ func TestDeleteCommands(t *testing.T) {
 					Id: "command3",
 					CommandUnion: v1.CommandUnion{
 						Composite: &v1.CompositeCommand{
-							Commands: []string{"command2"},
-						},
-					},
-				},
-			},
-			wantErr: false,
-		},
-		{
-			name:            "Commands that do not belong to Composite Command",
-			commandToDelete: "command3",
-			commands: []v1.Command{
-				{
-					Id: "command1",
-					CommandUnion: v1.CommandUnion{
-						Exec: &v1.ExecCommand{},
-					},
-				},
-				{
-					Id: "command2",
-					CommandUnion: v1.CommandUnion{
-						Composite: &v1.CompositeCommand{
-							Commands: []string{"command1"},
-						},
-					},
-				},
-				{
-					Id: "command3",
-					CommandUnion: v1.CommandUnion{
-						Composite: &v1.CompositeCommand{
-							Commands: []string{"command1"},
-						},
-					},
-				},
-			},
-			wantCommands: []v1.Command{
-				{
-					Id: "command1",
-					CommandUnion: v1.CommandUnion{
-						Exec: &v1.ExecCommand{},
-					},
-				},
-				{
-					Id: "command2",
-					CommandUnion: v1.CommandUnion{
-						Composite: &v1.CompositeCommand{
-							Commands: []string{"command1"},
+							Commands: []string{"command1", "command2", "command1"},
 						},
 					},
 				},

pkg/devfile/parser/data/v2/components.go

@@ -90,15 +90,7 @@ func (d *DevfileV2) DeleteComponent(name string) error {
 
 	found := false
 	for i := len(d.Components) - 1; i >= 0; i-- {
-		if d.Components[i].Container != nil && d.Components[i].Name != name {
-			var tmp []v1.VolumeMount
-			for _, volumeMount := range d.Components[i].Container.VolumeMounts {
-				if volumeMount.Name != name {
-					tmp = append(tmp, volumeMount)
-				}
-			}
-			d.Components[i].Container.VolumeMounts = tmp
-		} else if d.Components[i].Name == name {
+		if d.Components[i].Name == name {
 			found = true
 			d.Components = append(d.Components[:i], d.Components[i+1:]...)
 		}

pkg/devfile/parser/data/v2/components_test.go

@@ -454,6 +454,7 @@ func TestDeleteComponents(t *testing.T) {
 								VolumeMounts: []v1.VolumeMount{
 									testingutil.GetFakeVolumeMount("comp2", "/path"),
 									testingutil.GetFakeVolumeMount("comp2", "/path2"),
+									testingutil.GetFakeVolumeMount("comp3", "/path"),
 								},
 							},
 						},
@@ -468,51 +469,6 @@ func TestDeleteComponents(t *testing.T) {
 			},
 			wantErr: false,
 		},
-		{
-			name:              "Non Volume Component",
-			componentToDelete: "comp1",
-			components: []v1.Component{
-				{
-					Name: "comp1",
-					ComponentUnion: v1.ComponentUnion{
-						Container: &v1.ContainerComponent{
-							Container: v1.Container{
-								VolumeMounts: []v1.VolumeMount{
-									testingutil.GetFakeVolumeMount("comp2", "/path"),
-								},
-							},
-						},
-					},
-				},
-				{
-					Name: "comp2",
-					ComponentUnion: v1.ComponentUnion{
-						Volume: &v1.VolumeComponent{},
-					},
-				},
-				{
-					Name: "comp3",
-					ComponentUnion: v1.ComponentUnion{
-						Kubernetes: &v1.KubernetesComponent{},
-					},
-				},
-			},
-			wantComponents: []v1.Component{
-				{
-					Name: "comp2",
-					ComponentUnion: v1.ComponentUnion{
-						Volume: &v1.VolumeComponent{},
-					},
-				},
-				{
-					Name: "comp3",
-					ComponentUnion: v1.ComponentUnion{
-						Kubernetes: &v1.KubernetesComponent{},
-					},
-				},
-			},
-			wantErr: false,
-		},
 		{
 			name:              "Missing Component",
 			componentToDelete: "comp12",

pkg/devfile/parser/data/v2/volumes.go

@@ -8,75 +8,90 @@ import (
 	"github.com/devfile/library/pkg/devfile/parser/data/v2/common"
 )
 
-// AddVolume adds the volume to the devFile and mounts it to all the container components
-func (d *DevfileV2) AddVolume(volumeComponent v1.Component, path string) error {
-	volumeExists := false
+// AddVolumeMount adds the volume mount to the specified container component
+func (d *DevfileV2) AddVolumeMount(componentName, name, path string) error {
 	var pathErrorContainers []string
+	found := false
 	for _, component := range d.Components {
-		if component.Container != nil {
+		if component.Container != nil && component.Name == componentName {
+			found = true
 			for _, volumeMount := range component.Container.VolumeMounts {
 				if volumeMount.Path == path {
-					var err = fmt.Errorf("another volume, %s, is mounted to the same path: %s, on the container: %s", volumeMount.Name, path, component.Name)
+					var err = fmt.Errorf("another volume, %s, is mounted to the same path: %s, in the container: %s", volumeMount.Name, path, component.Name)
 					pathErrorContainers = append(pathErrorContainers, err.Error())
 				}
 			}
 			component.Container.VolumeMounts = append(component.Container.VolumeMounts, v1.VolumeMount{
-				Name: volumeComponent.Name,
+				Name: name,
 				Path: path,
 			})
-		} else if component.Volume != nil && component.Name == volumeComponent.Name {
-			volumeExists = true
-			break
 		}
 	}
 
-	if volumeExists {
-		return &common.FieldAlreadyExistError{
-			Field: "volume",
-			Name:  volumeComponent.Name,
+	if !found {
+		return &common.FieldNotFoundError{
+			Field: "container component",
+			Name:  componentName,
 		}
 	}
 
 	if len(pathErrorContainers) > 0 {
-		return fmt.Errorf("errors while creating volume:\n%s", strings.Join(pathErrorContainers, "\n"))
+		return fmt.Errorf("errors while adding volume mounts:\n%s", strings.Join(pathErrorContainers, "\n"))
 	}
 
-	d.Components = append(d.Components, volumeComponent)
-
 	return nil
 }
 
-// DeleteVolume removes the volume from the devFile and removes all the related volume mounts
-func (d *DevfileV2) DeleteVolume(name string) error {
+// DeleteVolumeMount deletes the volume mount from container components
+func (d *DevfileV2) DeleteVolumeMount(name string) error {
+	found := false
+	for i := range d.Components {
+		if d.Components[i].Container != nil && d.Components[i].Name != name {
+			for j := len(d.Components[i].Container.VolumeMounts) - 1; j >= 0; j-- {
+				if d.Components[i].Container.VolumeMounts[j].Name == name {
+					found = true
+					d.Components[i].Container.VolumeMounts = append(d.Components[i].Container.VolumeMounts[:j], d.Components[i].Container.VolumeMounts[j+1:]...)
+				}
+			}
+		}
+	}
 
-	return d.DeleteComponent(name)
+	if !found {
+		return &common.FieldNotFoundError{
+			Field: "volume mount",
+			Name:  name,
+		}
+	}
+
+	return nil
 }
 
-// GetVolumeMountPath gets the mount path of the required volume
-func (d *DevfileV2) GetVolumeMountPath(name string) (string, error) {
-	volumeFound := false
+// GetVolumeMountPath gets the mount path of the specified volume mount from the specified container component
+func (d *DevfileV2) GetVolumeMountPath(mountName, componentName string) (string, error) {
 	mountFound := false
-	path := ""
+	componentFound := false
+	var path string
 
 	for _, component := range d.Components {
-		if component.Container != nil {
+		if component.Container != nil && component.Name == componentName {
+			componentFound = true
 			for _, volumeMount := range component.Container.VolumeMounts {
-				if volumeMount.Name == name {
+				if volumeMount.Name == mountName {
 					mountFound = true
 					path = volumeMount.Path
 				}
 			}
-		} else if component.Volume != nil {
-			volumeFound = true
 		}
 	}
-	if volumeFound && mountFound {
-		return path, nil
-	} else if !mountFound && volumeFound {
-		return "", fmt.Errorf("volume not mounted to any component")
-	}
-	return "", &common.FieldNotFoundError{
-		Field: "volume",
-		Name:  "name",
+
+	if !componentFound {
+		return "", &common.FieldNotFoundError{
+			Field: "container component",
+			Name:  componentName,
+		}
+	} else if !mountFound {
+		return "", fmt.Errorf("volume %s not mounted to component %s", mountName, componentName)
 	}
+
+	return path, nil
 }