Add "alpha" feature gate tests (integration & examples) to our CI

docs/workspaces.md

@@ -186,8 +186,12 @@ spec:
       touch "$(workspaces.signals.path)/ready"
 ```
 
-**Note:** `Sidecars` _must_ explicitly opt-in to receiving the `Workspace` volume. Injected `Sidecars` from
-non-Tekton sources will not receive access to `Workspaces`.
+**Note:** Starting in Pipelines v0.24.0 `Sidecars` automatically get access to `Workspaces`.This is an
+alpha feature and requires Pipelines to have [the "alpha" feature gate enabled](./install.md#alpha-features).
+
+If a Sidecar already has a `volumeMount` at the location expected for a `workspace` then that `workspace` is
+not bound to the Sidecar. This preserves backwards-compatibility with any existing uses of the `volumeMount`
+trick described above.
 
 #### Isolating `Workspaces` to Specific `Steps` or `Sidecars`
 

pkg/workspace/apply.go

@@ -158,8 +158,7 @@ func mountAsSharedWorkspace(ts v1beta1.TaskSpec, volumeMount corev1.VolumeMount)
 	ts.StepTemplate.VolumeMounts = append(ts.StepTemplate.VolumeMounts, volumeMount)
 
 	for i := range ts.Sidecars {
-		sidecar := &ts.Sidecars[i]
-		sidecar.VolumeMounts = append(sidecar.VolumeMounts, volumeMount)
+		AddSidecarVolumeMount(&ts.Sidecars[i], volumeMount)
 	}
 }
 
@@ -193,3 +192,14 @@ func mountAsIsolatedWorkspace(ts v1beta1.TaskSpec, workspaceName string, volumeM
 		}
 	}
 }
+
+// AddSidecarVolumeMount is a helper to add a volumeMount to the sidecar unless its
+// MountPath would conflict with another of the sidecar's existing volume mounts.
+func AddSidecarVolumeMount(sidecar *v1beta1.Sidecar, volumeMount corev1.VolumeMount) {
+	for j := range sidecar.VolumeMounts {
+		if sidecar.VolumeMounts[j].MountPath == volumeMount.MountPath {
+			return
+		}
+	}
+	sidecar.VolumeMounts = append(sidecar.VolumeMounts, volumeMount)
+}

pkg/workspace/apply_test.go

@@ -794,6 +794,61 @@ func TestApply_IsolatedWorkspaces(t *testing.T) {
 				Name: "source",
 			}},
 		},
+	}, {
+		name: "existing sidecar volumeMounts are not displaced by workspace binding",
+		ts: v1beta1.TaskSpec{
+			Workspaces: []v1beta1.WorkspaceDeclaration{{
+				Name:      "custom",
+				MountPath: "/my/fancy/mount/path",
+				ReadOnly:  true,
+			}},
+			Sidecars: []v1beta1.Sidecar{{
+				Container: corev1.Container{
+					Name: "conflicting volume mount sidecar",
+					VolumeMounts: []corev1.VolumeMount{{
+						Name:      "mount-path-conflicts",
+						MountPath: "/my/fancy/mount/path",
+					}},
+				},
+			}},
+		},
+		workspaces: []v1beta1.WorkspaceBinding{{
+			Name: "custom",
+			PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
+				ClaimName: "mypvc",
+			},
+		}},
+		expectedTaskSpec: v1beta1.TaskSpec{
+			StepTemplate: &corev1.Container{
+				VolumeMounts: []corev1.VolumeMount{{
+					Name:      "ws-j2tds",
+					MountPath: "/my/fancy/mount/path",
+					ReadOnly:  true,
+				}},
+			},
+			Sidecars: []v1beta1.Sidecar{{
+				Container: corev1.Container{
+					Name: "conflicting volume mount sidecar",
+					VolumeMounts: []corev1.VolumeMount{{
+						Name:      "mount-path-conflicts",
+						MountPath: "/my/fancy/mount/path",
+					}},
+				},
+			}},
+			Volumes: []corev1.Volume{{
+				Name: "ws-j2tds",
+				VolumeSource: corev1.VolumeSource{
+					PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
+						ClaimName: "mypvc",
+					},
+				},
+			}},
+			Workspaces: []v1beta1.WorkspaceDeclaration{{
+				Name:      "custom",
+				MountPath: "/my/fancy/mount/path",
+				ReadOnly:  true,
+			}},
+		},
 	}} {
 		t.Run(tc.name, func(t *testing.T) {
 			ctx := config.ToContext(context.Background(), &config.Config{
@@ -831,3 +886,82 @@ func TestApplyWithMissingWorkspaceDeclaration(t *testing.T) {
 		t.Errorf("Expected error because workspace doesnt exist.")
 	}
 }
+
+// TestAddSidecarVolumeMount tests that sidecars dont receive a volume mount if
+// it has a mount that already shares the same MountPath.
+func TestAddSidecarVolumeMount(t *testing.T) {
+	for _, tc := range []struct {
+		sidecarMounts   []corev1.VolumeMount
+		volumeMount     corev1.VolumeMount
+		expectedSidecar v1beta1.Sidecar
+	}{{
+		sidecarMounts: nil,
+		volumeMount: corev1.VolumeMount{
+			Name:      "foo",
+			MountPath: "/workspace/foo",
+		},
+		expectedSidecar: v1beta1.Sidecar{
+			Container: corev1.Container{
+				VolumeMounts: []corev1.VolumeMount{{
+					Name:      "foo",
+					MountPath: "/workspace/foo",
+				}},
+			},
+		},
+	}, {
+		sidecarMounts: []corev1.VolumeMount{},
+		volumeMount: corev1.VolumeMount{
+			Name:      "foo",
+			MountPath: "/workspace/foo",
+		},
+		expectedSidecar: v1beta1.Sidecar{
+			Container: corev1.Container{
+				VolumeMounts: []corev1.VolumeMount{{
+					Name:      "foo",
+					MountPath: "/workspace/foo",
+				}},
+			},
+		},
+	}, {
+		sidecarMounts: []corev1.VolumeMount{{
+			Name:      "bar",
+			MountPath: "/workspace/bar",
+		}},
+		volumeMount: corev1.VolumeMount{
+			Name:      "workspace1",
+			MountPath: "/workspace/bar",
+		},
+		expectedSidecar: v1beta1.Sidecar{
+			Container: corev1.Container{
+				VolumeMounts: []corev1.VolumeMount{{
+					Name:      "bar",
+					MountPath: "/workspace/bar",
+				}},
+			},
+		},
+	}, {
+		sidecarMounts: []corev1.VolumeMount{{
+			Name:      "bar",
+			MountPath: "/workspace/bar",
+		}},
+		volumeMount: corev1.VolumeMount{
+			Name:      "foo",
+			MountPath: "/workspace/foo",
+		},
+		expectedSidecar: v1beta1.Sidecar{
+			Container: corev1.Container{
+				VolumeMounts: []corev1.VolumeMount{{
+					Name:      "bar",
+					MountPath: "/workspace/bar",
+				}, {
+					Name:      "foo",
+					MountPath: "/workspace/foo",
+				}},
+			},
+		},
+	}} {
+		sidecar := v1beta1.Sidecar{}
+		sidecar.Container.VolumeMounts = tc.sidecarMounts
+		workspace.AddSidecarVolumeMount(&v1beta1.Sidecar{}, corev1.VolumeMount{})
+	}
+}

test/e2e-tests.sh

@@ -29,14 +29,34 @@ install_pipeline_crd
 
 failed=0
 
-# Run the integration tests
-header "Running Go e2e tests"
-go_test_e2e -timeout=20m ./test/... || failed=1
-
-# Run these _after_ the integration tests b/c they don't quite work all the way
-# and they cause a lot of noise in the logs, making it harder to debug integration
-# test failures.
-go_test_e2e -tags=examples -timeout=20m ./test/ || failed=1
+function set_feature_gate() {
+  local gate="$1"
+  if [ "$gate" != "alpha" ] && [ "$gate" != "stable" ] && [ "$gate" != "beta" ] ; then
+    printf "Invalid gate %s\n" ${gate}
+    exit 255
+  fi
+  printf "Setting feature gate to %s\n", ${gate}
+  jsonpatch=$(printf "{\"data\": {\"enable-api-fields\": \"%s\"}}" $1)
+  echo "feature-flags ConfigMap patch: ${jsonpatch}"
+  kubectl patch configmap feature-flags -n tekton-pipelines -p "$jsonpatch"
+}
+
+function run_e2e() {
+  # Run the integration tests
+  header "Running Go e2e tests"
+  go_test_e2e -timeout=20m ./test/... || failed=1
+
+  # Run these _after_ the integration tests b/c they don't quite work all the way
+  # and they cause a lot of noise in the logs, making it harder to debug integration
+  # test failures.
+  go_test_e2e -tags=examples -timeout=20m ./test/ || failed=1
+}
+
+set_feature_gate "stable"
+run_e2e
+
+set_feature_gate "alpha"
+run_e2e
 
 (( failed )) && fail_test
 success

test/tektonbundles_test.go

@@ -39,6 +39,7 @@ import (
 	"github.com/google/go-containerregistry/pkg/v1/tarball"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1beta1"
 	"github.com/tektoncd/pipeline/pkg/pod"
+	"github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"knative.dev/pkg/apis"
@@ -269,7 +270,7 @@ func TestTektonBundlesUsingRegularImage(t *testing.T) {
 	if err := WaitForPipelineRunState(ctx, c, pipelineRunName, timeout,
 		Chain(
 			FailedWithReason(pod.ReasonCouldntGetTask, pipelineRunName),
-			FailedWithMessage("could not find object in image with kind: task and name: hello-world-dne", pipelineRunName),
+			FailedWithMessage("does not contain a dev.tekton.image.apiVersion annotation", pipelineRunName),
 		), "PipelineRunFailed"); err != nil {
 		t.Fatalf("Error waiting for PipelineRun to finish with expected error: %s", err)
 	}
@@ -345,6 +346,7 @@ func TestTektonBundlesUsingImproperFormat(t *testing.T) {
 	img, err = mutate.Append(img, mutate.Addendum{
 		Layer: taskLayer,
 		Annotations: map[string]string{
+			// intentionally invalid name annotation
 			"org.opencontainers.image.title": taskName,
 			"dev.tekton.image.kind":          strings.ToLower(task.Kind),
 			"dev.tekton.image.apiVersion":    task.APIVersion,
@@ -380,8 +382,8 @@ func TestTektonBundlesUsingImproperFormat(t *testing.T) {
 	t.Logf("Waiting for PipelineRun in namespace %s to finish", namespace)
 	if err := WaitForPipelineRunState(ctx, c, pipelineRunName, timeout,
 		Chain(
-			FailedWithReason(pod.ReasonCouldntGetTask, pipelineRunName),
-			FailedWithMessage("could not find object in image with kind: task and name: hello-world", pipelineRunName),
+			FailedWithReason(pipelinerun.ReasonCouldntGetPipeline, pipelineRunName),
+			FailedWithMessage("does not contain a dev.tekton.image.name annotation", pipelineRunName),
 		), "PipelineRunFailed"); err != nil {
 		t.Fatalf("Error waiting for PipelineRun to finish with expected error: %s", err)
 	}