update stagedUpdateRun to use latest resource snapshot if resourceIndex is unset

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

Author: britaniar

apis/placement/v1beta1/stageupdate_types.go

@@ -159,7 +159,7 @@ type UpdateRunSpec struct {
 
 	// The resource snapshot index of the selected resources to be updated across clusters.
 	// The index represents a group of resource snapshots that includes all the resources a ResourcePlacement selected.
-	// +kubebuilder:validation:Required
+	// +kubebuilder:validation:optional
 	ResourceSnapshotIndex string `json:"resourceSnapshotIndex"`
 
 	// The name of the update strategy that specifies the stages and the sequence

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

@@ -1998,7 +1998,7 @@ spec:
                           description: |-
                             MaxConcurrency specifies the maximum number of clusters that can be updated concurrently within this stage.
                             Value can be an absolute number (ex: 5) or a percentage of the total clusters in the stage (ex: 50%).
-                            Absolute number is calculated from percentage by rounding up.
+                            Fractional results are rounded down. A minimum of 1 update is enforced.
                             If not specified, all clusters in the stage are updated sequentially (effectively maxConcurrency = 1).
                             Defaults to 1.
                           pattern: ^((100|[0-9]{1,2})%|[0-9]+)$

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

@@ -312,7 +312,7 @@ spec:
                       description: |-
                         MaxConcurrency specifies the maximum number of clusters that can be updated concurrently within this stage.
                         Value can be an absolute number (ex: 5) or a percentage of the total clusters in the stage (ex: 50%).
-                        Absolute number is calculated from percentage by rounding up.
+                        Fractional results are rounded down. A minimum of 1 update is enforced.
                         If not specified, all clusters in the stage are updated sequentially (effectively maxConcurrency = 1).
                         Defaults to 1.
                       pattern: ^((100|[0-9]{1,2})%|[0-9]+)$

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

@@ -917,7 +917,7 @@ spec:
                           description: |-
                             MaxConcurrency specifies the maximum number of clusters that can be updated concurrently within this stage.
                             Value can be an absolute number (ex: 5) or a percentage of the total clusters in the stage (ex: 50%).
-                            Absolute number is calculated from percentage by rounding up.
+                            Fractional results are rounded down. A minimum of 1 update is enforced.
                             If not specified, all clusters in the stage are updated sequentially (effectively maxConcurrency = 1).
                             Defaults to 1.
                           pattern: ^((100|[0-9]{1,2})%|[0-9]+)$

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

@@ -174,7 +174,7 @@ spec:
                       description: |-
                         MaxConcurrency specifies the maximum number of clusters that can be updated concurrently within this stage.
                         Value can be an absolute number (ex: 5) or a percentage of the total clusters in the stage (ex: 50%).
-                        Absolute number is calculated from percentage by rounding up.
+                        Fractional results are rounded down. A minimum of 1 update is enforced.
                         If not specified, all clusters in the stage are updated sequentially (effectively maxConcurrency = 1).
                         Defaults to 1.
                       pattern: ^((100|[0-9]{1,2})%|[0-9]+)$

pkg/controllers/updaterun/initialization.go

@@ -469,27 +469,39 @@ func validateAfterStageTask(tasks []placementv1beta1.StageTask) error {
 
 // recordOverrideSnapshots finds all the override snapshots that are associated with each cluster and record them in the UpdateRun status.
 func (r *Reconciler) recordOverrideSnapshots(ctx context.Context, placementKey types.NamespacedName, updateRun placementv1beta1.UpdateRunObj) error {
+	var resourceSnapshotObjs []placementv1beta1.ResourceSnapshotObj
+	var snapshotIndex int
 	updateRunRef := klog.KObj(updateRun)
 	updateRunSpec := updateRun.GetUpdateRunSpec()
 	placementName := placementKey.Name
+	if updateRunSpec.ResourceSnapshotIndex != "" {
+		snapshotIndex, err := strconv.Atoi(updateRunSpec.ResourceSnapshotIndex)
+		if err != nil || snapshotIndex < 0 {
+			err := controller.NewUserError(fmt.Errorf("invalid resource snapshot index `%s` provided, expected an integer >= 0", updateRunSpec.ResourceSnapshotIndex))
+			klog.ErrorS(err, "Failed to parse the resource snapshot index", "updateRun", updateRunRef)
+			// no more retries here.
+			return fmt.Errorf("%w: %s", errInitializedFailed, err.Error())
+		}
 
-	snapshotIndex, err := strconv.Atoi(updateRunSpec.ResourceSnapshotIndex)
-	if err != nil || snapshotIndex < 0 {
-		err := controller.NewUserError(fmt.Errorf("invalid resource snapshot index `%s` provided, expected an integer >= 0", updateRunSpec.ResourceSnapshotIndex))
-		klog.ErrorS(err, "Failed to parse the resource snapshot index", "updateRun", updateRunRef)
-		// no more retries here.
-		return fmt.Errorf("%w: %s", errInitializedFailed, err.Error())
-	}
-
-	resourceSnapshotList, err := controller.ListAllResourceSnapshotWithAnIndex(ctx, r.Client, updateRunSpec.ResourceSnapshotIndex, placementName, placementKey.Namespace)
-	if err != nil {
-		klog.ErrorS(err, "Failed to list the resourceSnapshots associated with the placement",
-			"placement", placementKey, "resourceSnapshotIndex", snapshotIndex, "updateRun", updateRunRef)
-		// err can be retried.
-		return controller.NewAPIServerError(true, err)
+		resourceSnapshotList, err := controller.ListAllResourceSnapshotWithAnIndex(ctx, r.Client, updateRunSpec.ResourceSnapshotIndex, placementName, placementKey.Namespace)
+		if err != nil {
+			klog.ErrorS(err, "Failed to list the resourceSnapshots associated with the placement",
+				"placement", placementKey, "resourceSnapshotIndex", snapshotIndex, "updateRun", updateRunRef)
+			// err can be retried.
+			return controller.NewAPIServerError(true, err)
+		}
+		resourceSnapshotObjs = resourceSnapshotList.GetResourceSnapshotObjs()
+	} else {
+		latestResourceSnapshot, err := controller.ListLatestResourceSnapshots(ctx, r.Client, placementKey)
+		if err != nil {
+			klog.ErrorS(err, "Failed to list the latest resourceSnapshots associated with the placement",
+				"placement", placementKey, "updateRun", updateRunRef)
+			// err can be retried.
+			return controller.NewAPIServerError(true, err)
+		}
+		resourceSnapshotObjs = latestResourceSnapshot.GetResourceSnapshotObjs()
 	}
 
-	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)

pkg/controllers/updaterun/initialization_integration_test.go

@@ -748,7 +748,7 @@ var _ = Describe("Updaterun initialization tests", func() {
 			Expect(k8sClient.Create(ctx, updateStrategy)).To(Succeed())
 		})
 
-		It("Should fail to initialize if the specified resource snapshot index is invalid - not integer", func() {
+		FIt("Should fail to initialize if the specified resource snapshot index is invalid - not integer", func() {
 			By("Creating a new clusterStagedUpdateRun with invalid resource snapshot index")
 			updateRun.Spec.ResourceSnapshotIndex = "invalid-index"
 			Expect(k8sClient.Create(ctx, updateRun)).To(Succeed())
@@ -828,6 +828,29 @@ var _ = Describe("Updaterun initialization tests", func() {
 			validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(updateRun))
 		})
 
+		It("Should put related ClusterResourceOverrides in the status with no resource index defined", func() {
+			By("Creating a new resource snapshot")
+			Expect(k8sClient.Create(ctx, resourceSnapshot)).To(Succeed())
+
+			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)
+			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))
+		})
+
 		It("Should put related ClusterResourceOverrides in the status", func() {
 			By("Creating a new resource snapshot")
 			Expect(k8sClient.Create(ctx, resourceSnapshot)).To(Succeed())