fix: use index number in clusterStagedUpdateRun and add ApprovalAccepted status to ApprovalRequests

.codespellignore

@@ -3,4 +3,4 @@ AfterAll
 CROs
 NotIn
 fo
-allReady
+allReady

apis/placement/v1alpha1/stagedupdate_types.go

@@ -462,6 +462,11 @@ const (
 	// Its condition status can be:
 	// - "True": The request is approved.
 	ApprovalRequestConditionApproved ApprovalRequestConditionType = "Approved"
+
+	// ApprovalRequestConditionApprovalAccepted indicates if the approved approval request was accepted.
+	// Its condition status can be:
+	// - "True": The request is approved.
+	ApprovalRequestConditionApprovalAccepted ApprovalRequestConditionType = "ApprovalAccepted"
 )
 
 // ClusterApprovalRequestList contains a list of ClusterApprovalRequest.

apis/placement/v1beta1/stageupdate_types.go

@@ -463,6 +463,11 @@ const (
 	// Its condition status can be:
 	// - "True": The request is approved.
 	ApprovalRequestConditionApproved ApprovalRequestConditionType = "Approved"
+
+	// ApprovalRequestConditionApprovalAccepted indicates if the approved approval request was accepted.
+	// Its condition status can be:
+	// - "True": The request is approved.
+	ApprovalRequestConditionApprovalAccepted ApprovalRequestConditionType = "ApprovalAccepted"
 )
 
 // ClusterApprovalRequestList contains a list of ClusterApprovalRequest.

examples/stagedupdaterun/clusterStagedUpdateRun.yaml

@@ -4,10 +4,10 @@ metadata:
   name: example-run
 spec:
   placementName: example-placement
-  resourceSnapshotIndex: example-placement-0-snapshot
+  resourceSnapshotIndex: "0"
   stagedRolloutStrategyName: example-strategy
 status:
-  policySnapshotIndexUsed: example-placement-0
+  policySnapshotIndexUsed: "0"
   policyObservedClusterCount: 3
   appliedStrategy:
     type: Immediate

pkg/controllers/updaterun/controller.go

@@ -235,28 +235,39 @@ func (r *Reconciler) SetupWithManager(mgr runtime.Manager) error {
 			// We only care about when an approval request is approved.
 			UpdateFunc: func(ctx context.Context, e event.UpdateEvent, q workqueue.RateLimitingInterface) {
 				klog.V(2).InfoS("Handling a clusterApprovalRequest update event", "clusterApprovalRequest", klog.KObj(e.ObjectNew))
-				handleClusterApprovalRequest(e.ObjectNew, q)
-			},
-			GenericFunc: func(ctx context.Context, e event.GenericEvent, q workqueue.RateLimitingInterface) {
-				klog.V(2).InfoS("Handling a clusterApprovalRequest generic event", "clusterApprovalRequest", klog.KObj(e.Object))
-				handleClusterApprovalRequest(e.Object, q)
+				handleClusterApprovalRequest(e.ObjectOld, e.ObjectNew, q)
 			},
 		}).Complete(r)
 }
 
 // handleClusterApprovalRequest finds the ClusterStagedUpdateRun creating the ClusterApprovalRequest,
-// and enqueues it to the ClusterStagedUpdateRun controller queue.
-func handleClusterApprovalRequest(obj client.Object, q workqueue.RateLimitingInterface) {
-	approvalRequest, ok := obj.(*placementv1beta1.ClusterApprovalRequest)
+// and enqueues it to the ClusterStagedUpdateRun controller queue only when the approved condition gets changed.
+func handleClusterApprovalRequest(oldObj, newObj client.Object, q workqueue.RateLimitingInterface) {
+	oldAppReq, ok := oldObj.(*placementv1beta1.ClusterApprovalRequest)
+	if !ok {
+		klog.V(2).ErrorS(controller.NewUnexpectedBehaviorError(fmt.Errorf("cannot cast runtime object to ClusterApprovalRequest")),
+			"Invalid object type", "object", klog.KObj(oldObj))
+		return
+	}
+	newAppReq, ok := newObj.(*placementv1beta1.ClusterApprovalRequest)
 	if !ok {
 		klog.V(2).ErrorS(controller.NewUnexpectedBehaviorError(fmt.Errorf("cannot cast runtime object to ClusterApprovalRequest")),
-			"Invalid object type", "object", klog.KObj(obj))
+			"Invalid object type", "object", klog.KObj(newObj))
 		return
 	}
-	updateRun := approvalRequest.Spec.TargetUpdateRun
+
+	approvedInOld := condition.IsConditionStatusTrue(meta.FindStatusCondition(oldAppReq.Status.Conditions, string(placementv1beta1.ApprovalRequestConditionApproved)), oldAppReq.Generation)
+	approvedInNew := condition.IsConditionStatusTrue(meta.FindStatusCondition(newAppReq.Status.Conditions, string(placementv1beta1.ApprovalRequestConditionApproved)), newAppReq.Generation)
+
+	if approvedInOld == approvedInNew {
+		klog.V(2).InfoS("The approval status is not changed, ignore queueing", "clusterApprovalRequest", klog.KObj(newAppReq))
+		return
+	}
+
+	updateRun := newAppReq.Spec.TargetUpdateRun
 	if len(updateRun) == 0 {
 		klog.V(2).ErrorS(controller.NewUnexpectedBehaviorError(fmt.Errorf("TargetUpdateRun field in ClusterApprovalRequest is empty")),
-			"Invalid clusterApprovalRequest", "clusterApprovalRequest", klog.KObj(approvalRequest))
+			"Invalid clusterApprovalRequest", "clusterApprovalRequest", klog.KObj(newAppReq))
 		return
 	}
 	// enqueue to the updaterun controller queue.

pkg/controllers/updaterun/controller_integration_test.go

@@ -45,6 +45,9 @@ const (
 	numUnscheduledClusters = 3
 	// numberOfClustersAnnotation is the number of clusters in the test latest policy snapshot
 	numberOfClustersAnnotation = numTargetClusters
+
+	// testResourceSnapshotIndex is the index of the test resource snapshot
+	testResourceSnapshotIndex = "0"
 )
 
 var (
@@ -62,7 +65,7 @@ var _ = Describe("Test the clusterStagedUpdateRun controller", func() {
 	BeforeEach(func() {
 		testUpdateRunName = "updaterun-" + utils.RandStr()
 		testCRPName = "crp-" + utils.RandStr()
-		testResourceSnapshotName = "snapshot-" + utils.RandStr()
+		testResourceSnapshotName = testCRPName + "-" + testResourceSnapshotIndex + "-snapshot"
 		testUpdateStrategyName = "updatestrategy-" + utils.RandStr()
 		testCROName = "cro-" + utils.RandStr()
 		updateRunNamespacedName = types.NamespacedName{Name: testUpdateRunName}
@@ -216,7 +219,7 @@ func generateTestClusterStagedUpdateRun() *placementv1beta1.ClusterStagedUpdateR
 		},
 		Spec: placementv1beta1.StagedUpdateRunSpec{
 			PlacementName:            testCRPName,
-			ResourceSnapshotIndex:    testResourceSnapshotName,
+			ResourceSnapshotIndex:    testResourceSnapshotIndex,
 			StagedUpdateStrategyName: testUpdateStrategyName,
 		},
 	}
@@ -254,6 +257,7 @@ func generateTestClusterSchedulingPolicySnapshot(idx int) *placementv1beta1.Clus
 			Labels: map[string]string{
 				"kubernetes-fleet.io/parent-CRP":         testCRPName,
 				"kubernetes-fleet.io/is-latest-snapshot": "true",
+				"kubernetes-fleet.io/policy-index":       strconv.Itoa(idx),
 			},
 			Annotations: map[string]string{
 				"kubernetes-fleet.io/number-of-clusters": strconv.Itoa(numberOfClustersAnnotation),
@@ -360,6 +364,7 @@ func generateTestClusterResourceSnapshot() *placementv1beta1.ClusterResourceSnap
 			Labels: map[string]string{
 				placementv1beta1.CRPTrackingLabel:      testCRPName,
 				placementv1beta1.IsLatestSnapshotLabel: strconv.FormatBool(true),
+				placementv1beta1.ResourceIndexLabel:    testResourceSnapshotIndex,
 			},
 			Annotations: map[string]string{
 				placementv1beta1.ResourceGroupHashAnnotation:         "hash",