From 316423a69586361cb5da956d103ecd7e5e62b53b Mon Sep 17 00:00:00 2001 From: Maysun J Faisal Date: Mon, 22 Mar 2021 14:50:59 -0400 Subject: [PATCH 1/2] container can mount same vol at multiple places Signed-off-by: Maysun J Faisal --- pkg/devfile/parser/data/interface.go | 2 +- pkg/devfile/parser/data/v2/volumes.go | 16 +++++--- pkg/devfile/parser/data/v2/volumes_test.go | 48 ++++++++++++++-------- 3 files changed, 43 insertions(+), 23 deletions(-) diff --git a/pkg/devfile/parser/data/interface.go b/pkg/devfile/parser/data/interface.go index dce8e56c..9f9986e6 100644 --- a/pkg/devfile/parser/data/interface.go +++ b/pkg/devfile/parser/data/interface.go @@ -49,7 +49,7 @@ type DevfileData interface { // volume mount related methods AddVolumeMounts(componentName string, volumeMounts []v1.VolumeMount) error DeleteVolumeMount(name string) error - GetVolumeMountPath(mountName, componentName string) (string, error) + GetVolumeMountPaths(mountName, componentName string) ([]string, error) // workspace related methods GetDevfileWorkspace() *v1.DevWorkspaceTemplateSpecContent diff --git a/pkg/devfile/parser/data/v2/volumes.go b/pkg/devfile/parser/data/v2/volumes.go index 1f0abd48..2f287dd4 100644 --- a/pkg/devfile/parser/data/v2/volumes.go +++ b/pkg/devfile/parser/data/v2/volumes.go @@ -70,27 +70,33 @@ func (d *DevfileV2) DeleteVolumeMount(name string) error { return nil } -// GetVolumeMountPath gets the mount path of the specified volume mount from the specified container component -func (d *DevfileV2) GetVolumeMountPath(mountName, componentName string) (string, error) { +// GetVolumeMountPaths gets all the mount paths of the specified volume mount from the specified container component. +// A container can mount at different paths for a given volume. +func (d *DevfileV2) GetVolumeMountPaths(mountName, componentName string) ([]string, error) { componentFound := false + var mountPaths []string for _, component := range d.Components { if component.Container != nil && component.Name == componentName { componentFound = true for _, volumeMount := range component.Container.VolumeMounts { if volumeMount.Name == mountName { - return volumeMount.Path, nil + mountPaths = append(mountPaths, volumeMount.Path) } } } } if !componentFound { - return "", &common.FieldNotFoundError{ + return mountPaths, &common.FieldNotFoundError{ Field: "container component", Name: componentName, } } - return "", fmt.Errorf("volume %s not mounted to component %s", mountName, componentName) + if len(mountPaths) == 0 { + return mountPaths, fmt.Errorf("volume %s not mounted to component %s", mountName, componentName) + } + + return mountPaths, nil } diff --git a/pkg/devfile/parser/data/v2/volumes_test.go b/pkg/devfile/parser/data/v2/volumes_test.go index 0554ef0e..0e3c4588 100644 --- a/pkg/devfile/parser/data/v2/volumes_test.go +++ b/pkg/devfile/parser/data/v2/volumes_test.go @@ -263,16 +263,14 @@ func TestDevfile200_DeleteVolumeMounts(t *testing.T) { } -func TestDevfile200_GetVolumeMountPath(t *testing.T) { - volume1 := "volume1" - component1 := "component1" +func TestDevfile200_GetVolumeMountPaths(t *testing.T) { tests := []struct { name string currentComponents []v1.Component mountName string componentName string - wantPath string + wantPaths []string wantErr bool }{ { @@ -283,17 +281,18 @@ func TestDevfile200_GetVolumeMountPath(t *testing.T) { Container: &v1.ContainerComponent{ Container: v1.Container{ VolumeMounts: []v1.VolumeMount{ - testingutil.GetFakeVolumeMount(volume1, "/path"), + testingutil.GetFakeVolumeMount("volume1", "/path"), + testingutil.GetFakeVolumeMount("volume1", "/path2"), }, }, }, }, - Name: component1, + Name: "component1", }, }, - wantPath: "/path", - mountName: volume1, - componentName: component1, + wantPaths: []string{"/path", "/path2"}, + mountName: "volume1", + componentName: "component1", wantErr: false, }, { @@ -304,16 +303,16 @@ func TestDevfile200_GetVolumeMountPath(t *testing.T) { Container: &v1.ContainerComponent{ Container: v1.Container{ VolumeMounts: []v1.VolumeMount{ - testingutil.GetFakeVolumeMount(volume1, "/path"), + testingutil.GetFakeVolumeMount("volume1", "/path"), }, }, }, }, - Name: component1, + Name: "component1", }, }, mountName: "volume2", - componentName: component1, + componentName: "component1", wantErr: true, }, { @@ -324,15 +323,15 @@ func TestDevfile200_GetVolumeMountPath(t *testing.T) { Container: &v1.ContainerComponent{ Container: v1.Container{ VolumeMounts: []v1.VolumeMount{ - testingutil.GetFakeVolumeMount(volume1, "/path"), + testingutil.GetFakeVolumeMount("volume1", "/path"), }, }, }, }, - Name: component1, + Name: "component1", }, }, - mountName: volume1, + mountName: "volume1", componentName: "component2", wantErr: true, }, @@ -348,11 +347,26 @@ func TestDevfile200_GetVolumeMountPath(t *testing.T) { }, }, } - gotPath, err := d.GetVolumeMountPath(tt.mountName, tt.componentName) + gotPaths, err := d.GetVolumeMountPaths(tt.mountName, tt.componentName) if (err != nil) != tt.wantErr { t.Errorf("GetVolumeMountPath() error = %v, wantErr %v", err, tt.wantErr) } else if err == nil { - assert.Equal(t, tt.wantPath, gotPath, "The two values should be the same.") + if len(gotPaths) != len(tt.wantPaths) { + t.Error("expected mount paths length not the same as actual mount paths length") + } + + for _, wantPath := range tt.wantPaths { + matched := false + for _, gotPath := range gotPaths { + if wantPath == gotPath { + matched = true + } + } + + if !matched { + t.Errorf("unable to find the wanted mount path %s in the actual mount paths slice", wantPath) + } + } } }) } From b72a7ab6fcadbd62648d605b74b582076a39ef67 Mon Sep 17 00:00:00 2001 From: Maysun J Faisal Date: Mon, 22 Mar 2021 16:18:36 -0400 Subject: [PATCH 2/2] Update componentName to containerName to avoid confusion Signed-off-by: Maysun J Faisal --- pkg/devfile/parser/data/interface.go | 4 ++-- pkg/devfile/parser/data/v2/volumes.go | 14 +++++++------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/pkg/devfile/parser/data/interface.go b/pkg/devfile/parser/data/interface.go index 9f9986e6..ecbeb90a 100644 --- a/pkg/devfile/parser/data/interface.go +++ b/pkg/devfile/parser/data/interface.go @@ -47,9 +47,9 @@ type DevfileData interface { DeleteCommand(id string) error // volume mount related methods - AddVolumeMounts(componentName string, volumeMounts []v1.VolumeMount) error + AddVolumeMounts(containerName string, volumeMounts []v1.VolumeMount) error DeleteVolumeMount(name string) error - GetVolumeMountPaths(mountName, componentName string) ([]string, error) + GetVolumeMountPaths(mountName, containerName string) ([]string, error) // workspace related methods GetDevfileWorkspace() *v1.DevWorkspaceTemplateSpecContent diff --git a/pkg/devfile/parser/data/v2/volumes.go b/pkg/devfile/parser/data/v2/volumes.go index 2f287dd4..37a29fc7 100644 --- a/pkg/devfile/parser/data/v2/volumes.go +++ b/pkg/devfile/parser/data/v2/volumes.go @@ -9,11 +9,11 @@ import ( ) // AddVolumeMounts adds the volume mounts to the specified container component -func (d *DevfileV2) AddVolumeMounts(componentName string, volumeMounts []v1.VolumeMount) error { +func (d *DevfileV2) AddVolumeMounts(containerName string, volumeMounts []v1.VolumeMount) error { var pathErrorContainers []string found := false for _, component := range d.Components { - if component.Container != nil && component.Name == componentName { + if component.Container != nil && component.Name == containerName { found = true for _, devfileVolumeMount := range component.Container.VolumeMounts { for _, volumeMount := range volumeMounts { @@ -31,7 +31,7 @@ func (d *DevfileV2) AddVolumeMounts(componentName string, volumeMounts []v1.Volu if !found { return &common.FieldNotFoundError{ Field: "container component", - Name: componentName, + Name: containerName, } } @@ -72,12 +72,12 @@ func (d *DevfileV2) DeleteVolumeMount(name string) error { // GetVolumeMountPaths gets all the mount paths of the specified volume mount from the specified container component. // A container can mount at different paths for a given volume. -func (d *DevfileV2) GetVolumeMountPaths(mountName, componentName string) ([]string, error) { +func (d *DevfileV2) GetVolumeMountPaths(mountName, containerName string) ([]string, error) { componentFound := false var mountPaths []string for _, component := range d.Components { - if component.Container != nil && component.Name == componentName { + if component.Container != nil && component.Name == containerName { componentFound = true for _, volumeMount := range component.Container.VolumeMounts { if volumeMount.Name == mountName { @@ -90,12 +90,12 @@ func (d *DevfileV2) GetVolumeMountPaths(mountName, componentName string) ([]stri if !componentFound { return mountPaths, &common.FieldNotFoundError{ Field: "container component", - Name: componentName, + Name: containerName, } } if len(mountPaths) == 0 { - return mountPaths, fmt.Errorf("volume %s not mounted to component %s", mountName, componentName) + return mountPaths, fmt.Errorf("volume %s not mounted to component %s", mountName, containerName) } return mountPaths, nil