Skip to content

Commit

Permalink
Update with PR feedback
Browse files Browse the repository at this point in the history 
Signed-off-by: Maysun J Faisal <maysunaneek@gmail.com>
  • Loading branch information
@maysunfaisal
maysunfaisal committed Apr 21, 2021
1 parent 74c6802 commit 294012e
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 108 deletions.
11 changes: 11 additions & 0 deletions pkg/devfile/generator/generators.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import (
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"

v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
"github.com/devfile/library/pkg/devfile/parser"
"github.com/devfile/library/pkg/devfile/parser/data/v2/common"
)
Expand Down Expand Up @@ -326,3 +327,13 @@ func GetVolumesAndVolumeMounts(devfileObj parser.DevfileObj, volumeParams Volume
}
return pvcVols, nil
}

// GetVolumeMountPath gets the volume mount's path.
func GetVolumeMountPath(volumeMount v1.VolumeMount) string {
// if there is no volume mount path, default to volume mount name as per devfile schema
if volumeMount.Path == "" {
volumeMount.Path = "/" + volumeMount.Name
}

return volumeMount.Path
}
97 changes: 63 additions & 34 deletions pkg/devfile/generator/generators_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -417,50 +417,79 @@ func TestGetVolumesAndVolumeMounts(t *testing.T) {
}

pvcVols, err := GetVolumesAndVolumeMounts(devObj, volumeParams, options)
if !tt.wantErr && err != nil {
t.Errorf("TestGetVolumesAndVolumeMounts unexpected error: %v", err)
return
} else if tt.wantErr && err != nil {
return
} else if tt.wantErr && err == nil {
t.Error("TestGetVolumesAndVolumeMounts expected error but got nil")
return
}

// check if the pvc volumes returned are correct
for _, volInfo := range tt.volumeNameToVolInfo {
matched := false
for _, pvcVol := range pvcVols {
if volInfo.VolumeName == pvcVol.Name && pvcVol.PersistentVolumeClaim != nil && volInfo.PVCName == pvcVol.PersistentVolumeClaim.ClaimName {
matched = true
if tt.wantErr == (err == nil) {
t.Errorf("TestGetVolumesAndVolumeMounts() error = %v, wantErr %v", err, tt.wantErr)
} else if err == nil {
// check if the pvc volumes returned are correct
for _, volInfo := range tt.volumeNameToVolInfo {
matched := false
for _, pvcVol := range pvcVols {
if volInfo.VolumeName == pvcVol.Name && pvcVol.PersistentVolumeClaim != nil && volInfo.PVCName == pvcVol.PersistentVolumeClaim.ClaimName {
matched = true
}
}
}

if !matched {
t.Errorf("TestGetVolumesAndVolumeMounts error - could not find volume details %s in the actual result", volInfo.VolumeName)
if !matched {
t.Errorf("TestGetVolumesAndVolumeMounts error - could not find volume details %s in the actual result", volInfo.VolumeName)
}
}
}

// check the volume mounts of the containers
for _, container := range containers {
if volMounts, ok := tt.wantContainerToVol[container.Name]; !ok {
t.Errorf("TestGetVolumesAndVolumeMounts error - did not find the expected container %s", container.Name)
return
} else {
for _, expectedVolMount := range volMounts {
matched := false
for _, actualVolMount := range container.VolumeMounts {
if expectedVolMount.volumeName == actualVolMount.Name && expectedVolMount.mountPath == actualVolMount.MountPath {
matched = true
// check the volume mounts of the containers
for _, container := range containers {
if volMounts, ok := tt.wantContainerToVol[container.Name]; !ok {
t.Errorf("TestGetVolumesAndVolumeMounts error - did not find the expected container %s", container.Name)
return
} else {
for _, expectedVolMount := range volMounts {
matched := false
for _, actualVolMount := range container.VolumeMounts {
if expectedVolMount.volumeName == actualVolMount.Name && expectedVolMount.mountPath == actualVolMount.MountPath {
matched = true
}
}
}

if !matched {
t.Errorf("TestGetVolumesAndVolumeMounts error - could not find volume mount details for path %s in the actual result for container %s", expectedVolMount.mountPath, container.Name)
if !matched {
t.Errorf("TestGetVolumesAndVolumeMounts error - could not find volume mount details for path %s in the actual result for container %s", expectedVolMount.mountPath, container.Name)
}
}
}
}
}
})
}
}

func TestGetVolumeMountPath(t *testing.T) {

tests := []struct {
name string
volumeMount v1.VolumeMount
wantPath string
}{
{
name: "Mount Path is present",
volumeMount: v1.VolumeMount{
Name: "name1",
Path: "/path1",
},
wantPath: "/path1",
},
{
name: "Mount Path is absent",
volumeMount: v1.VolumeMount{
Name: "name1",
},
wantPath: "/name1",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
path := GetVolumeMountPath(tt.volumeMount)

if path != tt.wantPath {
t.Errorf("TestGetVolumeMountPath error: mount path mismatch, expected: %v got: %v", tt.wantPath, path)
}
})
}

}
11 changes: 0 additions & 11 deletions pkg/devfile/generator/utils.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -420,16 +420,6 @@ func getBuildConfigSpec(buildConfigSpecParams BuildConfigSpecParams) *buildv1.Bu
}
}

// GetVolumeMountPath gets the volume mount's path.
func GetVolumeMountPath(volumeMount v1.VolumeMount) string {
// if there is no volume mount path, default to volume mount name as per devfile schema
if volumeMount.Path == "" {
volumeMount.Path = "/" + volumeMount.Name
}

return volumeMount.Path
}

// getPVC gets a pvc type volume with the given volume name and pvc name.
func getPVC(volumeName, pvcName string) corev1.Volume {

Expand All @@ -454,7 +444,6 @@ func addVolumeMountToContainers(containers []corev1.Container, volumeName string
containers[i].VolumeMounts = append(containers[i].VolumeMounts, corev1.VolumeMount{
Name: volumeName,
MountPath: mountPath,
SubPath: "",
},
)
}
Expand Down
72 changes: 9 additions & 63 deletions pkg/devfile/generator/utils_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1311,55 +1311,22 @@ func TestGetBuildConfigSpec(t *testing.T) {

}

func TestGetVolumeMountPath(t *testing.T) {

tests := []struct {
name string
volumeMount v1.VolumeMount
wantPath string
}{
{
name: "Mount Path is present",
volumeMount: v1.VolumeMount{
Name: "name1",
Path: "/path1",
},
wantPath: "/path1",
},
{
name: "Mount Path is absent",
volumeMount: v1.VolumeMount{
Name: "name1",
},
wantPath: "/name1",
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
path := GetVolumeMountPath(tt.volumeMount)

if path != tt.wantPath {
t.Errorf("TestGetVolumeMountPath error: mount path mismatch, expected: %v got: %v", tt.wantPath, path)
}
})
}

}

func TestGetPVC(t *testing.T) {

tests := []struct {
name string
pvc string
volumeName string
}{
{
name: "Get PVC vol for given pvc name and volume name",
pvc: "mypvc",
volumeName: "myvolume",
},
}

for _, tt := range tests {
t.Run(tt.volumeName, func(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
volume := getPVC(tt.volumeName, tt.pvc)

if volume.Name != tt.volumeName {
Expand All @@ -1376,22 +1343,14 @@ func TestGetPVC(t *testing.T) {
func TestAddVolumeMountToContainers(t *testing.T) {

tests := []struct {
podName string
namespace string
serviceAccount string
pvc string
name string
volumeName string
containerMountPathsMap map[string][]string
container corev1.Container
labels map[string]string
wantErr bool
}{
{
podName: "podSpecTest",
namespace: "default",
serviceAccount: "default",
pvc: "mypvc",
volumeName: "myvolume",
name: "Successfully mount volume to container",
volumeName: "myvolume",
containerMountPathsMap: map[string][]string{
"container1": {"/tmp/path1", "/tmp/path2"},
},
Expand All @@ -1404,32 +1363,19 @@ func TestAddVolumeMountToContainers(t *testing.T) {
Args: []string{"-f", "/dev/null"},
Env: []corev1.EnvVar{},
},
labels: map[string]string{
"app": "app",
"component": "frontend",
},
wantErr: false,
},
{
podName: "podSpecTest",
namespace: "default",
serviceAccount: "default",
pvc: "mypvc",
volumeName: "myvolume",
name: "No Container present to mount volume",
volumeName: "myvolume",
containerMountPathsMap: map[string][]string{
"container1": {"/tmp/path1", "/tmp/path2"},
},
container: corev1.Container{},
labels: map[string]string{
"app": "app",
"component": "frontend",
},
wantErr: true,
},
}

for _, tt := range tests {
t.Run(tt.podName, func(t *testing.T) {
t.Run(tt.name, func(t *testing.T) {
containers := []corev1.Container{tt.container}
addVolumeMountToContainers(containers, tt.volumeName, tt.containerMountPathsMap)

Expand Down

0 comments on commit 294012e

Please sign in to comment.