Merge pull request #81 from maysunfaisal/181-1

Migrate devfile specific vol code from odo to devfile/library

Author: maysunfaisal

pkg/devfile/generator/generators.go

@@ -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"
 )
@@ -284,3 +285,55 @@ func GetImageStream(imageStreamParams ImageStreamParams) imagev1.ImageStream {
 	}
 	return imageStream
 }
+
+// VolumeInfo is a struct to hold the pvc name and the volume name to create a volume.
+type VolumeInfo struct {
+	PVCName    string
+	VolumeName string
+}
+
+// VolumeParams is a struct that contains the required data to create Kubernetes Volumes and mount Volumes in Containers
+type VolumeParams struct {
+	// Containers is a list of containers that needs to be updated for the volume mounts
+	Containers []corev1.Container
+
+	// VolumeNameToVolumeInfo is a map of the devfile volume name to the volume info containing the pvc name and the volume name.
+	VolumeNameToVolumeInfo map[string]VolumeInfo
+}
+
+// GetVolumesAndVolumeMounts gets the PVC volumes and updates the containers with the volume mounts.
+func GetVolumesAndVolumeMounts(devfileObj parser.DevfileObj, volumeParams VolumeParams, options common.DevfileOptions) ([]corev1.Volume, error) {
+
+	containerComponents, err := devfileObj.Data.GetDevfileContainerComponents(options)
+	if err != nil {
+		return nil, err
+	}
+
+	var pvcVols []corev1.Volume
+	for volName, volInfo := range volumeParams.VolumeNameToVolumeInfo {
+		pvcVols = append(pvcVols, getPVC(volInfo.VolumeName, volInfo.PVCName))
+
+		// containerNameToMountPaths is a map of the Devfile container name to their Devfile Volume Mount Paths for a given Volume Name
+		containerNameToMountPaths := make(map[string][]string)
+		for _, containerComp := range containerComponents {
+			for _, volumeMount := range containerComp.Container.VolumeMounts {
+				if volName == volumeMount.Name {
+					containerNameToMountPaths[containerComp.Name] = append(containerNameToMountPaths[containerComp.Name], GetVolumeMountPath(volumeMount))
+				}
+			}
+		}
+
+		addVolumeMountToContainers(volumeParams.Containers, volInfo.VolumeName, containerNameToMountPaths)
+	}
+	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
+}

pkg/devfile/generator/generators_test.go

@@ -4,7 +4,6 @@ import (
 	"reflect"
 	"testing"
 
-	"github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 	v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
 	"github.com/devfile/api/v2/pkg/attributes"
 	"github.com/devfile/library/pkg/devfile/parser"
@@ -28,7 +27,7 @@ func TestGetContainers(t *testing.T) {
 	trueMountSources := true
 	falseMountSources := false
 
-	project := v1alpha2.Project{
+	project := v1.Project{
 		ClonePath: "test-project/",
 		Name:      "project0",
 		ProjectSource: v1.ProjectSource{
@@ -189,7 +188,7 @@ func TestGetContainers(t *testing.T) {
 						DevWorkspaceTemplateSpec: v1.DevWorkspaceTemplateSpec{
 							DevWorkspaceTemplateSpecContent: v1.DevWorkspaceTemplateSpecContent{
 								Components: tt.containerComponents,
-								Projects: []v1alpha2.Project{
+								Projects: []v1.Project{
 									project,
 								},
 							},
@@ -228,3 +227,269 @@ func TestGetContainers(t *testing.T) {
 	}
 
 }
+
+func TestGetVolumesAndVolumeMounts(t *testing.T) {
+
+	type testVolumeMountInfo struct {
+		mountPath  string
+		volumeName string
+	}
+
+	tests := []struct {
+		name                string
+		components          []v1.Component
+		volumeNameToVolInfo map[string]VolumeInfo
+		wantContainerToVol  map[string][]testVolumeMountInfo
+		wantErr             bool
+	}{
+		{
+			name:       "One volume mounted",
+			components: []v1.Component{testingutil.GetFakeContainerComponent("comp1"), testingutil.GetFakeContainerComponent("comp2")},
+			volumeNameToVolInfo: map[string]VolumeInfo{
+				"myvolume1": {
+					PVCName:    "volume1-pvc",
+					VolumeName: "volume1-pvc-vol",
+				},
+			},
+			wantContainerToVol: map[string][]testVolumeMountInfo{
+				"comp1": {
+					{
+						mountPath:  "/my/volume/mount/path1",
+						volumeName: "volume1-pvc-vol",
+					},
+				},
+				"comp2": {
+					{
+						mountPath:  "/my/volume/mount/path1",
+						volumeName: "volume1-pvc-vol",
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "One volume mounted at diff locations",
+			components: []v1.Component{
+				{
+					Name: "container1",
+					ComponentUnion: v1.ComponentUnion{
+						Container: &v1.ContainerComponent{
+							Container: v1.Container{
+								VolumeMounts: []v1.VolumeMount{
+									{
+										Name: "volume1",
+										Path: "/path1",
+									},
+									{
+										Name: "volume1",
+										Path: "/path2",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			volumeNameToVolInfo: map[string]VolumeInfo{
+				"volume1": {
+					PVCName:    "volume1-pvc",
+					VolumeName: "volume1-pvc-vol",
+				},
+			},
+			wantContainerToVol: map[string][]testVolumeMountInfo{
+				"container1": {
+					{
+						mountPath:  "/path1",
+						volumeName: "volume1-pvc-vol",
+					},
+					{
+						mountPath:  "/path2",
+						volumeName: "volume1-pvc-vol",
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "One volume mounted at diff container components",
+			components: []v1.Component{
+				{
+					Name: "container1",
+					ComponentUnion: v1.ComponentUnion{
+						Container: &v1.ContainerComponent{
+							Container: v1.Container{
+								VolumeMounts: []v1.VolumeMount{
+									{
+										Name: "volume1",
+										Path: "/path1",
+									},
+								},
+							},
+						},
+					},
+				},
+				{
+					Name: "container2",
+					ComponentUnion: v1.ComponentUnion{
+						Container: &v1.ContainerComponent{
+							Container: v1.Container{
+								VolumeMounts: []v1.VolumeMount{
+									{
+										Name: "volume1",
+										Path: "/path2",
+									},
+								},
+							},
+						},
+					},
+				},
+			},
+			volumeNameToVolInfo: map[string]VolumeInfo{
+				"volume1": {
+					PVCName:    "volume1-pvc",
+					VolumeName: "volume1-pvc-vol",
+				},
+			},
+			wantContainerToVol: map[string][]testVolumeMountInfo{
+				"container1": {
+					{
+						mountPath:  "/path1",
+						volumeName: "volume1-pvc-vol",
+					},
+				},
+				"container2": {
+					{
+						mountPath:  "/path2",
+						volumeName: "volume1-pvc-vol",
+					},
+				},
+			},
+			wantErr: false,
+		},
+		{
+			name: "Invalid case",
+			components: []v1.Component{
+				{
+					Name: "container1",
+					Attributes: attributes.Attributes{}.FromStringMap(map[string]string{
+						"firstString": "firstStringValue",
+					}),
+					ComponentUnion: v1.ComponentUnion{},
+				},
+			},
+			wantErr: true,
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+
+			devObj := parser.DevfileObj{
+				Data: &v2.DevfileV2{
+					Devfile: v1.Devfile{
+						DevWorkspaceTemplateSpec: v1.DevWorkspaceTemplateSpec{
+							DevWorkspaceTemplateSpecContent: v1.DevWorkspaceTemplateSpecContent{
+								Components: tt.components,
+							},
+						},
+					},
+				},
+			}
+
+			containers, err := GetContainers(devObj, common.DevfileOptions{})
+			if err != nil {
+				t.Errorf("TestGetVolumesAndVolumeMounts error - %v", err)
+				return
+			}
+
+			var options common.DevfileOptions
+			if tt.wantErr {
+				options = common.DevfileOptions{
+					Filter: map[string]interface{}{
+						"firstString": "firstStringValue",
+					},
+				}
+			}
+
+			volumeParams := VolumeParams{
+				Containers:             containers,
+				VolumeNameToVolumeInfo: tt.volumeNameToVolInfo,
+			}
+
+			pvcVols, err := GetVolumesAndVolumeMounts(devObj, volumeParams, options)
+			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)
+					}
+				}
+
+				// 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)
+							}
+						}
+					}
+				}
+			}
+		})
+	}
+}
+
+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)
+			}
+		})
+	}
+
+}

pkg/devfile/generator/utils.go

@@ -419,3 +419,35 @@ func getBuildConfigSpec(buildConfigSpecParams BuildConfigSpecParams) *buildv1.Bu
 		},
 	}
 }
+
+// getPVC gets a pvc type volume with the given volume name and pvc name.
+func getPVC(volumeName, pvcName string) corev1.Volume {
+
+	return corev1.Volume{
+		Name: volumeName,
+		VolumeSource: corev1.VolumeSource{
+			PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
+				ClaimName: pvcName,
+			},
+		},
+	}
+}
+
+// addVolumeMountToContainers adds the Volume Mounts in containerNameToMountPaths to the containers for a given volumeName.
+// containerNameToMountPaths is a map of a container name to an array of its Mount Paths.
+func addVolumeMountToContainers(containers []corev1.Container, volumeName string, containerNameToMountPaths map[string][]string) {
+
+	for containerName, mountPaths := range containerNameToMountPaths {
+		for i := range containers {
+			if containers[i].Name == containerName {
+				for _, mountPath := range mountPaths {
+					containers[i].VolumeMounts = append(containers[i].VolumeMounts, corev1.VolumeMount{
+						Name:      volumeName,
+						MountPath: mountPath,
+					},
+					)
+				}
+			}
+		}
+	}
+}