updateRun e2e test

apis/placement/v1alpha1/stagedupdate_types.go

@@ -18,8 +18,8 @@ import (
 // +kubebuilder:resource:scope=Cluster,categories={fleet,fleet-placement},shortName=crsur
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 // +kubebuilder:printcolumn:JSONPath=`.spec.placementName`,name="Placement",type=string
-// +kubebuilder:printcolumn:JSONPath=`.spec.resourceSnapshotIndex`,name="Resource-Snapshot",type=string
-// +kubebuilder:printcolumn:JSONPath=`.status.policySnapshotIndexUsed`,name="Policy-Snapshot",type=string
+// +kubebuilder:printcolumn:JSONPath=`.spec.resourceSnapshotIndex`,name="Resource-Snapshot-Index",type=string
+// +kubebuilder:printcolumn:JSONPath=`.status.policySnapshotIndexUsed`,name="Policy-Snapshot-Index",type=string
 // +kubebuilder:printcolumn:JSONPath=`.status.conditions[?(@.type=="Initialized")].status`,name="Initialized",type=string
 // +kubebuilder:printcolumn:JSONPath=`.status.conditions[?(@.type=="Succeeded")].status`,name="Succeeded",type=string
 // +kubebuilder:printcolumn:JSONPath=`.metadata.creationTimestamp`,name="Age",type=date

apis/placement/v1beta1/stageupdate_types.go

@@ -17,8 +17,8 @@ import (
 // +k8s:deepcopy-gen:interfaces=k8s.io/apimachinery/pkg/runtime.Object
 // +kubebuilder:storageversion
 // +kubebuilder:printcolumn:JSONPath=`.spec.placementName`,name="Placement",type=string
-// +kubebuilder:printcolumn:JSONPath=`.spec.resourceSnapshotIndex`,name="Resource-Snapshot",type=string
-// +kubebuilder:printcolumn:JSONPath=`.status.policySnapshotIndexUsed`,name="Policy-Snapshot",type=string
+// +kubebuilder:printcolumn:JSONPath=`.spec.resourceSnapshotIndex`,name="Resource-Snapshot-Index",type=string
+// +kubebuilder:printcolumn:JSONPath=`.status.policySnapshotIndexUsed`,name="Policy-Snapshot-Index",type=string
 // +kubebuilder:printcolumn:JSONPath=`.status.conditions[?(@.type=="Initialized")].status`,name="Initialized",type=string
 // +kubebuilder:printcolumn:JSONPath=`.status.conditions[?(@.type=="Succeeded")].status`,name="Succeeded",type=string
 // +kubebuilder:printcolumn:JSONPath=`.metadata.creationTimestamp`,name="Age",type=date

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

@@ -24,10 +24,10 @@ spec:
       name: Placement
       type: string
     - jsonPath: .spec.resourceSnapshotIndex
-      name: Resource-Snapshot
+      name: Resource-Snapshot-Index
       type: string
     - jsonPath: .status.policySnapshotIndexUsed
-      name: Policy-Snapshot
+      name: Policy-Snapshot-Index
       type: string
     - jsonPath: .status.conditions[?(@.type=="Initialized")].status
       name: Initialized
@@ -1244,10 +1244,10 @@ spec:
       name: Placement
       type: string
     - jsonPath: .spec.resourceSnapshotIndex
-      name: Resource-Snapshot
+      name: Resource-Snapshot-Index
       type: string
     - jsonPath: .status.policySnapshotIndexUsed
-      name: Policy-Snapshot
+      name: Policy-Snapshot-Index
       type: string
     - jsonPath: .status.conditions[?(@.type=="Initialized")].status
       name: Initialized

pkg/controllers/updaterun/execution.go

@@ -200,7 +200,8 @@ func (r *Reconciler) executeDeleteStage(
 	for i := range existingDeleteStageStatus.Clusters {
 		existingDeleteStageClusterMap[existingDeleteStageStatus.Clusters[i].ClusterName] = &existingDeleteStageStatus.Clusters[i]
 	}
-	deletingBinding := 0
+	// Mark the delete stage as started in case it's not.
+	markStageUpdatingStarted(updateRun.Status.DeletionStageStatus, updateRun.Generation)
 	for _, binding := range toBeDeletedBindings {
 		curCluster, exist := existingDeleteStageClusterMap[binding.Spec.TargetCluster]
 		if !exist {
@@ -225,7 +226,6 @@ func (r *Reconciler) executeDeleteStage(
 				klog.ErrorS(unexpectedErr, "The binding should be deleting before we mark a cluster deleting", "clusterStatus", curCluster, "clusterStagedUpdateRun", updateRunRef)
 				return false, fmt.Errorf("%w: %s", errStagedUpdatedAborted, unexpectedErr.Error())
 			}
-			deletingBinding++
 			continue
 		}
 		// The cluster status is not deleting yet
@@ -235,10 +235,6 @@ func (r *Reconciler) executeDeleteStage(
 		}
 		klog.V(2).InfoS("Deleted a binding pointing to a to be deleted cluster", "binding", klog.KObj(binding), "cluster", curCluster.ClusterName, "clusterStagedUpdateRun", updateRunRef)
 		markClusterUpdatingStarted(curCluster, updateRun.Generation)
-		if deletingBinding == 0 {
-			markStageUpdatingStarted(updateRun.Status.DeletionStageStatus, updateRun.Generation)
-		}
-		deletingBinding++
 	}
 	// The rest of the clusters in the stage are not in the toBeDeletedBindings so it should be marked as delete succeeded.
 	for _, clusterStatus := range existingDeleteStageClusterMap {

pkg/controllers/updaterun/initialization.go

@@ -126,26 +126,23 @@ func (r *Reconciler) determinePolicySnapshot(
 	}
 	updateRun.Status.PolicySnapshotIndexUsed = policyIndex
 
-	// Get the cluster count from the policy snapshot.
-	if latestPolicySnapshot.Spec.Policy == nil {
-		nopolicyErr := controller.NewUnexpectedBehaviorError(fmt.Errorf("policy snapshot `%s` does not have a policy", latestPolicySnapshot.Name))
-		klog.ErrorS(nopolicyErr, "Failed to get the policy from the latestPolicySnapshot", "clusterResourcePlacement", placementName, "latestPolicySnapshot", latestPolicySnapshot.Name, "clusterStagedUpdateRun", updateRunRef)
-		// no more retries here.
-		return nil, -1, fmt.Errorf("%w: %s", errInitializedFailed, nopolicyErr.Error())
-	}
-	// for pickAll policy, the observed cluster count is not included in the policy snapshot. We set it to -1. It will be validated in the binding stages.
+	// For pickAll policy, the observed cluster count is not included in the policy snapshot.
+	// We set it to -1. It will be validated in the binding stages.
+	// If policy is nil, it's default to pickAll.
 	clusterCount := -1
-	if latestPolicySnapshot.Spec.Policy.PlacementType == placementv1beta1.PickNPlacementType {
-		count, err := annotations.ExtractNumOfClustersFromPolicySnapshot(&latestPolicySnapshot)
-		if err != nil {
-			annErr := controller.NewUnexpectedBehaviorError(fmt.Errorf("%w: the policy snapshot `%s` doesn't have valid cluster count annotation", err, latestPolicySnapshot.Name))
-			klog.ErrorS(annErr, "Failed to get the cluster count from the latestPolicySnapshot", "clusterResourcePlacement", placementName, "latestPolicySnapshot", latestPolicySnapshot.Name, "clusterStagedUpdateRun", updateRunRef)
-			// no more retries here.
-			return nil, -1, fmt.Errorf("%w, %s", errInitializedFailed, annErr.Error())
+	if latestPolicySnapshot.Spec.Policy != nil {
+		if latestPolicySnapshot.Spec.Policy.PlacementType == placementv1beta1.PickNPlacementType {
+			count, err := annotations.ExtractNumOfClustersFromPolicySnapshot(&latestPolicySnapshot)
+			if err != nil {
+				annErr := controller.NewUnexpectedBehaviorError(fmt.Errorf("%w: the policy snapshot `%s` doesn't have valid cluster count annotation", err, latestPolicySnapshot.Name))
+				klog.ErrorS(annErr, "Failed to get the cluster count from the latestPolicySnapshot", "clusterResourcePlacement", placementName, "latestPolicySnapshot", latestPolicySnapshot.Name, "clusterStagedUpdateRun", updateRunRef)
+				// no more retries here.
+				return nil, -1, fmt.Errorf("%w, %s", errInitializedFailed, annErr.Error())
+			}
+			clusterCount = count
+		} else if latestPolicySnapshot.Spec.Policy.PlacementType == placementv1beta1.PickFixedPlacementType {
+			clusterCount = len(latestPolicySnapshot.Spec.Policy.ClusterNames)
 		}
-		clusterCount = count
-	} else if latestPolicySnapshot.Spec.Policy.PlacementType == placementv1beta1.PickFixedPlacementType {
-		clusterCount = len(latestPolicySnapshot.Spec.Policy.ClusterNames)
 	}
 	updateRun.Status.PolicyObservedClusterCount = clusterCount
 	klog.V(2).InfoS("Found the latest policy snapshot", "latestPolicySnapshot", latestPolicySnapshot.Name, "observedClusterCount", updateRun.Status.PolicyObservedClusterCount, "clusterStagedUpdateRun", updateRunRef)
@@ -209,6 +206,7 @@ func (r *Reconciler) collectScheduledClusters(
 
 	if updateRun.Status.PolicyObservedClusterCount == -1 {
 		// For pickAll policy, the observed cluster count is not included in the policy snapshot. We set it to the number of selected bindings.
+		// TODO (wantjian): refactor this part to update PolicyObservedClusterCount in one place.
 		updateRun.Status.PolicyObservedClusterCount = len(selectedBindings)
 	} else if updateRun.Status.PolicyObservedClusterCount != len(selectedBindings) {
 		countErr := controller.NewUnexpectedBehaviorError(fmt.Errorf("the number of selected bindings %d is not equal to the observed cluster count %d", len(selectedBindings), updateRun.Status.PolicyObservedClusterCount))

pkg/controllers/updaterun/initialization_integration_test.go

@@ -232,18 +232,6 @@ var _ = Describe("Updaterun initialization tests", func() {
 			validateFailedInitCondition(ctx, updateRun, "does not have a policy index label")
 		})
 
-		It("Should fail to initialize if the latest policy snapshot has a nil policy", func() {
-			By("Creating scheduling policy snapshot with nil policy")
-			policySnapshot.Spec.Policy = nil
-			Expect(k8sClient.Create(ctx, policySnapshot)).To(Succeed())
-
-			By("Creating a new clusterStagedUpdateRun")
-			Expect(k8sClient.Create(ctx, updateRun)).To(Succeed())
-
-			By("Validating the initialization failed")
-			validateFailedInitCondition(ctx, updateRun, "does not have a policy")
-		})
-
 		It("Should fail to initialize if the latest policy snapshot does not have valid cluster count annotation", func() {
 			By("Creating scheduling policy snapshot with invalid cluster count annotation")
 			delete(policySnapshot.Annotations, placementv1beta1.NumberOfClustersAnnotation)

test/e2e/actuals_test.go

@@ -653,6 +653,7 @@ func crpStatusWithOverrideUpdatedFailedActual(
 		return nil
 	}
 }
+
 func crpStatusWithWorkSynchronizedUpdatedFailedActual(
 	wantSelectedResourceIdentifiers []placementv1beta1.ResourceIdentifier,
 	wantSelectedClusters []string,
@@ -949,3 +950,184 @@ func validateCRPSnapshotRevisions(crpName string, wantPolicySnapshotRevision, wa
 	}
 	return nil
 }
+
+func updateRunClusterRolloutSucceedConditions(generation int64) []metav1.Condition {
+	return []metav1.Condition{
+		{
+			Type:               string(placementv1beta1.ClusterUpdatingConditionStarted),
+			Status:             metav1.ConditionTrue,
+			Reason:             condition.ClusterUpdatingStartedReason,
+			ObservedGeneration: generation,
+		},
+		{
+			Type:               string(placementv1beta1.ClusterUpdatingConditionSucceeded),
+			Status:             metav1.ConditionTrue,
+			Reason:             condition.ClusterUpdatingSucceededReason,
+			ObservedGeneration: generation,
+		},
+	}
+}
+
+func updateRunStageRolloutSucceedConditions(generation int64, wait bool) []metav1.Condition {
+	startedCond := metav1.Condition{
+		Type:               string(placementv1beta1.StageUpdatingConditionProgressing),
+		Status:             metav1.ConditionTrue,
+		Reason:             condition.StageUpdatingStartedReason,
+		ObservedGeneration: generation,
+	}
+	if wait {
+		startedCond.Status = metav1.ConditionFalse
+		startedCond.Reason = condition.StageUpdatingWaitingReason
+	}
+	return []metav1.Condition{
+		startedCond,
+		{
+			Type:               string(placementv1beta1.StageUpdatingConditionSucceeded),
+			Status:             metav1.ConditionTrue,
+			Reason:             condition.StageUpdatingSucceededReason,
+			ObservedGeneration: generation,
+		},
+	}
+}
+
+func updateRunAfterStageTaskSucceedConditions(generation int64, taskType placementv1beta1.AfterStageTaskType) []metav1.Condition {
+	if taskType == placementv1beta1.AfterStageTaskTypeApproval {
+		return []metav1.Condition{
+			{
+				Type:               string(placementv1beta1.AfterStageTaskConditionApprovalRequestCreated),
+				Status:             metav1.ConditionTrue,
+				Reason:             condition.AfterStageTaskApprovalRequestCreatedReason,
+				ObservedGeneration: generation,
+			},
+			{
+				Type:               string(placementv1beta1.AfterStageTaskConditionApprovalRequestApproved),
+				Status:             metav1.ConditionTrue,
+				Reason:             condition.AfterStageTaskApprovalRequestApprovedReason,
+				ObservedGeneration: generation,
+			},
+		}
+	}
+	return []metav1.Condition{
+		{
+			Type:               string(placementv1beta1.AfterStageTaskConditionWaitTimeElapsed),
+			Status:             metav1.ConditionTrue,
+			Reason:             condition.AfterStageTaskWaitTimeElapsedReason,
+			ObservedGeneration: generation,
+		},
+	}
+}
+
+func updateRunSucceedConditions(generation int64) []metav1.Condition {
+	return []metav1.Condition{
+		{
+			Type:               string(placementv1beta1.StagedUpdateRunConditionInitialized),
+			Status:             metav1.ConditionTrue,
+			Reason:             condition.UpdateRunInitializeSucceededReason,
+			ObservedGeneration: generation,
+		},
+		{
+			Type:               string(placementv1beta1.StagedUpdateRunConditionProgressing),
+			Status:             metav1.ConditionTrue,
+			Reason:             condition.UpdateRunStartedReason,
+			ObservedGeneration: generation,
+		},
+		{
+			Type:               string(placementv1beta1.StagedUpdateRunConditionSucceeded),
+			Status:             metav1.ConditionTrue,
+			Reason:             condition.UpdateRunSucceededReason,
+			ObservedGeneration: generation,
+		},
+	}
+}
+
+func updateRunStatusSucceededActual(
+	updateRunName string,
+	wantPolicyIndex string,
+	wantClusterCount int,
+	wantApplyStrategy *placementv1beta1.ApplyStrategy,
+	wantStrategySpec *placementv1beta1.StagedUpdateStrategySpec,
+	wantSelectedClusters [][]string,
+	wantUnscheduledClusters []string,
+	wantCROs map[string][]string,
+	wantROs map[string][]placementv1beta1.NamespacedName,
+) func() error {
+	return func() error {
+		updateRun := &placementv1beta1.ClusterStagedUpdateRun{}
+		if err := hubClient.Get(ctx, types.NamespacedName{Name: updateRunName}, updateRun); err != nil {
+			return err
+		}
+
+		wantStatus := placementv1beta1.StagedUpdateRunStatus{
+			PolicySnapshotIndexUsed:      wantPolicyIndex,
+			PolicyObservedClusterCount:   wantClusterCount,
+			ApplyStrategy:                wantApplyStrategy.DeepCopy(),
+			StagedUpdateStrategySnapshot: wantStrategySpec,
+		}
+		stagesStatus := make([]placementv1beta1.StageUpdatingStatus, len(wantStrategySpec.Stages))
+		for i, stage := range wantStrategySpec.Stages {
+			stagesStatus[i].StageName = stage.Name
+			stagesStatus[i].Clusters = make([]placementv1beta1.ClusterUpdatingStatus, len(wantSelectedClusters[i]))
+			for j := range stagesStatus[i].Clusters {
+				stagesStatus[i].Clusters[j].ClusterName = wantSelectedClusters[i][j]
+				stagesStatus[i].Clusters[j].ClusterResourceOverrideSnapshots = wantCROs[wantSelectedClusters[i][j]]
+				stagesStatus[i].Clusters[j].ResourceOverrideSnapshots = wantROs[wantSelectedClusters[i][j]]
+				stagesStatus[i].Clusters[j].Conditions = updateRunClusterRolloutSucceedConditions(updateRun.Generation)
+			}
+			stagesStatus[i].AfterStageTaskStatus = make([]placementv1beta1.AfterStageTaskStatus, len(stage.AfterStageTasks))
+			for j, task := range stage.AfterStageTasks {
+				stagesStatus[i].AfterStageTaskStatus[j].Type = task.Type
+				if task.Type == placementv1beta1.AfterStageTaskTypeApproval {
+					stagesStatus[i].AfterStageTaskStatus[j].ApprovalRequestName = fmt.Sprintf(placementv1beta1.ApprovalTaskNameFmt, updateRun.Name, stage.Name)
+				}
+				stagesStatus[i].AfterStageTaskStatus[j].Conditions = updateRunAfterStageTaskSucceedConditions(updateRun.Generation, task.Type)
+			}
+			stagesStatus[i].Conditions = updateRunStageRolloutSucceedConditions(updateRun.Generation, true)
+		}
+
+		deleteStageStatus := &placementv1beta1.StageUpdatingStatus{
+			StageName: "kubernetes-fleet.io/deleteStage",
+		}
+		deleteStageStatus.Clusters = make([]placementv1beta1.ClusterUpdatingStatus, len(wantUnscheduledClusters))
+		for i := range deleteStageStatus.Clusters {
+			deleteStageStatus.Clusters[i].ClusterName = wantUnscheduledClusters[i]
+			deleteStageStatus.Clusters[i].Conditions = updateRunClusterRolloutSucceedConditions(updateRun.Generation)
+		}
+		deleteStageStatus.Conditions = updateRunStageRolloutSucceedConditions(updateRun.Generation, false)
+
+		wantStatus.StagesStatus = stagesStatus
+		wantStatus.DeletionStageStatus = deleteStageStatus
+		wantStatus.Conditions = updateRunSucceedConditions(updateRun.Generation)
+		if diff := cmp.Diff(updateRun.Status, wantStatus, updateRunStatusCmpOption...); diff != "" {
+			return fmt.Errorf("CRP status diff (-got, +want): %s", diff)
+		}
+		return nil
+	}
+}
+
+func updateRunAndApprovalRequestsRemovedActual(updateRunName string) func() error {
+	return func() error {
+		if err := hubClient.Get(ctx, types.NamespacedName{Name: updateRunName}, &placementv1beta1.ClusterStagedUpdateRun{}); !errors.IsNotFound(err) {
+			return fmt.Errorf("UpdateRun still exists or an unexpected error occurred: %w", err)
+		}
+
+		appReqList := &placementv1beta1.ClusterApprovalRequestList{}
+		if err := hubClient.List(ctx, appReqList, client.MatchingLabels{
+			placementv1beta1.TargetUpdateRunLabel: updateRunName,
+		}); err != nil {
+			return fmt.Errorf("failed to list ClusterApprovalRequests: %w", err)
+		}
+		if len(appReqList.Items) > 0 {
+			return fmt.Errorf("ClusterApprovalRequests still exist: %v", appReqList.Items)
+		}
+		return nil
+	}
+}
+
+func updateRunStrategyRemovedActual(strategyName string) func() error {
+	return func() error {
+		if err := hubClient.Get(ctx, types.NamespacedName{Name: strategyName}, &placementv1beta1.ClusterStagedUpdateStrategy{}); !errors.IsNotFound(err) {
+			return fmt.Errorf("ClusterStagedUpdateStrategy still exists or an unexpected error occurred: %w", err)
+		}
+		return nil
+	}
+}

test/e2e/resources_test.go

@@ -34,6 +34,8 @@ const (
 	internalServiceImportNameTemplate = "isi-%d"
 	endpointSliceExportNameTemplate   = "ep-%d"
 	crpEvictionNameTemplate           = "crpe-%d"
+	updateRunStrategyNameTemplate     = "curs-%d"
+	updateRunNameWithSubIndexTemplate = "cur-%d-%d"
 
 	customDeletionBlockerFinalizer = "custom-deletion-blocker-finalizer"
 	workNamespaceLabelName         = "process"

test/e2e/setup_test.go

@@ -211,6 +211,12 @@ var (
 		ignoreClusterNameField,
 		cmpopts.EquateEmpty(),
 	}
+
+	updateRunStatusCmpOption = cmp.Options{
+		utils.IgnoreConditionLTTAndMessageFields,
+		cmpopts.IgnoreFields(placementv1beta1.StageUpdatingStatus{}, "StartTime", "EndTime"),
+		cmpopts.EquateEmpty(),
+	}
 )
 
 // TestMain sets up the E2E test environment.