From 9c58415e4f6a767dab29e0674c4dc4c8dc3c8975 Mon Sep 17 00:00:00 2001 From: Wantong Jiang Date: Fri, 28 Feb 2025 19:31:02 +0000 Subject: [PATCH 1/2] fix waitTime and approval task blocking each other --- .../updaterun/controller_integration_test.go | 9 +++ pkg/controllers/updaterun/execution.go | 44 +++++++----- .../updaterun/execution_integration_test.go | 69 +++++++++++++++++-- .../initialization_integration_test.go | 5 ++ 4 files changed, 104 insertions(+), 23 deletions(-) diff --git a/pkg/controllers/updaterun/controller_integration_test.go b/pkg/controllers/updaterun/controller_integration_test.go index 6f6b30b7f..92eee41ad 100644 --- a/pkg/controllers/updaterun/controller_integration_test.go +++ b/pkg/controllers/updaterun/controller_integration_test.go @@ -335,6 +335,9 @@ func generateTestClusterStagedUpdateStrategy() *placementv1beta1.ClusterStagedUp Duration: time.Second * 4, }, }, + { + Type: placementv1beta1.AfterStageTaskTypeApproval, + }, }, }, { @@ -350,6 +353,12 @@ func generateTestClusterStagedUpdateStrategy() *placementv1beta1.ClusterStagedUp { Type: placementv1beta1.AfterStageTaskTypeApproval, }, + { + Type: placementv1beta1.AfterStageTaskTypeTimedWait, + WaitTime: metav1.Duration{ + Duration: time.Second * 4, + }, + }, }, }, }, diff --git a/pkg/controllers/updaterun/execution.go b/pkg/controllers/updaterun/execution.go index 799f79191..84113acd4 100644 --- a/pkg/controllers/updaterun/execution.go +++ b/pkg/controllers/updaterun/execution.go @@ -267,6 +267,8 @@ func (r *Reconciler) checkAfterStageTasksStatus(ctx context.Context, updatingSta klog.V(2).InfoS("There is no after stage task for this stage", "stage", updatingStage.Name, "clusterStagedUpdateRun", updateRunRef) return true, 0, nil } + passed := true + afterStageWaitTime := time.Duration(-1) for i, task := range updatingStage.AfterStageTasks { switch task.Type { case placementv1beta1.AfterStageTaskTypeTimedWait: @@ -275,10 +277,12 @@ func (r *Reconciler) checkAfterStageTasksStatus(ctx context.Context, updatingSta waitTime := time.Until(waitStartTime.Add(task.WaitTime.Duration)) if waitTime > 0 { klog.V(2).InfoS("The after stage task still need to wait", "waitStartTime", waitStartTime, "waitTime", task.WaitTime, "stage", updatingStage.Name, "clusterStagedUpdateRun", updateRunRef) - return false, waitTime, nil + passed = false + afterStageWaitTime = waitTime + } else { + markAfterStageWaitTimeElapsed(&updatingStageStatus.AfterStageTaskStatus[i], updateRun.Generation) + klog.V(2).InfoS("The after stage wait task has completed", "stage", updatingStage.Name, "clusterStagedUpdateRun", updateRunRef) } - markAfterStageWaitTimeElapsed(&updatingStageStatus.AfterStageTaskStatus[i], updateRun.Generation) - klog.V(2).InfoS("The after stage wait task has completed", "stage", updatingStage.Name, "clusterStagedUpdateRun", updateRunRef) case placementv1beta1.AfterStageTaskTypeApproval: // Check if the approval request has been created. approvalRequest := placementv1beta1.ClusterApprovalRequest{ @@ -310,20 +314,26 @@ func (r *Reconciler) checkAfterStageTasksStatus(ctx context.Context, updatingSta return false, -1, fmt.Errorf("%w: %s", errStagedUpdatedAborted, unexpectedErr.Error()) } approvalAccepted := condition.IsConditionStatusTrue(meta.FindStatusCondition(approvalRequest.Status.Conditions, string(placementv1beta1.ApprovalRequestConditionApprovalAccepted)), approvalRequest.Generation) + approved := condition.IsConditionStatusTrue(meta.FindStatusCondition(approvalRequest.Status.Conditions, string(placementv1beta1.ApprovalRequestConditionApproved)), approvalRequest.Generation) // Approved state should not change once the approval is accepted. - if !approvalAccepted && !condition.IsConditionStatusTrue(meta.FindStatusCondition(approvalRequest.Status.Conditions, string(placementv1beta1.ApprovalRequestConditionApproved)), approvalRequest.Generation) { + if !approvalAccepted && !approved { klog.V(2).InfoS("The approval request has not been approved yet", "approvalRequestTask", requestRef, "stage", updatingStage.Name, "clusterStagedUpdateRun", updateRunRef) - return false, -1, nil - } - klog.V(2).InfoS("The approval request has been approved", "approvalRequestTask", requestRef, "stage", updatingStage.Name, "clusterStagedUpdateRun", 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, "clusterStagedUpdateRun", updateRunRef) - // retriable err - return false, 0, err + passed = false + } else { + if !approved { + klog.V(2).InfoS("The approval request has been approval-accepted, ignoring changing back to unapproved", "approvalRequestTask", requestRef, "stage", updatingStage.Name, "clusterStagedUpdateRun", updateRunRef) + } else { + klog.V(2).InfoS("The approval request has been approved", "approvalRequestTask", requestRef, "stage", updatingStage.Name, "clusterStagedUpdateRun", 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, "clusterStagedUpdateRun", updateRunRef) + // retriable err + return false, -1, err + } + } + markAfterStageRequestApproved(&updatingStageStatus.AfterStageTaskStatus[i], updateRun.Generation) } - markAfterStageRequestApproved(&updatingStageStatus.AfterStageTaskStatus[i], updateRun.Generation) } else { // retriable error klog.ErrorS(err, "Failed to create the approval request", "approvalRequest", requestRef, "stage", updatingStage.Name, "clusterStagedUpdateRun", updateRunRef) @@ -333,12 +343,14 @@ func (r *Reconciler) checkAfterStageTasksStatus(ctx context.Context, updatingSta // 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, "clusterStagedUpdateRun", updateRunRef) markAfterStageRequestCreated(&updatingStageStatus.AfterStageTaskStatus[i], updateRun.Generation) - return false, -1, nil + passed = false } } } - // All the after stage tasks have been finished or the for loop will return before this line. - return true, 0, nil + if passed { + afterStageWaitTime = 0 + } + return passed, afterStageWaitTime, nil } // updateBindingRolloutStarted updates the binding status to indicate the rollout has started. diff --git a/pkg/controllers/updaterun/execution_integration_test.go b/pkg/controllers/updaterun/execution_integration_test.go index bf7ab7379..712c2c423 100644 --- a/pkg/controllers/updaterun/execution_integration_test.go +++ b/pkg/controllers/updaterun/execution_integration_test.go @@ -254,19 +254,57 @@ var _ = Describe("UpdateRun execution tests", 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 AfterStageTask") + By("Validating the 5th cluster has succeeded and stage waiting for AfterStageTasks") stageWaitingCondition := generateFalseCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing) stageWaitingCondition.Reason = condition.StageUpdatingWaitingReason wantStatus.StagesStatus[0].Clusters[4].Conditions = append(wantStatus.StagesStatus[0].Clusters[4].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) wantStatus.StagesStatus[0].Conditions[0] = stageWaitingCondition // 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.AfterStageTaskConditionApprovalRequestCreated)) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") }) - It("Should complete the 1st stage after wait time passed and move on to the 2nd stage", func() { - By("Validating the waitTime after stage task has completed and 2nd stage has started") - // AfterStageTask completed. + It("Should complete the 1st stage after wait time passed and approval request approved and move on to the 2nd stage", func() { + By("Validating the approvalRequest has been created") + approvalRequest := &placementv1beta1.ClusterApprovalRequest{} + wantApprovalRequest := &placementv1beta1.ClusterApprovalRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: updateRun.Status.StagesStatus[0].AfterStageTaskStatus[1].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, + }, + } + Eventually(func() error { + if err := k8sClient.Get(ctx, types.NamespacedName{Name: wantApprovalRequest.Name}, approvalRequest); err != nil { + return err + } + if diff := cmp.Diff(wantApprovalRequest.Spec, approvalRequest.Spec); diff != "" { + return fmt.Errorf("approvalRequest has different spec (-want +got):\n%s", diff) + } + if diff := cmp.Diff(wantApprovalRequest.Labels, approvalRequest.Labels); diff != "" { + return fmt.Errorf("approvalRequest has different labels (-want +got):\n%s", diff) + } + return nil + }, timeout, interval).Should(Succeed(), "failed to validate the approvalRequest") + + By("Approving the approvalRequest") + meta.SetStatusCondition(&approvalRequest.Status.Conditions, generateTrueCondition(approvalRequest, placementv1beta1.ApprovalRequestConditionApproved)) + Expect(k8sClient.Status().Update(ctx, approvalRequest)).Should(Succeed(), "failed to update the approvalRequest status") + + By("Validating both after stage tasks have completed and 2nd stage has started") + // Timedwait afterStageTask completed. wantStatus.StagesStatus[0].AfterStageTaskStatus[0].Conditions = append(wantStatus.StagesStatus[0].AfterStageTaskStatus[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.AfterStageTaskConditionWaitTimeElapsed)) + // Approval afterStageTask completed. + wantStatus.StagesStatus[0].AfterStageTaskStatus[1].Conditions = append(wantStatus.StagesStatus[0].AfterStageTaskStatus[1].Conditions, + generateTrueCondition(updateRun, placementv1beta1.AfterStageTaskConditionApprovalRequestApproved)) // 1st stage completed. wantStatus.StagesStatus[0].Conditions = append(wantStatus.StagesStatus[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // 2nd stage started. @@ -281,10 +319,12 @@ var _ = Describe("UpdateRun execution tests", func() { By("Validating the waitTime after stage task only completes after the wait time") waitStartTime := meta.FindStatusCondition(updateRun.Status.StagesStatus[0].Conditions, string(placementv1beta1.StageUpdatingConditionProgressing)).LastTransitionTime.Time waitEndTime := meta.FindStatusCondition(updateRun.Status.StagesStatus[0].AfterStageTaskStatus[0].Conditions, string(placementv1beta1.AfterStageTaskConditionWaitTimeElapsed)).LastTransitionTime.Time - // In this test, I set wait time to be 4 seconds, while stageClusterUpdatingWaitTime is 3 seconds. - // So it needs 2 rounds of reconcile to wait for the waitTime to elapse, waitEndTime - waitStartTime should be around 6 seconds. Expect(waitStartTime.Add(updateStrategy.Spec.Stages[0].AfterStageTasks[0].WaitTime.Duration).After(waitEndTime)).Should(BeFalse(), fmt.Sprintf("waitEndTime %v did not pass waitStartTime %v long enough, want at least %v", waitEndTime, waitStartTime, updateStrategy.Spec.Stages[0].AfterStageTasks[0].WaitTime.Duration)) + + By("Validating the creation time of the approval request is before the complete time of the timedwait task") + approvalCreateTime := meta.FindStatusCondition(updateRun.Status.StagesStatus[0].AfterStageTaskStatus[1].Conditions, string(placementv1beta1.AfterStageTaskConditionApprovalRequestCreated)).LastTransitionTime.Time + Expect(approvalCreateTime.Before(waitEndTime)).Should(BeTrue()) }) It("Should mark the 1st cluster in the 2nd stage as succeeded after marking the binding available", func() { @@ -369,7 +409,7 @@ var _ = Describe("UpdateRun execution tests", func() { validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") }) - It("Should complete the 2nd stage after the ApprovalRequest is approved and move on to the delete stage", func() { + It("Should complete the 2nd stage after both after stage tasks are completed and move on to the delete stage", func() { By("Validating the approvalRequest has been created") approvalRequest := &placementv1beta1.ClusterApprovalRequest{} wantApprovalRequest := &placementv1beta1.ClusterApprovalRequest{ @@ -406,6 +446,8 @@ var _ = Describe("UpdateRun execution tests", func() { By("Validating the 2nd stage has completed and the delete stage has started") wantStatus.StagesStatus[1].AfterStageTaskStatus[0].Conditions = append(wantStatus.StagesStatus[1].AfterStageTaskStatus[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.AfterStageTaskConditionApprovalRequestApproved)) + wantStatus.StagesStatus[1].AfterStageTaskStatus[1].Conditions = append(wantStatus.StagesStatus[1].AfterStageTaskStatus[1].Conditions, + generateTrueCondition(updateRun, placementv1beta1.AfterStageTaskConditionWaitTimeElapsed)) wantStatus.StagesStatus[1].Conditions = append(wantStatus.StagesStatus[1].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing)) @@ -414,6 +456,19 @@ var _ = Describe("UpdateRun execution tests", func() { } validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + By("Validating the 2nd stage has endTime set") + Expect(updateRun.Status.StagesStatus[1].EndTime).ShouldNot(BeNil()) + + By("Validating the waitTime after stage task only completes after the wait time") + waitStartTime := meta.FindStatusCondition(updateRun.Status.StagesStatus[1].Conditions, string(placementv1beta1.StageUpdatingConditionProgressing)).LastTransitionTime.Time + waitEndTime := meta.FindStatusCondition(updateRun.Status.StagesStatus[1].AfterStageTaskStatus[1].Conditions, string(placementv1beta1.AfterStageTaskConditionWaitTimeElapsed)).LastTransitionTime.Time + Expect(waitStartTime.Add(updateStrategy.Spec.Stages[1].AfterStageTasks[1].WaitTime.Duration).After(waitEndTime)).Should(BeFalse(), + fmt.Sprintf("waitEndTime %v did not pass waitStartTime %v long enough, want at least %v", waitEndTime, waitStartTime, updateStrategy.Spec.Stages[1].AfterStageTasks[1].WaitTime.Duration)) + + By("Validating the creation time of the approval request is before the complete time of the timedwait task") + approvalCreateTime := meta.FindStatusCondition(updateRun.Status.StagesStatus[1].AfterStageTaskStatus[0].Conditions, string(placementv1beta1.AfterStageTaskConditionApprovalRequestCreated)).LastTransitionTime.Time + Expect(approvalCreateTime.Before(waitEndTime)).Should(BeTrue()) + By("Validating the approvalRequest has ApprovalAccepted status") Eventually(func() (bool, error) { if err := k8sClient.Get(ctx, types.NamespacedName{Name: wantApprovalRequest.Name}, approvalRequest); err != nil { diff --git a/pkg/controllers/updaterun/initialization_integration_test.go b/pkg/controllers/updaterun/initialization_integration_test.go index 581003eb8..2e649f948 100644 --- a/pkg/controllers/updaterun/initialization_integration_test.go +++ b/pkg/controllers/updaterun/initialization_integration_test.go @@ -770,6 +770,10 @@ func generateSucceededInitializationStatus( }, AfterStageTaskStatus: []placementv1beta1.AfterStageTaskStatus{ {Type: placementv1beta1.AfterStageTaskTypeTimedWait}, + { + Type: placementv1beta1.AfterStageTaskTypeApproval, + ApprovalRequestName: updateRun.Name + "-stage1", + }, }, }, { @@ -786,6 +790,7 @@ func generateSucceededInitializationStatus( Type: placementv1beta1.AfterStageTaskTypeApproval, ApprovalRequestName: updateRun.Name + "-stage2", }, + {Type: placementv1beta1.AfterStageTaskTypeTimedWait}, }, }, }, From 83f9e25c9569adf87fca71a4d2174f9189ff5343 Mon Sep 17 00:00:00 2001 From: Wantong Jiang Date: Tue, 4 Mar 2025 08:19:19 +0000 Subject: [PATCH 2/2] fix comments --- pkg/controllers/updaterun/execution.go | 16 +- .../updaterun/execution_integration_test.go | 314 +++++++++++++++++- .../initialization_integration_test.go | 28 +- 3 files changed, 326 insertions(+), 32 deletions(-) diff --git a/pkg/controllers/updaterun/execution.go b/pkg/controllers/updaterun/execution.go index 84113acd4..5f8994d3b 100644 --- a/pkg/controllers/updaterun/execution.go +++ b/pkg/controllers/updaterun/execution.go @@ -315,16 +315,13 @@ func (r *Reconciler) checkAfterStageTasksStatus(ctx context.Context, updatingSta } approvalAccepted := condition.IsConditionStatusTrue(meta.FindStatusCondition(approvalRequest.Status.Conditions, string(placementv1beta1.ApprovalRequestConditionApprovalAccepted)), approvalRequest.Generation) approved := condition.IsConditionStatusTrue(meta.FindStatusCondition(approvalRequest.Status.Conditions, string(placementv1beta1.ApprovalRequestConditionApproved)), approvalRequest.Generation) - // Approved state should not change once the approval is accepted. if !approvalAccepted && !approved { klog.V(2).InfoS("The approval request has not been approved yet", "approvalRequestTask", requestRef, "stage", updatingStage.Name, "clusterStagedUpdateRun", updateRunRef) passed = false - } else { - if !approved { - klog.V(2).InfoS("The approval request has been approval-accepted, ignoring changing back to unapproved", "approvalRequestTask", requestRef, "stage", updatingStage.Name, "clusterStagedUpdateRun", updateRunRef) - } else { - klog.V(2).InfoS("The approval request has been approved", "approvalRequestTask", requestRef, "stage", updatingStage.Name, "clusterStagedUpdateRun", updateRunRef) - } + continue + } + if approved { + klog.V(2).InfoS("The approval request has been approved", "approvalRequestTask", requestRef, "stage", updatingStage.Name, "clusterStagedUpdateRun", 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, "clusterStagedUpdateRun", updateRunRef) @@ -332,8 +329,11 @@ func (r *Reconciler) checkAfterStageTasksStatus(ctx context.Context, updatingSta return false, -1, err } } - markAfterStageRequestApproved(&updatingStageStatus.AfterStageTaskStatus[i], updateRun.Generation) + } 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, "clusterStagedUpdateRun", updateRunRef) } + markAfterStageRequestApproved(&updatingStageStatus.AfterStageTaskStatus[i], updateRun.Generation) } else { // retriable error klog.ErrorS(err, "Failed to create the approval request", "approvalRequest", requestRef, "stage", updatingStage.Name, "clusterStagedUpdateRun", updateRunRef) diff --git a/pkg/controllers/updaterun/execution_integration_test.go b/pkg/controllers/updaterun/execution_integration_test.go index 712c2c423..21f95936e 100644 --- a/pkg/controllers/updaterun/execution_integration_test.go +++ b/pkg/controllers/updaterun/execution_integration_test.go @@ -130,14 +130,6 @@ var _ = Describe("UpdateRun execution tests", func() { By("Creating a new cluster resource override") Expect(k8sClient.Create(ctx, clusterResourceOverride)).To(Succeed()) - - By("Creating a new clusterStagedUpdateRun") - Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) - - By("Validating the initialization succeeded and the execution started") - initialized := generateSucceededInitializationStatus(crp, updateRun, policySnapshot, updateStrategy, clusterResourceOverride) - wantStatus = generateExecutionStartedStatus(updateRun, initialized) - validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") }) AfterEach(OncePerOrdered, func() { @@ -181,7 +173,17 @@ var _ = Describe("UpdateRun execution tests", func() { clusterResourceOverride = nil }) - Context("Cluster staged update run should update clusters one by one", Ordered, func() { + Context("Cluster staged update run should update clusters one by one - strategy with double afterStageTasks", Ordered, func() { + BeforeAll(func() { + By("Creating a new clusterStagedUpdateRun") + Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) + + By("Validating the initialization succeeded and the execution started") + initialized := generateSucceededInitializationStatus(crp, updateRun, policySnapshot, updateStrategy, clusterResourceOverride) + wantStatus = generateExecutionStartedStatus(updateRun, initialized) + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + }) + It("Should mark the 1st cluster in the 1st stage as succeeded after marking the binding available", func() { By("Validating the 1st clusterResourceBinding is updated to Bound") binding := resourceBindings[numTargetClusters-1] // cluster-9 @@ -504,7 +506,301 @@ var _ = Describe("UpdateRun execution tests", func() { }) }) + Context("Cluster staged update run should update clusters one by one - strategy with single afterStageTask", Ordered, func() { + BeforeAll(func() { + By("Updating the strategy to have single afterStageTask") + updateStrategy.Spec.Stages[0].AfterStageTasks = updateStrategy.Spec.Stages[0].AfterStageTasks[:1] + updateStrategy.Spec.Stages[1].AfterStageTasks = updateStrategy.Spec.Stages[1].AfterStageTasks[:1] + Expect(k8sClient.Update(ctx, updateStrategy)).To(Succeed()) + + By("Creating a new clusterStagedUpdateRun") + Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) + + By("Validating the initialization succeeded and the execution started") + initialized := generateSucceededInitializationStatus(crp, updateRun, policySnapshot, updateStrategy, clusterResourceOverride) + wantStatus = generateExecutionStartedStatus(updateRun, initialized) + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + }) + + It("Should mark the 1st cluster in the 1st stage as succeeded after marking the binding available", func() { + By("Validating the 1st clusterResourceBinding is updated to Bound") + binding := resourceBindings[numTargetClusters-1] // cluster-9 + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + + 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") + + 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)) + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + + By("Validating the 1st stage has startTime set") + Expect(updateRun.Status.StagesStatus[0].StartTime).ShouldNot(BeNil()) + }) + + It("Should mark the 2nd cluster in the 1st stage as succeeded after marking the binding available", func() { + By("Validating the 2nd clusterResourceBinding is updated to Bound") + binding := resourceBindings[numTargetClusters-3] // cluster-7 + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + + By("Updating the 2nd 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") + + By("Validating the 2nd cluster has succeeded and 3rd cluster has started") + wantStatus.StagesStatus[0].Clusters[1].Conditions = append(wantStatus.StagesStatus[0].Clusters[1].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) + wantStatus.StagesStatus[0].Clusters[2].Conditions = append(wantStatus.StagesStatus[0].Clusters[2].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionStarted)) + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + }) + + It("Should mark the 3rd cluster in the 1st stage as succeeded after marking the binding available", func() { + By("Validating the 3rd clusterResourceBinding is updated to Bound") + binding := resourceBindings[numTargetClusters-5] // cluster-5 + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + + By("Updating the 3rd 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") + + By("Validating the 3rd cluster has succeeded and 4th cluster has started") + wantStatus.StagesStatus[0].Clusters[2].Conditions = append(wantStatus.StagesStatus[0].Clusters[2].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) + wantStatus.StagesStatus[0].Clusters[3].Conditions = append(wantStatus.StagesStatus[0].Clusters[3].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionStarted)) + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + }) + + It("Should mark the 4th cluster in the 1st stage as succeeded after marking the binding available", func() { + By("Validating the 4th clusterResourceBinding is updated to Bound") + binding := resourceBindings[numTargetClusters-7] // cluster-3 + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + + By("Updating the 4th 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") + + By("Validating the 4th cluster has succeeded and 5th cluster has started") + wantStatus.StagesStatus[0].Clusters[3].Conditions = append(wantStatus.StagesStatus[0].Clusters[3].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) + wantStatus.StagesStatus[0].Clusters[4].Conditions = append(wantStatus.StagesStatus[0].Clusters[4].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionStarted)) + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + }) + + It("Should mark the 5th cluster in the 1st stage as succeeded after marking the binding available", func() { + By("Validating the 5th clusterResourceBinding is updated to Bound") + binding := resourceBindings[numTargetClusters-9] // cluster-1 + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + + By("Updating the 5th 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") + + By("Validating the 5th cluster has succeeded and stage waiting for AfterStageTasks") + stageWaitingCondition := generateFalseCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing) + stageWaitingCondition.Reason = condition.StageUpdatingWaitingReason + wantStatus.StagesStatus[0].Clusters[4].Conditions = append(wantStatus.StagesStatus[0].Clusters[4].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) + wantStatus.StagesStatus[0].Conditions[0] = stageWaitingCondition // The progressing condition now becomes false with waiting reason. + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + }) + + It("Should complete the 1st stage after wait time passed and move on to the 2nd stage", func() { + By("Validating the TimedWait after stage task has completed and 2nd stage has started") + // Timedwait afterStageTask completed. + wantStatus.StagesStatus[0].AfterStageTaskStatus[0].Conditions = append(wantStatus.StagesStatus[0].AfterStageTaskStatus[0].Conditions, + generateTrueCondition(updateRun, placementv1beta1.AfterStageTaskConditionWaitTimeElapsed)) + // 1st stage completed. + 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)) + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + + By("Validating the 1st stage has endTime set") + Expect(updateRun.Status.StagesStatus[0].EndTime).ShouldNot(BeNil()) + + By("Validating the waitTime after stage task only completes after the wait time") + waitStartTime := meta.FindStatusCondition(updateRun.Status.StagesStatus[0].Conditions, string(placementv1beta1.StageUpdatingConditionProgressing)).LastTransitionTime.Time + waitEndTime := meta.FindStatusCondition(updateRun.Status.StagesStatus[0].AfterStageTaskStatus[0].Conditions, string(placementv1beta1.AfterStageTaskConditionWaitTimeElapsed)).LastTransitionTime.Time + Expect(waitStartTime.Add(updateStrategy.Spec.Stages[0].AfterStageTasks[0].WaitTime.Duration).After(waitEndTime)).Should(BeFalse(), + fmt.Sprintf("waitEndTime %v did not pass waitStartTime %v long enough, want at least %v", waitEndTime, waitStartTime, updateStrategy.Spec.Stages[0].AfterStageTasks[0].WaitTime.Duration)) + }) + + It("Should mark the 1st cluster in the 2nd stage as succeeded after 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) + + 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") + + 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)) + wantStatus.StagesStatus[1].Clusters[1].Conditions = append(wantStatus.StagesStatus[1].Clusters[1].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionStarted)) + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + + By("Validating the 2nd stage has startTime set") + Expect(updateRun.Status.StagesStatus[0].StartTime).ShouldNot(BeNil()) + }) + + It("Should mark the 2nd cluster in the 2nd stage as succeeded after marking the binding available", func() { + By("Validating the 2nd clusterResourceBinding is updated to Bound") + binding := resourceBindings[2] // cluster-2 + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 1) + + By("Updating the 2nd 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") + + By("Validating the 2nd cluster has succeeded and 3rd cluster has started") + wantStatus.StagesStatus[1].Clusters[1].Conditions = append(wantStatus.StagesStatus[1].Clusters[1].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) + wantStatus.StagesStatus[1].Clusters[2].Conditions = append(wantStatus.StagesStatus[1].Clusters[2].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionStarted)) + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + }) + + It("Should mark the 3rd cluster in the 2nd stage as succeeded after marking the binding available", func() { + By("Validating the 3rd clusterResourceBinding is updated to Bound") + binding := resourceBindings[4] // cluster-4 + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 1) + + By("Updating the 3rd 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") + + By("Validating the 3rd cluster has succeeded and 4th cluster has started") + wantStatus.StagesStatus[1].Clusters[2].Conditions = append(wantStatus.StagesStatus[1].Clusters[2].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) + wantStatus.StagesStatus[1].Clusters[3].Conditions = append(wantStatus.StagesStatus[1].Clusters[3].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionStarted)) + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + }) + + It("Should mark the 4th cluster in the 2nd stage as succeeded after marking the binding available", func() { + By("Validating the 4th clusterResourceBinding is updated to Bound") + binding := resourceBindings[6] // cluster-6 + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 1) + + By("Updating the 4th 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") + + By("Validating the 4th cluster has succeeded and 5th cluster has started") + wantStatus.StagesStatus[1].Clusters[3].Conditions = append(wantStatus.StagesStatus[1].Clusters[3].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) + wantStatus.StagesStatus[1].Clusters[4].Conditions = append(wantStatus.StagesStatus[1].Clusters[4].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionStarted)) + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + }) + + It("Should mark the 5th cluster in the 2nd stage as succeeded after marking the binding available", func() { + By("Validating the 5th clusterResourceBinding is updated to Bound") + binding := resourceBindings[8] // cluster-8 + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 1) + + By("Updating the 5th 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") + + By("Validating the 5th cluster has succeeded and the stage waiting for AfterStageTask") + wantStatus.StagesStatus[1].Clusters[4].Conditions = append(wantStatus.StagesStatus[1].Clusters[4].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) + stageWaitingCondition := generateFalseCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing) + stageWaitingCondition.Reason = condition.StageUpdatingWaitingReason + wantStatus.StagesStatus[1].Conditions[0] = stageWaitingCondition // The progressing condition now becomes false with waiting reason. + wantStatus.StagesStatus[1].AfterStageTaskStatus[0].Conditions = append(wantStatus.StagesStatus[1].AfterStageTaskStatus[0].Conditions, + generateTrueCondition(updateRun, placementv1beta1.AfterStageTaskConditionApprovalRequestCreated)) + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + }) + + It("Should complete the 2nd stage after the after stage task is completed and move on to the delete stage", func() { + By("Validating the approvalRequest has been created") + approvalRequest := &placementv1beta1.ClusterApprovalRequest{} + wantApprovalRequest := &placementv1beta1.ClusterApprovalRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: updateRun.Status.StagesStatus[1].AfterStageTaskStatus[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, + }, + } + Eventually(func() error { + if err := k8sClient.Get(ctx, types.NamespacedName{Name: wantApprovalRequest.Name}, approvalRequest); err != nil { + return err + } + if diff := cmp.Diff(wantApprovalRequest.Spec, approvalRequest.Spec); diff != "" { + return fmt.Errorf("approvalRequest has different spec (-want +got):\n%s", diff) + } + if diff := cmp.Diff(wantApprovalRequest.Labels, approvalRequest.Labels); diff != "" { + return fmt.Errorf("approvalRequest has different labels (-want +got):\n%s", diff) + } + return nil + }, timeout, interval).Should(Succeed(), "failed to validate the approvalRequest") + + By("Approving the approvalRequest") + meta.SetStatusCondition(&approvalRequest.Status.Conditions, generateTrueCondition(approvalRequest, placementv1beta1.ApprovalRequestConditionApproved)) + Expect(k8sClient.Status().Update(ctx, approvalRequest)).Should(Succeed(), "failed to update the approvalRequest status") + + By("Validating the 2nd stage has completed and the delete stage has started") + wantStatus.StagesStatus[1].AfterStageTaskStatus[0].Conditions = append(wantStatus.StagesStatus[1].AfterStageTaskStatus[0].Conditions, + generateTrueCondition(updateRun, placementv1beta1.AfterStageTaskConditionApprovalRequestApproved)) + wantStatus.StagesStatus[1].Conditions = append(wantStatus.StagesStatus[1].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) + + wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing)) + for i := range wantStatus.DeletionStageStatus.Clusters { + wantStatus.DeletionStageStatus.Clusters[i].Conditions = append(wantStatus.DeletionStageStatus.Clusters[i].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionStarted)) + } + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + + By("Validating the 2nd stage has endTime set") + Expect(updateRun.Status.StagesStatus[1].EndTime).ShouldNot(BeNil()) + + By("Validating the approvalRequest has ApprovalAccepted status") + Eventually(func() (bool, error) { + 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") + }) + + It("Should delete all the clusterResourceBindings in the delete stage and complete the update run", func() { + By("Validating the to-be-deleted bindings are all deleted") + Eventually(func() error { + for i := numTargetClusters; i < numTargetClusters+numUnscheduledClusters; i++ { + binding := &placementv1beta1.ClusterResourceBinding{} + err := k8sClient.Get(ctx, types.NamespacedName{Name: resourceBindings[i].Name}, binding) + if err == nil { + return fmt.Errorf("binding %s is not deleted", binding.Name) + } + if !apierrors.IsNotFound(err) { + return fmt.Errorf("Get binding %s does not return a not-found error: %w", binding.Name, err) + } + } + return nil + }, timeout, interval).Should(Succeed(), "failed to validate the deletion of the to-be-deleted bindings") + + By("Validating the delete stage and the clusterStagedUpdateRun has completed") + for i := range wantStatus.DeletionStageStatus.Clusters { + wantStatus.DeletionStageStatus.Clusters[i].Conditions = append(wantStatus.DeletionStageStatus.Clusters[i].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) + } + wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) + wantStatus.Conditions = append(wantStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionSucceeded)) + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + }) + }) + Context("Cluster staged update run should abort the execution within a failed updating stage", Ordered, func() { + BeforeAll(func() { + By("Creating a new clusterStagedUpdateRun") + Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) + + By("Validating the initialization succeeded and the execution started") + initialized := generateSucceededInitializationStatus(crp, updateRun, policySnapshot, updateStrategy, clusterResourceOverride) + wantStatus = generateExecutionStartedStatus(updateRun, initialized) + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + }) + It("Should keep waiting for the 1st cluster while it's not available", func() { By("Validating the 1st clusterResourceBinding is updated to Bound") binding := resourceBindings[numTargetClusters-1] // cluster-9 diff --git a/pkg/controllers/updaterun/initialization_integration_test.go b/pkg/controllers/updaterun/initialization_integration_test.go index 2e649f948..a89349e0e 100644 --- a/pkg/controllers/updaterun/initialization_integration_test.go +++ b/pkg/controllers/updaterun/initialization_integration_test.go @@ -753,7 +753,7 @@ func generateSucceededInitializationStatus( updateStrategy *placementv1beta1.ClusterStagedUpdateStrategy, clusterResourceOverride *placementv1alpha1.ClusterResourceOverrideSnapshot, ) *placementv1beta1.StagedUpdateRunStatus { - return &placementv1beta1.StagedUpdateRunStatus{ + status := &placementv1beta1.StagedUpdateRunStatus{ PolicySnapshotIndexUsed: policySnapshot.Labels[placementv1beta1.PolicyIndexLabel], PolicyObservedClusterCount: numberOfClustersAnnotation, ApplyStrategy: crp.Spec.Strategy.ApplyStrategy.DeepCopy(), @@ -768,13 +768,6 @@ func generateSucceededInitializationStatus( {ClusterName: "cluster-3", ClusterResourceOverrideSnapshots: []string{clusterResourceOverride.Name}}, {ClusterName: "cluster-1", ClusterResourceOverrideSnapshots: []string{clusterResourceOverride.Name}}, }, - AfterStageTaskStatus: []placementv1beta1.AfterStageTaskStatus{ - {Type: placementv1beta1.AfterStageTaskTypeTimedWait}, - { - Type: placementv1beta1.AfterStageTaskTypeApproval, - ApprovalRequestName: updateRun.Name + "-stage1", - }, - }, }, { StageName: "stage2", @@ -785,13 +778,6 @@ func generateSucceededInitializationStatus( {ClusterName: "cluster-6"}, {ClusterName: "cluster-8"}, }, - AfterStageTaskStatus: []placementv1beta1.AfterStageTaskStatus{ - { - Type: placementv1beta1.AfterStageTaskTypeApproval, - ApprovalRequestName: updateRun.Name + "-stage2", - }, - {Type: placementv1beta1.AfterStageTaskTypeTimedWait}, - }, }, }, DeletionStageStatus: &placementv1beta1.StageUpdatingStatus{ @@ -807,6 +793,18 @@ func generateSucceededInitializationStatus( generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionInitialized), }, } + for i := range status.StagesStatus { + var tasks []placementv1beta1.AfterStageTaskStatus + for _, task := range updateStrategy.Spec.Stages[i].AfterStageTasks { + taskStatus := placementv1beta1.AfterStageTaskStatus{Type: task.Type} + if task.Type == placementv1beta1.AfterStageTaskTypeApproval { + taskStatus.ApprovalRequestName = updateRun.Name + "-" + status.StagesStatus[i].StageName + } + tasks = append(tasks, taskStatus) + } + status.StagesStatus[i].AfterStageTaskStatus = tasks + } + return status } func generateExecutionStartedStatus(