Generate Pod name using names package

For some reason we weren't using the standard name generation package for generating Pod names. It's possible this didn't work before, but it seems to work now, and makes Pod name generation less unique.
tektoncd · Dec 3, 2019 · e7d19e0 · e7d19e0
1 parent 780d9bd
commit e7d19e0
Show file tree

Hide file tree

Showing 3 changed files with 17 additions and 36 deletions.
diff --git a/pkg/pod/pod.go b/pkg/pod/pod.go
@@ -17,11 +17,7 @@ limitations under the License.
 package pod
 
 import (
-	"crypto/rand"
-	"encoding/hex"
 	"fmt"
-	"io"
-	"io/ioutil"
 	"path/filepath"
 
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
@@ -75,10 +71,6 @@ var (
 		Name:         "tekton-home",
 		VolumeSource: corev1.VolumeSource{EmptyDir: &corev1.EmptyDirVolumeSource{}},
 	}}
-
-	// Random byte reader used for pod name generation.
-	// var for testing.
-	randReader = rand.Reader
 )
 
 // MakePod converts TaskRun and TaskSpec objects to a Pod which implements the taskrun specified
@@ -185,13 +177,6 @@ func MakePod(images pipeline.Images, taskRun *v1alpha1.TaskRun, taskSpec v1alpha
 		return nil, err
 	}
 
-	// Generate a short random hex string.
-	b, err := ioutil.ReadAll(io.LimitReader(randReader, 3))
-	if err != nil {
-		return nil, err
-	}
-	gibberish := hex.EncodeToString(b)
-
 	// Merge sidecar containers with step containers.
 	mergedPodContainers := stepContainers
 	for _, sc := range taskSpec.Sidecars {
@@ -208,7 +193,7 @@ func MakePod(images pipeline.Images, taskRun *v1alpha1.TaskRun, taskSpec v1alpha
 			// Add a unique suffix to avoid confusion when a build
 			// is deleted and re-created with the same name.
 			// We don't use RestrictLengthWithRandomSuffix here because k8s fakes don't support it.
-			Name: fmt.Sprintf("%s-pod-%s", taskRun.Name, gibberish),
+			Name: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("%s-pod", taskRun.Name)),
 			// If our parent TaskRun is deleted, then we should be as well.
 			OwnerReferences: []metav1.OwnerReference{
 				*metav1.NewControllerRef(taskRun, groupVersionKind),

diff --git a/pkg/pod/pod_test.go b/pkg/pod/pod_test.go
@@ -17,7 +17,6 @@ limitations under the License.
 package pod
 
 import (
-	"crypto/rand"
 	"fmt"
 	"path/filepath"
 	"strings"
@@ -62,9 +61,6 @@ func TestMakePod(t *testing.T) {
 
 	runtimeClassName := "gvisor"
 
-	randReader = strings.NewReader(strings.Repeat("a", 10000))
-	defer func() { randReader = rand.Reader }()
-
 	for _, c := range []struct {
 		desc            string
 		trs             v1alpha1.TaskRunSpec
@@ -580,10 +576,8 @@ script-heredoc-randomly-generated-6nl7g
 				t.Fatalf("MakePod: %v", err)
 			}
 
-			// Generated name from hexlifying a stream of 'a's.
-			wantName := "taskrun-name-pod-616161"
-			if got.Name != wantName {
-				t.Errorf("Pod name got %q, want %q", got.Name, wantName)
+			if !strings.HasPrefix(got.Name, "taskrun-name-pod-") {
+				t.Errorf("Pod name %q should have prefix 'taskrun-name-pod-'", got.Name)
 			}
 
 			if d := cmp.Diff(c.want, &got.Spec, resourceQuantityCmp); d != "" {

diff --git a/pkg/reconciler/taskrun/taskrun_test.go b/pkg/reconciler/taskrun/taskrun_test.go
@@ -72,11 +72,12 @@ var (
 		ImageDigestExporterImage: "override-with-imagedigest-exporter-image:latest",
 	}
 	ignoreLastTransitionTime = cmpopts.IgnoreTypes(apis.Condition{}.LastTransitionTime.Inner.Time)
-	// Pods are created with a random 3-byte (6 hex character) suffix that we want to ignore in our diffs.
+	// Pods are created with a random 5-character suffix that we want to
+	// ignore in our diffs.
 	ignoreRandomPodNameSuffix = cmp.FilterPath(func(path cmp.Path) bool {
 		return path.GoString() == "{v1.ObjectMeta}.Name"
 	}, cmp.Comparer(func(name1, name2 string) bool {
-		return name1[:len(name1)-6] == name2[:len(name2)-6]
+		return name1[:len(name1)-5] == name2[:len(name2)-5]
 	}))
 	resourceQuantityCmp = cmp.Comparer(func(x, y resource.Quantity) bool {
 		return x.Cmp(y) == 0
@@ -285,7 +286,7 @@ func TestReconcile_ExplicitDefaultSA(t *testing.T) {
 	}{{
 		name:    "success",
 		taskRun: taskRunSuccess,
-		wantPod: tb.Pod("test-taskrun-run-success-pod-123456", "foo",
+		wantPod: tb.Pod("test-taskrun-run-success-pod-abcde", "foo",
 			tb.PodLabel(taskNameLabelKey, "test-task"),
 			tb.PodLabel(taskRunNameLabelKey, "test-taskrun-run-success"),
 			tb.PodLabel(podconvert.ManagedByLabelKey, podconvert.ManagedByLabelValue),
@@ -325,7 +326,7 @@ func TestReconcile_ExplicitDefaultSA(t *testing.T) {
 	}, {
 		name:    "serviceaccount",
 		taskRun: taskRunWithSaSuccess,
-		wantPod: tb.Pod("test-taskrun-with-sa-run-success-pod-123456", "foo",
+		wantPod: tb.Pod("test-taskrun-with-sa-run-success-pod-abcde", "foo",
 			tb.PodLabel(taskNameLabelKey, "test-with-sa"),
 			tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-sa-run-success"),
 			tb.PodLabel(podconvert.ManagedByLabelKey, podconvert.ManagedByLabelValue),
@@ -526,7 +527,7 @@ func TestReconcile(t *testing.T) {
 
 	taskRunWithPod := tb.TaskRun("test-taskrun-with-pod", "foo",
 		tb.TaskRunSpec(tb.TaskRunTaskRef(simpleTask.Name)),
-		tb.TaskRunStatus(tb.PodName("some-pod-that-no-longer-exists")),
+		tb.TaskRunStatus(tb.PodName("some-pod-abcdethat-no-longer-exists")),
 	)
 
 	taskruns := []*v1alpha1.TaskRun{
@@ -549,7 +550,7 @@ func TestReconcile(t *testing.T) {
 	}{{
 		name:    "success",
 		taskRun: taskRunSuccess,
-		wantPod: tb.Pod("test-taskrun-run-success-pod-123456", "foo",
+		wantPod: tb.Pod("test-taskrun-run-success-pod-abcde", "foo",
 			tb.PodLabel(taskNameLabelKey, "test-task"),
 			tb.PodLabel(taskRunNameLabelKey, "test-taskrun-run-success"),
 			tb.PodLabel(podconvert.ManagedByLabelKey, podconvert.ManagedByLabelValue),
@@ -588,7 +589,7 @@ func TestReconcile(t *testing.T) {
 	}, {
 		name:    "serviceaccount",
 		taskRun: taskRunWithSaSuccess,
-		wantPod: tb.Pod("test-taskrun-with-sa-run-success-pod-123456", "foo",
+		wantPod: tb.Pod("test-taskrun-with-sa-run-success-pod-abcde", "foo",
 			tb.PodLabel(taskNameLabelKey, "test-with-sa"),
 			tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-sa-run-success"),
 			tb.PodLabel(podconvert.ManagedByLabelKey, podconvert.ManagedByLabelValue),
@@ -628,7 +629,7 @@ func TestReconcile(t *testing.T) {
 	}, {
 		name:    "params",
 		taskRun: taskRunSubstitution,
-		wantPod: tb.Pod("test-taskrun-substitution-pod-123456", "foo",
+		wantPod: tb.Pod("test-taskrun-substitution-pod-abcde", "foo",
 			tb.PodLabel(taskNameLabelKey, "test-task-with-substitution"),
 			tb.PodLabel(taskRunNameLabelKey, "test-taskrun-substitution"),
 			tb.PodLabel(podconvert.ManagedByLabelKey, podconvert.ManagedByLabelValue),
@@ -723,7 +724,7 @@ func TestReconcile(t *testing.T) {
 	}, {
 		name:    "taskrun-with-taskspec",
 		taskRun: taskRunWithTaskSpec,
-		wantPod: tb.Pod("test-taskrun-with-taskspec-pod-123456", "foo",
+		wantPod: tb.Pod("test-taskrun-with-taskspec-pod-abcde", "foo",
 			tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-taskspec"),
 			tb.PodLabel(podconvert.ManagedByLabelKey, podconvert.ManagedByLabelValue),
 			tb.PodOwnerReference("TaskRun", "test-taskrun-with-taskspec",
@@ -784,7 +785,7 @@ func TestReconcile(t *testing.T) {
 	}, {
 		name:    "success-with-cluster-task",
 		taskRun: taskRunWithClusterTask,
-		wantPod: tb.Pod("test-taskrun-with-cluster-task-pod-123456", "foo",
+		wantPod: tb.Pod("test-taskrun-with-cluster-task-pod-abcde", "foo",
 			tb.PodLabel(taskNameLabelKey, "test-cluster-task"),
 			tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-cluster-task"),
 			tb.PodLabel(podconvert.ManagedByLabelKey, podconvert.ManagedByLabelValue),
@@ -823,7 +824,7 @@ func TestReconcile(t *testing.T) {
 	}, {
 		name:    "taskrun-with-resource-spec-task-spec",
 		taskRun: taskRunWithResourceSpecAndTaskSpec,
-		wantPod: tb.Pod("test-taskrun-with-resource-spec-pod-123456", "foo",
+		wantPod: tb.Pod("test-taskrun-with-resource-spec-pod-abcde", "foo",
 			tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-resource-spec"),
 			tb.PodLabel(podconvert.ManagedByLabelKey, podconvert.ManagedByLabelValue),
 			tb.PodOwnerReference("TaskRun", "test-taskrun-with-resource-spec",
@@ -882,7 +883,7 @@ func TestReconcile(t *testing.T) {
 	}, {
 		name:    "taskrun-with-pod",
 		taskRun: taskRunWithPod,
-		wantPod: tb.Pod("test-taskrun-with-pod-pod-123456", "foo",
+		wantPod: tb.Pod("test-taskrun-with-pod-pod-abcde", "foo",
 			tb.PodLabel(taskNameLabelKey, "test-task"),
 			tb.PodLabel(taskRunNameLabelKey, "test-taskrun-with-pod"),
 			tb.PodLabel(podconvert.ManagedByLabelKey, podconvert.ManagedByLabelValue),
@@ -974,6 +975,7 @@ func TestReconcile(t *testing.T) {
 				t.Errorf("Pod metadata doesn't match (-want, +got): %s", d)
 			}
 
+			pod.Name = tc.wantPod.Name // Ignore pod name differences, the pod name is generated and tested in pod_test.go
 			if d := cmp.Diff(tc.wantPod.Spec, pod.Spec, resourceQuantityCmp); d != "" {
 				t.Errorf("Pod spec doesn't match (-want, +got): %s", d)
 			}