Skip to content

Commit

Permalink
Run integration tests in "alpha" as part of pipelines' CI
Browse files Browse the repository at this point in the history 
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.
  • Loading branch information
@tekton-robot
Scott authored and tekton-robot committed May 10, 2021
1 parent f4b56b1 commit ea9158f
Show file tree
Hide file tree
Showing 5 changed files with 185 additions and 15 deletions.
8 changes: 6 additions & 2 deletions docs/workspaces.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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`

Expand Down
14 changes: 12 additions & 2 deletions pkg/workspace/apply.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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)
}
134 changes: 134 additions & 0 deletions pkg/workspace/apply_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{})
}
}
36 changes: 28 additions & 8 deletions test/e2e-tests.sh
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
8 changes: 5 additions & 3 deletions test/tektonbundles_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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)
}
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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)
}
Expand Down

0 comments on commit ea9158f

Please sign in to comment.