Run integration tests in "alpha" as part of pipelines' CI

Prior to this commit the integration tests run against PRs
were only those for "stable" features.

This PR adds additional integration test runs for "alpha"
features as well. `test/e2e-scripts.sh` now patches the feature-flags
ConfigMap before executing integration tests and examples tests, first
with "enable-api-fields: stable" and then with "enable-api-fields: alpha".

As part of switching on alpha tests two issues have also been fixed
with alpha features:

1. Sidecar workspaces could collide with existing volumeMounts attached
to the sidecar if the mountpath of the workspace matched an existing
volumeMount. This has been fixed by preventing the additional workspace
volumeMount if one already exists at that mountpath.

2. Tekton bundles integration tests needed to be updated to account
for validation errors that are returned when a bundle is invalid.

Author: Scott

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)
 	}