fix: Revert "fix: fix race condition in envelope work object creation via deterministic naming" (kubefleet-dev#454)

Author: britaniar

pkg/controllers/workgenerator/envelope.go

@@ -18,14 +18,13 @@ package workgenerator
 
 import (
 	"context"
-	"crypto/sha256"
-	"encoding/hex"
 	"fmt"
 	"sort"
 	"strings"
 
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/apis/meta/v1/unstructured"
+	"k8s.io/apimachinery/pkg/util/uuid"
 	"k8s.io/klog/v2"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
@@ -82,19 +81,12 @@ func (r *Reconciler) createOrUpdateEnvelopeCRWorkObj(
 	var work *fleetv1beta1.Work
 	switch {
 	case len(workList.Items) > 1:
-		// Multiple matching work objects found; this should never occur under normal conditions
-		// with deterministic naming. Log details for investigation.
+		// Multiple matching work objects found; this should never occur under normal conditions.
 		wrappedErr := fmt.Errorf("%d work objects found for the same envelope %v, only one expected", len(workList.Items), envelopeReader.GetEnvelopeObjRef())
 		klog.ErrorS(wrappedErr, "Failed to create or update work object for envelope",
 			"resourceBinding", klog.KObj(binding),
 			"resourceSnapshot", klog.KObj(resourceSnapshot),
 			"envelope", envelopeReader.GetEnvelopeObjRef())
-		// Log the work object names to help debug
-		for i := range workList.Items {
-			klog.ErrorS(wrappedErr, "Duplicate work object found",
-				"work", klog.KObj(&workList.Items[i]),
-				"creationTimestamp", workList.Items[i].CreationTimestamp)
-		}
 		return nil, controller.NewUnexpectedBehaviorError(wrappedErr)
 	case len(workList.Items) == 1:
 		klog.V(2).InfoS("Found existing work object for the envelope; updating it",
@@ -204,11 +196,7 @@ func buildNewWorkForEnvelopeCR(
 	manifests []fleetv1beta1.Manifest,
 	resourceOverrideSnapshotHash, clusterResourceOverrideSnapshotHash string,
 ) *fleetv1beta1.Work {
-	// Generate a deterministic work name based on the envelope identity to prevent duplicate work objects
-	// from being created by concurrent reconciliations. The name is stable across reconciliations for
-	// the same envelope, allowing Kubernetes' built-in duplicate prevention to work correctly.
-	envelopeIdentifier := generateEnvelopeIdentifier(envelopeReader)
-	workName := fmt.Sprintf(fleetv1beta1.WorkNameWithEnvelopeCRFmt, workNamePrefix, envelopeIdentifier)
+	workName := fmt.Sprintf(fleetv1beta1.WorkNameWithEnvelopeCRFmt, workNamePrefix, uuid.NewUUID())
 	workNamespace := fmt.Sprintf(utils.NamespaceNameFormat, resourceBinding.GetBindingSpec().TargetCluster)
 
 	// Create the labels map
@@ -246,18 +234,3 @@ func buildNewWorkForEnvelopeCR(
 		},
 	}
 }
-
-// generateEnvelopeIdentifier generates a stable, deterministic identifier for an envelope.
-// This identifier is used in the work name to ensure that the same envelope always produces
-// the same work name, enabling Kubernetes' atomic create operations to prevent duplicates.
-func generateEnvelopeIdentifier(envelopeReader fleetv1beta1.EnvelopeReader) string {
-	// Create a stable identifier based on envelope type, name, and namespace.
-	// For cluster-scoped envelopes, namespace is empty, so we include the type to ensure uniqueness.
-	identifier := fmt.Sprintf("%s.%s.%s", envelopeReader.GetEnvelopeType(), envelopeReader.GetNamespace(), envelopeReader.GetName())
-
-	// Use SHA256 hash to create a deterministic identifier
-	hash := sha256.Sum256([]byte(identifier))
-	// Take first 8 characters of the hex-encoded hash to keep work names reasonably short
-	// while maintaining uniqueness (8 hex chars = 4 bytes = 2^32 combinations, sufficient for envelope uniqueness)
-	return hex.EncodeToString(hash[:])[:8]
-}

pkg/controllers/workgenerator/envelope_test.go

@@ -483,18 +483,6 @@ func TestCreateOrUpdateEnvelopeCRWorkObj(t *testing.T) {
 			want:                                nil,
 			wantErr:                             true,
 		},
-		{
-			name:                                "two existing works should result in error",
-			envelopeReader:                      resourceEnvelope,
-			resourceOverrideSnapshotHash:        "new-resource-hash",
-			clusterResourceOverrideSnapshotHash: "new-cluster-resource-hash",
-			existingObjects: func() []client.Object {
-				existingWork1 := existingWork.DeepCopy()
-				existingWork1.Name = "test-work-1"
-				return []client.Object{existingWork, existingWork1}
-			}(),
-			wantErr: true,
-		},
 	}
 
 	for _, tt := range tests {

test/e2e/enveloped_object_placement_test.go

@@ -140,7 +140,7 @@ var _ = Describe("placing wrapped resources using a CRP", func() {
 			for idx := range allMemberClusters {
 				memberCluster := allMemberClusters[idx]
 				workResourcesPlacedActual := checkAllResourcesPlacement(memberCluster)
-				Eventually(workResourcesPlacedActual, workloadEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to place work resources on member cluster %s", memberCluster.ClusterName)
+				Eventually(workResourcesPlacedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to place work resources on member cluster %s", memberCluster.ClusterName)
 			}
 		})