Use kmeta.ChildName for child resources

When a reconciler generates a child resource, it risks creating a
duplicate resource because of stale informer cache. This may happen
for instance when two reconciliations of the same resource happen
almost at the same time.
To avoid this risk, children resources should use a name that is uniquely
associated to that of the parent, so that any attempt of recreation would
fail. Such failure, when it happens, is considered as a transient error,
and it does not cause the TaskRun or the PipelineRun to fail.

We use kmeta.ChildName to generate the names. This applies to Pods
generated by TaskRuns and TaskRuns generated by PipelineRuns.
ChildName combines a base name with a suffix, and if the resulting name is
too long it shortens it by truncating it and appending an hash of the
original name: https://pkg.go.dev/github.com/knative/pkg/kmeta#ChildName

Resources that are not k8s resources, such as steps or scripts, are not
affected by this change.

Fixes #4358

Signed-off-by: Andrea Frittoli <[email protected]>

docs/events.md

@@ -143,7 +143,7 @@ of the event. Inside the root key, the whole `spec` and `status` of the resource
           "type": "Succeeded"
         }
       ],
-      "podName": "curl-run-6gplk-pod-rsgn2",
+      "podName": "curl-run-6gplk-pod",
       "startTime": "2021-01-29T14:47:57Z",
       "steps": [
         {

docs/pipelineruns.md

@@ -547,7 +547,7 @@ conditions:
     type: Succeeded
 startTime: "2020-05-04T02:00:11Z"
 taskRuns:
-  triggers-release-nightly-frwmw-build-ng2qk:
+  triggers-release-nightly-frwmw-build:
     pipelineTaskName: build
     status:
       completionTime: "2020-05-04T02:10:49Z"
@@ -557,7 +557,7 @@ taskRuns:
           reason: Succeeded
           status: "True"
           type: Succeeded
-      podName: triggers-release-nightly-frwmw-build-ng2qk-pod-8vj99
+      podName: triggers-release-nightly-frwmw-build-pod
       resourcesResult:
         - key: commit
           resourceRef:
@@ -616,7 +616,7 @@ Skipped Tasks:
     Values:
       foo
 Task Runs:
-  pipelinerun-to-skip-task-run-this-task-r2djj:
+  pipelinerun-to-skip-task-run-this-task:
     Pipeline Task Name:  run-this-task
     Status:
       ...
@@ -626,6 +626,10 @@ Task Runs:
       Values:
         foo
 ```
+The name of the `TaskRuns` owned by a `PipelineRun`  are univocally associated to the owning resource.
+If a `PipelineRun` resource is deleted and created with the same name, the child `TaskRuns` will be created with the
+same name as before. The base format of the name is `<pipelinerun-name>-<pipelinetask-name>`. The name may vary
+according the logic of [`kmeta.ChildName`](https://pkg.go.dev/github.com/knative/pkg/kmeta#ChildName).
 
 ## Cancelling a `PipelineRun`
 

docs/taskruns.md

@@ -428,7 +428,7 @@ conditions:
     reason: Succeeded
     status: "True"
     type: Succeeded
-podName: status-taskrun-pod-6488ef
+podName: status-taskrun-pod
 startTime: "2019-08-12T18:22:51Z"
 steps:
   - container: step-hello
@@ -459,6 +459,12 @@ False|TaskRunTimeout|Yes|The TaskRun timed out.
 
 When a `TaskRun` changes status, [events](events.md#taskruns) are triggered accordingly.
 
+The name of the `Pod` owned by a `TaskRun`  is univocally associated to the owning resource. 
+If a `TaskRun` resource is deleted and created with the same name, the child `Pod` will be created with the same name
+as before. The base format of the name is `<taskrun-name>-pod`. The name may vary according to the logic of
+[`kmeta.ChildName`](https://pkg.go.dev/github.com/knative/pkg/kmeta#ChildName).
+
+
 ### Monitoring `Steps`
 
 If multiple `Steps` are defined in the `Task` invoked by the `TaskRun`, you can monitor their execution
@@ -535,7 +541,7 @@ change done by the user (running the debug-continue or debug-fail-continue scrip
 During this time, the user/client can get remote shell access to the step container with a command such as the following.
 
 ```bash
-kubectl exec -it print-date-d7tj5-pod-w5qrn -c step-print-date-human-readable
+kubectl exec -it print-date-d7tj5-pod -c step-print-date-human-readable
 ```
 
 ### Debug Environment

docs/tutorial.md

@@ -546,9 +546,9 @@ Params
 No params
 
 Taskruns
-NAME                                               TASK NAME            STARTED       DURATION     STATUS
-tutorial-pipeline-run-1-deploy-web-jjf2l           deploy-web           4 hours ago   14 seconds   Succeeded
-tutorial-pipeline-run-1-build-skaffold-web-7jgjh   build-skaffold-web   4 hours ago   1 minute     Succeeded
+NAME                                         TASK NAME            STARTED       DURATION     STATUS
+tutorial-pipeline-run-1-deploy-web           deploy-web           4 hours ago   14 seconds   Succeeded
+tutorial-pipeline-run-1-build-skaffold-web   build-skaffold-web   4 hours ago   1 minute     Succeeded
 ```
 
 The `Succeeded` status indicates that your `PipelineRun` completed without errors.

pkg/pod/pod.go

@@ -22,6 +22,8 @@ import (
 	"path/filepath"
 	"strconv"
 
+	"knative.dev/pkg/kmeta"
+
 	"github.com/tektoncd/pipeline/pkg/apis/config"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline"
 	"github.com/tektoncd/pipeline/pkg/apis/pipeline/pod"
@@ -126,7 +128,7 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 	volumeMounts = append(volumeMounts, credVolumeMounts...)
 
 	// Merge step template with steps.
-	// TODO(#1605): Move MergeSteps to pkg/pod
+	// TODO(#1605): Move MergeSteps to pkg/newPod
 	steps, err := v1beta1.MergeStepsWithStepTemplate(taskSpec.StepTemplate, taskSpec.Steps)
 	if err != nil {
 		return nil, err
@@ -251,7 +253,7 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 		stepContainers[i].Name = names.SimpleNameGenerator.RestrictLength(StepName(s.Name, i))
 	}
 
-	// By default, use an empty pod template and take the one defined in the task run spec if any
+	// By default, use an empty newPod template and take the one defined in the task run spec if any
 	podTemplate := pod.Template{}
 
 	if taskRun.Spec.PodTemplate != nil {
@@ -296,16 +298,15 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 	}
 	activeDeadlineSeconds := int64(taskRun.GetTimeout(ctx).Seconds() * deadlineFactor)
 
-	pod := &corev1.Pod{
+	newPod := &corev1.Pod{
 		ObjectMeta: metav1.ObjectMeta{
-			// We execute the build's pod in the same namespace as where the build was
+			// We execute the build's newPod in the same namespace as where the build was
 			// created so that it can access colocated resources.
 			Namespace: taskRun.Namespace,
 			// Generate a unique name based on the build's name.
-			// 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: names.SimpleNameGenerator.RestrictLengthWithRandomSuffix(fmt.Sprintf("%s-pod", taskRun.Name)),
+			// The name is univocally generated so that in case of
+			// stale informer cache, we never create duplicate Pods
+			Name: kmeta.ChildName(taskRun.Name, "-pod"),
 			// If our parent TaskRun is deleted, then we should be as well.
 			OwnerReferences: []metav1.OwnerReference{
 				*metav1.NewControllerRef(taskRun, groupVersionKind),
@@ -333,18 +334,18 @@ func (b *Builder) Build(ctx context.Context, taskRun *v1beta1.TaskRun, taskSpec
 			PriorityClassName:            priorityClassName,
 			ImagePullSecrets:             podTemplate.ImagePullSecrets,
 			HostAliases:                  podTemplate.HostAliases,
-			ActiveDeadlineSeconds:        &activeDeadlineSeconds, // Set ActiveDeadlineSeconds to mark the pod as "terminating" (like a Job)
+			ActiveDeadlineSeconds:        &activeDeadlineSeconds, // Set ActiveDeadlineSeconds to mark the newPod as "terminating" (like a Job)
 		},
 	}
 
 	for _, f := range transformers {
-		pod, err = f(pod)
+		newPod, err = f(newPod)
 		if err != nil {
-			return pod, err
+			return newPod, err
 		}
 	}
 
-	return pod, nil
+	return newPod, nil
 }
 
 // makeLabels constructs the labels we will propagate from TaskRuns to Pods.

pkg/pod/pod_test.go

@@ -21,10 +21,11 @@ import (
 	"fmt"
 	"os"
 	"path/filepath"
-	"strings"
 	"testing"
 	"time"
 
+	"knative.dev/pkg/kmeta"
+
 	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/tektoncd/pipeline/pkg/apis/config"
@@ -1487,9 +1488,9 @@ _EOF_
 			if err != nil {
 				t.Fatalf("builder.Build: %v", err)
 			}
-
-			if !strings.HasPrefix(got.Name, "taskrun-name-pod-") {
-				t.Errorf("Pod name %q should have prefix 'taskrun-name-pod-'", got.Name)
+			expectedName := kmeta.ChildName(tr.Name, "-pod")
+			if d := cmp.Diff(expectedName, got.Name); d != "" {
+				t.Errorf("Pod name does not matc: %q", d)
 			}
 
 			if d := cmp.Diff(c.want, &got.Spec, resourceQuantityCmp, volumeSort, volumeMountSort); d != "" {
@@ -1640,8 +1641,9 @@ func TestPodBuildwithAlphaAPIEnabled(t *testing.T) {
 				t.Fatalf("builder.Build: %v", err)
 			}
 
-			if !strings.HasPrefix(got.Name, "taskrun-name-pod-") {
-				t.Errorf("Pod name %q should have prefix 'taskrun-name-pod-'", got.Name)
+			expectedName := kmeta.ChildName(tr.Name, "-pod")
+			if d := cmp.Diff(expectedName, got.Name); d != "" {
+				t.Errorf("Pod name does not matc: %q", d)
 			}
 
 			if d := cmp.Diff(c.want, &got.Spec, resourceQuantityCmp, volumeSort, volumeMountSort); d != "" {