address comments

Signed-off-by: Britania Rodriguez Reyes <britaniar@microsoft.com>

Author: britaniar

apis/placement/v1beta1/stageupdate_types.go

@@ -374,7 +374,7 @@ type UpdateRunStatus struct {
 	// +kubebuilder:validation:Optional
 	PolicyObservedClusterCount int `json:"policyObservedClusterCount,omitempty"`
 
-	// ResourceSnapshotName records the name of the resource snapshot that the update run is based on.
+	// ResourceSnapshotName records the name of the master resource snapshot that the update run is based on.
 	// +kubebuilder:validation:Optional
 	ResourceSnapshotName string `json:"resourceSnapshotName,omitempty"`
 

apis/placement/v1beta1/zz_generated.deepcopy.go

@@ -1895,7 +1895,7 @@ spec:
                   to the current policy.
                 type: string
               resourceSnapshotName:
-                description: ResourceSnapshotName records the name of the resource
+                description: ResourceSnapshotName records the name of the master resource
                   snapshot that the update run is based on.
                 type: string
               stagedUpdateStrategySnapshot:

config/crd/bases/placement.kubernetes-fleet.io_clusterstagedupdateruns.yaml

@@ -815,7 +815,7 @@ spec:
                   to the current policy.
                 type: string
               resourceSnapshotName:
-                description: ResourceSnapshotName records the name of the resource
+                description: ResourceSnapshotName records the name of the master resource
                   snapshot that the update run is based on.
                 type: string
               stagedUpdateStrategySnapshot:

config/crd/bases/placement.kubernetes-fleet.io_stagedupdateruns.yaml

@@ -478,19 +478,13 @@ func (r *Reconciler) recordOverrideSnapshots(ctx context.Context, placementKey t
 		return err
 	}
 
-	if len(resourceSnapshotObjs) == 0 {
-		err := controller.NewUserError(fmt.Errorf("no resourceSnapshots found for placement `%s`", placementKey))
-		klog.ErrorS(err, "No resourceSnapshots found", "updateRun", updateRunRef)
-		// no more retries here.
-		return fmt.Errorf("%w: %s", errInitializedFailed, err.Error())
-	}
-
 	// Look for the master resourceSnapshot.
 	var masterResourceSnapshot placementv1beta1.ResourceSnapshotObj
 	for _, resourceSnapshot := range resourceSnapshotObjs {
 		// only master has this annotation.
 		if len(resourceSnapshot.GetAnnotations()[placementv1beta1.ResourceGroupHashAnnotation]) != 0 {
 			masterResourceSnapshot = resourceSnapshot
+
 			break
 		}
 	}
@@ -502,10 +496,12 @@ func (r *Reconciler) recordOverrideSnapshots(ctx context.Context, placementKey t
 		return fmt.Errorf("%w: %s", errInitializedFailed, err.Error())
 	}
 
-	klog.InfoS("Found master resourceSnapshot", "placement", placementKey, "index", updateRun.GetUpdateRunSpec().ResourceSnapshotIndex, "updateRun", updateRunRef)
+	klog.InfoS("Found master resourceSnapshot", "placement", placementKey, "masterResourceSnapshot", masterResourceSnapshot.GetName(), "updateRun", updateRunRef)
 
 	// Update the resource snapshot name in the UpdateRun status.
-	updateRun.GetUpdateRunStatus().ResourceSnapshotName = masterResourceSnapshot.GetName()
+	updateRunStatus := updateRun.GetUpdateRunStatus()
+	updateRunStatus.ResourceSnapshotName = masterResourceSnapshot.GetName()
+	updateRun.SetUpdateRunStatus(*updateRunStatus)
 	if updateErr := r.Client.Status().Update(ctx, updateRun); updateErr != nil {
 		klog.ErrorS(updateErr, "Failed to update the UpdateRun status with resource snapshot name", "updateRun", klog.KObj(updateRun), "resourceSnapshot", klog.KObj(masterResourceSnapshot))
 		// updateErr can be retried.
@@ -522,7 +518,6 @@ func (r *Reconciler) recordOverrideSnapshots(ctx context.Context, placementKey t
 	}
 
 	// Pick the overrides associated with each target cluster.
-	updateRunStatus := updateRun.GetUpdateRunStatus()
 	for _, stageStatus := range updateRunStatus.StagesStatus {
 		for i := range stageStatus.Clusters {
 			clusterStatus := &stageStatus.Clusters[i]
@@ -539,8 +534,10 @@ func (r *Reconciler) recordOverrideSnapshots(ctx context.Context, placementKey t
 	return nil
 }
 
-// getResourceSnapshotObjs retrieves the resource snapshot objects either by index or latest snapshots.
+// getResourceSnapshotObjs retrieves the list of resource snapshot objects from the specified ResourceSnapshotIndex.
+// If ResourceSnapshotIndex is unspecified, it returns the list of latest resource snapshots.
 func (r *Reconciler) getResourceSnapshotObjs(ctx context.Context, updateRunSpec *placementv1beta1.UpdateRunSpec, placementName string, placementKey types.NamespacedName, updateRunRef klog.ObjectRef) ([]placementv1beta1.ResourceSnapshotObj, error) {
+	var resourceSnapshotObjs []placementv1beta1.ResourceSnapshotObj
 	if updateRunSpec.ResourceSnapshotIndex != "" {
 		snapshotIndex, err := strconv.Atoi(updateRunSpec.ResourceSnapshotIndex)
 		if err != nil || snapshotIndex < 0 {
@@ -555,7 +552,15 @@ func (r *Reconciler) getResourceSnapshotObjs(ctx context.Context, updateRunSpec
 				"placement", placementKey, "resourceSnapshotIndex", snapshotIndex, "updateRun", updateRunRef)
 			return nil, controller.NewAPIServerError(true, err)
 		}
-		return resourceSnapshotList.GetResourceSnapshotObjs(), nil
+
+		resourceSnapshotObjs = resourceSnapshotList.GetResourceSnapshotObjs()
+		if len(resourceSnapshotObjs) == 0 {
+			err := controller.NewUserError(fmt.Errorf("no resourceSnapshots with index `%d` found for placement `%s`", snapshotIndex, placementKey))
+			klog.ErrorS(err, "No specified resourceSnapshots found", "updateRun", updateRunRef)
+			// no more retries here.
+			return resourceSnapshotObjs, fmt.Errorf("%w: %s", errInitializedFailed, err.Error())
+		}
+		return resourceSnapshotObjs, nil
 	}
 
 	latestResourceSnapshots, err := controller.ListLatestResourceSnapshots(ctx, r.Client, placementKey)
@@ -564,7 +569,15 @@ func (r *Reconciler) getResourceSnapshotObjs(ctx context.Context, updateRunSpec
 			"placement", placementKey, "updateRun", updateRunRef)
 		return nil, controller.NewAPIServerError(true, err)
 	}
-	return latestResourceSnapshots.GetResourceSnapshotObjs(), nil
+
+	resourceSnapshotObjs = latestResourceSnapshots.GetResourceSnapshotObjs()
+	if len(resourceSnapshotObjs) == 0 {
+		err := controller.NewUserError(fmt.Errorf("no resourceSnapshots found for placement `%s`", placementKey))
+		klog.ErrorS(err, "No resourceSnapshots found", "updateRun", updateRunRef)
+		// no more retries here.
+		return resourceSnapshotObjs, fmt.Errorf("%w: %s", errInitializedFailed, err.Error())
+	}
+	return resourceSnapshotObjs, nil
 }
 
 // recordInitializationSucceeded records the successful initialization condition in the UpdateRun status.

pkg/controllers/updaterun/initialization.go

@@ -877,6 +877,80 @@ var _ = Describe("Updaterun initialization tests", func() {
 			validateUpdateRunMetricsEmitted(generateProgressingMetric(updateRun))
 		})
 	})
+
+	Context("Test multiple resource snapshots", func() {
+		var resourceSnapshot2, resourceSnapshot3 *placementv1beta1.ClusterResourceSnapshot
+		BeforeEach(func() {
+			By("Creating a new clusterResourcePlacement")
+			Expect(k8sClient.Create(ctx, crp)).To(Succeed())
+
+			By("Creating scheduling policy snapshot")
+			Expect(k8sClient.Create(ctx, policySnapshot)).To(Succeed())
+
+			By("Setting the latest policy snapshot condition as fully scheduled")
+			meta.SetStatusCondition(&policySnapshot.Status.Conditions, metav1.Condition{
+				Type:               string(placementv1beta1.PolicySnapshotScheduled),
+				Status:             metav1.ConditionTrue,
+				ObservedGeneration: policySnapshot.Generation,
+				Reason:             "scheduled",
+			})
+			Expect(k8sClient.Status().Update(ctx, policySnapshot)).Should(Succeed(), "failed to update the policy snapshot condition")
+
+			By("Creating a new resource snapshot")
+			resourceSnapshot.Labels[placementv1beta1.IsLatestSnapshotLabel] = "false"
+			Expect(k8sClient.Create(ctx, resourceSnapshot)).To(Succeed())
+
+			By("Creating a another new resource snapshot")
+			resourceSnapshot2 = generateTestClusterResourceSnapshot()
+			resourceSnapshot2.Name = testCRPName + "-1-snapshot"
+			resourceSnapshot2.Labels[placementv1beta1.IsLatestSnapshotLabel] = "false"
+			resourceSnapshot2.Labels[placementv1beta1.ResourceIndexLabel] = "1"
+			Expect(k8sClient.Create(ctx, resourceSnapshot2)).To(Succeed())
+
+			By("Creating a latest master resource snapshot")
+			resourceSnapshot3 = generateTestClusterResourceSnapshot()
+			resourceSnapshot3.Name = testCRPName + "-2-snapshot"
+			resourceSnapshot3.Labels[placementv1beta1.ResourceIndexLabel] = "2"
+			Expect(k8sClient.Create(ctx, resourceSnapshot3)).To(Succeed())
+
+			By("Creating the member clusters")
+			for _, cluster := range targetClusters {
+				Expect(k8sClient.Create(ctx, cluster)).To(Succeed())
+			}
+			for _, cluster := range unscheduledClusters {
+				Expect(k8sClient.Create(ctx, cluster)).To(Succeed())
+			}
+
+			By("Creating a bunch of ClusterResourceBindings")
+			for _, binding := range resourceBindings {
+				Expect(k8sClient.Create(ctx, binding)).To(Succeed())
+			}
+
+			By("Creating a clusterStagedUpdateStrategy")
+			Expect(k8sClient.Create(ctx, updateStrategy)).To(Succeed())
+		})
+
+		It("Should pick latest master resource snapshot", func() {
+			By("Creating a new cluster resource override")
+			Expect(k8sClient.Create(ctx, clusterResourceOverride)).To(Succeed())
+
+			By("Creating a new clusterStagedUpdateRun")
+			updateRun.Spec.ResourceSnapshotIndex = ""
+			Expect(k8sClient.Create(ctx, updateRun)).To(Succeed())
+
+			By("Validating the clusterStagedUpdateRun stats")
+			initialized := generateSucceededInitializationStatus(crp, updateRun, policySnapshot, updateStrategy, clusterResourceOverride)
+			initialized.ResourceSnapshotName = resourceSnapshot3.Name
+			want := generateExecutionStartedStatus(updateRun, initialized)
+			validateClusterStagedUpdateRunStatus(ctx, updateRun, want, "")
+
+			By("Validating the clusterStagedUpdateRun initialized consistently")
+			validateClusterStagedUpdateRunStatusConsistently(ctx, updateRun, want, "")
+
+			By("Checking update run status metrics are emitted")
+			validateUpdateRunMetricsEmitted(generateProgressingMetric(updateRun))
+		})
+	})
 })
 
 func validateFailedInitCondition(ctx context.Context, updateRun *placementv1beta1.ClusterStagedUpdateRun, message string) {