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