Merge pull request #3498 from vieux/fix-fake-client-generatename-retry

🐛 fakeclient: retry GenerateName on AlreadyExists collisions

Author: k8s-ci-robot

pkg/client/fake/client.go

@@ -633,12 +633,12 @@ func (c *fakeClient) Create(ctx context.Context, obj client.Object, opts ...clie
 		return err
 	}
 
+	var generateNameBase string
 	if accessor.GetName() == "" && accessor.GetGenerateName() != "" {
-		base := accessor.GetGenerateName()
-		if len(base) > maxGeneratedNameLength {
-			base = base[:maxGeneratedNameLength]
+		generateNameBase = accessor.GetGenerateName()
+		if len(generateNameBase) > maxGeneratedNameLength {
+			generateNameBase = generateNameBase[:maxGeneratedNameLength]
 		}
-		accessor.SetName(fmt.Sprintf("%s%s", base, utilrand.String(randomLength)))
 	}
 	// Ignore attempts to set deletion timestamp
 	if !accessor.GetDeletionTimestamp().IsZero() {
@@ -653,10 +653,21 @@ func (c *fakeClient) Create(ctx context.Context, obj client.Object, opts ...clie
 	c.trackerWriteLock.Lock()
 	defer c.trackerWriteLock.Unlock()
 
-	if err := c.tracker.Create(gvr, obj, accessor.GetNamespace(), *createOptions.AsCreateOptions()); err != nil {
+	const maxRetries = 7
+	var createErr error
+	for range maxRetries {
+		if generateNameBase != "" {
+			accessor.SetName(fmt.Sprintf("%s%s", generateNameBase, utilrand.String(randomLength)))
+		}
+		createErr = c.tracker.Create(gvr, obj, accessor.GetNamespace(), *createOptions.AsCreateOptions())
+		if createErr == nil || generateNameBase == "" || !apierrors.IsAlreadyExists(createErr) {
+			break
+		}
+	}
+	if createErr != nil {
 		// The managed fields tracker sets gvk even on errors
 		_ = ensureTypeMeta(obj, gvk)
-		return err
+		return createErr
 	}
 
 	if !c.returnManagedFields {

pkg/client/fake/client_test.go

@@ -44,6 +44,7 @@ import (
 	"k8s.io/apimachinery/pkg/runtime/schema"
 	"k8s.io/apimachinery/pkg/runtime/serializer"
 	"k8s.io/apimachinery/pkg/types"
+	"k8s.io/apimachinery/pkg/util/sets"
 	"k8s.io/apimachinery/pkg/watch"
 	clientgoapplyconfigurations "k8s.io/client-go/applyconfigurations"
 	corev1applyconfigurations "k8s.io/client-go/applyconfigurations/core/v1"
@@ -485,6 +486,47 @@ var _ = Describe("Fake client", func() {
 			Expect(list.Items[0].Name).NotTo(BeEmpty())
 		})
 
+		It("should retry GenerateName on name collision and succeed", func(ctx SpecContext) {
+			// Create many objects with the same short GenerateName prefix so that
+			// birthday-paradox collisions are highly likely. The retry loop
+			// (max 7 attempts per object) must absorb every collision.
+			const n = 500
+			names := sets.New[string]()
+			for i := range n {
+				cm := &corev1.ConfigMap{
+					ObjectMeta: metav1.ObjectMeta{
+						GenerateName: "this-generate-name-prefix-is-longer-than-max-generated-name-length-and-will-be-truncated-",
+						Namespace:    "ns2",
+					},
+				}
+				Expect(cl.Create(ctx, cm)).To(Succeed(), "create #%d failed", i)
+				Expect(cm.Name).NotTo(BeEmpty())
+				Expect(cm.Name).To(HaveLen(maxNameLength))
+				Expect(names.Has(cm.Name)).To(BeFalse(), "duplicate name %q", cm.Name)
+				names.Insert(cm.Name)
+			}
+		})
+
+		It("should not retry AlreadyExists when Name is set explicitly", func(ctx SpecContext) {
+			cm := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "explicit-name",
+					Namespace: "ns2",
+				},
+			}
+			Expect(cl.Create(ctx, cm)).To(Succeed())
+
+			// Second create with the same explicit name must fail immediately.
+			cm2 := &corev1.ConfigMap{
+				ObjectMeta: metav1.ObjectMeta{
+					Name:      "explicit-name",
+					Namespace: "ns2",
+				},
+			}
+			err := cl.Create(ctx, cm2)
+			Expect(apierrors.IsAlreadyExists(err)).To(BeTrue())
+		})
+
 		It("should be able to Update", func(ctx SpecContext) {
 			By("Updating a new configmap")
 			newcm := &corev1.ConfigMap{