Skip to content

Commit

Permalink
Merge pull request #81 from maysunfaisal/181-1
Browse files Browse the repository at this point in the history
Migrate devfile specific vol code from odo to devfile/library
  • Loading branch information
maysunfaisal authored Apr 21, 2021
2 parents ffe3a0a + 294012e commit 1017f66
Show file tree
Hide file tree
Showing 4 changed files with 443 additions and 3 deletions.
53 changes: 53 additions & 0 deletions pkg/devfile/generator/generators.go
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 @@ -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
}
271 changes: 268 additions & 3 deletions pkg/devfile/generator/generators_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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{
Expand Down Expand Up @@ -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,
},
},
Expand Down Expand Up @@ -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)
}
})
}

}
32 changes: 32 additions & 0 deletions pkg/devfile/generator/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
)
}
}
}
}
}
Loading

0 comments on commit 1017f66

Please sign in to comment.