From ea9158f95d8ccc3260408f0b518a441fefbf14f2 Mon Sep 17 00:00:00 2001 From: Scott Date: Thu, 6 May 2021 13:32:18 -0400 Subject: [PATCH] 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. --- docs/workspaces.md | 8 ++- pkg/workspace/apply.go | 14 +++- pkg/workspace/apply_test.go | 134 ++++++++++++++++++++++++++++++++++++ test/e2e-tests.sh | 36 +++++++--- test/tektonbundles_test.go | 8 ++- 5 files changed, 185 insertions(+), 15 deletions(-) diff --git a/docs/workspaces.md b/docs/workspaces.md index 9cb7ee09fb1..98658359e11 100644 --- a/docs/workspaces.md +++ b/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` diff --git a/pkg/workspace/apply.go b/pkg/workspace/apply.go index 6af116a4943..3afc3f088dd 100644 --- a/pkg/workspace/apply.go +++ b/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) +} diff --git a/pkg/workspace/apply_test.go b/pkg/workspace/apply_test.go index 66f518057e5..98704db4624 100644 --- a/pkg/workspace/apply_test.go +++ b/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{}) + } +} diff --git a/test/e2e-tests.sh b/test/e2e-tests.sh index 9fd46940045..8480bbc1afb 100755 --- a/test/e2e-tests.sh +++ b/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 diff --git a/test/tektonbundles_test.go b/test/tektonbundles_test.go index a6d7c1364e7..596d1cf142f 100644 --- a/test/tektonbundles_test.go +++ b/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) }