PVC: Use Owner UIDs instead of Owner names.

We've noticed that using Owner names can lead to race conditions in
deleting a volumeClaimTemplate PVC in where a PipelineRun is quickly deleted
then recreated (see #3855).

Using the UID allows us to uniquely identify independent runs with the
same name, avoiding this issue while keeping the same semantic.

pkg/reconciler/pipelinerun/pipelinerun_test.go

@@ -41,6 +41,7 @@ import (
 	"github.com/tektoncd/pipeline/pkg/reconciler/pipelinerun/resources"
 	taskrunresources "github.com/tektoncd/pipeline/pkg/reconciler/taskrun/resources"
 	ttesting "github.com/tektoncd/pipeline/pkg/reconciler/testing"
+	"github.com/tektoncd/pipeline/pkg/reconciler/volumeclaim"
 	"github.com/tektoncd/pipeline/test"
 	"github.com/tektoncd/pipeline/test/diff"
 	"github.com/tektoncd/pipeline/test/names"
@@ -2846,9 +2847,18 @@ func TestReconcileWithVolumeClaimTemplateWorkspace(t *testing.T) {
 		t.Errorf("expected one PVC created. %d was created", len(pvcNames))
 	}
 
-	expectedPVCName := fmt.Sprintf("%s-%s", claimName, "cab465d09a")
-	if pvcNames[0] != expectedPVCName {
-		t.Errorf("expected the created PVC to be named %s. It was named %s", expectedPVCName, pvcNames[0])
+	for _, pr := range prs {
+		for _, w := range pr.Spec.Workspaces {
+			expectedPVCName := volumeclaim.GetPersistentVolumeClaimName(&corev1.PersistentVolumeClaim{
+				ObjectMeta: metav1.ObjectMeta{
+					Name: w.VolumeClaimTemplate.Name,
+				},
+			}, w, pr.GetOwnerReference())
+			_, err := clients.Kube.CoreV1().PersistentVolumeClaims(pr.Namespace).Get(prt.TestAssets.Ctx, expectedPVCName, metav1.GetOptions{})
+			if err != nil {
+				t.Fatalf("expected PVC %s to exist but instead got error when getting it: %v", expectedPVCName, err)
+			}
+		}
 	}
 
 	taskRuns, err := clients.Pipeline.TektonV1beta1().TaskRuns("foo").List(prt.TestAssets.Ctx, metav1.ListOptions{})

pkg/reconciler/taskrun/taskrun_test.go

@@ -2982,12 +2982,15 @@ func TestReconcileWorkspaceWithVolumeClaimTemplate(t *testing.T) {
 		if w.PersistentVolumeClaim != nil {
 			t.Fatalf("expected workspace from volumeClaimTemplate to be translated to PVC")
 		}
-	}
-
-	expectedPVCName := fmt.Sprintf("%s-%s", claimName, "a521418087")
-	_, err = clients.Kube.CoreV1().PersistentVolumeClaims(taskRun.Namespace).Get(testAssets.Ctx, expectedPVCName, metav1.GetOptions{})
-	if err != nil {
-		t.Fatalf("expected PVC %s to exist but instead got error when getting it: %v", expectedPVCName, err)
+		expectedPVCName := volumeclaim.GetPersistentVolumeClaimName(&corev1.PersistentVolumeClaim{
+			ObjectMeta: metav1.ObjectMeta{
+				Name: w.VolumeClaimTemplate.Name,
+			},
+		}, w, ttt.GetOwnerReference())
+		_, err = clients.Kube.CoreV1().PersistentVolumeClaims(taskRun.Namespace).Get(testAssets.Ctx, expectedPVCName, metav1.GetOptions{})
+		if err != nil {
+			t.Fatalf("expected PVC %s to exist but instead got error when getting it: %v", expectedPVCName, err)
+		}
 	}
 }
 

pkg/reconciler/volumeclaim/pvchandler.go

@@ -91,13 +91,14 @@ func getPersistentVolumeClaims(workspaceBindings []v1beta1.WorkspaceBinding, own
 
 // GetPersistentVolumeClaimName gets the name of PersistentVolumeClaim for a Workspace and PipelineRun or TaskRun. claim
 // must be a PersistentVolumeClaim from a volumeClaimTemplate. The returned name must be consistent given the same
-// workspaceBinding name and ownerReference name - because it is first used for creating a PVC and later,
+// workspaceBinding name and ownerReference UID - because it is first used for creating a PVC and later,
 // possibly several TaskRuns to lookup the PVC to mount.
+// We use ownerReference UID over ownerReference name to distinguish runs with the same name.
 func GetPersistentVolumeClaimName(claim *corev1.PersistentVolumeClaim, wb v1beta1.WorkspaceBinding, owner metav1.OwnerReference) string {
 	if claim.Name == "" {
-		return fmt.Sprintf("%s-%s", "pvc", getPersistentVolumeClaimIdentity(wb.Name, owner.Name))
+		return fmt.Sprintf("%s-%s", "pvc", getPersistentVolumeClaimIdentity(wb.Name, string(owner.UID)))
 	}
-	return fmt.Sprintf("%s-%s", claim.Name, getPersistentVolumeClaimIdentity(wb.Name, owner.Name))
+	return fmt.Sprintf("%s-%s", claim.Name, getPersistentVolumeClaimIdentity(wb.Name, string(owner.UID)))
 }
 
 func getPersistentVolumeClaimIdentity(workspaceName, ownerName string) string {

pkg/reconciler/volumeclaim/pvchandler_test.go

@@ -25,6 +25,7 @@ import (
 	"go.uber.org/zap"
 	corev1 "k8s.io/api/core/v1"
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
+	"k8s.io/apimachinery/pkg/types"
 	fakek8s "k8s.io/client-go/kubernetes/fake"
 )
 
@@ -69,7 +70,7 @@ func TestCreatePersistentVolumeClaimsForWorkspaces(t *testing.T) {
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
-	ownerRef := metav1.OwnerReference{Name: ownerName}
+	ownerRef := metav1.OwnerReference{UID: types.UID(ownerName)}
 	namespace := "ns"
 	fakekubeclient := fakek8s.NewSimpleClientset()
 	pvcHandler := defaultPVCHandler{fakekubeclient, zap.NewExample().Sugar()}
@@ -110,7 +111,7 @@ func TestCreatePersistentVolumeClaimsForWorkspaces(t *testing.T) {
 		t.Fatalf("unexpected number of ownerreferences on created PVC; expected: %d got %d", expectedNumberOfOwnerRefs, len(pvc.OwnerReferences))
 	}
 
-	if pvc.OwnerReferences[0].Name != ownerName {
+	if string(pvc.OwnerReferences[0].UID) != ownerName {
 		t.Fatalf("unexptected name in ownerreference on created PVC; expected: %s got %s", ownerName, pvc.OwnerReferences[0].Name)
 	}
 }
@@ -134,7 +135,7 @@ func TestCreatePersistentVolumeClaimsForWorkspacesWithoutMetadata(t *testing.T)
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
-	ownerRef := metav1.OwnerReference{Name: ownerName}
+	ownerRef := metav1.OwnerReference{UID: types.UID(ownerName)}
 	namespace := "ns"
 	fakekubeclient := fakek8s.NewSimpleClientset()
 	pvcHandler := defaultPVCHandler{fakekubeclient, zap.NewExample().Sugar()}