diff --git a/apis/placement/v1beta1/commons.go b/apis/placement/v1beta1/commons.go index 03e8e9dc0..9ced5c1b5 100644 --- a/apis/placement/v1beta1/commons.go +++ b/apis/placement/v1beta1/commons.go @@ -167,8 +167,11 @@ const ( // TargetUpdatingStageNameLabel indicates the updating stage name on a staged run related object. TargetUpdatingStageNameLabel = FleetPrefix + "targetUpdatingStage" - // ApprovalTaskNameFmt is the format of the approval task name. - ApprovalTaskNameFmt = "%s-%s" + // BeforeStageApprovalTaskNameFmt is the format of the before stage approval task name. + BeforeStageApprovalTaskNameFmt = "%s-before-%s" + + // AfterStageApprovalTaskNameFmt is the format of the after stage approval task name. + AfterStageApprovalTaskNameFmt = "%s-after-%s" ) var ( diff --git a/pkg/controllers/updaterun/controller.go b/pkg/controllers/updaterun/controller.go index 9a0ee4862..4e8fa695c 100644 --- a/pkg/controllers/updaterun/controller.go +++ b/pkg/controllers/updaterun/controller.go @@ -79,7 +79,7 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim } runObjRef := klog.KObj(updateRun) - // Remove waitTime from the updateRun status for AfterStageTask for type Approval. + // Remove waitTime from the updateRun status for BeforeStageTask and AfterStageTask for type Approval. removeWaitTimeFromUpdateRunStatus(updateRun) // Handle the deletion of the updateRun. @@ -455,10 +455,15 @@ func emitUpdateRunStatusMetric(updateRun placementv1beta1.UpdateRunObj) { } func removeWaitTimeFromUpdateRunStatus(updateRun placementv1beta1.UpdateRunObj) { - // Remove waitTime from the updateRun status for AfterStageTask for type Approval. + // Remove waitTime from the updateRun status for BeforeStageTask and AfterStageTask for type Approval. updateRunStatus := updateRun.GetUpdateRunStatus() if updateRunStatus.UpdateStrategySnapshot != nil { for i := range updateRunStatus.UpdateStrategySnapshot.Stages { + for j := range updateRunStatus.UpdateStrategySnapshot.Stages[i].BeforeStageTasks { + if updateRunStatus.UpdateStrategySnapshot.Stages[i].BeforeStageTasks[j].Type == placementv1beta1.StageTaskTypeApproval { + updateRunStatus.UpdateStrategySnapshot.Stages[i].BeforeStageTasks[j].WaitTime = nil + } + } for j := range updateRunStatus.UpdateStrategySnapshot.Stages[i].AfterStageTasks { if updateRunStatus.UpdateStrategySnapshot.Stages[i].AfterStageTasks[j].Type == placementv1beta1.StageTaskTypeApproval { updateRunStatus.UpdateStrategySnapshot.Stages[i].AfterStageTasks[j].WaitTime = nil diff --git a/pkg/controllers/updaterun/controller_integration_test.go b/pkg/controllers/updaterun/controller_integration_test.go index 8c8e38d06..88cd5f774 100644 --- a/pkg/controllers/updaterun/controller_integration_test.go +++ b/pkg/controllers/updaterun/controller_integration_test.go @@ -543,7 +543,7 @@ func generateTestClusterStagedUpdateStrategy() *placementv1beta1.ClusterStagedUp } } -func generateTestClusterStagedUpdateStrategyWithSingleStage(afterStageTasks []placementv1beta1.StageTask) *placementv1beta1.ClusterStagedUpdateStrategy { +func generateTestClusterStagedUpdateStrategyWithSingleStage(beforeStageTasks, afterStageTasks []placementv1beta1.StageTask) *placementv1beta1.ClusterStagedUpdateStrategy { return &placementv1beta1.ClusterStagedUpdateStrategy{ ObjectMeta: metav1.ObjectMeta{ Name: testUpdateStrategyName, @@ -551,9 +551,10 @@ func generateTestClusterStagedUpdateStrategyWithSingleStage(afterStageTasks []pl Spec: placementv1beta1.UpdateStrategySpec{ Stages: []placementv1beta1.StageConfig{ { - Name: "stage1", - LabelSelector: &metav1.LabelSelector{}, // Select all clusters. - AfterStageTasks: afterStageTasks, + Name: "stage1", + LabelSelector: &metav1.LabelSelector{}, // Select all clusters. + AfterStageTasks: afterStageTasks, + BeforeStageTasks: beforeStageTasks, }, }, }, @@ -724,9 +725,9 @@ func generateTrueCondition(obj client.Object, condType any) metav1.Condition { case placementv1beta1.StageTaskConditionWaitTimeElapsed: reason = condition.AfterStageTaskWaitTimeElapsedReason case placementv1beta1.StageTaskConditionApprovalRequestCreated: - reason = condition.AfterStageTaskApprovalRequestCreatedReason + reason = condition.StageTaskApprovalRequestCreatedReason case placementv1beta1.StageTaskConditionApprovalRequestApproved: - reason = condition.AfterStageTaskApprovalRequestApprovedReason + reason = condition.StageTaskApprovalRequestApprovedReason } typeStr = string(cond) case placementv1beta1.ApprovalRequestConditionType: diff --git a/pkg/controllers/updaterun/controller_test.go b/pkg/controllers/updaterun/controller_test.go index f842960f2..7fe56913f 100644 --- a/pkg/controllers/updaterun/controller_test.go +++ b/pkg/controllers/updaterun/controller_test.go @@ -912,7 +912,7 @@ func TestRemoveWaitTimeFromUpdateRunStatus(t *testing.T) { }, }, }, - "should remove waitTime from Approval tasks only": { + "should remove waitTime from Approval tasks only for AfterStageTasks": { inputUpdateRun: &placementv1beta1.ClusterStagedUpdateRun{ Status: placementv1beta1.UpdateRunStatus{ UpdateStrategySnapshot: &placementv1beta1.UpdateStrategySpec{ @@ -953,12 +953,51 @@ func TestRemoveWaitTimeFromUpdateRunStatus(t *testing.T) { }, }, }, + "should remove waitTime from Approval tasks only for BeforeStageTasks": { + inputUpdateRun: &placementv1beta1.ClusterStagedUpdateRun{ + Status: placementv1beta1.UpdateRunStatus{ + UpdateStrategySnapshot: &placementv1beta1.UpdateStrategySpec{ + Stages: []placementv1beta1.StageConfig{ + { + BeforeStageTasks: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + WaitTime: &waitTime, + }, + }, + }, + }, + }, + }, + }, + wantUpdateRun: &placementv1beta1.ClusterStagedUpdateRun{ + Status: placementv1beta1.UpdateRunStatus{ + UpdateStrategySnapshot: &placementv1beta1.UpdateStrategySpec{ + Stages: []placementv1beta1.StageConfig{ + { + BeforeStageTasks: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + }, + }, + }, + }, + }, + }, + }, "should handle multiple stages": { inputUpdateRun: &placementv1beta1.ClusterStagedUpdateRun{ Status: placementv1beta1.UpdateRunStatus{ UpdateStrategySnapshot: &placementv1beta1.UpdateStrategySpec{ Stages: []placementv1beta1.StageConfig{ { + BeforeStageTasks: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + WaitTime: &waitTime, + }, + }, AfterStageTasks: []placementv1beta1.StageTask{ { Type: placementv1beta1.StageTaskTypeApproval, @@ -978,6 +1017,14 @@ func TestRemoveWaitTimeFromUpdateRunStatus(t *testing.T) { }, }, }, + { + BeforeStageTasks: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + WaitTime: &waitTime, + }, + }, + }, }, }, }, @@ -987,6 +1034,11 @@ func TestRemoveWaitTimeFromUpdateRunStatus(t *testing.T) { UpdateStrategySnapshot: &placementv1beta1.UpdateStrategySpec{ Stages: []placementv1beta1.StageConfig{ { + BeforeStageTasks: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + }, AfterStageTasks: []placementv1beta1.StageTask{ { Type: placementv1beta1.StageTaskTypeApproval, @@ -1004,6 +1056,13 @@ func TestRemoveWaitTimeFromUpdateRunStatus(t *testing.T) { }, }, }, + { + BeforeStageTasks: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + }, + }, }, }, }, diff --git a/pkg/controllers/updaterun/execution.go b/pkg/controllers/updaterun/execution.go index 198980749..62de3664e 100644 --- a/pkg/controllers/updaterun/execution.go +++ b/pkg/controllers/updaterun/execution.go @@ -62,34 +62,80 @@ func (r *Reconciler) execute( updateRun placementv1beta1.UpdateRunObj, updatingStageIndex int, toBeUpdatedBindings, toBeDeletedBindings []placementv1beta1.BindingObj, -) (bool, time.Duration, error) { +) (finished bool, waitTime time.Duration, err error) { + updateRunStatus := updateRun.GetUpdateRunStatus() + var updatingStage *placementv1beta1.StageUpdatingStatus + + // Set up defer function to handle errStagedUpdatedAborted + defer func() { + if errors.Is(err, errStagedUpdatedAborted) { + if updatingStage != nil { + markStageUpdatingFailed(updatingStage, updateRun.GetGeneration(), err.Error()) + } else { + // Handle deletion stage case + markStageUpdatingFailed(updateRunStatus.DeletionStageStatus, updateRun.GetGeneration(), err.Error()) + } + waitTime = 0 + finished = true + } + }() + // Mark updateRun as progressing if it's not already marked as waiting or stuck. // This avoids triggering an unnecessary in-memory transition from stuck (waiting) -> progressing -> stuck (waiting), // which would update the lastTransitionTime even though the status hasn't effectively changed. markUpdateRunProgressingIfNotWaitingOrStuck(updateRun) - - updateRunStatus := updateRun.GetUpdateRunStatus() if updatingStageIndex < len(updateRunStatus.StagesStatus) { maxConcurrency, err := calculateMaxConcurrencyValue(updateRunStatus, updatingStageIndex) if err != nil { return false, 0, err } - updatingStage := &updateRunStatus.StagesStatus[updatingStageIndex] - waitTime, execErr := r.executeUpdatingStage(ctx, updateRun, updatingStageIndex, toBeUpdatedBindings, maxConcurrency) - if errors.Is(execErr, errStagedUpdatedAborted) { - markStageUpdatingFailed(updatingStage, updateRun.GetGeneration(), execErr.Error()) - return true, waitTime, execErr + updatingStage = &updateRunStatus.StagesStatus[updatingStageIndex] + approved, err := r.checkBeforeStageTasksStatus(ctx, updatingStageIndex, updateRun) + if err != nil { + return false, 0, err } + if !approved { + markStageUpdatingWaiting(updatingStage, updateRun.GetGeneration(), "Not all before-stage tasks are completed, waiting for approval") + markUpdateRunWaiting(updateRun, fmt.Sprintf(condition.UpdateRunWaitingMessageFmt, "before-stage", updatingStage.StageName)) + return false, stageUpdatingWaitTime, nil + } + waitTime, err = r.executeUpdatingStage(ctx, updateRun, updatingStageIndex, toBeUpdatedBindings, maxConcurrency) // The execution has not finished yet. - return false, waitTime, execErr + return false, waitTime, err } // All the stages have finished, now start the delete stage. - finished, execErr := r.executeDeleteStage(ctx, updateRun, toBeDeletedBindings) - if errors.Is(execErr, errStagedUpdatedAborted) { - markStageUpdatingFailed(updateRunStatus.DeletionStageStatus, updateRun.GetGeneration(), execErr.Error()) - return true, 0, execErr + finished, err = r.executeDeleteStage(ctx, updateRun, toBeDeletedBindings) + return finished, clusterUpdatingWaitTime, err +} + +// checkBeforeStageTasksStatus checks if the before stage tasks have finished. +// It returns if the before stage tasks have finished or error if the before stage tasks failed. +func (r *Reconciler) checkBeforeStageTasksStatus(ctx context.Context, updatingStageIndex int, updateRun placementv1beta1.UpdateRunObj) (bool, error) { + updateRunRef := klog.KObj(updateRun) + updateRunStatus := updateRun.GetUpdateRunStatus() + updatingStageStatus := &updateRunStatus.StagesStatus[updatingStageIndex] + updatingStage := &updateRunStatus.UpdateStrategySnapshot.Stages[updatingStageIndex] + if updatingStage.BeforeStageTasks == nil { + klog.V(2).InfoS("There is no before stage task for this stage", "stage", updatingStage.Name, "updateRun", updateRunRef) + return true, nil } - return finished, clusterUpdatingWaitTime, execErr + + for i, task := range updatingStage.BeforeStageTasks { + switch task.Type { + case placementv1beta1.StageTaskTypeApproval: + approved, err := r.handleStageApprovalTask(ctx, &updatingStageStatus.BeforeStageTaskStatus[i], updatingStage, updateRun) + if err != nil { + return false, err + } + return approved, nil // Ideally there should be only one approval task in before stage tasks. + default: + // Approval is the only supported before stage task. + unexpectedErr := controller.NewUnexpectedBehaviorError(fmt.Errorf("found unsupported task type in before stage tasks: %s", task.Type)) + klog.ErrorS(unexpectedErr, "Task type is not supported in before stage tasks", "stage", updatingStage.Name, "updateRun", updateRunRef, "taskType", task.Type) + return false, fmt.Errorf("%w: %s", errStagedUpdatedAborted, unexpectedErr.Error()) + } + } + return true, nil } // executeUpdatingStage executes a single updating stage by updating the bindings. @@ -242,8 +288,8 @@ func (r *Reconciler) executeUpdatingStage( if finishedClusterCount == len(updatingStageStatus.Clusters) { // All the clusters in the stage have been updated. - markUpdateRunWaiting(updateRun, updatingStageStatus.StageName) - markStageUpdatingWaiting(updatingStageStatus, updateRun.GetGeneration()) + markUpdateRunWaiting(updateRun, fmt.Sprintf(condition.UpdateRunWaitingMessageFmt, "after-stage", updatingStageStatus.StageName)) + markStageUpdatingWaiting(updatingStageStatus, updateRun.GetGeneration(), "All clusters in the stage are updated, waiting for after-stage tasks to complete") klog.V(2).InfoS("The stage has finished all cluster updating", "stage", updatingStageStatus.StageName, "updateRun", updateRunRef) // Check if the after stage tasks are ready. approved, waitTime, err := r.checkAfterStageTasksStatus(ctx, updatingStageIndex, updateRun) @@ -360,59 +406,11 @@ func (r *Reconciler) checkAfterStageTasksStatus(ctx context.Context, updatingSta klog.V(2).InfoS("The after stage wait task has completed", "stage", updatingStage.Name, "updateRun", updateRunRef) } case placementv1beta1.StageTaskTypeApproval: - afterStageTaskApproved := condition.IsConditionStatusTrue(meta.FindStatusCondition(updatingStageStatus.AfterStageTaskStatus[i].Conditions, string(placementv1beta1.StageTaskConditionApprovalRequestApproved)), updateRun.GetGeneration()) - if afterStageTaskApproved { - // The afterStageTask has been approved. - continue + approved, err := r.handleStageApprovalTask(ctx, &updatingStageStatus.AfterStageTaskStatus[i], updatingStage, updateRun) + if err != nil { + return false, -1, err } - // Check if the approval request has been created. - approvalRequest := buildApprovalRequestObject(types.NamespacedName{Name: updatingStageStatus.AfterStageTaskStatus[i].ApprovalRequestName, Namespace: updateRun.GetNamespace()}, updatingStage.Name, updateRun.GetName()) - requestRef := klog.KObj(approvalRequest) - if err := r.Client.Create(ctx, approvalRequest); err != nil { - if apierrors.IsAlreadyExists(err) { - // The approval task already exists. - markAfterStageRequestCreated(&updatingStageStatus.AfterStageTaskStatus[i], updateRun.GetGeneration()) - if err = r.Client.Get(ctx, client.ObjectKeyFromObject(approvalRequest), approvalRequest); err != nil { - klog.ErrorS(err, "Failed to get the already existing approval request", "approvalRequest", requestRef, "stage", updatingStage.Name, "updateRun", updateRunRef) - return false, -1, controller.NewAPIServerError(true, err) - } - approvalRequestSpec := approvalRequest.GetApprovalRequestSpec() - if approvalRequestSpec.TargetStage != updatingStage.Name || approvalRequestSpec.TargetUpdateRun != updateRun.GetName() { - unexpectedErr := controller.NewUnexpectedBehaviorError(fmt.Errorf("the approval request task `%s/%s` is targeting update run `%s/%s` and stage `%s`", approvalRequest.GetNamespace(), approvalRequest.GetName(), approvalRequest.GetNamespace(), approvalRequestSpec.TargetUpdateRun, approvalRequestSpec.TargetStage)) - klog.ErrorS(unexpectedErr, "Found an approval request targeting wrong stage", "approvalRequestTask", requestRef, "stage", updatingStage.Name, "updateRun", updateRunRef) - return false, -1, fmt.Errorf("%w: %s", errStagedUpdatedAborted, unexpectedErr.Error()) - } - approvalRequestStatus := approvalRequest.GetApprovalRequestStatus() - approvalAccepted := condition.IsConditionStatusTrue(meta.FindStatusCondition(approvalRequestStatus.Conditions, string(placementv1beta1.ApprovalRequestConditionApprovalAccepted)), approvalRequest.GetGeneration()) - approved := condition.IsConditionStatusTrue(meta.FindStatusCondition(approvalRequestStatus.Conditions, string(placementv1beta1.ApprovalRequestConditionApproved)), approvalRequest.GetGeneration()) - if !approvalAccepted && !approved { - klog.V(2).InfoS("The approval request has not been approved yet", "approvalRequestTask", requestRef, "stage", updatingStage.Name, "updateRun", updateRunRef) - passed = false - continue - } - if approved { - klog.V(2).InfoS("The approval request has been approved", "approvalRequestTask", requestRef, "stage", updatingStage.Name, "updateRun", updateRunRef) - if !approvalAccepted { - if err = r.updateApprovalRequestAccepted(ctx, approvalRequest); err != nil { - klog.ErrorS(err, "Failed to accept the approved approval request", "approvalRequest", requestRef, "stage", updatingStage.Name, "updateRun", updateRunRef) - // retriable err - return false, -1, err - } - } - } else { - // Approved state should not change once the approval is accepted. - klog.V(2).InfoS("The approval request has been approval-accepted, ignoring changing back to unapproved", "approvalRequestTask", requestRef, "stage", updatingStage.Name, "updateRun", updateRunRef) - } - markAfterStageRequestApproved(&updatingStageStatus.AfterStageTaskStatus[i], updateRun.GetGeneration()) - } else { - // retriable error - klog.ErrorS(err, "Failed to create the approval request", "approvalRequest", requestRef, "stage", updatingStage.Name, "updateRun", updateRunRef) - return false, -1, controller.NewAPIServerError(false, err) - } - } else { - // The approval request has been created for the first time. - klog.V(2).InfoS("The approval request has been created", "approvalRequestTask", requestRef, "stage", updatingStage.Name, "updateRun", updateRunRef) - markAfterStageRequestCreated(&updatingStageStatus.AfterStageTaskStatus[i], updateRun.GetGeneration()) + if !approved { passed = false } } @@ -423,6 +421,74 @@ func (r *Reconciler) checkAfterStageTasksStatus(ctx context.Context, updatingSta return passed, afterStageWaitTime, nil } +// handleStageApprovalTask handles the approval task logic for before or after stage tasks. +// It returns true if the task is approved, false otherwise, and any error encountered. +func (r *Reconciler) handleStageApprovalTask( + ctx context.Context, + stageTaskStatus *placementv1beta1.StageTaskStatus, + updatingStage *placementv1beta1.StageConfig, + updateRun placementv1beta1.UpdateRunObj, +) (bool, error) { + updateRunRef := klog.KObj(updateRun) + + stageTaskApproved := condition.IsConditionStatusTrue(meta.FindStatusCondition(stageTaskStatus.Conditions, string(placementv1beta1.StageTaskConditionApprovalRequestApproved)), updateRun.GetGeneration()) + if stageTaskApproved { + // The stageTask has been approved. + return true, nil + } + + // Check if the approval request has been created. + approvalRequest := buildApprovalRequestObject(types.NamespacedName{Name: stageTaskStatus.ApprovalRequestName, Namespace: updateRun.GetNamespace()}, updatingStage.Name, updateRun.GetName()) + requestRef := klog.KObj(approvalRequest) + if err := r.Client.Create(ctx, approvalRequest); err != nil { + if apierrors.IsAlreadyExists(err) { + // The approval task already exists. + markStageTaskRequestCreated(stageTaskStatus, updateRun.GetGeneration()) + if err = r.Client.Get(ctx, client.ObjectKeyFromObject(approvalRequest), approvalRequest); err != nil { + klog.ErrorS(err, "Failed to get the already existing approval request", "approvalRequest", requestRef, "stage", updatingStage.Name, "updateRun", updateRunRef) + return false, controller.NewAPIServerError(true, err) + } + approvalRequestSpec := approvalRequest.GetApprovalRequestSpec() + if approvalRequestSpec.TargetStage != updatingStage.Name || approvalRequestSpec.TargetUpdateRun != updateRun.GetName() { + unexpectedErr := controller.NewUnexpectedBehaviorError(fmt.Errorf("the approval request task `%s/%s` is targeting update run `%s/%s` and stage `%s`, want target update run `%s/%s and stage `%s`", approvalRequest.GetNamespace(), approvalRequest.GetName(), approvalRequest.GetNamespace(), approvalRequestSpec.TargetUpdateRun, approvalRequestSpec.TargetStage, approvalRequest.GetNamespace(), updateRun.GetName(), updatingStage.Name)) + klog.ErrorS(unexpectedErr, "Found an approval request targeting wrong stage", "approvalRequestTask", requestRef, "stage", updatingStage.Name, "updateRun", updateRunRef) + return false, fmt.Errorf("%w: %s", errStagedUpdatedAborted, unexpectedErr.Error()) + } + approvalRequestStatus := approvalRequest.GetApprovalRequestStatus() + approvalAccepted := condition.IsConditionStatusTrue(meta.FindStatusCondition(approvalRequestStatus.Conditions, string(placementv1beta1.ApprovalRequestConditionApprovalAccepted)), approvalRequest.GetGeneration()) + approved := condition.IsConditionStatusTrue(meta.FindStatusCondition(approvalRequestStatus.Conditions, string(placementv1beta1.ApprovalRequestConditionApproved)), approvalRequest.GetGeneration()) + if !approvalAccepted && !approved { + klog.V(2).InfoS("The approval request has not been approved yet", "approvalRequestTask", requestRef, "stage", updatingStage.Name, "updateRun", updateRunRef) + return false, nil + } + if approved { + klog.V(2).InfoS("The approval request has been approved", "approvalRequestTask", requestRef, "stage", updatingStage.Name, "updateRun", updateRunRef) + if !approvalAccepted { + if err = r.updateApprovalRequestAccepted(ctx, approvalRequest); err != nil { + klog.ErrorS(err, "Failed to accept the approved approval request", "approvalRequest", requestRef, "stage", updatingStage.Name, "updateRun", updateRunRef) + // retriable err + return false, err + } + } + } else { + // Approved state should not change once the approval is accepted. + klog.V(2).InfoS("The approval request has been approval-accepted, ignoring changing back to unapproved", "approvalRequestTask", requestRef, "stage", updatingStage.Name, "updateRun", updateRunRef) + } + markStageTaskRequestApproved(stageTaskStatus, updateRun.GetGeneration()) + } else { + // retriable error + klog.ErrorS(err, "Failed to create the approval request", "approvalRequest", requestRef, "stage", updatingStage.Name, "updateRun", updateRunRef) + return false, controller.NewAPIServerError(false, err) + } + } else { + // The approval request has been created for the first time. + klog.V(2).InfoS("The approval request has been created", "approvalRequestTask", requestRef, "stage", updatingStage.Name, "updateRun", updateRunRef) + markStageTaskRequestCreated(stageTaskStatus, updateRun.GetGeneration()) + return false, nil + } + return true, nil +} + // updateBindingRolloutStarted updates the binding status to indicate the rollout has started. func (r *Reconciler) updateBindingRolloutStarted(ctx context.Context, binding placementv1beta1.BindingObj, updateRun placementv1beta1.UpdateRunObj) error { // first reset the condition to reflect the latest lastTransitionTime @@ -616,14 +682,14 @@ func markUpdateRunStuck(updateRun placementv1beta1.UpdateRunObj, stageName, clus } // markUpdateRunWaiting marks the updateRun as waiting in memory. -func markUpdateRunWaiting(updateRun placementv1beta1.UpdateRunObj, stageName string) { +func markUpdateRunWaiting(updateRun placementv1beta1.UpdateRunObj, message string) { updateRunStatus := updateRun.GetUpdateRunStatus() meta.SetStatusCondition(&updateRunStatus.Conditions, metav1.Condition{ Type: string(placementv1beta1.StagedUpdateRunConditionProgressing), Status: metav1.ConditionFalse, ObservedGeneration: updateRun.GetGeneration(), Reason: condition.UpdateRunWaitingReason, - Message: fmt.Sprintf("The updateRun is waiting for after-stage tasks in stage %s to complete", stageName), + Message: message, }) } @@ -642,13 +708,13 @@ func markStageUpdatingStarted(stageUpdatingStatus *placementv1beta1.StageUpdatin } // markStageUpdatingWaiting marks the stage updating status as waiting in memory. -func markStageUpdatingWaiting(stageUpdatingStatus *placementv1beta1.StageUpdatingStatus, generation int64) { +func markStageUpdatingWaiting(stageUpdatingStatus *placementv1beta1.StageUpdatingStatus, generation int64, message string) { meta.SetStatusCondition(&stageUpdatingStatus.Conditions, metav1.Condition{ Type: string(placementv1beta1.StageUpdatingConditionProgressing), Status: metav1.ConditionFalse, ObservedGeneration: generation, Reason: condition.StageUpdatingWaitingReason, - Message: "All clusters in the stage are updated, waiting for after-stage tasks to complete", + Message: message, }) } @@ -727,24 +793,24 @@ func markClusterUpdatingFailed(clusterUpdatingStatus *placementv1beta1.ClusterUp }) } -// markAfterStageRequestCreated marks the Approval after stage task as ApprovalRequestCreated in memory. -func markAfterStageRequestCreated(afterStageTaskStatus *placementv1beta1.StageTaskStatus, generation int64) { - meta.SetStatusCondition(&afterStageTaskStatus.Conditions, metav1.Condition{ +// markStageTaskRequestCreated marks the Approval for the before or after stage task as ApprovalRequestCreated in memory. +func markStageTaskRequestCreated(stageTaskStatus *placementv1beta1.StageTaskStatus, generation int64) { + meta.SetStatusCondition(&stageTaskStatus.Conditions, metav1.Condition{ Type: string(placementv1beta1.StageTaskConditionApprovalRequestCreated), Status: metav1.ConditionTrue, ObservedGeneration: generation, - Reason: condition.AfterStageTaskApprovalRequestCreatedReason, + Reason: condition.StageTaskApprovalRequestCreatedReason, Message: "ApprovalRequest object is created", }) } -// markAfterStageRequestApproved marks the Approval after stage task as Approved in memory. -func markAfterStageRequestApproved(afterStageTaskStatus *placementv1beta1.StageTaskStatus, generation int64) { - meta.SetStatusCondition(&afterStageTaskStatus.Conditions, metav1.Condition{ +// markStageTaskRequestApproved marks the Approval for the before or after stage task as Approved in memory. +func markStageTaskRequestApproved(stageTaskStatus *placementv1beta1.StageTaskStatus, generation int64) { + meta.SetStatusCondition(&stageTaskStatus.Conditions, metav1.Condition{ Type: string(placementv1beta1.StageTaskConditionApprovalRequestApproved), Status: metav1.ConditionTrue, ObservedGeneration: generation, - Reason: condition.AfterStageTaskApprovalRequestApprovedReason, + Reason: condition.StageTaskApprovalRequestApprovedReason, Message: "ApprovalRequest object is approved", }) } diff --git a/pkg/controllers/updaterun/execution_integration_test.go b/pkg/controllers/updaterun/execution_integration_test.go index 980283497..01e3219f8 100644 --- a/pkg/controllers/updaterun/execution_integration_test.go +++ b/pkg/controllers/updaterun/execution_integration_test.go @@ -60,6 +60,16 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { updateRun = generateTestClusterStagedUpdateRun() crp = generateTestClusterResourcePlacement() updateStrategy = generateTestClusterStagedUpdateStrategy() + updateStrategy.Spec.Stages[0].BeforeStageTasks = []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + } + updateStrategy.Spec.Stages[1].BeforeStageTasks = []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + } clusterResourceOverride = generateTestClusterResourceOverride() resourceBindings, targetClusters, unscheduledClusters = generateTestClusterResourceBindingsAndClusters(1) policySnapshot = generateTestClusterSchedulingPolicySnapshot(1, len(targetClusters)) @@ -155,17 +165,64 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { }) Context("Cluster staged update run should update clusters one by one", Ordered, func() { + var wantApprovalRequest *placementv1beta1.ClusterApprovalRequest BeforeAll(func() { By("Creating a new clusterStagedUpdateRun") Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) - By("Validating the initialization succeeded and the execution started") + By("Validating the initialization succeeded and the execution has not started") initialized := generateSucceededInitializationStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, clusterResourceOverride) - wantStatus = generateExecutionStartedStatus(updateRun, initialized) + wantStatus = generateExecutionNotStartedStatus(updateRun, initialized) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + By("Validating the first beforeStage approvalRequest has been created") + wantApprovalRequest = &placementv1beta1.ClusterApprovalRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: updateRun.Status.StagesStatus[0].BeforeStageTaskStatus[0].ApprovalRequestName, + Labels: map[string]string{ + placementv1beta1.TargetUpdatingStageNameLabel: updateRun.Status.StagesStatus[0].StageName, + placementv1beta1.TargetUpdateRunLabel: updateRun.Name, + placementv1beta1.IsLatestUpdateRunApprovalLabel: "true", + }, + }, + Spec: placementv1beta1.ApprovalRequestSpec{ + TargetUpdateRun: updateRun.Name, + TargetStage: updateRun.Status.StagesStatus[0].StageName, + }, + } + validateApprovalRequestCreated(wantApprovalRequest) + By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateProgressingMetric(updateRun)) + validateUpdateRunMetricsEmitted(generateWaitingMetric(updateRun)) + }) + + It("Should not start rolling out 1st stage", func() { + By("Validating the 1st clusterResourceBinding is not updated to Bound") + binding := resourceBindings[numTargetClusters-1] // cluster-9 + validateNotBoundBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + + By("Validating the 1st stage does not have startTime set") + Expect(updateRun.Status.StagesStatus[0].StartTime).Should(BeNil()) + + By("Checking update run status metrics are emitted") + validateUpdateRunMetricsEmitted(generateWaitingMetric(updateRun)) + }) + + It("Should accept the approval request and start to rollout 1st stage", func() { + By("Approving the approvalRequest") + approveClusterApprovalRequest(ctx, wantApprovalRequest.Name) + + By("Validating the approvalRequest has ApprovalAccepted status") + Eventually(func() (bool, error) { + var approvalRequest placementv1beta1.ClusterApprovalRequest + if err := k8sClient.Get(ctx, types.NamespacedName{Name: wantApprovalRequest.Name}, &approvalRequest); err != nil { + return false, err + } + return condition.IsConditionStatusTrue(meta.FindStatusCondition(approvalRequest.Status.Conditions, string(placementv1beta1.ApprovalRequestConditionApprovalAccepted)), approvalRequest.Generation), nil + }, timeout, interval).Should(BeTrue(), "failed to validate the approvalRequest approval accepted") + // Approval task has been approved. + wantStatus.StagesStatus[0].BeforeStageTaskStatus[0].Conditions = append(wantStatus.StagesStatus[0].BeforeStageTaskStatus[0].Conditions, + generateTrueCondition(updateRun, placementv1beta1.StageTaskConditionApprovalRequestApproved)) }) It("Should mark the 1st cluster in the 1st stage as succeeded after marking the binding available", func() { @@ -177,6 +234,9 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) Expect(k8sClient.Status().Update(ctx, binding)).Should(Succeed(), "failed to update the binding status") + // 1st stage started. + wantStatus = generateExecutionStartedStatus(updateRun, wantStatus) + By("Validating the 1st cluster has succeeded and 2nd cluster has started") wantStatus.StagesStatus[0].Clusters[0].Conditions = append(wantStatus.StagesStatus[0].Clusters[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) wantStatus.StagesStatus[0].Clusters[1].Conditions = append(wantStatus.StagesStatus[0].Clusters[1].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionStarted)) @@ -186,7 +246,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { Expect(updateRun.Status.StagesStatus[0].StartTime).ShouldNot(BeNil()) By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateProgressingMetric(updateRun)) + validateUpdateRunMetricsEmitted(generateWaitingMetric(updateRun), generateProgressingMetric(updateRun)) }) It("Should mark the 2nd cluster in the 1st stage as succeeded after marking the binding available", func() { @@ -204,7 +264,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateProgressingMetric(updateRun)) + validateUpdateRunMetricsEmitted(generateWaitingMetric(updateRun), generateProgressingMetric(updateRun)) }) It("Should mark the 3rd cluster in the 1st stage as succeeded after marking the binding available", func() { @@ -222,7 +282,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateProgressingMetric(updateRun)) + validateUpdateRunMetricsEmitted(generateWaitingMetric(updateRun), generateProgressingMetric(updateRun)) }) It("Should mark the 4th cluster in the 1st stage as succeeded after marking the binding available", func() { @@ -240,7 +300,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateProgressingMetric(updateRun)) + validateUpdateRunMetricsEmitted(generateWaitingMetric(updateRun), generateProgressingMetric(updateRun)) }) It("Should mark the 5th cluster in the 1st stage as succeeded after marking the binding available", func() { @@ -252,12 +312,13 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) Expect(k8sClient.Status().Update(ctx, binding)).Should(Succeed(), "failed to update the binding status") - By("Validating the 5th cluster has succeeded and stage waiting for AfterStageTasks") + By("Validating the 5th cluster has succeeded and 1st stage has completed and is waiting for AfterStageTasks") + // 5th cluster succeeded. wantStatus.StagesStatus[0].Clusters[4].Conditions = append(wantStatus.StagesStatus[0].Clusters[4].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) - wantStatus.StagesStatus[0].Conditions[0] = generateFalseCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing) // The progressing condition now becomes false with waiting reason. - wantStatus.StagesStatus[0].AfterStageTaskStatus[1].Conditions = append(wantStatus.StagesStatus[0].AfterStageTaskStatus[1].Conditions, - generateTrueCondition(updateRun, placementv1beta1.StageTaskConditionApprovalRequestCreated)) - wantStatus.Conditions[1] = generateFalseCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing) + // Now waiting for after stage tasks of 1st stage. + meta.SetStatusCondition(&wantStatus.StagesStatus[0].Conditions, generateFalseCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing)) + wantStatus.StagesStatus[0].AfterStageTaskStatus[1].Conditions = append(wantStatus.StagesStatus[0].AfterStageTaskStatus[1].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageTaskConditionApprovalRequestCreated)) + meta.SetStatusCondition(&wantStatus.Conditions, generateFalseCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing)) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") By("Checking update run status metrics are emitted") @@ -295,11 +356,11 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { // 1st stage completed, mark progressing condition reason as succeeded and add succeeded condition. wantStatus.StagesStatus[0].Conditions[0] = generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, true) wantStatus.StagesStatus[0].Conditions = append(wantStatus.StagesStatus[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) - // 2nd stage started. - wantStatus.StagesStatus[1].Conditions = append(wantStatus.StagesStatus[1].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing)) - // 1st cluster in 2nd stage started. - wantStatus.StagesStatus[1].Clusters[0].Conditions = append(wantStatus.StagesStatus[1].Clusters[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionStarted)) - wantStatus.Conditions[1] = generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing) + // 2nd stage waiting for before stage tasks. + wantStatus.StagesStatus[1].Conditions = append(wantStatus.StagesStatus[1].Conditions, generateFalseCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing)) + wantStatus.StagesStatus[1].BeforeStageTaskStatus[0].Conditions = append(wantStatus.StagesStatus[1].BeforeStageTaskStatus[0].Conditions, + generateTrueCondition(updateRun, placementv1beta1.StageTaskConditionApprovalRequestCreated)) + wantStatus.Conditions[1] = generateFalseCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") By("Validating the 1st stage has endTime set") @@ -316,10 +377,61 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { Expect(approvalCreateTime.Before(waitEndTime)).Should(BeTrue()) By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateWaitingMetric(updateRun), generateProgressingMetric(updateRun)) + validateUpdateRunMetricsEmitted(generateProgressingMetric(updateRun), generateWaitingMetric(updateRun)) + }) + + It("Should create approval request before 2nd stage", func() { + By("Validating the approvalRequest has been created") + wantApprovalRequest = &placementv1beta1.ClusterApprovalRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: updateRun.Status.StagesStatus[1].BeforeStageTaskStatus[0].ApprovalRequestName, + Labels: map[string]string{ + placementv1beta1.TargetUpdatingStageNameLabel: updateRun.Status.StagesStatus[1].StageName, + placementv1beta1.TargetUpdateRunLabel: updateRun.Name, + placementv1beta1.IsLatestUpdateRunApprovalLabel: "true", + }, + }, + Spec: placementv1beta1.ApprovalRequestSpec{ + TargetUpdateRun: updateRun.Name, + TargetStage: updateRun.Status.StagesStatus[1].StageName, + }, + } + validateApprovalRequestCreated(wantApprovalRequest) + + By("Checking update run status metrics are emitted") + validateUpdateRunMetricsEmitted(generateProgressingMetric(updateRun), generateWaitingMetric(updateRun)) + }) + + It("Should not start rolling out 2nd stage", func() { + By("Validating the 1st clusterResourceBinding is not updated to Bound") + binding := resourceBindings[0] // cluster-0 + validateNotBoundBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 1) + + By("Validating the 1st stage does not have startTime set") + Expect(updateRun.Status.StagesStatus[1].StartTime).Should(BeNil()) + + By("Checking update run status metrics are emitted") + validateUpdateRunMetricsEmitted(generateProgressingMetric(updateRun), generateWaitingMetric(updateRun)) + }) + + It("Should accept the approval request and start to rollout 2nd stage", func() { + By("Approving the approvalRequest") + approveClusterApprovalRequest(ctx, wantApprovalRequest.Name) + + By("Validating the approvalRequest has ApprovalAccepted status") + Eventually(func() (bool, error) { + var approvalRequest placementv1beta1.ClusterApprovalRequest + if err := k8sClient.Get(ctx, types.NamespacedName{Name: wantApprovalRequest.Name}, &approvalRequest); err != nil { + return false, err + } + return condition.IsConditionStatusTrue(meta.FindStatusCondition(approvalRequest.Status.Conditions, string(placementv1beta1.ApprovalRequestConditionApprovalAccepted)), approvalRequest.Generation), nil + }, timeout, interval).Should(BeTrue(), "failed to validate the approvalRequest approval accepted") + // Approval task has been approved. + wantStatus.StagesStatus[1].BeforeStageTaskStatus[0].Conditions = append(wantStatus.StagesStatus[1].BeforeStageTaskStatus[0].Conditions, + generateTrueCondition(updateRun, placementv1beta1.StageTaskConditionApprovalRequestApproved)) }) - It("Should mark the 1st cluster in the 2nd stage as succeeded after marking the binding available", func() { + It("Should mark the 1st cluster in the 2nd stage as succeeded after approving request and marking the binding available", func() { By("Validating the 1st clusterResourceBinding is updated to Bound") binding := resourceBindings[0] // cluster-0 validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 1) @@ -327,6 +439,11 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { By("Updating the 1st clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) Expect(k8sClient.Status().Update(ctx, binding)).Should(Succeed(), "failed to update the binding status") + // 2nd stage started. + wantStatus.StagesStatus[1].Conditions[0] = generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing) + meta.SetStatusCondition(&wantStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing)) + // 1st cluster started. + wantStatus.StagesStatus[1].Clusters[0].Conditions = append(wantStatus.StagesStatus[1].Clusters[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionStarted)) By("Validating the 1st cluster has succeeded and 2nd cluster has started") wantStatus.StagesStatus[1].Clusters[0].Conditions = append(wantStatus.StagesStatus[1].Clusters[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) @@ -520,13 +637,13 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { By("Creating a new clusterStagedUpdateRun") Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) - By("Validating the initialization succeeded and the execution started") + By("Validating the initialization succeeded and the execution has not started") initialized := generateSucceededInitializationStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, clusterResourceOverride) - wantStatus = generateExecutionStartedStatus(updateRun, initialized) + wantStatus = generateExecutionNotStartedStatus(updateRun, initialized) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateProgressingMetric(updateRun)) + validateUpdateRunMetricsEmitted(generateWaitingMetric(updateRun)) }) AfterAll(func() { @@ -535,6 +652,24 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { }) It("Should keep waiting for the 1st cluster while it's not available", func() { + By("Approving the approvalRequest") + approvalName := updateRun.Status.StagesStatus[0].BeforeStageTaskStatus[0].ApprovalRequestName + approveClusterApprovalRequest(ctx, approvalName) + + By("Validating the approvalRequest has ApprovalAccepted status") + Eventually(func() (bool, error) { + var approvalRequest placementv1beta1.ClusterApprovalRequest + if err := k8sClient.Get(ctx, types.NamespacedName{Name: approvalName}, &approvalRequest); err != nil { + return false, err + } + return condition.IsConditionStatusTrue(meta.FindStatusCondition(approvalRequest.Status.Conditions, string(placementv1beta1.ApprovalRequestConditionApprovalAccepted)), approvalRequest.Generation), nil + }, timeout, interval).Should(BeTrue(), "failed to validate the approvalRequest approval accepted") + // Approval task has been approved. + wantStatus.StagesStatus[0].BeforeStageTaskStatus[0].Conditions = append(wantStatus.StagesStatus[0].BeforeStageTaskStatus[0].Conditions, + generateTrueCondition(updateRun, placementv1beta1.StageTaskConditionApprovalRequestApproved)) + // 1st stage started. + wantStatus = generateExecutionStartedStatus(updateRun, wantStatus) + By("Validating the 1st clusterResourceBinding is updated to Bound") binding := resourceBindings[numTargetClusters-1] // cluster-9 validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) @@ -552,7 +687,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { It("Should emit stuck status metrics after time waiting for the 1st cluster reaches threshold", func() { By("Checking update run stuck metrics is emitted") - validateUpdateRunMetricsEmitted(generateProgressingMetric(updateRun), generateStuckMetric(updateRun)) + validateUpdateRunMetricsEmitted(generateWaitingMetric(updateRun), generateProgressingMetric(updateRun), generateStuckMetric(updateRun)) }) It("Should abort the execution if the binding has unexpected state", func() { @@ -573,7 +708,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateProgressingMetric(updateRun), generateStuckMetric(updateRun), generateFailedMetric(updateRun)) + validateUpdateRunMetricsEmitted(generateWaitingMetric(updateRun), generateProgressingMetric(updateRun), generateStuckMetric(updateRun), generateFailedMetric(updateRun)) }) }) }) @@ -602,7 +737,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { policySnapshot = generateTestClusterSchedulingPolicySnapshot(1, len(targetClusters)) resourceSnapshot = generateTestClusterResourceSnapshot() resourceSnapshot = generateTestClusterResourceSnapshot() - updateStrategy = generateTestClusterStagedUpdateStrategyWithSingleStage(nil) + updateStrategy = generateTestClusterStagedUpdateStrategyWithSingleStage(nil, nil) // Set smaller wait time for testing stageUpdatingWaitTime = time.Second * 3 @@ -1142,8 +1277,14 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { }) Context("Cluster staged update run should recreate deleted approvalRequest", Ordered, func() { + var wantApprovalRequest *placementv1beta1.ClusterApprovalRequest BeforeAll(func() { By("Creating a strategy with single stage and both after stage tasks") + updateStrategy.Spec.Stages[0].BeforeStageTasks = []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + } updateStrategy.Spec.Stages[0].AfterStageTasks = []placementv1beta1.StageTask{ { Type: placementv1beta1.StageTaskTypeApproval, @@ -1162,10 +1303,91 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { By("Creating a new clusterStagedUpdateRun") Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) - By("Validating the initialization succeeded and the execution started") + By("Validating the initialization succeeded and the execution has not started") initialized := generateSucceededInitializationStatusForSmallClusters(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy) - wantStatus = generateExecutionStartedStatus(updateRun, initialized) + wantStatus = generateExecutionNotStartedStatus(updateRun, initialized) + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + + By("Validating the approvalRequest has been created") + wantApprovalRequest = &placementv1beta1.ClusterApprovalRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: updateRun.Status.StagesStatus[0].BeforeStageTaskStatus[0].ApprovalRequestName, + Labels: map[string]string{ + placementv1beta1.TargetUpdatingStageNameLabel: updateRun.Status.StagesStatus[0].StageName, + placementv1beta1.TargetUpdateRunLabel: updateRun.Name, + placementv1beta1.IsLatestUpdateRunApprovalLabel: "true", + }, + }, + Spec: placementv1beta1.ApprovalRequestSpec{ + TargetUpdateRun: updateRun.Name, + TargetStage: updateRun.Status.StagesStatus[0].StageName, + }, + } + validateApprovalRequestCreated(wantApprovalRequest) + + }) + + It("Should not start rolling out", func() { + By("Validating the 1st clusterResourceBinding is not updated to Bound") + binding := resourceBindings[0] // cluster-0 + validateNotBoundBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + + By("Validating the 1st stage does not have startTime set") + Expect(updateRun.Status.StagesStatus[0].StartTime).Should(BeNil()) + + By("Checking update run status metrics are emitted") + validateUpdateRunMetricsEmitted(generateWaitingMetric(updateRun)) + }) + + It("Should start the 1st stage after approval request is approved", func() { + By("Validating the approvalRequest has been created") + approvalRequest := &placementv1beta1.ClusterApprovalRequest{} + wantApprovalRequest := &placementv1beta1.ClusterApprovalRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: updateRun.Status.StagesStatus[0].BeforeStageTaskStatus[0].ApprovalRequestName, + Labels: map[string]string{ + placementv1beta1.TargetUpdatingStageNameLabel: updateRun.Status.StagesStatus[0].StageName, + placementv1beta1.TargetUpdateRunLabel: updateRun.Name, + placementv1beta1.IsLatestUpdateRunApprovalLabel: "true", + }, + }, + Spec: placementv1beta1.ApprovalRequestSpec{ + TargetUpdateRun: updateRun.Name, + TargetStage: updateRun.Status.StagesStatus[0].StageName, + }, + } + validateApprovalRequestCreated(wantApprovalRequest) + + By("Deleting the approvalRequest") + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: wantApprovalRequest.Name}, approvalRequest)).Should(Succeed()) + Expect(k8sClient.Delete(ctx, approvalRequest)).Should(Succeed()) + + By("Validating the approvalRequest has been recreated immediately") + validateApprovalRequestCreated(wantApprovalRequest) + + By("Approving the approvalRequest") + approveClusterApprovalRequest(ctx, wantApprovalRequest.Name) + + By("Check the updateRun status") + wantStatus.StagesStatus[0].BeforeStageTaskStatus[0].Conditions = append(wantStatus.StagesStatus[0].BeforeStageTaskStatus[0].Conditions, + generateTrueCondition(updateRun, placementv1beta1.StageTaskConditionApprovalRequestApproved)) + wantStatus.StagesStatus[0].Clusters[0].Conditions = append(wantStatus.StagesStatus[0].Clusters[0].Conditions, + generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionStarted)) + wantStatus.StagesStatus[0].Conditions[0] = generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing) // The progressing condition now becomes false with progressing reason. + meta.SetStatusCondition(&wantStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing)) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + + By("Deleting the approvalRequest") + Expect(k8sClient.Get(ctx, types.NamespacedName{Name: wantApprovalRequest.Name}, approvalRequest)).Should(Succeed()) + Expect(k8sClient.Delete(ctx, approvalRequest)).Should(Succeed(), "failed to delete the approvalRequest") + + By("Validating the approvalRequest has not been recreated") + Eventually(func() bool { + return apierrors.IsNotFound(k8sClient.Get(ctx, types.NamespacedName{Name: wantApprovalRequest.Name}, approvalRequest)) + }, timeout, interval).Should(BeTrue(), "failed to ensure the approvalRequest is not recreated") + Consistently(func() bool { + return apierrors.IsNotFound(k8sClient.Get(ctx, types.NamespacedName{Name: wantApprovalRequest.Name}, approvalRequest)) + }, timeout, interval).Should(BeTrue(), "failed to ensure the approvalRequest is not recreated") }) It("Should mark the 1st cluster in the 1st stage as succeeded after marking the binding available", func() { @@ -1336,6 +1558,23 @@ func validateBindingState(ctx context.Context, binding *placementv1beta1.Cluster }, timeout, interval).Should(Succeed(), "failed to validate the binding state") } +func validateNotBoundBindingState(ctx context.Context, binding *placementv1beta1.ClusterResourceBinding, resourceSnapshotName string, updateRun *placementv1beta1.ClusterStagedUpdateRun, stage int) { + Consistently(func() error { + if err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.Name}, binding); err != nil { + return err + } + + if binding.Spec.State == placementv1beta1.BindingStateBound { + return fmt.Errorf("binding %s is in Bound state, got %s", binding.Name, binding.Spec.State) + } + rolloutStartedCond := binding.GetCondition(string(placementv1beta1.ResourceBindingRolloutStarted)) + if condition.IsConditionStatusTrue(rolloutStartedCond, binding.Generation) { + return fmt.Errorf("binding %s rollout has started", binding.Name) + } + return nil + }, duration, interval).Should(Succeed(), "failed to validate the binding state") +} + func approveClusterApprovalRequest(ctx context.Context, approvalRequestName string) { Eventually(func() error { var approvalRequest placementv1beta1.ClusterApprovalRequest diff --git a/pkg/controllers/updaterun/execution_test.go b/pkg/controllers/updaterun/execution_test.go index ce775ea50..6d8650d60 100644 --- a/pkg/controllers/updaterun/execution_test.go +++ b/pkg/controllers/updaterun/execution_test.go @@ -19,6 +19,7 @@ package updaterun import ( "context" "errors" + "fmt" "strings" "testing" "time" @@ -943,3 +944,256 @@ func TestCalculateMaxConcurrencyValue(t *testing.T) { }) } } + +func TestCheckBeforeStageTasksStatus_NegativeCases(t *testing.T) { + stageName := "stage-0" + testUpdateRunName = "test-update-run" + approvalRequestName := fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, testUpdateRunName, stageName) + tests := []struct { + name string + stageIndex int + updateRun *placementv1beta1.ClusterStagedUpdateRun + approvalRequest *placementv1beta1.ClusterApprovalRequest + wantErrMsg string + wantErrAborted bool + }{ + // Negative test cases only + { + name: "should return err if before stage task is TimedWait", + stageIndex: 0, + updateRun: &placementv1beta1.ClusterStagedUpdateRun{ + Status: placementv1beta1.UpdateRunStatus{ + UpdateStrategySnapshot: &placementv1beta1.UpdateStrategySpec{ + Stages: []placementv1beta1.StageConfig{ + { + Name: stageName, + BeforeStageTasks: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeTimedWait, + }, + }, + }, + }, + }, + StagesStatus: []placementv1beta1.StageUpdatingStatus{ + { + StageName: stageName, + BeforeStageTaskStatus: []placementv1beta1.StageTaskStatus{ + { + Type: placementv1beta1.StageTaskTypeTimedWait, + }, + }, + }, + }, + }, + }, + wantErrMsg: fmt.Sprintf("found unsupported task type in before stage tasks: %s", placementv1beta1.StageTaskTypeTimedWait), + wantErrAborted: true, + }, + { + name: "should return err if Approval request has wrong target stage in spec", + stageIndex: 0, + updateRun: &placementv1beta1.ClusterStagedUpdateRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: testUpdateRunName, + }, + Status: placementv1beta1.UpdateRunStatus{ + UpdateStrategySnapshot: &placementv1beta1.UpdateStrategySpec{ + Stages: []placementv1beta1.StageConfig{ + { + Name: stageName, + BeforeStageTasks: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + }, + }, + }, + }, + StagesStatus: []placementv1beta1.StageUpdatingStatus{ + { + StageName: stageName, + BeforeStageTaskStatus: []placementv1beta1.StageTaskStatus{ + { + Type: placementv1beta1.StageTaskTypeApproval, + ApprovalRequestName: fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, testUpdateRunName, stageName), + Conditions: []metav1.Condition{ + { + Type: string(placementv1beta1.StageTaskConditionApprovalRequestCreated), + Status: metav1.ConditionTrue, + }, + }, + }, + }, + }, + }, + }, + }, + approvalRequest: &placementv1beta1.ClusterApprovalRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: approvalRequestName, + Labels: map[string]string{ + placementv1beta1.TargetUpdatingStageNameLabel: stageName, + placementv1beta1.TargetUpdateRunLabel: testUpdateRunName, + placementv1beta1.IsLatestUpdateRunApprovalLabel: "true", + }, + }, + Spec: placementv1beta1.ApprovalRequestSpec{ + TargetUpdateRun: testUpdateRunName, + TargetStage: "stage-1", + }, + }, + wantErrMsg: fmt.Sprintf("the approval request task `/%s` is targeting update run `/%s` and stage `stage-1`", approvalRequestName, testUpdateRunName), + wantErrAborted: true, + }, + { + name: "should return err if Approval request has wrong target update run in spec", + stageIndex: 0, + updateRun: &placementv1beta1.ClusterStagedUpdateRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: testUpdateRunName, + }, + Status: placementv1beta1.UpdateRunStatus{ + UpdateStrategySnapshot: &placementv1beta1.UpdateStrategySpec{ + Stages: []placementv1beta1.StageConfig{ + { + Name: stageName, + BeforeStageTasks: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + }, + }, + }, + }, + StagesStatus: []placementv1beta1.StageUpdatingStatus{ + { + StageName: stageName, + BeforeStageTaskStatus: []placementv1beta1.StageTaskStatus{ + { + Type: placementv1beta1.StageTaskTypeApproval, + ApprovalRequestName: fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, testUpdateRunName, stageName), + Conditions: []metav1.Condition{ + { + Type: string(placementv1beta1.StageTaskConditionApprovalRequestCreated), + Status: metav1.ConditionTrue, + }, + }, + }, + }, + }, + }, + }, + }, + approvalRequest: &placementv1beta1.ClusterApprovalRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, testUpdateRunName, stageName), + Labels: map[string]string{ + placementv1beta1.TargetUpdatingStageNameLabel: stageName, + placementv1beta1.TargetUpdateRunLabel: testUpdateRunName, + placementv1beta1.IsLatestUpdateRunApprovalLabel: "true", + }, + }, + Spec: placementv1beta1.ApprovalRequestSpec{ + TargetUpdateRun: "wrong-update-run", + TargetStage: stageName, + }, + }, + wantErrMsg: fmt.Sprintf("the approval request task `/%s` is targeting update run `/wrong-update-run` and stage `%s`", approvalRequestName, stageName), + wantErrAborted: true, + }, + { + name: "should return err if cannot update Approval request that is approved as accepted", + stageIndex: 0, + updateRun: &placementv1beta1.ClusterStagedUpdateRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: testUpdateRunName, + }, + Status: placementv1beta1.UpdateRunStatus{ + UpdateStrategySnapshot: &placementv1beta1.UpdateStrategySpec{ + Stages: []placementv1beta1.StageConfig{ + { + Name: stageName, + BeforeStageTasks: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + }, + }, + }, + }, + StagesStatus: []placementv1beta1.StageUpdatingStatus{ + { + StageName: stageName, + BeforeStageTaskStatus: []placementv1beta1.StageTaskStatus{ + { + Type: placementv1beta1.StageTaskTypeApproval, + ApprovalRequestName: fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, testUpdateRunName, stageName), + Conditions: []metav1.Condition{ + { + Type: string(placementv1beta1.StageTaskConditionApprovalRequestCreated), + Status: metav1.ConditionTrue, + }, + }, + }, + }, + }, + }, + }, + }, + approvalRequest: &placementv1beta1.ClusterApprovalRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, testUpdateRunName, stageName), + Labels: map[string]string{ + placementv1beta1.TargetUpdatingStageNameLabel: stageName, + placementv1beta1.TargetUpdateRunLabel: testUpdateRunName, + placementv1beta1.IsLatestUpdateRunApprovalLabel: "true", + }, + }, + Spec: placementv1beta1.ApprovalRequestSpec{ + TargetUpdateRun: testUpdateRunName, + TargetStage: stageName, + }, + Status: placementv1beta1.ApprovalRequestStatus{ + Conditions: []metav1.Condition{ + { + Type: string(placementv1beta1.ApprovalRequestConditionApproved), + Status: metav1.ConditionTrue, + }, + }, + }, + }, + wantErrMsg: fmt.Sprintf("error returned by the API server: clusterapprovalrequests.placement.kubernetes-fleet.io \"%s\" not found", approvalRequestName), + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + objects := []client.Object{tt.updateRun} + if tt.approvalRequest != nil { + objects = append(objects, tt.approvalRequest) + } + objectsWithStatus := []client.Object{tt.updateRun} + scheme := runtime.NewScheme() + _ = placementv1beta1.AddToScheme(scheme) + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(objects...). + WithStatusSubresource(objectsWithStatus...). + Build() + r := Reconciler{ + Client: fakeClient, + } + ctx := context.Background() + _, gotErr := r.checkBeforeStageTasksStatus(ctx, tt.stageIndex, tt.updateRun) + if gotErr == nil { + t.Fatalf("checkBeforeStageTasksStatus() want error but got nil") + } + if !strings.Contains(gotErr.Error(), tt.wantErrMsg) { + t.Fatalf("checkBeforeStageTasksStatus() error = %v, wantErr %v", gotErr, tt.wantErrMsg) + } + if tt.wantErrAborted && !errors.Is(gotErr, errStagedUpdatedAborted) { + t.Fatalf("checkBeforeStageTasksStatus() want aborted error but got different error: %v", gotErr) + } + }) + } +} diff --git a/pkg/controllers/updaterun/initialization.go b/pkg/controllers/updaterun/initialization.go index 1296008c2..baa07be2b 100644 --- a/pkg/controllers/updaterun/initialization.go +++ b/pkg/controllers/updaterun/initialization.go @@ -292,7 +292,7 @@ func (r *Reconciler) generateStagesByStrategy( updateStrategySpec := updateStrategy.GetUpdateStrategySpec() updateRunStatus.UpdateStrategySnapshot = updateStrategySpec - // Remove waitTime from the updateRun status for AfterStageTask for type Approval. + // Remove waitTime from the updateRun status for BeforeStageTask and AfterStageTask for type Approval. removeWaitTimeFromUpdateRunStatus(updateRun) // Compute the update stages. @@ -344,6 +344,12 @@ func (r *Reconciler) computeRunStageStatus( // Apply the label selectors from the UpdateStrategy to filter the clusters. for _, stage := range updateRunStatus.UpdateStrategySnapshot.Stages { + if err := validateBeforeStageTask(stage.BeforeStageTasks); err != nil { + klog.ErrorS(err, "Failed to validate the before stage tasks", "updateStrategy", strategyKey, "stageName", stage.Name, "updateRun", updateRunRef) + // no more retries here. + invalidBeforeStageErr := controller.NewUserError(fmt.Errorf("the before stage tasks are invalid, updateStrategy: `%s`, stage: %s, err: %s", strategyKey, stage.Name, err.Error())) + return fmt.Errorf("%w: %s", errInitializedFailed, invalidBeforeStageErr.Error()) + } if err := validateAfterStageTask(stage.AfterStageTasks); err != nil { klog.ErrorS(err, "Failed to validate the after stage tasks", "updateStrategy", strategyKey, "stageName", stage.Name, "updateRun", updateRunRef) // no more retries here. @@ -418,12 +424,20 @@ func (r *Reconciler) computeRunStageStatus( curStageUpdatingStatus.Clusters[i].ClusterName = cluster.Name } + // Create the before stage tasks. + curStageUpdatingStatus.BeforeStageTaskStatus = make([]placementv1beta1.StageTaskStatus, len(stage.BeforeStageTasks)) + for i, task := range stage.BeforeStageTasks { + curStageUpdatingStatus.BeforeStageTaskStatus[i].Type = task.Type + if task.Type == placementv1beta1.StageTaskTypeApproval { + curStageUpdatingStatus.BeforeStageTaskStatus[i].ApprovalRequestName = fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, updateRun.GetName(), stage.Name) + } + } // Create the after stage tasks. curStageUpdatingStatus.AfterStageTaskStatus = make([]placementv1beta1.StageTaskStatus, len(stage.AfterStageTasks)) for i, task := range stage.AfterStageTasks { curStageUpdatingStatus.AfterStageTaskStatus[i].Type = task.Type if task.Type == placementv1beta1.StageTaskTypeApproval { - curStageUpdatingStatus.AfterStageTaskStatus[i].ApprovalRequestName = fmt.Sprintf(placementv1beta1.ApprovalTaskNameFmt, updateRun.GetName(), stage.Name) + curStageUpdatingStatus.AfterStageTaskStatus[i].ApprovalRequestName = fmt.Sprintf(placementv1beta1.AfterStageApprovalTaskNameFmt, updateRun.GetName(), stage.Name) } } stagesStatus = append(stagesStatus, curStageUpdatingStatus) @@ -448,8 +462,25 @@ func (r *Reconciler) computeRunStageStatus( return nil } -// validateAfterStageTask valides the afterStageTasks in the stage defined in the UpdateStrategy. -// The error returned from this function is not retryable. +// validateBeforeStageTask validates the beforeStageTasks in the stage defined in the UpdateStrategy. +// The error returned from this function is not retriable. +func validateBeforeStageTask(tasks []placementv1beta1.StageTask) error { + if len(tasks) > 1 { + return fmt.Errorf("beforeStageTasks can have at most one task") + } + for i, task := range tasks { + if task.Type != placementv1beta1.StageTaskTypeApproval { + return fmt.Errorf("task %d of type %s is not allowed in beforeStageTasks, allowed type: Approval", i, task.Type) + } + if task.WaitTime != nil { + return fmt.Errorf("task %d of type Approval cannot have wait duration set", i) + } + } + return nil +} + +// validateAfterStageTask validates the afterStageTasks in the stage defined in the UpdateStrategy. +// The error returned from this function is not retriable. func validateAfterStageTask(tasks []placementv1beta1.StageTask) error { if len(tasks) == 2 && tasks[0].Type == tasks[1].Type { return fmt.Errorf("afterStageTasks cannot have two tasks of the same type: %s", tasks[0].Type) diff --git a/pkg/controllers/updaterun/initialization_integration_test.go b/pkg/controllers/updaterun/initialization_integration_test.go index 9a4507eee..2cbff93e7 100644 --- a/pkg/controllers/updaterun/initialization_integration_test.go +++ b/pkg/controllers/updaterun/initialization_integration_test.go @@ -1010,15 +1010,25 @@ func generateSucceededInitializationStatus( }, } for i := range status.StagesStatus { - var tasks []placementv1beta1.StageTaskStatus + var afterTasks []placementv1beta1.StageTaskStatus for _, task := range updateStrategy.Spec.Stages[i].AfterStageTasks { taskStatus := placementv1beta1.StageTaskStatus{Type: task.Type} if task.Type == placementv1beta1.StageTaskTypeApproval { - taskStatus.ApprovalRequestName = updateRun.Name + "-" + status.StagesStatus[i].StageName + taskStatus.ApprovalRequestName = fmt.Sprintf(placementv1beta1.AfterStageApprovalTaskNameFmt, updateRun.Name, status.StagesStatus[i].StageName) } - tasks = append(tasks, taskStatus) + afterTasks = append(afterTasks, taskStatus) } - status.StagesStatus[i].AfterStageTaskStatus = tasks + status.StagesStatus[i].AfterStageTaskStatus = afterTasks + + var beforeTasks []placementv1beta1.StageTaskStatus + for _, task := range updateStrategy.Spec.Stages[i].BeforeStageTasks { + taskStatus := placementv1beta1.StageTaskStatus{Type: task.Type} + if task.Type == placementv1beta1.StageTaskTypeApproval { + taskStatus.ApprovalRequestName = fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, updateRun.Name, status.StagesStatus[i].StageName) + } + beforeTasks = append(beforeTasks, taskStatus) + } + status.StagesStatus[i].BeforeStageTaskStatus = beforeTasks } return status } @@ -1056,28 +1066,56 @@ func generateSucceededInitializationStatusForSmallClusters( }, } for i := range status.StagesStatus { - var tasks []placementv1beta1.StageTaskStatus + var beforeTasks []placementv1beta1.StageTaskStatus + for _, task := range updateStrategy.Spec.Stages[i].BeforeStageTasks { + taskStatus := placementv1beta1.StageTaskStatus{Type: task.Type} + if task.Type == placementv1beta1.StageTaskTypeApproval { + taskStatus.ApprovalRequestName = fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, updateRun.Name, status.StagesStatus[i].StageName) + } + beforeTasks = append(beforeTasks, taskStatus) + } + status.StagesStatus[i].BeforeStageTaskStatus = beforeTasks + + var afterTasks []placementv1beta1.StageTaskStatus for _, task := range updateStrategy.Spec.Stages[i].AfterStageTasks { taskStatus := placementv1beta1.StageTaskStatus{Type: task.Type} if task.Type == placementv1beta1.StageTaskTypeApproval { - taskStatus.ApprovalRequestName = updateRun.Name + "-" + status.StagesStatus[i].StageName + taskStatus.ApprovalRequestName = fmt.Sprintf(placementv1beta1.AfterStageApprovalTaskNameFmt, updateRun.Name, status.StagesStatus[i].StageName) } - tasks = append(tasks, taskStatus) + afterTasks = append(afterTasks, taskStatus) } - status.StagesStatus[i].AfterStageTaskStatus = tasks + status.StagesStatus[i].AfterStageTaskStatus = afterTasks } return status } func generateExecutionStartedStatus( updateRun *placementv1beta1.ClusterStagedUpdateRun, - initialized *placementv1beta1.UpdateRunStatus, + status *placementv1beta1.UpdateRunStatus, ) *placementv1beta1.UpdateRunStatus { // Mark updateRun execution has started. - initialized.Conditions = append(initialized.Conditions, generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing)) + meta.SetStatusCondition(&status.Conditions, generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing)) + // Mark updateRun 1st stage has started. - initialized.StagesStatus[0].Conditions = append(initialized.StagesStatus[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing)) + meta.SetStatusCondition(&status.StagesStatus[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing)) + // Mark updateRun 1st cluster in the 1st stage has started. - initialized.StagesStatus[0].Clusters[0].Conditions = []metav1.Condition{generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionStarted)} - return initialized + status.StagesStatus[0].Clusters[0].Conditions = []metav1.Condition{generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionStarted)} + return status +} + +func generateExecutionNotStartedStatus( + updateRun *placementv1beta1.ClusterStagedUpdateRun, + status *placementv1beta1.UpdateRunStatus, +) *placementv1beta1.UpdateRunStatus { + // Mark updateRun execution has not started. + status.Conditions = append(status.Conditions, generateFalseCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing)) + + // Mark updateRun 1st stage has not started. + status.StagesStatus[0].Conditions = append(status.StagesStatus[0].Conditions, generateFalseCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing)) + + // Mark updateRun 1st stage BeforeStageTasks has created approval request. + status.StagesStatus[0].BeforeStageTaskStatus[0].Conditions = append(status.StagesStatus[0].BeforeStageTaskStatus[0].Conditions, + generateTrueCondition(updateRun, placementv1beta1.StageTaskConditionApprovalRequestCreated)) + return status } diff --git a/pkg/controllers/updaterun/initialization_test.go b/pkg/controllers/updaterun/initialization_test.go index 0c470d38e..6ce3689a8 100644 --- a/pkg/controllers/updaterun/initialization_test.go +++ b/pkg/controllers/updaterun/initialization_test.go @@ -1,30 +1,114 @@ +/* +Copyright 2025 The KubeFleet Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + package updaterun import ( + "fmt" "testing" "time" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/utils/ptr" - "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1" + placementv1beta1 "github.com/kubefleet-dev/kubefleet/apis/placement/v1beta1" ) +func TestValidateBeforeStageTask(t *testing.T) { + tests := []struct { + name string + task []placementv1beta1.StageTask + wantErr bool + wantErrMsg string + }{ + { + name: "valid BeforeTasks", + task: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + }, + wantErr: false, + }, + { + name: "invalid BeforeTasks, greater than 1 task", + task: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + }, + wantErr: true, + wantErrMsg: "beforeStageTasks can have at most one task", + }, + { + name: "invalid BeforeTasks, with invalid task type", + task: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeTimedWait, + WaitTime: ptr.To(metav1.Duration{Duration: 5 * time.Minute}), + }, + }, + wantErr: true, + wantErrMsg: fmt.Sprintf("task %d of type %s is not allowed in beforeStageTasks, allowed type: Approval", 0, placementv1beta1.StageTaskTypeTimedWait), + }, + { + name: "invalid BeforeTasks, with duration for Approval", + task: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + WaitTime: ptr.To(metav1.Duration{Duration: 1 * time.Minute}), + }, + }, + wantErr: true, + wantErrMsg: fmt.Sprintf("task %d of type Approval cannot have wait duration set", 0), + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + gotErr := validateBeforeStageTask(tt.task) + if tt.wantErr { + if gotErr == nil || gotErr.Error() != tt.wantErrMsg { + t.Fatalf("validateBeforeStageTask() error = %v, wantErr %v", gotErr, tt.wantErrMsg) + } + } else if gotErr != nil { + t.Fatalf("validateBeforeStageTask() error = %v, wantErr %v", gotErr, tt.wantErr) + } + }) + } +} + func TestValidateAfterStageTask(t *testing.T) { tests := []struct { name string - task []v1beta1.StageTask + task []placementv1beta1.StageTask wantErr bool errMsg string }{ { name: "valid AfterTasks", - task: []v1beta1.StageTask{ + task: []placementv1beta1.StageTask{ { - Type: v1beta1.StageTaskTypeApproval, + Type: placementv1beta1.StageTaskTypeApproval, }, { - Type: v1beta1.StageTaskTypeTimedWait, + Type: placementv1beta1.StageTaskTypeTimedWait, WaitTime: ptr.To(metav1.Duration{Duration: 5 * time.Minute}), }, }, @@ -32,13 +116,13 @@ func TestValidateAfterStageTask(t *testing.T) { }, { name: "invalid AfterTasks, same type of tasks", - task: []v1beta1.StageTask{ + task: []placementv1beta1.StageTask{ { - Type: v1beta1.StageTaskTypeTimedWait, + Type: placementv1beta1.StageTaskTypeTimedWait, WaitTime: ptr.To(metav1.Duration{Duration: 1 * time.Minute}), }, { - Type: v1beta1.StageTaskTypeTimedWait, + Type: placementv1beta1.StageTaskTypeTimedWait, WaitTime: ptr.To(metav1.Duration{Duration: 5 * time.Minute}), }, }, @@ -47,9 +131,9 @@ func TestValidateAfterStageTask(t *testing.T) { }, { name: "invalid AfterTasks, with nil duration for TimedWait", - task: []v1beta1.StageTask{ + task: []placementv1beta1.StageTask{ { - Type: v1beta1.StageTaskTypeTimedWait, + Type: placementv1beta1.StageTaskTypeTimedWait, }, }, wantErr: true, @@ -57,9 +141,9 @@ func TestValidateAfterStageTask(t *testing.T) { }, { name: "invalid AfterTasks, with zero duration for TimedWait", - task: []v1beta1.StageTask{ + task: []placementv1beta1.StageTask{ { - Type: v1beta1.StageTaskTypeTimedWait, + Type: placementv1beta1.StageTaskTypeTimedWait, WaitTime: ptr.To(metav1.Duration{Duration: 0 * time.Minute}), }, }, diff --git a/pkg/utils/condition/reason.go b/pkg/utils/condition/reason.go index b1d963e58..9566ee42e 100644 --- a/pkg/utils/condition/reason.go +++ b/pkg/utils/condition/reason.go @@ -194,17 +194,20 @@ const ( // ClusterUpdatingSucceededReason is the reason string of condition if the cluster updating succeeded. ClusterUpdatingSucceededReason = "ClusterUpdatingSucceeded" - // AfterStageTaskApprovalRequestApprovedReason is the reason string of condition if the approval request for after stage task has been approved. - AfterStageTaskApprovalRequestApprovedReason = "AfterStageTaskApprovalRequestApproved" + // StageTaskApprovalRequestApprovedReason is the reason string of condition if the approval request for before or after stage task has been approved. + StageTaskApprovalRequestApprovedReason = "StageTaskApprovalRequestApproved" - // AfterStageTaskApprovalRequestCreatedReason is the reason string of condition if the approval request for after stage task has been created. - AfterStageTaskApprovalRequestCreatedReason = "AfterStageTaskApprovalRequestCreated" + // StageTaskApprovalRequestCreatedReason is the reason string of condition if the approval request for before or after stage task has been created. + StageTaskApprovalRequestCreatedReason = "StageTaskApprovalRequestCreated" // AfterStageTaskWaitTimeElapsedReason is the reason string of condition if the wait time for after stage task has elapsed. AfterStageTaskWaitTimeElapsedReason = "AfterStageTaskWaitTimeElapsed" // ApprovalRequestApprovalAcceptedReason is the reason string of condition if the approval of the approval request has been accepted. ApprovalRequestApprovalAcceptedReason = "ApprovalRequestApprovalAccepted" + + // UpdateRunWaitingMessageFmt is the message format string of condition if the staged update run is waiting for stage tasks in a stage to complete. + UpdateRunWaitingMessageFmt = "The updateRun is waiting for %s tasks in stage %s to complete" ) // A group of condition reason & message string which is used to populate the ClusterResourcePlacementEviction condition. diff --git a/test/e2e/actuals_test.go b/test/e2e/actuals_test.go index 344faf6f4..09904965b 100644 --- a/test/e2e/actuals_test.go +++ b/test/e2e/actuals_test.go @@ -2040,19 +2040,19 @@ func updateRunStageRolloutSucceedConditions(generation int64) []metav1.Condition } } -func updateRunAfterStageTaskSucceedConditions(generation int64, taskType placementv1beta1.StageTaskType) []metav1.Condition { +func updateRunStageTaskSucceedConditions(generation int64, taskType placementv1beta1.StageTaskType) []metav1.Condition { if taskType == placementv1beta1.StageTaskTypeApproval { return []metav1.Condition{ { Type: string(placementv1beta1.StageTaskConditionApprovalRequestCreated), Status: metav1.ConditionTrue, - Reason: condition.AfterStageTaskApprovalRequestCreatedReason, + Reason: condition.StageTaskApprovalRequestCreatedReason, ObservedGeneration: generation, }, { Type: string(placementv1beta1.StageTaskConditionApprovalRequestApproved), Status: metav1.ConditionTrue, - Reason: condition.AfterStageTaskApprovalRequestApprovedReason, + Reason: condition.StageTaskApprovalRequestApprovedReason, ObservedGeneration: generation, }, } @@ -2182,9 +2182,17 @@ func buildStageUpdatingStatuses( for j, task := range stage.AfterStageTasks { stagesStatus[i].AfterStageTaskStatus[j].Type = task.Type if task.Type == placementv1beta1.StageTaskTypeApproval { - stagesStatus[i].AfterStageTaskStatus[j].ApprovalRequestName = fmt.Sprintf(placementv1beta1.ApprovalTaskNameFmt, updateRun.GetName(), stage.Name) + stagesStatus[i].AfterStageTaskStatus[j].ApprovalRequestName = fmt.Sprintf(placementv1beta1.AfterStageApprovalTaskNameFmt, updateRun.GetName(), stage.Name) } - stagesStatus[i].AfterStageTaskStatus[j].Conditions = updateRunAfterStageTaskSucceedConditions(updateRun.GetGeneration(), task.Type) + stagesStatus[i].AfterStageTaskStatus[j].Conditions = updateRunStageTaskSucceedConditions(updateRun.GetGeneration(), task.Type) + } + stagesStatus[i].BeforeStageTaskStatus = make([]placementv1beta1.StageTaskStatus, len(stage.BeforeStageTasks)) + for j, task := range stage.BeforeStageTasks { + stagesStatus[i].BeforeStageTaskStatus[j].Type = task.Type + if task.Type == placementv1beta1.StageTaskTypeApproval { + stagesStatus[i].BeforeStageTaskStatus[j].ApprovalRequestName = fmt.Sprintf(placementv1beta1.BeforeStageApprovalTaskNameFmt, updateRun.GetName(), stage.Name) + } + stagesStatus[i].BeforeStageTaskStatus[j].Conditions = updateRunStageTaskSucceedConditions(updateRun.GetGeneration(), task.Type) } stagesStatus[i].Conditions = updateRunStageRolloutSucceedConditions(updateRun.GetGeneration()) } diff --git a/test/e2e/cluster_staged_updaterun_test.go b/test/e2e/cluster_staged_updaterun_test.go index 8e8b28822..3c29d0068 100644 --- a/test/e2e/cluster_staged_updaterun_test.go +++ b/test/e2e/cluster_staged_updaterun_test.go @@ -144,14 +144,16 @@ var _ = Describe("test CRP rollout with staged update run", func() { crpStatusUpdatedActual := crpStatusWithExternalStrategyActual(nil, "", false, allMemberClusterNames, []string{"", resourceSnapshotIndex1st, ""}, []bool{false, true, false}, nil, nil) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) - validateAndApproveClusterApprovalRequests(updateRunNames[0], envCanary) + validateAndApproveClusterApprovalRequests(updateRunNames[0], envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) }) - It("Should rollout resources to member-cluster-1 first because of its name", func() { - checkIfPlacedWorkResourcesOnMemberClustersInUpdateRun([]*framework.Cluster{allMemberClusters[0]}) + It("Should not rollout resources to prod stage until approved", func() { + checkIfRemovedWorkResourcesFromMemberClustersConsistently([]*framework.Cluster{allMemberClusters[0], allMemberClusters[2]}) }) - It("Should rollout resources to all the members and complete the cluster staged update run successfully", func() { + It("Should rollout resources to all the members after approval and complete the cluster staged update run successfully", func() { + validateAndApproveClusterApprovalRequests(updateRunNames[0], envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + csurSucceededActual := clusterStagedUpdateRunStatusSucceededActual(updateRunNames[0], resourceSnapshotIndex1st, policySnapshotIndex1st, len(allMemberClusters), defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(csurSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s succeeded", updateRunNames[0]) checkIfPlacedWorkResourcesOnMemberClustersInUpdateRun(allMemberClusters) @@ -214,10 +216,20 @@ var _ = Describe("test CRP rollout with staged update run", func() { []string{resourceSnapshotIndex1st, resourceSnapshotIndex2nd, resourceSnapshotIndex1st}, []bool{true, true, true}, nil, nil) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) - validateAndApproveClusterApprovalRequests(updateRunNames[1], envCanary) + validateAndApproveClusterApprovalRequests(updateRunNames[1], envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) + }) + + It("Should not rollout resources to prod stage until approved", func() { + By("Verify that the configmap is not updated on member-cluster-1 and member-cluster-3") + for _, cluster := range []*framework.Cluster{allMemberClusters[0], allMemberClusters[2]} { + configMapActual := configMapPlacedOnClusterActual(cluster, &oldConfigMap) + Consistently(configMapActual, consistentlyDuration, consistentlyInterval).Should(Succeed(), "Failed to keep configmap %s data as expected", newConfigMap.Name) + } }) - It("Should rollout resources to member-cluster-1 and member-cluster-3 too and complete the cluster staged update run successfully", func() { + It("Should rollout resources to all the members after approval and complete the cluster staged update run successfully", func() { + validateAndApproveClusterApprovalRequests(updateRunNames[1], envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + csurSucceededActual := clusterStagedUpdateRunStatusSucceededActual(updateRunNames[1], resourceSnapshotIndex2nd, policySnapshotIndex1st, len(allMemberClusters), defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(csurSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s succeeded", updateRunNames[1]) By("Verify that new the configmap is updated on all member clusters") @@ -312,14 +324,16 @@ var _ = Describe("test CRP rollout with staged update run", func() { crpStatusUpdatedActual := crpStatusWithExternalStrategyActual(nil, "", false, allMemberClusterNames, []string{"", resourceSnapshotIndex1st, ""}, []bool{false, true, false}, nil, nil) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) - validateAndApproveClusterApprovalRequests(updateRunNames[0], envCanary) + validateAndApproveClusterApprovalRequests(updateRunNames[0], envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) }) - It("Should rollout resources to member-cluster-1 first because of its name", func() { - checkIfPlacedWorkResourcesOnMemberClustersInUpdateRun([]*framework.Cluster{allMemberClusters[0]}) + It("Should not rollout resources to prod stage until approved", func() { + checkIfRemovedWorkResourcesFromMemberClustersConsistently([]*framework.Cluster{allMemberClusters[0], allMemberClusters[2]}) }) - It("Should rollout resources to all the members and complete the cluster staged update run successfully", func() { + It("Should rollout resources to all the members after approval and complete the cluster staged update run successfully", func() { + validateAndApproveClusterApprovalRequests(updateRunNames[0], envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + csurSucceededActual := clusterStagedUpdateRunStatusSucceededActual(updateRunNames[0], resourceSnapshotIndex1st, policySnapshotIndex1st, len(allMemberClusters), defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(csurSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s succeeded", updateRunNames[0]) checkIfPlacedWorkResourcesOnMemberClustersInUpdateRun(allMemberClusters) @@ -381,10 +395,20 @@ var _ = Describe("test CRP rollout with staged update run", func() { []string{resourceSnapshotIndex1st, resourceSnapshotIndex2nd, resourceSnapshotIndex1st}, []bool{true, true, true}, nil, nil) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) - validateAndApproveClusterApprovalRequests(updateRunNames[1], envCanary) + validateAndApproveClusterApprovalRequests(updateRunNames[1], envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) }) - It("Should rollout resources to member-cluster-1 and member-cluster-3 too and complete the cluster staged update run successfully", func() { + It("Should not rollout resources to prod stage until approved", func() { + By("Verify that the configmap is not updated on member-cluster-1 and member-cluster-3") + for _, cluster := range []*framework.Cluster{allMemberClusters[0], allMemberClusters[2]} { + configMapActual := configMapPlacedOnClusterActual(cluster, &oldConfigMap) + Consistently(configMapActual, consistentlyDuration, consistentlyInterval).Should(Succeed(), "Failed to keep configmap %s data as expected", newConfigMap.Name) + } + }) + + It("Should rollout resources to member-cluster-1 and member-cluster-3 after approval and complete the cluster staged update run successfully", func() { + validateAndApproveClusterApprovalRequests(updateRunNames[1], envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + csurSucceededActual := clusterStagedUpdateRunStatusSucceededActual(updateRunNames[1], resourceSnapshotIndex2nd, policySnapshotIndex1st, len(allMemberClusters), defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(csurSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s succeeded", updateRunNames[1]) By("Verify that new the configmap is updated on all member clusters") @@ -419,10 +443,20 @@ var _ = Describe("test CRP rollout with staged update run", func() { []string{resourceSnapshotIndex2nd, resourceSnapshotIndex1st, resourceSnapshotIndex2nd}, []bool{true, true, true}, nil, nil) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) - validateAndApproveClusterApprovalRequests(updateRunNames[2], envCanary) + validateAndApproveClusterApprovalRequests(updateRunNames[2], envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) + }) + + It("Should not rollback resources to prod stage until approved", func() { + By("Verify that the configmap is not rolled back on member-cluster-1 and member-cluster-3") + for _, cluster := range []*framework.Cluster{allMemberClusters[0], allMemberClusters[2]} { + configMapActual := configMapPlacedOnClusterActual(cluster, &newConfigMap) + Consistently(configMapActual, consistentlyDuration, consistentlyInterval).Should(Succeed(), "Failed to keep configmap %s data as expected", newConfigMap.Name) + } }) - It("Should rollback resources to member-cluster-1 and member-cluster-3 too and complete the cluster staged update run successfully", func() { + It("Should rollback resources to member-cluster-1 and member-cluster-3 after approval and complete the cluster staged update run successfully", func() { + validateAndApproveClusterApprovalRequests(updateRunNames[2], envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + csurSucceededActual := clusterStagedUpdateRunStatusSucceededActual(updateRunNames[2], resourceSnapshotIndex1st, policySnapshotIndex1st, len(allMemberClusters), defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(csurSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s succeeded", updateRunNames[1]) for idx := range allMemberClusters { @@ -515,10 +549,16 @@ var _ = Describe("test CRP rollout with staged update run", func() { crpStatusUpdatedActual := crpStatusWithExternalStrategyActual(nil, "", false, allMemberClusterNames[:2], []string{"", resourceSnapshotIndex1st}, []bool{false, true}, nil, nil) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) - validateAndApproveClusterApprovalRequests(updateRunNames[0], envCanary) + validateAndApproveClusterApprovalRequests(updateRunNames[0], envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) + }) + + It("Should not rollout resources to prod stage until approved", func() { + checkIfRemovedWorkResourcesFromMemberClustersConsistently([]*framework.Cluster{allMemberClusters[0], allMemberClusters[2]}) }) - It("Should rollout resources to member-cluster-1 too but not member-cluster-3 and complete the cluster staged update run successfully", func() { + It("Should rollout resources to member-cluster-1 after approval but not member-cluster-3 and complete the cluster staged update run successfully", func() { + validateAndApproveClusterApprovalRequests(updateRunNames[0], envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + csurSucceededActual := clusterStagedUpdateRunStatusSucceededActual(updateRunNames[0], resourceSnapshotIndex1st, policySnapshotIndex1st, 2, defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0]}}, nil, nil, nil) Eventually(csurSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s succeeded", updateRunNames[0]) checkIfPlacedWorkResourcesOnMemberClustersInUpdateRun([]*framework.Cluster{allMemberClusters[0], allMemberClusters[1]}) @@ -565,10 +605,16 @@ var _ = Describe("test CRP rollout with staged update run", func() { crpStatusUpdatedActual := crpStatusWithExternalStrategyActual(nil, "", false, allMemberClusterNames, []string{resourceSnapshotIndex1st, resourceSnapshotIndex1st, ""}, []bool{false, true, false}, nil, nil) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to keep CRP %s status as expected", crpName) - validateAndApproveClusterApprovalRequests(updateRunNames[1], envCanary) + validateAndApproveClusterApprovalRequests(updateRunNames[1], envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) }) - It("Should rollout resources to member-cluster-3 too and complete the cluster staged update run successfully", func() { + It("Should not rollout resources to prod stage until approved", func() { + checkIfRemovedWorkResourcesFromMemberClustersConsistently([]*framework.Cluster{allMemberClusters[2]}) + }) + + It("Should rollout resources to member-cluster-3 after approval and complete the cluster staged update run successfully", func() { + validateAndApproveClusterApprovalRequests(updateRunNames[1], envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + csurSucceededActual := clusterStagedUpdateRunStatusSucceededActual(updateRunNames[1], resourceSnapshotIndex1st, policySnapshotIndex2nd, 3, defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(csurSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s succeeded", updateRunNames[1]) checkIfPlacedWorkResourcesOnMemberClustersInUpdateRun(allMemberClusters) @@ -611,10 +657,12 @@ var _ = Describe("test CRP rollout with staged update run", func() { crpStatusUpdatedActual := crpStatusWithExternalStrategyActual(workResourceIdentifiers(), resourceSnapshotIndex1st, false, []string{allMemberClusterNames[2]}, []string{resourceSnapshotIndex1st}, []bool{false}, nil, nil) Consistently(crpStatusUpdatedActual, consistentlyDuration, consistentlyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) - validateAndApproveClusterApprovalRequests(updateRunNames[2], envCanary) + validateAndApproveClusterApprovalRequests(updateRunNames[2], envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) }) - It("Should remove resources on member-cluster-1 and member-cluster-2 and complete the cluster staged update run successfully", func() { + It("Should remove resources on member-cluster-1 and member-cluster-2 after approval and complete the cluster staged update run successfully", func() { + validateAndApproveClusterApprovalRequests(updateRunNames[2], envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + // need to go through two stages csurSucceededActual := clusterStagedUpdateRunStatusSucceededActual(updateRunNames[2], resourceSnapshotIndex1st, policySnapshotIndex3rd, 1, defaultApplyStrategy, &strategy.Spec, [][]string{{}, {allMemberClusterNames[2]}}, []string{allMemberClusterNames[0], allMemberClusterNames[1]}, nil, nil) Eventually(csurSucceededActual, 2*updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s succeeded", updateRunNames[2]) @@ -704,10 +752,16 @@ var _ = Describe("test CRP rollout with staged update run", func() { crpStatusUpdatedActual := crpStatusWithExternalStrategyActual(nil, "", false, allMemberClusterNames[2:], []string{""}, []bool{false}, nil, nil) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) - validateAndApproveClusterApprovalRequests(updateRunNames[0], envCanary) + validateAndApproveClusterApprovalRequests(updateRunNames[0], envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) + }) + + It("Should not rollout resources to prod stage until approved", func() { + checkIfRemovedWorkResourcesFromMemberClustersConsistently([]*framework.Cluster{allMemberClusters[0], allMemberClusters[1]}) }) - It("Should rollout resources to member-cluster-3 and complete the cluster staged update run successfully", func() { + It("Should rollout resources to member-cluster-3 after approval and complete the cluster staged update run successfully", func() { + validateAndApproveClusterApprovalRequests(updateRunNames[0], envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + csurSucceededActual := clusterStagedUpdateRunStatusSucceededActual(updateRunNames[0], resourceSnapshotIndex1st, policySnapshotIndex1st, 1, defaultApplyStrategy, &strategy.Spec, [][]string{{}, {allMemberClusterNames[2]}}, nil, nil, nil) Eventually(csurSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s succeeded", updateRunNames[0]) checkIfPlacedWorkResourcesOnMemberClustersInUpdateRun([]*framework.Cluster{allMemberClusters[2]}) @@ -753,10 +807,16 @@ var _ = Describe("test CRP rollout with staged update run", func() { crpStatusUpdatedActual := crpStatusWithExternalStrategyActual(nil, "", false, allMemberClusterNames, []string{"", resourceSnapshotIndex1st, resourceSnapshotIndex1st}, []bool{false, true, true}, nil, nil) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to keep CRP %s status as expected", crpName) - validateAndApproveClusterApprovalRequests(updateRunNames[1], envCanary) + validateAndApproveClusterApprovalRequests(updateRunNames[1], envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) + }) + + It("Should not rollout resources to member-cluster-1 until approved", func() { + checkIfRemovedWorkResourcesFromMemberClustersConsistently([]*framework.Cluster{allMemberClusters[0]}) }) - It("Should rollout resources to member-cluster-1 too and complete the cluster staged update run successfully", func() { + It("Should rollout resources to member-cluster-1 after approval and complete the cluster staged update run successfully", func() { + validateAndApproveClusterApprovalRequests(updateRunNames[1], envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + csurSucceededActual := clusterStagedUpdateRunStatusSucceededActual(updateRunNames[1], resourceSnapshotIndex1st, policySnapshotIndex1st, 3, defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(csurSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s succeeded", updateRunNames[1]) checkIfPlacedWorkResourcesOnMemberClustersInUpdateRun(allMemberClusters) @@ -799,10 +859,16 @@ var _ = Describe("test CRP rollout with staged update run", func() { crpStatusUpdatedActual := crpStatusWithExternalStrategyActual(workResourceIdentifiers(), resourceSnapshotIndex1st, true, allMemberClusterNames[1:], []string{resourceSnapshotIndex1st, resourceSnapshotIndex1st}, []bool{true, true}, nil, nil) Consistently(crpStatusUpdatedActual, consistentlyDuration, consistentlyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) - validateAndApproveClusterApprovalRequests(updateRunNames[2], envCanary) + validateAndApproveClusterApprovalRequests(updateRunNames[2], envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) }) - It("Should remove resources on member-cluster-1 and complete the cluster staged update run successfully", func() { + It("Should not remove resources from member-cluster-1 until approved", func() { + checkIfPlacedWorkResourcesOnMemberClustersConsistently(allMemberClusters) + }) + + It("Should remove resources on member-cluster-1 after approval and complete the cluster staged update run successfully", func() { + validateAndApproveClusterApprovalRequests(updateRunNames[2], envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + csurSucceededActual := clusterStagedUpdateRunStatusSucceededActual(updateRunNames[2], resourceSnapshotIndex1st, policySnapshotIndex1st, 2, defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[2]}}, []string{allMemberClusterNames[0]}, nil, nil) Eventually(csurSucceededActual, 2*updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s succeeded", updateRunNames[2]) checkIfRemovedWorkResourcesFromMemberClusters([]*framework.Cluster{allMemberClusters[0]}) @@ -971,10 +1037,16 @@ var _ = Describe("test CRP rollout with staged update run", func() { []string{"", resourceSnapshotIndex1st, ""}, []bool{false, true, false}, nil, wantROs) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) - validateAndApproveClusterApprovalRequests(updateRunName, envCanary) + validateAndApproveClusterApprovalRequests(updateRunName, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) + }) + + It("Should not rollout resources to member-cluster-1 and member-cluster-3 until approved", func() { + checkIfRemovedWorkResourcesFromMemberClustersConsistently([]*framework.Cluster{allMemberClusters[0], allMemberClusters[2]}) }) - It("Should rollout resources to member-cluster-1 and member-cluster-3 too and complete the cluster staged update run successfully", func() { + It("Should rollout resources to member-cluster-1 and member-cluster-3 after approval and complete the cluster staged update run successfully", func() { + validateAndApproveClusterApprovalRequests(updateRunName, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + csurSucceededActual := clusterStagedUpdateRunStatusSucceededActual(updateRunName, resourceSnapshotIndex1st, policySnapshotIndex1st, len(allMemberClusters), defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, wantCROs, wantROs) Eventually(csurSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s succeeded", updateRunName) checkIfPlacedWorkResourcesOnMemberClustersInUpdateRun(allMemberClusters) @@ -1071,10 +1143,12 @@ var _ = Describe("test CRP rollout with staged update run", func() { []string{"", resourceSnapshotIndex1st, ""}, []bool{false, true, false}, nil, nil) Eventually(crpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update CRP %s status as expected", crpName) - validateAndApproveClusterApprovalRequests(updateRunName, envCanary) + validateAndApproveClusterApprovalRequests(updateRunName, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) }) - It("Should report diff for member-cluster-1 and member-cluster-3 too and complete the cluster staged update run successfully", func() { + It("Should report diff for member-cluster-1 and member-cluster-3 after approval and complete the cluster staged update run successfully", func() { + validateAndApproveClusterApprovalRequests(updateRunName, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + csurSucceededActual := clusterStagedUpdateRunStatusSucceededActual(updateRunName, resourceSnapshotIndex1st, policySnapshotIndex1st, len(allMemberClusters), applyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(csurSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s succeeded", updateRunName) }) @@ -1183,7 +1257,11 @@ var _ = Describe("test CRP rollout with staged update run", func() { configMapActual := configMapPlacedOnClusterActual(allMemberClusters[1], &newConfigMap) Eventually(configMapActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update to the new configmap %s on cluster %s", newConfigMap.Name, allMemberClusterNames[1]) - validateAndApproveClusterApprovalRequests(updateRunName, envCanary) + // Approval for AfterStageTasks of canary stage + validateAndApproveClusterApprovalRequests(updateRunName, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) + + // Approval for BeforeStageTasks of prod stage + validateAndApproveClusterApprovalRequests(updateRunName, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) // Verify complete rollout csurSucceededActual := clusterStagedUpdateRunStatusSucceededActual(updateRunName, resourceSnapshotIndex2nd, policySnapshotIndex1st, len(allMemberClusters), defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) @@ -1265,7 +1343,7 @@ var _ = Describe("test CRP rollout with staged update run", func() { }, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to find cluster approval request") }) - It("Should approve cluster approval request using kubectl-fleet approve plugin", func() { + It("Should approve after-stage cluster approval request using kubectl-fleet approve plugin for canary stage", func() { var approvalRequestName string // Get the cluster approval request name. @@ -1311,6 +1389,52 @@ var _ = Describe("test CRP rollout with staged update run", func() { }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to verify approval request is approved") }) + It("Should approve before-stage cluster approval request using kubectl-fleet approve plugin for prod stage", func() { + var approvalRequestName string + + // Get the cluster approval request name. + Eventually(func() error { + appReqList := &placementv1beta1.ClusterApprovalRequestList{} + if err := hubClient.List(ctx, appReqList, client.MatchingLabels{ + placementv1beta1.TargetUpdatingStageNameLabel: envProd, + placementv1beta1.TargetUpdateRunLabel: updateRunName, + }); err != nil { + return fmt.Errorf("failed to list approval requests: %w", err) + } + + if len(appReqList.Items) != 1 { + return fmt.Errorf("want 1 approval request, got %d", len(appReqList.Items)) + } + + approvalRequestName = appReqList.Items[0].Name + return nil + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to get approval request name") + + // Use kubectl-fleet approve plugin to approve the request + cmd := exec.Command(fleetBinaryPath, "approve", "clusterapprovalrequest", + "--hubClusterContext", "kind-hub", + "--name", approvalRequestName) + output, err := cmd.CombinedOutput() + Expect(err).ToNot(HaveOccurred(), "kubectl-fleet approve failed: %s", string(output)) + + // Verify the approval request is approved + Eventually(func() error { + var appReq placementv1beta1.ClusterApprovalRequest + if err := hubClient.Get(ctx, client.ObjectKey{Name: approvalRequestName}, &appReq); err != nil { + return fmt.Errorf("failed to get approval request: %w", err) + } + + approvedCondition := meta.FindStatusCondition(appReq.Status.Conditions, string(placementv1beta1.ApprovalRequestConditionApproved)) + if approvedCondition == nil { + return fmt.Errorf("approved condition not found") + } + if approvedCondition.Status != metav1.ConditionTrue { + return fmt.Errorf("approved condition status is %s, want True", approvedCondition.Status) + } + return nil + }, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to verify approval request is approved") + }) + It("Should complete the staged update run after approval", func() { csurSucceededActual := clusterStagedUpdateRunStatusSucceededActual(updateRunName, resourceSnapshotIndex1st, policySnapshotIndex1st, len(allMemberClusters), defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(csurSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s succeeded", updateRunName) @@ -1383,7 +1507,11 @@ var _ = Describe("test CRP rollout with staged update run", func() { It("Create updateRun and verify resources are rolled out", func() { createClusterStagedUpdateRunSucceed(updateRunName, crpName, resourceSnapshotIndex1st, strategyName) - validateAndApproveClusterApprovalRequests(updateRunName, envCanary) + // Approval for AfterStageTasks of canary stage + validateAndApproveClusterApprovalRequests(updateRunName, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) + + // Approval for BeforeStageTasks of prod stage + validateAndApproveClusterApprovalRequests(updateRunName, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) csurSucceededActual := clusterStagedUpdateRunStatusSucceededActual(updateRunName, resourceSnapshotIndex1st, policySnapshotIndex1st, len(allMemberClusters), defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(csurSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s succeeded", updateRunName) @@ -1956,6 +2084,11 @@ func createClusterStagedUpdateStrategySucceed(strategyName string) *placementv1b envLabelName: envProd, // member-cluster-1 and member-cluster-3 }, }, + BeforeStageTasks: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + }, }, }, }, @@ -2037,7 +2170,7 @@ func createClusterStagedUpdateRunSucceedWithNoResourceSnapshotIndex(updateRunNam Expect(hubClient.Create(ctx, updateRun)).To(Succeed(), "Failed to create ClusterStagedUpdateRun %s", updateRunName) } -func validateAndApproveClusterApprovalRequests(updateRunName, stageName string) { +func validateAndApproveClusterApprovalRequests(updateRunName, stageName, approvalRequestNameFmt string) { Eventually(func() error { appReqList := &placementv1beta1.ClusterApprovalRequestList{} if err := hubClient.List(ctx, appReqList, client.MatchingLabels{ @@ -2051,6 +2184,10 @@ func validateAndApproveClusterApprovalRequests(updateRunName, stageName string) return fmt.Errorf("got %d approval requests, want 1", len(appReqList.Items)) } appReq := &appReqList.Items[0] + approvalRequestName := fmt.Sprintf(approvalRequestNameFmt, updateRunName, stageName) + if appReq.Name != approvalRequestName { + return fmt.Errorf("got approval request %s, want %s", appReq.Name, approvalRequestName) + } meta.SetStatusCondition(&appReq.Status.Conditions, metav1.Condition{ Status: metav1.ConditionTrue, Type: string(placementv1beta1.ApprovalRequestConditionApproved), diff --git a/test/e2e/staged_updaterun_test.go b/test/e2e/staged_updaterun_test.go index 2f03affa9..60dcfe885 100644 --- a/test/e2e/staged_updaterun_test.go +++ b/test/e2e/staged_updaterun_test.go @@ -135,14 +135,16 @@ var _ = Describe("test RP rollout with staged update run", Label("resourceplacem rpStatusUpdatedActual := rpStatusWithExternalStrategyActual(nil, "", false, allMemberClusterNames, []string{"", resourceSnapshotIndex1st, ""}, []bool{false, true, false}, nil, nil) Eventually(rpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update RP %s/%s status as expected", testNamespace, rpName) - validateAndApproveNamespacedApprovalRequests(updateRunNames[0], testNamespace, envCanary) + validateAndApproveNamespacedApprovalRequests(updateRunNames[0], testNamespace, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) }) - It("Should rollout resources to member-cluster-1 first because of its name", func() { - checkIfPlacedWorkResourcesOnMemberClustersInUpdateRun([]*framework.Cluster{allMemberClusters[0]}) + It("Should not rollout resources to prod stage until approved", func() { + checkIfRemovedConfigMapFromMemberClustersConsistently([]*framework.Cluster{allMemberClusters[0], allMemberClusters[2]}) }) - It("Should rollout resources to all the members and complete the staged update run successfully", func() { + It("Should rollout resources to all the members after approval and complete the staged update run successfully", func() { + validateAndApproveNamespacedApprovalRequests(updateRunNames[0], testNamespace, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + surSucceededActual := stagedUpdateRunStatusSucceededActual(updateRunNames[0], testNamespace, resourceSnapshotIndex1st, policySnapshotIndex1st, len(allMemberClusters), defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(surSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s/%s succeeded", testNamespace, updateRunNames[0]) checkIfPlacedWorkResourcesOnMemberClustersInUpdateRun(allMemberClusters) @@ -204,10 +206,19 @@ var _ = Describe("test RP rollout with staged update run", Label("resourceplacem []string{resourceSnapshotIndex1st, resourceSnapshotIndex2nd, resourceSnapshotIndex1st}, []bool{true, true, true}, nil, nil) Eventually(rpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update RP %s/%s status as expected", testNamespace, rpName) - validateAndApproveNamespacedApprovalRequests(updateRunNames[1], testNamespace, envCanary) + validateAndApproveNamespacedApprovalRequests(updateRunNames[1], testNamespace, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) + }) + It("Should not rollout resources to prod stage until approved", func() { + By("Verify that the configmap is not updated on member-cluster-1 and member-cluster-3") + for _, cluster := range []*framework.Cluster{allMemberClusters[0], allMemberClusters[2]} { + configMapActual := configMapPlacedOnClusterActual(cluster, &oldConfigMap) + Consistently(configMapActual, consistentlyDuration, consistentlyInterval).Should(Succeed(), "Failed to keep configmap %s data as expected", newConfigMap.Name) + } }) - It("Should rollout resources to member-cluster-1 and member-cluster-3 too and complete the staged update run successfully", func() { + It("Should rollout resources to all the members after approval and complete the staged update run successfully", func() { + validateAndApproveNamespacedApprovalRequests(updateRunNames[1], testNamespace, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + surSucceededActual := stagedUpdateRunStatusSucceededActual(updateRunNames[1], testNamespace, resourceSnapshotIndex2nd, policySnapshotIndex1st, len(allMemberClusters), defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(surSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s/%s succeeded", testNamespace, updateRunNames[1]) By("Verify that new the configmap is updated on all member clusters") @@ -300,14 +311,16 @@ var _ = Describe("test RP rollout with staged update run", Label("resourceplacem rpStatusUpdatedActual := rpStatusWithExternalStrategyActual(nil, "", false, allMemberClusterNames, []string{"", resourceSnapshotIndex1st, ""}, []bool{false, true, false}, nil, nil) Eventually(rpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update RP %s/%s status as expected", testNamespace, rpName) - validateAndApproveNamespacedApprovalRequests(updateRunNames[0], testNamespace, envCanary) + validateAndApproveNamespacedApprovalRequests(updateRunNames[0], testNamespace, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) }) - It("Should rollout resources to member-cluster-1 first because of its name", func() { - checkIfPlacedWorkResourcesOnMemberClustersInUpdateRun([]*framework.Cluster{allMemberClusters[0]}) + It("Should not rollout resources to prod stage until approved", func() { + checkIfRemovedConfigMapFromMemberClustersConsistently([]*framework.Cluster{allMemberClusters[0], allMemberClusters[2]}) }) - It("Should rollout resources to all the members and complete the staged update run successfully", func() { + It("Should rollout resources to all the members after approval and complete the staged update run successfully", func() { + validateAndApproveNamespacedApprovalRequests(updateRunNames[0], testNamespace, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + surSucceededActual := stagedUpdateRunStatusSucceededActual(updateRunNames[0], testNamespace, resourceSnapshotIndex1st, policySnapshotIndex1st, len(allMemberClusters), defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(surSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s/%s succeeded", testNamespace, updateRunNames[0]) checkIfPlacedWorkResourcesOnMemberClustersInUpdateRun(allMemberClusters) @@ -369,10 +382,20 @@ var _ = Describe("test RP rollout with staged update run", Label("resourceplacem []string{resourceSnapshotIndex1st, resourceSnapshotIndex2nd, resourceSnapshotIndex1st}, []bool{true, true, true}, nil, nil) Eventually(rpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update RP %s/%s status as expected", testNamespace, rpName) - validateAndApproveNamespacedApprovalRequests(updateRunNames[1], testNamespace, envCanary) + validateAndApproveNamespacedApprovalRequests(updateRunNames[1], testNamespace, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) + }) + + It("Should not rollout resources to prod stage until approved", func() { + By("Verify that the configmap is not updated on member-cluster-1 and member-cluster-3") + for _, cluster := range []*framework.Cluster{allMemberClusters[0], allMemberClusters[2]} { + configMapActual := configMapPlacedOnClusterActual(cluster, &oldConfigMap) + Consistently(configMapActual, consistentlyDuration, consistentlyInterval).Should(Succeed(), "Failed to keep configmap %s data as expected", newConfigMap.Name) + } }) - It("Should rollout resources to member-cluster-1 and member-cluster-3 too and complete the staged update run successfully", func() { + It("Should rollout resources to member-cluster-1 and member-cluster-3 after approval and complete the staged update run successfully", func() { + validateAndApproveNamespacedApprovalRequests(updateRunNames[1], testNamespace, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + surSucceededActual := stagedUpdateRunStatusSucceededActual(updateRunNames[1], testNamespace, resourceSnapshotIndex2nd, policySnapshotIndex1st, len(allMemberClusters), defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(surSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s/%s succeeded", testNamespace, updateRunNames[1]) By("Verify that new the configmap is updated on all member clusters") @@ -407,10 +430,20 @@ var _ = Describe("test RP rollout with staged update run", Label("resourceplacem []string{resourceSnapshotIndex2nd, resourceSnapshotIndex1st, resourceSnapshotIndex2nd}, []bool{true, true, true}, nil, nil) Eventually(rpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update RP %s/%s status as expected", testNamespace, rpName) - validateAndApproveNamespacedApprovalRequests(updateRunNames[2], testNamespace, envCanary) + validateAndApproveNamespacedApprovalRequests(updateRunNames[2], testNamespace, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) }) - It("Should rollback resources to member-cluster-1 and member-cluster-3 too and complete the staged update run successfully", func() { + It("Should not rollback resources to prod stage until approved", func() { + By("Verify that the configmap is not rolled back on member-cluster-1 and member-cluster-3") + for _, cluster := range []*framework.Cluster{allMemberClusters[0], allMemberClusters[2]} { + configMapActual := configMapPlacedOnClusterActual(cluster, &newConfigMap) + Consistently(configMapActual, consistentlyDuration, consistentlyInterval).Should(Succeed(), "Failed to keep configmap %s data as expected", newConfigMap.Name) + } + }) + + It("Should rollback resources to member-cluster-1 and member-cluster-3 after approval and complete the staged update run successfully", func() { + validateAndApproveNamespacedApprovalRequests(updateRunNames[2], testNamespace, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + surSucceededActual := stagedUpdateRunStatusSucceededActual(updateRunNames[2], testNamespace, resourceSnapshotIndex1st, policySnapshotIndex1st, len(allMemberClusters), defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(surSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s succeeded", updateRunNames[1]) for idx := range allMemberClusters { @@ -501,10 +534,16 @@ var _ = Describe("test RP rollout with staged update run", Label("resourceplacem rpStatusUpdatedActual := rpStatusWithExternalStrategyActual(nil, "", false, allMemberClusterNames[:2], []string{"", resourceSnapshotIndex1st}, []bool{false, true}, nil, nil) Eventually(rpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update RP %s/%s status as expected", testNamespace, rpName) - validateAndApproveNamespacedApprovalRequests(updateRunNames[0], testNamespace, envCanary) + validateAndApproveNamespacedApprovalRequests(updateRunNames[0], testNamespace, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) + }) + + It("Should not rollout resources to prod stage until approved", func() { + checkIfRemovedConfigMapFromMemberClustersConsistently([]*framework.Cluster{allMemberClusters[0], allMemberClusters[2]}) }) - It("Should rollout resources to member-cluster-1 too but not member-cluster-3 and complete the staged update run successfully", func() { + It("Should rollout resources to member-cluster-1 after approval but not member-cluster-3 and complete the staged update run successfully", func() { + validateAndApproveNamespacedApprovalRequests(updateRunNames[0], testNamespace, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + surSucceededActual := stagedUpdateRunStatusSucceededActual(updateRunNames[0], testNamespace, resourceSnapshotIndex1st, policySnapshotIndex1st, 2, defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0]}}, nil, nil, nil) Eventually(surSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s succeeded", updateRunNames[0]) checkIfPlacedWorkResourcesOnMemberClustersInUpdateRun([]*framework.Cluster{allMemberClusters[0], allMemberClusters[1]}) @@ -551,10 +590,16 @@ var _ = Describe("test RP rollout with staged update run", Label("resourceplacem rpStatusUpdatedActual := rpStatusWithExternalStrategyActual(nil, "", false, allMemberClusterNames, []string{resourceSnapshotIndex1st, resourceSnapshotIndex1st, ""}, []bool{false, true, false}, nil, nil) Eventually(rpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to keep RP %s/%s status as expected", testNamespace, rpName) - validateAndApproveNamespacedApprovalRequests(updateRunNames[1], testNamespace, envCanary) + validateAndApproveNamespacedApprovalRequests(updateRunNames[1], testNamespace, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) + }) + + It("Should not rollout resources to prod stage until approved", func() { + checkIfRemovedConfigMapFromMemberClustersConsistently([]*framework.Cluster{allMemberClusters[2]}) }) - It("Should rollout resources to member-cluster-3 too and complete the staged update run successfully", func() { + It("Should rollout resources to member-cluster-3 after approval and complete the staged update run successfully", func() { + validateAndApproveNamespacedApprovalRequests(updateRunNames[1], testNamespace, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + surSucceededActual := stagedUpdateRunStatusSucceededActual(updateRunNames[1], testNamespace, resourceSnapshotIndex1st, policySnapshotIndex2nd, 3, defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(surSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s/%s succeeded", testNamespace, updateRunNames[1]) checkIfPlacedWorkResourcesOnMemberClustersInUpdateRun(allMemberClusters) @@ -597,10 +642,12 @@ var _ = Describe("test RP rollout with staged update run", Label("resourceplacem rpStatusUpdatedActual := rpStatusWithExternalStrategyActual(appConfigMapIdentifiers(), resourceSnapshotIndex1st, false, []string{allMemberClusterNames[2]}, []string{resourceSnapshotIndex1st}, []bool{false}, nil, nil) Consistently(rpStatusUpdatedActual, consistentlyDuration, consistentlyInterval).Should(Succeed(), "Failed to update RP %s/%s status as expected", testNamespace, rpName) - validateAndApproveNamespacedApprovalRequests(updateRunNames[2], testNamespace, envCanary) + validateAndApproveNamespacedApprovalRequests(updateRunNames[2], testNamespace, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) }) - It("Should remove resources on member-cluster-1 and member-cluster-2 and complete the staged update run successfully", func() { + It("Should remove resources on member-cluster-1 and member-cluster-2 after approval and complete the staged update run successfully", func() { + validateAndApproveNamespacedApprovalRequests(updateRunNames[2], testNamespace, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + // need to go through two stages surSucceededActual := stagedUpdateRunStatusSucceededActual(updateRunNames[2], testNamespace, resourceSnapshotIndex1st, policySnapshotIndex3rd, 1, defaultApplyStrategy, &strategy.Spec, [][]string{{}, {allMemberClusterNames[2]}}, []string{allMemberClusterNames[0], allMemberClusterNames[1]}, nil, nil) Eventually(surSucceededActual, 2*updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s/%s succeeded", testNamespace, updateRunNames[2]) @@ -688,10 +735,16 @@ var _ = Describe("test RP rollout with staged update run", Label("resourceplacem rpStatusUpdatedActual := rpStatusWithExternalStrategyActual(nil, "", false, allMemberClusterNames[2:], []string{""}, []bool{false}, nil, nil) Eventually(rpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update RP %s/%s status as expected", testNamespace, rpName) - validateAndApproveNamespacedApprovalRequests(updateRunNames[0], testNamespace, envCanary) + validateAndApproveNamespacedApprovalRequests(updateRunNames[0], testNamespace, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) }) - It("Should rollout resources to member-cluster-3 and complete the staged update run successfully", func() { + It("Should not rollout resources to prod stage until approved", func() { + checkIfRemovedConfigMapFromMemberClustersConsistently([]*framework.Cluster{allMemberClusters[0], allMemberClusters[1]}) + }) + + It("Should rollout resources to member-cluster-3 after approval and complete the cluster staged update run successfully", func() { + validateAndApproveNamespacedApprovalRequests(updateRunNames[0], testNamespace, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + surSucceededActual := stagedUpdateRunStatusSucceededActual(updateRunNames[0], testNamespace, resourceSnapshotIndex1st, policySnapshotIndex1st, 1, defaultApplyStrategy, &strategy.Spec, [][]string{{}, {allMemberClusterNames[2]}}, nil, nil, nil) Eventually(surSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s/%s succeeded", testNamespace, updateRunNames[0]) checkIfPlacedWorkResourcesOnMemberClustersInUpdateRun([]*framework.Cluster{allMemberClusters[2]}) @@ -737,10 +790,16 @@ var _ = Describe("test RP rollout with staged update run", Label("resourceplacem rpStatusUpdatedActual := rpStatusWithExternalStrategyActual(nil, "", false, allMemberClusterNames, []string{"", resourceSnapshotIndex1st, resourceSnapshotIndex1st}, []bool{false, true, true}, nil, nil) Eventually(rpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to keep RP %s/%s status as expected", testNamespace, rpName) - validateAndApproveNamespacedApprovalRequests(updateRunNames[1], testNamespace, envCanary) + validateAndApproveNamespacedApprovalRequests(updateRunNames[1], testNamespace, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) }) - It("Should rollout resources to member-cluster-1 too and complete the staged update run successfully", func() { + It("Should not rollout resources to member-cluster-1 until approved", func() { + checkIfRemovedConfigMapFromMemberClustersConsistently([]*framework.Cluster{allMemberClusters[0]}) + }) + + It("Should rollout resources to member-cluster-1 after approval and complete the staged update run successfully", func() { + validateAndApproveNamespacedApprovalRequests(updateRunNames[1], testNamespace, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + surSucceededActual := stagedUpdateRunStatusSucceededActual(updateRunNames[1], testNamespace, resourceSnapshotIndex1st, policySnapshotIndex1st, 3, defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(surSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s/%s succeeded", testNamespace, updateRunNames[1]) checkIfPlacedWorkResourcesOnMemberClustersInUpdateRun(allMemberClusters) @@ -783,10 +842,16 @@ var _ = Describe("test RP rollout with staged update run", Label("resourceplacem rpStatusUpdatedActual := rpStatusWithExternalStrategyActual(appConfigMapIdentifiers(), resourceSnapshotIndex1st, true, allMemberClusterNames[1:], []string{resourceSnapshotIndex1st, resourceSnapshotIndex1st}, []bool{true, true}, nil, nil) Consistently(rpStatusUpdatedActual, consistentlyDuration, consistentlyInterval).Should(Succeed(), "Failed to update RP %s/%s status as expected", testNamespace, rpName) - validateAndApproveNamespacedApprovalRequests(updateRunNames[2], testNamespace, envCanary) + validateAndApproveNamespacedApprovalRequests(updateRunNames[2], testNamespace, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) + }) + + It("Should not remove resources from member-cluster-1 until approved", func() { + checkIfPlacedWorkResourcesOnMemberClustersConsistently(allMemberClusters) }) - It("Should remove resources on member-cluster-1 and complete the staged update run successfully", func() { + It("Should remove resources on member-cluster-1 after approval and complete the cluster staged update run successfully", func() { + validateAndApproveNamespacedApprovalRequests(updateRunNames[2], testNamespace, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + surSucceededActual := stagedUpdateRunStatusSucceededActual(updateRunNames[2], testNamespace, resourceSnapshotIndex1st, policySnapshotIndex1st, 2, defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[2]}}, []string{allMemberClusterNames[0]}, nil, nil) Eventually(surSucceededActual, 2*updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s/%s succeeded", testNamespace, updateRunNames[2]) checkIfRemovedConfigMapFromMemberClusters([]*framework.Cluster{allMemberClusters[0]}) @@ -928,10 +993,16 @@ var _ = Describe("test RP rollout with staged update run", Label("resourceplacem rpStatusUpdatedActual := rpStatusWithExternalStrategyActual(nil, "", false, allMemberClusterNames, []string{"", resourceSnapshotIndex1st, ""}, []bool{false, true, false}, nil, wantROs) Eventually(rpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update RP %s/%s status as expected", testNamespace, rpName) - validateAndApproveNamespacedApprovalRequests(updateRunName, testNamespace, envCanary) + validateAndApproveNamespacedApprovalRequests(updateRunName, testNamespace, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) + }) + + It("Should not rollout resources to member-cluster-1 and member-cluster-3 until approved", func() { + checkIfRemovedConfigMapFromMemberClustersConsistently([]*framework.Cluster{allMemberClusters[0], allMemberClusters[2]}) }) - It("Should rollout resources to member-cluster-1 and member-cluster-3 too and complete the staged update run successfully", func() { + It("Should rollout resources to member-cluster-1 and member-cluster-3 after approval and complete the cluster staged update run successfully", func() { + validateAndApproveNamespacedApprovalRequests(updateRunName, testNamespace, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + surSucceededActual := stagedUpdateRunStatusSucceededActual(updateRunName, testNamespace, resourceSnapshotIndex1st, policySnapshotIndex1st, len(allMemberClusters), defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, wantROs) Eventually(surSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s/%s succeeded", testNamespace, updateRunName) checkIfPlacedWorkResourcesOnMemberClustersInUpdateRun(allMemberClusters) @@ -1022,10 +1093,12 @@ var _ = Describe("test RP rollout with staged update run", Label("resourceplacem []string{"", resourceSnapshotIndex1st, ""}, []bool{false, true, false}, nil, nil) Eventually(rpStatusUpdatedActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update RP %s/%s status as expected", testNamespace, rpName) - validateAndApproveNamespacedApprovalRequests(updateRunName, testNamespace, envCanary) + validateAndApproveNamespacedApprovalRequests(updateRunName, testNamespace, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) }) - It("Should report diff for member-cluster-1 and member-cluster-3 too and complete the staged update run successfully", func() { + It("Should report diff for member-cluster-1 and member-cluster-3 after approval and complete the cluster staged update run successfully", func() { + validateAndApproveNamespacedApprovalRequests(updateRunName, testNamespace, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) + surSucceededActual := stagedUpdateRunStatusSucceededActual(updateRunName, testNamespace, resourceSnapshotIndex1st, policySnapshotIndex1st, len(allMemberClusters), applyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(surSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s/%s succeeded", testNamespace, updateRunName) }) @@ -1132,7 +1205,11 @@ var _ = Describe("test RP rollout with staged update run", Label("resourceplacem configMapActual := configMapPlacedOnClusterActual(allMemberClusters[1], &newConfigMap) Eventually(configMapActual, eventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to update to the new configmap %s on cluster %s", newConfigMap.Name, allMemberClusterNames[1]) - validateAndApproveNamespacedApprovalRequests(updateRunName, testNamespace, envCanary) + // Approval for AfterStageTask of canary stage + validateAndApproveNamespacedApprovalRequests(updateRunName, testNamespace, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) + + // Approval for BeforeStageTask of prod stage + validateAndApproveNamespacedApprovalRequests(updateRunName, testNamespace, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) // Verify complete rollout. surSucceededActual := stagedUpdateRunStatusSucceededActual(updateRunName, testNamespace, resourceSnapshotIndex2nd, policySnapshotIndex1st, len(allMemberClusters), defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) @@ -1209,7 +1286,11 @@ var _ = Describe("test RP rollout with staged update run", Label("resourceplacem It("Create updateRun and verify resources are rolled out", func() { createStagedUpdateRunSucceed(updateRunName, testNamespace, rpName, resourceSnapshotIndex1st, strategyName) - validateAndApproveNamespacedApprovalRequests(updateRunName, testNamespace, envCanary) + // Approval for AfterStageTask of canary stage + validateAndApproveNamespacedApprovalRequests(updateRunName, testNamespace, envCanary, placementv1beta1.AfterStageApprovalTaskNameFmt) + + // Approval for BeforeStageTask of prod stage + validateAndApproveNamespacedApprovalRequests(updateRunName, testNamespace, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt) surSucceededActual := stagedUpdateRunStatusSucceededActual(updateRunName, testNamespace, resourceSnapshotIndex1st, policySnapshotIndex1st, len(allMemberClusters), defaultApplyStrategy, &strategy.Spec, [][]string{{allMemberClusterNames[1]}, {allMemberClusterNames[0], allMemberClusterNames[2]}}, nil, nil, nil) Eventually(surSucceededActual, updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s/%s succeeded", testNamespace, updateRunName) @@ -1481,6 +1562,11 @@ func createStagedUpdateStrategySucceed(strategyName, namespace string) *placemen envLabelName: envProd, // member-cluster-1 and member-cluster-3 }, }, + BeforeStageTasks: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + }, }, }, }, @@ -1564,7 +1650,7 @@ func createStagedUpdateRunSucceedWithNoResourceSnapshotIndex(updateRunName, name Expect(hubClient.Create(ctx, updateRun)).To(Succeed(), "Failed to create StagedUpdateRun %s", updateRunName) } -func validateAndApproveNamespacedApprovalRequests(updateRunName, namespace, stageName string) { +func validateAndApproveNamespacedApprovalRequests(updateRunName, namespace, stageName, approvalRequestNameFmt string) { Eventually(func() error { appReqList := &placementv1beta1.ApprovalRequestList{} if err := hubClient.List(ctx, appReqList, client.InNamespace(namespace), client.MatchingLabels{ @@ -1578,6 +1664,10 @@ func validateAndApproveNamespacedApprovalRequests(updateRunName, namespace, stag return fmt.Errorf("got %d approval requests, want 1", len(appReqList.Items)) } appReq := &appReqList.Items[0] + approvalRequestName := fmt.Sprintf(approvalRequestNameFmt, updateRunName, stageName) + if appReq.Name != approvalRequestName { + return fmt.Errorf("got approval request %s, want %s", appReq.Name, approvalRequestName) + } meta.SetStatusCondition(&appReq.Status.Conditions, metav1.Condition{ Status: metav1.ConditionTrue, Type: string(placementv1beta1.ApprovalRequestConditionApproved),