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