Skip to content

Commit 510e26a

Browse files
committed
fix: resolve pod creation failure on retry when using workspace.<name>.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.
1 parent 4fe52d4 commit 510e26a

File tree

5 files changed

+299
-71
lines changed

5 files changed

+299
-71
lines changed

pkg/reconciler/taskrun/resources/apply_test.go

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1141,29 +1141,29 @@ func TestApplyWorkspaces(t *testing.T) {
11411141
EmptyDir: &corev1.EmptyDirVolumeSource{},
11421142
}},
11431143
want: applyMutation(ts, func(spec *v1.TaskSpec) {
1144-
spec.StepTemplate.Env[0].Value = "ws-9l9zj"
1144+
spec.StepTemplate.Env[0].Value = "ws-b31db"
11451145
spec.StepTemplate.Env[1].Value = "foo"
11461146
spec.StepTemplate.Env[2].Value = ""
11471147

1148-
spec.Steps[0].Name = "ws-9l9zj"
1149-
spec.Steps[0].Image = "ws-mz4c7"
1150-
spec.Steps[0].WorkingDir = "ws-mz4c7"
1148+
spec.Steps[0].Name = "ws-b31db"
1149+
spec.Steps[0].Image = "ws-a6f34"
1150+
spec.Steps[0].WorkingDir = "ws-a6f34"
11511151
spec.Steps[0].Args = []string{"/workspace/myws"}
11521152

1153-
spec.Steps[1].VolumeMounts[0].Name = "ws-9l9zj"
1153+
spec.Steps[1].VolumeMounts[0].Name = "ws-b31db"
11541154
spec.Steps[1].VolumeMounts[0].MountPath = "path/to//foo"
1155-
spec.Steps[1].VolumeMounts[0].SubPath = "ws-9l9zj"
1155+
spec.Steps[1].VolumeMounts[0].SubPath = "ws-b31db"
11561156

1157-
spec.Steps[2].Env[0].Value = "ws-9l9zj"
1158-
spec.Steps[2].Env[1].ValueFrom.SecretKeyRef.LocalObjectReference.Name = "ws-9l9zj"
1159-
spec.Steps[2].Env[1].ValueFrom.SecretKeyRef.Key = "ws-9l9zj"
1160-
spec.Steps[2].EnvFrom[0].Prefix = "ws-9l9zj"
1161-
spec.Steps[2].EnvFrom[0].ConfigMapRef.LocalObjectReference.Name = "ws-9l9zj"
1157+
spec.Steps[2].Env[0].Value = "ws-b31db"
1158+
spec.Steps[2].Env[1].ValueFrom.SecretKeyRef.LocalObjectReference.Name = "ws-b31db"
1159+
spec.Steps[2].Env[1].ValueFrom.SecretKeyRef.Key = "ws-b31db"
1160+
spec.Steps[2].EnvFrom[0].Prefix = "ws-b31db"
1161+
spec.Steps[2].EnvFrom[0].ConfigMapRef.LocalObjectReference.Name = "ws-b31db"
11621162

1163-
spec.Volumes[0].Name = "ws-9l9zj"
1164-
spec.Volumes[0].VolumeSource.ConfigMap.LocalObjectReference.Name = "ws-9l9zj"
1165-
spec.Volumes[1].VolumeSource.Secret.SecretName = "ws-9l9zj"
1166-
spec.Volumes[2].VolumeSource.PersistentVolumeClaim.ClaimName = "ws-9l9zj"
1163+
spec.Volumes[0].Name = "ws-b31db"
1164+
spec.Volumes[0].VolumeSource.ConfigMap.LocalObjectReference.Name = "ws-b31db"
1165+
spec.Volumes[1].VolumeSource.Secret.SecretName = "ws-b31db"
1166+
spec.Volumes[2].VolumeSource.PersistentVolumeClaim.ClaimName = "ws-b31db"
11671167
}),
11681168
}, {
11691169
name: "optional-workspace-provided-variable-replacement",

pkg/reconciler/taskrun/taskrun_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1046,7 +1046,7 @@ spec:
10461046
},
10471047
wantPod: addVolumeMounts(expectedPod("test-taskrun-with-output-config-ws-pod", "", "test-taskrun-with-output-config-ws", "foo", config.DefaultServiceAccountValue, false,
10481048
[]corev1.Volume{{
1049-
Name: "ws-9l9zj",
1049+
Name: "ws-d872e",
10501050
VolumeSource: corev1.VolumeSource{
10511051
EmptyDir: &corev1.EmptyDirVolumeSource{},
10521052
},
@@ -1058,7 +1058,7 @@ spec:
10581058
cmd: "/mycmd",
10591059
}}),
10601060
[]corev1.VolumeMount{{
1061-
Name: "ws-9l9zj",
1061+
Name: "ws-d872e",
10621062
MountPath: "/workspace/data",
10631063
}}),
10641064
}} {
@@ -4031,8 +4031,8 @@ spec:
40314031
t.Fatalf("create pod threw error %v", err)
40324032
}
40334033

4034-
if vm := pod.Spec.Containers[0].VolumeMounts[0]; !strings.HasPrefix(vm.Name, "ws-9l9zj") || vm.MountPath != expectedMountPath {
4035-
t.Fatalf("failed to find expanded Workspace mountpath %v", expectedMountPath)
4034+
if vm := pod.Spec.Containers[0].VolumeMounts[0]; !strings.HasPrefix(vm.Name, "ws-f888c") || vm.MountPath != expectedMountPath {
4035+
t.Fatalf("failed to find expanded Workspace mountpath %v for %v", expectedMountPath, vm.Name)
40364036
}
40374037

40384038
if a := pod.Spec.Containers[0].Args; a[len(a)-1] != expectedReplacedArgs {

pkg/workspace/apply.go

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,16 +19,19 @@ package workspace
1919
import (
2020
"context"
2121
"fmt"
22+
"hash/fnv"
23+
"strconv"
24+
"strings"
2225

2326
v1 "github.com/tektoncd/pipeline/pkg/apis/pipeline/v1"
24-
"github.com/tektoncd/pipeline/pkg/names"
2527
"github.com/tektoncd/pipeline/pkg/substitution"
2628
corev1 "k8s.io/api/core/v1"
2729
"k8s.io/apimachinery/pkg/util/sets"
2830
)
2931

3032
const (
31-
volumeNameBase = "ws"
33+
volumeNameBase = "ws"
34+
defaultRandomLength = 5
3235
)
3336

3437
// nameVolumeMap is a map from a workspace's name to its Volume.
@@ -42,15 +45,47 @@ func (nvm nameVolumeMap) setVolumeSource(workspaceName string, volumeName string
4245
}
4346
}
4447

48+
// GenerateHashedName generates a unique name for a volume based on the workspace name.
49+
func GenerateHashedName(prefix, name string, randomLength int) string {
50+
if randomLength <= 0 {
51+
randomLength = defaultRandomLength
52+
}
53+
h := fnv.New32a()
54+
h.Write([]byte(name))
55+
suffix := strconv.FormatUint(uint64(h.Sum32()), 16)
56+
if ln := len(suffix); ln > randomLength {
57+
suffix = suffix[:randomLength]
58+
} else if ln < randomLength {
59+
suffix += strings.Repeat("0", randomLength-ln)
60+
}
61+
return fmt.Sprintf("%s-%s", prefix, suffix)
62+
}
63+
64+
// generateVolumeName generates a unique name for a volume based on the workspace name.
65+
func generateVolumeName(name string) string {
66+
return GenerateHashedName(volumeNameBase, name, defaultRandomLength)
67+
}
68+
4569
// CreateVolumes will return a dictionary where the keys are the names of the workspaces bound in
4670
// wb and the value is a newly-created Volume to use. If the same Volume is bound twice, the
4771
// resulting volumes will both have the same name to prevent the same Volume from being attached
48-
// to a pod twice. The names of the returned volumes will be a short random string starting "ws-".
72+
// to a pod twice. The names of the returned volumes will be a short hash string starting "ws-".
4973
func CreateVolumes(wb []v1.WorkspaceBinding) map[string]corev1.Volume {
5074
pvcs := map[string]corev1.Volume{}
51-
v := make(nameVolumeMap)
75+
v := make(nameVolumeMap, len(wb))
76+
// Track the names we've used so far to avoid collisions
77+
usedNames := make(map[string]struct{}, len(wb))
78+
5279
for _, w := range wb {
53-
name := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(volumeNameBase)
80+
name := generateVolumeName(w.Name)
81+
82+
// If we've already generated this name, try appending extra characters until we find a unique name
83+
for _, exists := usedNames[name]; exists; _, exists = usedNames[name] {
84+
name = generateVolumeName(name + "$")
85+
}
86+
// Track the name we've used
87+
usedNames[name] = struct{}{}
88+
5489
switch {
5590
case w.PersistentVolumeClaim != nil:
5691
// If it's a PVC, we need to check if we've encountered it before so we avoid mounting it twice

0 commit comments

Comments
 (0)