From e556bc734f781a90ec32e93231025bf7fc156a54 Mon Sep 17 00:00:00 2001 From: qingliu Date: Mon, 15 Apr 2024 21:09:13 +0800 Subject: [PATCH] fix: resolve pod creation failure on retry when using `workspace..volume` fix #7886 Change the naming method of the workspace volume from completely random to hashed, ensuring that the name generated during a single taskRun lifecycle is consistent each time, and is unique within all current workspaces. This way, we can reuse the logic of retrieving the taskSpec from the status, and also store the content after variable expansion in the taskSpec of the status for easy debugging; it will also not affect the reconstruction of the pod when retrying. --- pkg/names/generate.go | 19 +++ pkg/names/generate_test.go | 61 ++++++++ .../taskrun/resources/apply_test.go | 30 ++-- pkg/reconciler/taskrun/taskrun_test.go | 8 +- pkg/workspace/apply.go | 27 +++- pkg/workspace/apply_test.go | 135 ++++++++++++------ test/taskrun_test.go | 91 ++++++++++++ 7 files changed, 300 insertions(+), 71 deletions(-) diff --git a/pkg/names/generate.go b/pkg/names/generate.go index deab93f5097..cc1b33399f9 100644 --- a/pkg/names/generate.go +++ b/pkg/names/generate.go @@ -18,7 +18,10 @@ package names import ( "fmt" + "hash/fnv" "regexp" + "strconv" + "strings" utilrand "k8s.io/apimachinery/pkg/util/rand" ) @@ -73,3 +76,19 @@ func (simpleNameGenerator) RestrictLength(base string) string { } return base } + +// GenerateHashedName creates a unique name with a hashed suffix. +func GenerateHashedName(prefix, name string, hashedLength int) string { + if hashedLength <= 0 { + hashedLength = randomLength + } + h := fnv.New32a() + h.Write([]byte(name)) + suffix := strconv.FormatUint(uint64(h.Sum32()), 16) + if ln := len(suffix); ln > hashedLength { + suffix = suffix[:hashedLength] + } else if ln < hashedLength { + suffix += strings.Repeat("0", hashedLength-ln) + } + return fmt.Sprintf("%s-%s", prefix, suffix) +} diff --git a/pkg/names/generate_test.go b/pkg/names/generate_test.go index bf14eb82dd8..bca8c03d02d 100644 --- a/pkg/names/generate_test.go +++ b/pkg/names/generate_test.go @@ -67,3 +67,64 @@ func TestRestrictLength(t *testing.T) { }) } } + +func TestGenerateHashedName(t *testing.T) { + tests := []struct { + title string + prefix string + name string + randomLength int + expectedHashedName string + }{{ + title: "generate hashed name with custom random length", + prefix: "ws", + name: "workspace-name", + randomLength: 10, + expectedHashedName: "ws-d70baf7a00", + }, { + title: "generate hashed name with default random length", + prefix: "ws", + name: "workspace-name", + randomLength: -1, + expectedHashedName: "ws-d70ba", + }, { + title: "generate hashed name with empty prefix", + prefix: "", + name: "workspace-name", + randomLength: 0, + expectedHashedName: "-d70ba", + }, { + title: "consistent hashed name for different inputs - 1", + prefix: "ws", + name: "test-01097628", + randomLength: 5, + expectedHashedName: "ws-f32ff", + }, { + title: "consistent hashed name for different inputs - 2", + prefix: "ws", + name: "test-01617609", + randomLength: 5, + expectedHashedName: "ws-f32ff", + }, { + title: "consistent hashed name for different inputs - 3", + prefix: "ws", + name: "test-01675975", + randomLength: 5, + expectedHashedName: "ws-f32ff", + }, { + title: "consistent hashed name for different inputs - 4", + prefix: "ws", + name: "test-01809743", + randomLength: 5, + expectedHashedName: "ws-f32ff", + }} + + for _, tc := range tests { + t.Run(tc.title, func(t *testing.T) { + hashedName := pkgnames.GenerateHashedName(tc.prefix, tc.name, tc.randomLength) + if hashedName != tc.expectedHashedName { + t.Errorf("expected %q, got %q", tc.expectedHashedName, hashedName) + } + }) + } +} diff --git a/pkg/reconciler/taskrun/resources/apply_test.go b/pkg/reconciler/taskrun/resources/apply_test.go index 6d657d8c20f..9601df0b4bf 100644 --- a/pkg/reconciler/taskrun/resources/apply_test.go +++ b/pkg/reconciler/taskrun/resources/apply_test.go @@ -1141,29 +1141,29 @@ func TestApplyWorkspaces(t *testing.T) { EmptyDir: &corev1.EmptyDirVolumeSource{}, }}, want: applyMutation(ts, func(spec *v1.TaskSpec) { - spec.StepTemplate.Env[0].Value = "ws-9l9zj" + spec.StepTemplate.Env[0].Value = "ws-b31db" spec.StepTemplate.Env[1].Value = "foo" spec.StepTemplate.Env[2].Value = "" - spec.Steps[0].Name = "ws-9l9zj" - spec.Steps[0].Image = "ws-mz4c7" - spec.Steps[0].WorkingDir = "ws-mz4c7" + spec.Steps[0].Name = "ws-b31db" + spec.Steps[0].Image = "ws-a6f34" + spec.Steps[0].WorkingDir = "ws-a6f34" spec.Steps[0].Args = []string{"/workspace/myws"} - spec.Steps[1].VolumeMounts[0].Name = "ws-9l9zj" + spec.Steps[1].VolumeMounts[0].Name = "ws-b31db" spec.Steps[1].VolumeMounts[0].MountPath = "path/to//foo" - spec.Steps[1].VolumeMounts[0].SubPath = "ws-9l9zj" + spec.Steps[1].VolumeMounts[0].SubPath = "ws-b31db" - spec.Steps[2].Env[0].Value = "ws-9l9zj" - spec.Steps[2].Env[1].ValueFrom.SecretKeyRef.LocalObjectReference.Name = "ws-9l9zj" - spec.Steps[2].Env[1].ValueFrom.SecretKeyRef.Key = "ws-9l9zj" - spec.Steps[2].EnvFrom[0].Prefix = "ws-9l9zj" - spec.Steps[2].EnvFrom[0].ConfigMapRef.LocalObjectReference.Name = "ws-9l9zj" + spec.Steps[2].Env[0].Value = "ws-b31db" + spec.Steps[2].Env[1].ValueFrom.SecretKeyRef.LocalObjectReference.Name = "ws-b31db" + spec.Steps[2].Env[1].ValueFrom.SecretKeyRef.Key = "ws-b31db" + spec.Steps[2].EnvFrom[0].Prefix = "ws-b31db" + spec.Steps[2].EnvFrom[0].ConfigMapRef.LocalObjectReference.Name = "ws-b31db" - spec.Volumes[0].Name = "ws-9l9zj" - spec.Volumes[0].VolumeSource.ConfigMap.LocalObjectReference.Name = "ws-9l9zj" - spec.Volumes[1].VolumeSource.Secret.SecretName = "ws-9l9zj" - spec.Volumes[2].VolumeSource.PersistentVolumeClaim.ClaimName = "ws-9l9zj" + spec.Volumes[0].Name = "ws-b31db" + spec.Volumes[0].VolumeSource.ConfigMap.LocalObjectReference.Name = "ws-b31db" + spec.Volumes[1].VolumeSource.Secret.SecretName = "ws-b31db" + spec.Volumes[2].VolumeSource.PersistentVolumeClaim.ClaimName = "ws-b31db" }), }, { name: "optional-workspace-provided-variable-replacement", diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go index 31f2ba22d1e..ce23ebfa491 100644 --- a/pkg/reconciler/taskrun/taskrun_test.go +++ b/pkg/reconciler/taskrun/taskrun_test.go @@ -1046,7 +1046,7 @@ spec: }, wantPod: addVolumeMounts(expectedPod("test-taskrun-with-output-config-ws-pod", "", "test-taskrun-with-output-config-ws", "foo", config.DefaultServiceAccountValue, false, []corev1.Volume{{ - Name: "ws-9l9zj", + Name: "ws-d872e", VolumeSource: corev1.VolumeSource{ EmptyDir: &corev1.EmptyDirVolumeSource{}, }, @@ -1058,7 +1058,7 @@ spec: cmd: "/mycmd", }}), []corev1.VolumeMount{{ - Name: "ws-9l9zj", + Name: "ws-d872e", MountPath: "/workspace/data", }}), }} { @@ -4034,8 +4034,8 @@ spec: t.Fatalf("create pod threw error %v", err) } - if vm := pod.Spec.Containers[0].VolumeMounts[0]; !strings.HasPrefix(vm.Name, "ws-9l9zj") || vm.MountPath != expectedMountPath { - t.Fatalf("failed to find expanded Workspace mountpath %v", expectedMountPath) + if vm := pod.Spec.Containers[0].VolumeMounts[0]; !strings.HasPrefix(vm.Name, "ws-f888c") || vm.MountPath != expectedMountPath { + t.Fatalf("failed to find expanded Workspace mountpath %v for %v", expectedMountPath, vm.Name) } if a := pod.Spec.Containers[0].Args; a[len(a)-1] != expectedReplacedArgs { diff --git a/pkg/workspace/apply.go b/pkg/workspace/apply.go index 993e0f1c9da..e4b1581d2af 100644 --- a/pkg/workspace/apply.go +++ b/pkg/workspace/apply.go @@ -21,14 +21,15 @@ import ( "fmt" v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1" - "github.com/tektoncd/pipeline/pkg/names" + pkgnames "github.com/tektoncd/pipeline/pkg/names" "github.com/tektoncd/pipeline/pkg/substitution" corev1 "k8s.io/api/core/v1" "k8s.io/apimachinery/pkg/util/sets" ) const ( - volumeNameBase = "ws" + volumeNameBase = "ws" + defaultRandomLength = 5 ) // nameVolumeMap is a map from a workspace's name to its Volume. @@ -42,15 +43,31 @@ func (nvm nameVolumeMap) setVolumeSource(workspaceName string, volumeName string } } +// generateVolumeName generates a unique name for a volume based on the workspace name. +func generateVolumeName(name string) string { + return pkgnames.GenerateHashedName(volumeNameBase, name, defaultRandomLength) +} + // CreateVolumes will return a dictionary where the keys are the names of the workspaces bound in // wb and the value is a newly-created Volume to use. If the same Volume is bound twice, the // resulting volumes will both have the same name to prevent the same Volume from being attached -// to a pod twice. The names of the returned volumes will be a short random string starting "ws-". +// to a pod twice. The names of the returned volumes will be a short hash string starting "ws-". func CreateVolumes(wb []v1.WorkspaceBinding) map[string]corev1.Volume { pvcs := map[string]corev1.Volume{} - v := make(nameVolumeMap) + v := make(nameVolumeMap, len(wb)) + // Track the names we've used so far to avoid collisions + usedNames := make(map[string]struct{}, len(wb)) + for _, w := range wb { - name := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(volumeNameBase) + name := generateVolumeName(w.Name) + + // If we've already generated this name, try appending extra characters until we find a unique name + for _, exists := usedNames[name]; exists; _, exists = usedNames[name] { + name = generateVolumeName(name + "$") + } + // Track the name we've used + usedNames[name] = struct{}{} + switch { case w.PersistentVolumeClaim != nil: // If it's a PVC, we need to check if we've encountered it before so we avoid mounting it twice diff --git a/pkg/workspace/apply_test.go b/pkg/workspace/apply_test.go index 8e64881ecca..01d5811f11a 100644 --- a/pkg/workspace/apply_test.go +++ b/pkg/workspace/apply_test.go @@ -47,7 +47,7 @@ func TestCreateVolumes(t *testing.T) { }}, expectedVolumes: map[string]corev1.Volume{ "custom": { - Name: "ws-9l9zj", + Name: "ws-20573", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: "mypvc", @@ -66,7 +66,7 @@ func TestCreateVolumes(t *testing.T) { }}, expectedVolumes: map[string]corev1.Volume{ "custom": { - Name: "ws-mz4c7", + Name: "ws-20573", VolumeSource: corev1.VolumeSource{ EmptyDir: &corev1.EmptyDirVolumeSource{ Medium: corev1.StorageMediumMemory, @@ -91,7 +91,7 @@ func TestCreateVolumes(t *testing.T) { }}, expectedVolumes: map[string]corev1.Volume{ "custom": { - Name: "ws-mssqb", + Name: "ws-20573", VolumeSource: corev1.VolumeSource{ ConfigMap: &corev1.ConfigMapVolumeSource{ LocalObjectReference: corev1.LocalObjectReference{ @@ -120,7 +120,7 @@ func TestCreateVolumes(t *testing.T) { }}, expectedVolumes: map[string]corev1.Volume{ "custom": { - Name: "ws-78c5n", + Name: "ws-20573", VolumeSource: corev1.VolumeSource{ Secret: &corev1.SecretVolumeSource{ SecretName: "foobarsecret", @@ -163,7 +163,7 @@ func TestCreateVolumes(t *testing.T) { }}, expectedVolumes: map[string]corev1.Volume{ "custom": { - Name: "ws-6nl7g", + Name: "ws-20573", VolumeSource: corev1.VolumeSource{ Projected: &corev1.ProjectedVolumeSource{ Sources: []corev1.VolumeProjection{{ @@ -211,7 +211,7 @@ func TestCreateVolumes(t *testing.T) { }}, expectedVolumes: map[string]corev1.Volume{ "custom": { - Name: "ws-j2tds", + Name: "ws-20573", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: "mypvc", @@ -219,7 +219,7 @@ func TestCreateVolumes(t *testing.T) { }, }, "even-more-custom": { - Name: "ws-vr6ds", + Name: "ws-338c2", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: "myotherpvc", @@ -244,7 +244,7 @@ func TestCreateVolumes(t *testing.T) { }}, expectedVolumes: map[string]corev1.Volume{ "custom": { - Name: "ws-l22wn", + Name: "ws-20573", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: "mypvc", @@ -253,7 +253,7 @@ func TestCreateVolumes(t *testing.T) { }, "custom2": { // Since it is the same PVC source, it can't be added twice with two different names - Name: "ws-l22wn", + Name: "ws-20573", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: "mypvc", @@ -261,6 +261,47 @@ func TestCreateVolumes(t *testing.T) { }, }, }, + }, { + name: "consistent hashed name for different inputs", + workspaces: []v1.WorkspaceBinding{{ + Name: "test-01097628", + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, { + Name: "test-01617609", + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, { + Name: "test-01675975", + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, { + Name: "test-01809743", + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }}, + expectedVolumes: map[string]corev1.Volume{ + "test-01097628": { + Name: "ws-f32ff", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + "test-01617609": { + Name: "ws-a0814", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + "test-01675975": { + Name: "ws-b2d09", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + "test-01809743": { + Name: "ws-60320", + VolumeSource: corev1.VolumeSource{ + EmptyDir: &corev1.EmptyDirVolumeSource{}, + }, + }, + }, }} { t.Run(tc.name, func(t *testing.T) { v := workspace.CreateVolumes(tc.workspaces) @@ -295,13 +336,13 @@ func TestApply(t *testing.T) { expectedTaskSpec: v1.TaskSpec{ StepTemplate: &v1.StepTemplate{ VolumeMounts: []corev1.VolumeMount{{ - Name: "ws-9l9zj", + Name: "ws-20573", MountPath: "/workspace/custom", SubPath: "/foo/bar/baz", }}, }, Volumes: []corev1.Volume{{ - Name: "ws-9l9zj", + Name: "ws-20573", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: "mypvc", @@ -329,13 +370,13 @@ func TestApply(t *testing.T) { expectedTaskSpec: v1.TaskSpec{ StepTemplate: &v1.StepTemplate{ VolumeMounts: []corev1.VolumeMount{{ - Name: "ws-mz4c7", + Name: "ws-20573", MountPath: "/workspace/custom", SubPath: "/foo/bar/baz", // TODO: what happens when you use subPath with emptyDir }}, }, Volumes: []corev1.Volume{{ - Name: "ws-mz4c7", + Name: "ws-20573", VolumeSource: corev1.VolumeSource{ EmptyDir: &corev1.EmptyDirVolumeSource{ Medium: corev1.StorageMediumMemory, @@ -383,13 +424,13 @@ func TestApply(t *testing.T) { expectedTaskSpec: v1.TaskSpec{ StepTemplate: &v1.StepTemplate{ VolumeMounts: []corev1.VolumeMount{{ - Name: "ws-mssqb", + Name: "ws-20573", MountPath: "/workspace/custom", SubPath: "/foo/bar/baz", // TODO: what happens when you use subPath with emptyDir }}, }, Volumes: []corev1.Volume{{ - Name: "ws-mssqb", + Name: "ws-20573", VolumeSource: corev1.VolumeSource{ Projected: &corev1.ProjectedVolumeSource{ Sources: []corev1.VolumeProjection{{ @@ -452,7 +493,7 @@ func TestApply(t *testing.T) { Name: "awesome-volume", MountPath: "/", }, { - Name: "ws-78c5n", + Name: "ws-20573", MountPath: "/workspace/custom", SubPath: "/foo/bar/baz", // TODO: what happens when you use subPath with emptyDir }}, @@ -463,7 +504,7 @@ func TestApply(t *testing.T) { EmptyDir: &corev1.EmptyDirVolumeSource{}, }, }, { - Name: "ws-78c5n", + Name: "ws-20573", VolumeSource: corev1.VolumeSource{ EmptyDir: &corev1.EmptyDirVolumeSource{ Medium: corev1.StorageMediumMemory, @@ -511,24 +552,24 @@ func TestApply(t *testing.T) { expectedTaskSpec: v1.TaskSpec{ StepTemplate: &v1.StepTemplate{ VolumeMounts: []corev1.VolumeMount{{ - Name: "ws-6nl7g", + Name: "ws-20573", MountPath: "/workspace/custom", SubPath: "/foo/bar/baz", }, { - Name: "ws-j2tds", + Name: "ws-338c2", MountPath: "/workspace/even-more-custom", SubPath: "", }}, }, Volumes: []corev1.Volume{{ - Name: "ws-6nl7g", + Name: "ws-20573", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: "mypvc", }, }, }, { - Name: "ws-j2tds", + Name: "ws-338c2", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: "myotherpvc", @@ -565,17 +606,17 @@ func TestApply(t *testing.T) { expectedTaskSpec: v1.TaskSpec{ StepTemplate: &v1.StepTemplate{ VolumeMounts: []corev1.VolumeMount{{ - Name: "ws-vr6ds", + Name: "ws-20573", MountPath: "/workspace/custom", SubPath: "/foo/bar/baz", }, { - Name: "ws-vr6ds", + Name: "ws-20573", MountPath: "/workspace/custom2", SubPath: "/very/professional/work/space", }}, }, Volumes: []corev1.Volume{{ - Name: "ws-vr6ds", + Name: "ws-20573", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: "mypvc", @@ -605,12 +646,12 @@ func TestApply(t *testing.T) { expectedTaskSpec: v1.TaskSpec{ StepTemplate: &v1.StepTemplate{ VolumeMounts: []corev1.VolumeMount{{ - Name: "ws-twkr2", + Name: "ws-20573", MountPath: "/my/fancy/mount/path", }}, }, Volumes: []corev1.Volume{{ - Name: "ws-twkr2", + Name: "ws-20573", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: "mypvc", @@ -640,13 +681,13 @@ func TestApply(t *testing.T) { expectedTaskSpec: v1.TaskSpec{ StepTemplate: &v1.StepTemplate{ VolumeMounts: []corev1.VolumeMount{{ - Name: "ws-mnq6l", + Name: "ws-20573", MountPath: "/my/fancy/mount/path", ReadOnly: true, }}, }, Volumes: []corev1.Volume{{ - Name: "ws-mnq6l", + Name: "ws-20573", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: "mypvc", @@ -678,14 +719,14 @@ func TestApply(t *testing.T) { expectedTaskSpec: v1.TaskSpec{ StepTemplate: &v1.StepTemplate{ VolumeMounts: []corev1.VolumeMount{{ - Name: "ws-hvpvf", + Name: "ws-20573", MountPath: "/workspace/csi", SubPath: "/foo/bar/baz", ReadOnly: true, }}, }, Volumes: []corev1.Volume{{ - Name: "ws-hvpvf", + Name: "ws-20573", VolumeSource: corev1.VolumeSource{ CSI: &corev1.CSIVolumeSource{ Driver: "secrets-store.csi.k8s.io", @@ -735,7 +776,7 @@ func TestApply_PropagatedWorkspacesFromWorkspaceBindingToDeclarations(t *testing }}, expectedTaskSpec: v1.TaskSpec{ Volumes: []corev1.Volume{{ - Name: "ws-9l9zj", + Name: "ws-c88ff", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: "mypvc", @@ -752,7 +793,7 @@ func TestApply_PropagatedWorkspacesFromWorkspaceBindingToDeclarations(t *testing ReadOnly: false, }}, StepTemplate: &v1.StepTemplate{ - VolumeMounts: []corev1.VolumeMount{{Name: "ws-9l9zj", MountPath: "/workspace/workspace2"}}, + VolumeMounts: []corev1.VolumeMount{{Name: "ws-c88ff", MountPath: "/workspace/workspace2"}}, }, }, }} { @@ -798,7 +839,7 @@ func TestApply_IsolatedWorkspaces(t *testing.T) { expectedTaskSpec: v1.TaskSpec{ StepTemplate: &v1.StepTemplate{}, Volumes: []corev1.Volume{{ - Name: "ws-9l9zj", + Name: "ws-1bcf2", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: "testpvc", @@ -807,7 +848,7 @@ func TestApply_IsolatedWorkspaces(t *testing.T) { }}, Steps: []v1.Step{{ VolumeMounts: []corev1.VolumeMount{{ - Name: "ws-9l9zj", + Name: "ws-1bcf2", MountPath: "/workspace/source", }}, Workspaces: []v1.WorkspaceUsage{{ @@ -843,7 +884,7 @@ func TestApply_IsolatedWorkspaces(t *testing.T) { expectedTaskSpec: v1.TaskSpec{ StepTemplate: &v1.StepTemplate{}, Volumes: []corev1.Volume{{ - Name: "ws-mz4c7", + Name: "ws-1bcf2", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: "testpvc", @@ -855,7 +896,7 @@ func TestApply_IsolatedWorkspaces(t *testing.T) { }}, Sidecars: []v1.Sidecar{{ VolumeMounts: []corev1.VolumeMount{{ - Name: "ws-mz4c7", + Name: "ws-1bcf2", MountPath: "/workspace/source", }}, Workspaces: []v1.WorkspaceUsage{{ @@ -894,7 +935,7 @@ func TestApply_IsolatedWorkspaces(t *testing.T) { expectedTaskSpec: v1.TaskSpec{ StepTemplate: &v1.StepTemplate{}, Volumes: []corev1.Volume{{ - Name: "ws-mssqb", + Name: "ws-1bcf2", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: "testpvc", @@ -903,7 +944,7 @@ func TestApply_IsolatedWorkspaces(t *testing.T) { }}, Steps: []v1.Step{{ VolumeMounts: []corev1.VolumeMount{{ - Name: "ws-mssqb", + Name: "ws-1bcf2", MountPath: "/workspace/source", }}, Workspaces: []v1.WorkspaceUsage{{ @@ -914,7 +955,7 @@ func TestApply_IsolatedWorkspaces(t *testing.T) { }}, Sidecars: []v1.Sidecar{{ VolumeMounts: []corev1.VolumeMount{{ - Name: "ws-mssqb", + Name: "ws-1bcf2", MountPath: "/workspace/source", }}, Workspaces: []v1.WorkspaceUsage{{ @@ -943,11 +984,11 @@ func TestApply_IsolatedWorkspaces(t *testing.T) { expectedTaskSpec: v1.TaskSpec{ StepTemplate: &v1.StepTemplate{ VolumeMounts: []corev1.VolumeMount{{ - Name: "ws-78c5n", MountPath: "/workspace/source", + Name: "ws-1bcf2", MountPath: "/workspace/source", }}, }, Volumes: []corev1.Volume{{ - Name: "ws-78c5n", + Name: "ws-1bcf2", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: "testpvc", @@ -957,7 +998,7 @@ func TestApply_IsolatedWorkspaces(t *testing.T) { Steps: []v1.Step{{}}, Sidecars: []v1.Sidecar{{ VolumeMounts: []corev1.VolumeMount{{ - Name: "ws-78c5n", + Name: "ws-1bcf2", MountPath: "/workspace/source", }}, }}, @@ -993,7 +1034,7 @@ func TestApply_IsolatedWorkspaces(t *testing.T) { expectedTaskSpec: v1.TaskSpec{ StepTemplate: &v1.StepTemplate{}, Volumes: []corev1.Volume{{ - Name: "ws-6nl7g", + Name: "ws-1bcf2", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: "testpvc", @@ -1006,7 +1047,7 @@ func TestApply_IsolatedWorkspaces(t *testing.T) { MountPath: "/foo", }}, VolumeMounts: []corev1.VolumeMount{{ - Name: "ws-6nl7g", + Name: "ws-1bcf2", MountPath: "/foo", }}, }}, @@ -1016,7 +1057,7 @@ func TestApply_IsolatedWorkspaces(t *testing.T) { MountPath: "/bar", }}, VolumeMounts: []corev1.VolumeMount{{ - Name: "ws-6nl7g", + Name: "ws-1bcf2", MountPath: "/bar", }}, }}, @@ -1049,7 +1090,7 @@ func TestApply_IsolatedWorkspaces(t *testing.T) { expectedTaskSpec: v1.TaskSpec{ StepTemplate: &v1.StepTemplate{ VolumeMounts: []corev1.VolumeMount{{ - Name: "ws-j2tds", + Name: "ws-20573", MountPath: "/my/fancy/mount/path", ReadOnly: true, }}, @@ -1062,7 +1103,7 @@ func TestApply_IsolatedWorkspaces(t *testing.T) { }}, }}, Volumes: []corev1.Volume{{ - Name: "ws-j2tds", + Name: "ws-20573", VolumeSource: corev1.VolumeSource{ PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{ ClaimName: "mypvc", diff --git a/test/taskrun_test.go b/test/taskrun_test.go index fe38c6ae33c..ee277fc4daa 100644 --- a/test/taskrun_test.go +++ b/test/taskrun_test.go @@ -36,6 +36,7 @@ import ( corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/types" + "knative.dev/pkg/apis" knativetest "knative.dev/pkg/test" "knative.dev/pkg/test/helpers" ) @@ -490,3 +491,93 @@ func cancelTaskRun(t *testing.T, ctx context.Context, taskRunName string, c *cli return nil } + +func TestTaskRunRetryFailure(t *testing.T) { + ctx := context.Background() + ctx, cancel := context.WithCancel(ctx) + defer cancel() + + c, namespace := setup(ctx, t) + t.Parallel() + + knativetest.CleanupOnInterrupt(func() { tearDown(ctx, t, c, namespace) }, t.Logf) + defer tearDown(ctx, t, c, namespace) + + taskRunName := helpers.ObjectNameForTest(t) + + t.Logf("Creating Task and TaskRun in namespace %s", namespace) + task := parse.MustParseV1Task(t, fmt.Sprintf(` +metadata: + name: %s + namespace: %s +spec: + steps: + - image: busybox + command: ['/bin/sh'] + args: ['-c', 'exit 1'] + volumeMounts: + - mountPath: /cache + name: $(workspaces.cache.volume) + workspaces: + - description: cache + name: cache +`, helpers.ObjectNameForTest(t), namespace)) + if _, err := c.V1TaskClient.Create(ctx, task, metav1.CreateOptions{}); err != nil { + t.Fatalf("Failed to create Task: %s", err) + } + taskRun := parse.MustParseV1TaskRun(t, fmt.Sprintf(` +metadata: + name: %s + namespace: %s +spec: + taskRef: + name: %s + retries: 1 + workspaces: + - name: cache + emptyDir: {} +`, taskRunName, namespace, task.Name)) + if _, err := c.V1TaskRunClient.Create(ctx, taskRun, metav1.CreateOptions{}); err != nil { + t.Fatalf("Failed to create TaskRun: %s", err) + } + + t.Logf("Waiting for TaskRun in namespace %s to fail", namespace) + if err := WaitForTaskRunState(ctx, c, taskRunName, TaskRunFailed(taskRunName), "TaskRunFailed", v1Version); err != nil { + t.Errorf("Error waiting for TaskRun to finish: %s", err) + } + + taskrun, err := c.V1TaskRunClient.Get(ctx, taskRunName, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Couldn't get expected TaskRun %s: %s", taskRunName, err) + } + + if !isFailed(t, taskrun.GetName(), taskrun.Status.Conditions) { + t.Fatalf("task should have been a failure") + } + + expectedReason := "Failed" + actualReason := taskrun.Status.GetCondition(apis.ConditionSucceeded).GetReason() + if actualReason != expectedReason { + t.Fatalf("expected TaskRun to have failed reason %s, got %s", expectedReason, actualReason) + } + + expectedStepState := []v1.StepState{{ + ContainerState: corev1.ContainerState{ + Terminated: &corev1.ContainerStateTerminated{ + ExitCode: 1, + Reason: "Error", + }, + }, + TerminationReason: "Error", + Name: "unnamed-0", + Container: "step-unnamed-0", + }} + ignoreTerminatedFields := cmpopts.IgnoreFields(corev1.ContainerStateTerminated{}, "StartedAt", "FinishedAt", "ContainerID") + ignoreStepFields := cmpopts.IgnoreFields(v1.StepState{}, "ImageID") + if d := cmp.Diff(taskrun.Status.Steps, expectedStepState, ignoreTerminatedFields, ignoreStepFields); d != "" { + t.Fatalf("-got, +want: %v", d) + } + if len(taskrun.Status.RetriesStatus) != 1 { + t.Fatalf("expected 1 retry status, got %d", len(taskrun.Status.RetriesStatus)) + } +}