Don't add projects volumeMount if container mounts it manually

This avoids an issue where a container can mount the projects volume
directly and have `mountSources: true`, which results in a duplicate
mount and invalid deployment.

Related to issue devfile/api#290 and how
projects volume should be handled with `mountSources`.

Signed-off-by: Angel Misevski <[email protected]>

Author: amisevsk

pkg/library/container/container_test.go

@@ -18,6 +18,7 @@ import (
 	"path/filepath"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	corev1 "k8s.io/api/core/v1"
 
 	devworkspace "github.com/devfile/api/pkg/apis/workspaces/v1alpha2"
@@ -68,6 +69,7 @@ func TestGetKubeContainersFromDevfile(t *testing.T) {
 		loadTestCaseOrPanic(t, "handles-mountSources.yaml"),
 		loadTestCaseOrPanic(t, "handles-resources.yaml"),
 		loadTestCaseOrPanic(t, "handles-endpoints-with-common-port.yaml"),
+		loadTestCaseOrPanic(t, "handles-container-that-mounts-projects-directly.yaml"),
 		loadTestCaseOrPanic(t, "ignores-non-container-components.yaml"),
 		loadTestCaseOrPanic(t, "converts-all-fields.yaml"),
 		loadTestCaseOrPanic(t, "error-has-parent.yaml"),
@@ -87,7 +89,8 @@ func TestGetKubeContainersFromDevfile(t *testing.T) {
 				if !assert.NoError(t, err, "Should not return error") {
 					return
 				}
-				assert.Equal(t, tt.Output.PodAdditions, gotPodAdditions, "PodAdditions should match expected output")
+				assert.True(t, cmp.Equal(tt.Output.PodAdditions, gotPodAdditions),
+					"PodAdditions should match expected output: \n%s", cmp.Diff(tt.Output.PodAdditions, gotPodAdditions))
 			}
 		})
 	}

pkg/library/container/mountSources.go

@@ -40,16 +40,23 @@ func handleMountSources(k8sContainer *corev1.Container, devfileContainer *devwor
 	if !HasMountSources(devfileContainer) {
 		return
 	}
-	sourceMapping := devfileContainer.SourceMapping
-	if sourceMapping == "" {
-		// Sanity check -- this value should be defaulted to `/projects` but may not be
-		// if struct was not processed by k8s
-		sourceMapping = config.DefaultProjectsSourcesRoot
+	var sourceMapping string
+	if vm := getProjectsVolumeMount(k8sContainer); vm != nil {
+		// Container already mounts projects volume; need to set env vars according to mountPath
+		// TODO: see issue https://github.com/devfile/api/issues/290
+		sourceMapping = vm.MountPath
+	} else {
+		sourceMapping = devfileContainer.SourceMapping
+		if sourceMapping == "" {
+			// Sanity check -- this value should be defaulted to `/projects` but may not be
+			// if struct was not processed by k8s
+			sourceMapping = config.DefaultProjectsSourcesRoot
+		}
+		k8sContainer.VolumeMounts = append(k8sContainer.VolumeMounts, corev1.VolumeMount{
+			Name:      constants.ProjectsVolumeName,
+			MountPath: sourceMapping,
+		})
 	}
-	k8sContainer.VolumeMounts = append(k8sContainer.VolumeMounts, corev1.VolumeMount{
-		Name:      constants.ProjectsVolumeName,
-		MountPath: sourceMapping,
-	})
 	k8sContainer.Env = append(k8sContainer.Env, corev1.EnvVar{
 		Name:  constants.ProjectsRootEnvVar,
 		Value: sourceMapping,
@@ -58,3 +65,14 @@ func handleMountSources(k8sContainer *corev1.Container, devfileContainer *devwor
 		Value: sourceMapping, // TODO: Unclear how this should be handled in case of multiple projects
 	})
 }
+
+// getProjectsVolumeMount returns the projects volumeMount in a container, if it is defined; if it does not exist,
+// returns nil.
+func getProjectsVolumeMount(k8sContainer *corev1.Container) *corev1.VolumeMount {
+	for _, vm := range k8sContainer.VolumeMounts {
+		if vm.Name == constants.ProjectsVolumeName {
+			return &vm
+		}
+	}
+	return nil
+}

pkg/library/container/testdata/handles-container-that-mounts-projects-directly.yaml

@@ -0,0 +1,34 @@
+# This test is due to an inconsistency in devfile/api semantics; see
+# issue https://github.com/devfile/api/issues/290
+
+name: "Handles container with mountSources and explicit projects volume"
+
+input:
+  components:
+    - name: testing-container-1
+      container:
+        image: testing-image-1
+        memoryLimit: 999Mi  # isolate test to not include memoryLimit
+        sourceMapping: "/testdir1"
+        mountSources: true
+        volumeMounts:
+          - name: "projects"
+            path: "/not-source-mapping"
+
+output:
+  podAdditions:
+    containers:
+      - name: testing-container-1
+        image: testing-image-1
+        imagePullPolicy: Always
+        resources:
+          limits:
+            memory: "999Mi"
+        volumeMounts:
+          - name: "projects"
+            mountPath: "/not-source-mapping"
+        env:
+          - name: "PROJECTS_ROOT"
+            value: "/not-source-mapping"
+          - name: "PROJECTS_SOURCE"
+            value: "/not-source-mapping" # Temp value until projects is figured out