Skip to content

Commit

Permalink
Randomize names of volumes to avoid collisions 🎲
Browse files Browse the repository at this point in the history 
It's possible folks might be specifying volumes already in the Task or
via the stepTemplate that might collide with the names we are using for
the workspaces; instead of validating this and making the Task author
change these, we can instead randomize them!

In a follow up we'll want to add support for variable interpolation for
the volume name so folks can access it if they want to.
  • Loading branch information
@bobcatfish
bobcatfish committed Dec 6, 2019
1 parent 84df9c2 commit 3b2aa5b
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 24 deletions.
10 changes: 8 additions & 2 deletions pkg/workspace/apply.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,24 +4,30 @@ import (
"fmt"

"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
"github.com/tektoncd/pipeline/pkg/names"
corev1 "k8s.io/api/core/v1"
)

const (
volumeNameBase = "ws"
)

// GetVolumes will return a dictionary where the keys are the names fo the workspaces bound in
// wb and the value is the 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 pod twice.
func GetVolumes(wb []v1alpha1.WorkspaceBinding) map[string]corev1.Volume {
pvcs := map[string]corev1.Volume{}
v := map[string]corev1.Volume{}
for _, w := range wb {
name := names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(volumeNameBase)
if w.PersistentVolumeClaim != nil {
// If it's a PVC, we need to check if we've encountered it before so we avoid mounting it twice
if vv, ok := pvcs[w.PersistentVolumeClaim.ClaimName]; ok {
v[w.Name] = vv
} else {
pvc := *w.PersistentVolumeClaim
v[w.Name] = corev1.Volume{
Name: w.Name,
Name: name,
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &pvc,
},
Expand All @@ -31,7 +37,7 @@ func GetVolumes(wb []v1alpha1.WorkspaceBinding) map[string]corev1.Volume {
} else {
ed := *w.EmptyDir
v[w.Name] = corev1.Volume{
Name: w.Name,
Name: name,
VolumeSource: corev1.VolumeSource{
EmptyDir: &ed,
},
Expand Down
47 changes: 25 additions & 22 deletions pkg/workspace/apply_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@ import (
"github.com/google/go-cmp/cmp"
"github.com/tektoncd/pipeline/pkg/apis/pipeline/v1alpha1"
"github.com/tektoncd/pipeline/pkg/workspace"
"github.com/tektoncd/pipeline/test/names"
corev1 "k8s.io/api/core/v1"
)

func TestGetVolumes(t *testing.T) {
names.TestingSeed()
for _, tc := range []struct {
name string
workspaces []v1alpha1.WorkspaceBinding
Expand All @@ -25,7 +27,7 @@ func TestGetVolumes(t *testing.T) {
}},
expectedVolumes: map[string]corev1.Volume{
"custom": corev1.Volume{
Name: "custom",
Name: "ws-9l9zj",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: "mypvc",
Expand All @@ -44,7 +46,7 @@ func TestGetVolumes(t *testing.T) {
}},
expectedVolumes: map[string]corev1.Volume{
"custom": corev1.Volume{
Name: "custom",
Name: "ws-mz4c7",
VolumeSource: corev1.VolumeSource{
EmptyDir: &corev1.EmptyDirVolumeSource{
Medium: corev1.StorageMediumMemory,
Expand Down Expand Up @@ -72,15 +74,15 @@ func TestGetVolumes(t *testing.T) {
}},
expectedVolumes: map[string]corev1.Volume{
"custom": corev1.Volume{
Name: "custom",
Name: "ws-mssqb",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: "mypvc",
},
},
},
"even-more-custom": corev1.Volume{
Name: "even-more-custom",
Name: "ws-78c5n",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: "myotherpvc",
Expand All @@ -105,16 +107,16 @@ func TestGetVolumes(t *testing.T) {
}},
expectedVolumes: map[string]corev1.Volume{
"custom": corev1.Volume{
Name: "custom",
Name: "ws-6nl7g",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: "mypvc",
},
},
},
"custom2": corev1.Volume{
// Since it is the same volume, it can't be added twice with two different names
Name: "custom",
// Since it is the same PVC source, it can't be added twice with two different names
Name: "ws-6nl7g",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: "mypvc",
Expand All @@ -133,6 +135,7 @@ func TestGetVolumes(t *testing.T) {
}

func TestApply(t *testing.T) {
names.TestingSeed()
for _, tc := range []struct {
name string
ts v1alpha1.TaskSpec
Expand All @@ -155,13 +158,13 @@ func TestApply(t *testing.T) {
expectedTaskSpec: v1alpha1.TaskSpec{
StepTemplate: &corev1.Container{
VolumeMounts: []corev1.VolumeMount{{
Name: "custom",
Name: "ws-9l9zj",
MountPath: "/workspace/custom",
SubPath: "/foo/bar/baz",
}},
},
Volumes: []corev1.Volume{{
Name: "custom", // TODO: randomize names so that there aren't collisions?
Name: "ws-9l9zj",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: "mypvc",
Expand Down Expand Up @@ -189,13 +192,13 @@ func TestApply(t *testing.T) {
expectedTaskSpec: v1alpha1.TaskSpec{
StepTemplate: &corev1.Container{
VolumeMounts: []corev1.VolumeMount{{
Name: "custom",
Name: "ws-mz4c7",
MountPath: "/workspace/custom",
SubPath: "/foo/bar/baz", // TODO: what happens when you use subPath with emptyDir
}},
},
Volumes: []corev1.Volume{{
Name: "custom",
Name: "ws-mz4c7",
VolumeSource: corev1.VolumeSource{
EmptyDir: &corev1.EmptyDirVolumeSource{
Medium: corev1.StorageMediumMemory,
Expand Down Expand Up @@ -238,7 +241,7 @@ func TestApply(t *testing.T) {
Name: "awesome-volume",
MountPath: "/",
}, {
Name: "custom",
Name: "ws-mssqb",
MountPath: "/workspace/custom",
SubPath: "/foo/bar/baz", // TODO: what happens when you use subPath with emptyDir
}},
Expand All @@ -249,7 +252,7 @@ func TestApply(t *testing.T) {
EmptyDir: &corev1.EmptyDirVolumeSource{},
},
}, {
Name: "custom",
Name: "ws-mssqb",
VolumeSource: corev1.VolumeSource{
EmptyDir: &corev1.EmptyDirVolumeSource{
Medium: corev1.StorageMediumMemory,
Expand Down Expand Up @@ -299,24 +302,24 @@ func TestApply(t *testing.T) {
expectedTaskSpec: v1alpha1.TaskSpec{
StepTemplate: &corev1.Container{
VolumeMounts: []corev1.VolumeMount{{
Name: "custom",
Name: "ws-78c5n",
MountPath: "/workspace/custom",
SubPath: "/foo/bar/baz",
}, {
Name: "even-more-custom",
Name: "ws-6nl7g",
MountPath: "/workspace/even-more-custom",
SubPath: "",
}},
},
Volumes: []corev1.Volume{{
Name: "custom",
Name: "ws-78c5n",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: "mypvc",
},
},
}, {
Name: "even-more-custom",
Name: "ws-6nl7g",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: "myotherpvc",
Expand Down Expand Up @@ -353,17 +356,17 @@ func TestApply(t *testing.T) {
expectedTaskSpec: v1alpha1.TaskSpec{
StepTemplate: &corev1.Container{
VolumeMounts: []corev1.VolumeMount{{
Name: "custom",
Name: "ws-j2tds",
MountPath: "/workspace/custom",
SubPath: "/foo/bar/baz",
}, {
Name: "custom",
Name: "ws-j2tds",
MountPath: "/workspace/custom2",
SubPath: "/very/professional/work/space",
}},
},
Volumes: []corev1.Volume{{
Name: "custom",
Name: "ws-j2tds",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: "mypvc",
Expand Down Expand Up @@ -393,12 +396,12 @@ func TestApply(t *testing.T) {
expectedTaskSpec: v1alpha1.TaskSpec{
StepTemplate: &corev1.Container{
VolumeMounts: []corev1.VolumeMount{{
Name: "custom",
Name: "ws-l22wn",
MountPath: "/my/fancy/mount/path",
}},
},
Volumes: []corev1.Volume{{
Name: "custom",
Name: "ws-l22wn",
VolumeSource: corev1.VolumeSource{
PersistentVolumeClaim: &corev1.PersistentVolumeClaimVolumeSource{
ClaimName: "mypvc",
Expand Down

0 comments on commit 3b2aa5b

Please sign in to comment.