diff --git a/.github/workflows/codespell.yml b/.github/workflows/codespell.yml index 8489d593d..eda29e860 100644 --- a/.github/workflows/codespell.yml +++ b/.github/workflows/codespell.yml @@ -12,7 +12,7 @@ jobs: runs-on: ubuntu-latest steps: - name: Harden Runner - uses: step-security/harden-runner@6c3c2f2c1c457b00c10c4848d6f5491db3b629df # v2.18.0 + uses: step-security/harden-runner@8d3c67de8e2fe68ef647c8db1e6a09f647780f40 # v2.19.0 with: egress-policy: audit diff --git a/.github/workflows/trivy.yml b/.github/workflows/trivy.yml index e4836973a..a39f6b32c 100644 --- a/.github/workflows/trivy.yml +++ b/.github/workflows/trivy.yml @@ -64,7 +64,7 @@ jobs: TAG: ${{ env.IMAGE_VERSION }} - name: Scan ${{ env.REGISTRY }}/${{ env.HUB_AGENT_IMAGE_NAME }}:${{ env.IMAGE_VERSION }} - uses: aquasecurity/trivy-action@57a97c7e7821a5776cebc9bb87c984fa69cba8f1 # v0.35.0 + uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0 with: image-ref: ${{ env.REGISTRY }}/${{ env.HUB_AGENT_IMAGE_NAME }}:${{ env.IMAGE_VERSION }} format: 'table' @@ -80,7 +80,7 @@ jobs: - name: Scan ${{ env.REGISTRY }}/${{ env.MEMBER_AGENT_IMAGE_NAME }}:${{ env.IMAGE_VERSION }} - uses: aquasecurity/trivy-action@57a97c7e7821a5776cebc9bb87c984fa69cba8f1 # v0.35.0 + uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0 with: image-ref: ${{ env.REGISTRY }}/${{ env.MEMBER_AGENT_IMAGE_NAME }}:${{ env.IMAGE_VERSION }} format: 'table' @@ -95,7 +95,7 @@ jobs: TRIVY_DB_REPOSITORY: mcr.microsoft.com/mirror/ghcr/aquasecurity/trivy-db - name: Scan ${{ env.REGISTRY }}/${{ env.REFRESH_TOKEN_IMAGE_NAME }}:${{ env.IMAGE_VERSION }} - uses: aquasecurity/trivy-action@57a97c7e7821a5776cebc9bb87c984fa69cba8f1 # v0.35.0 + uses: aquasecurity/trivy-action@ed142fd0673e97e23eac54620cfb913e5ce36c25 # v0.36.0 with: image-ref: ${{ env.REGISTRY }}/${{ env.REFRESH_TOKEN_IMAGE_NAME }}:${{ env.IMAGE_VERSION }} format: 'table' diff --git a/pkg/controllers/updaterun/controller.go b/pkg/controllers/updaterun/controller.go index 3e5cb095e..fa0c70925 100644 --- a/pkg/controllers/updaterun/controller.go +++ b/pkg/controllers/updaterun/controller.go @@ -109,8 +109,12 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim return runtime.Result{}, err } + // Track errors for metrics emission. The error is used to determine the failure type + // (user_error vs internal_error) in the emitted metrics. + var reconcileErr error // Emit the update run status metric based on status conditions in the updateRun. - defer emitUpdateRunStatusMetric(updateRun) + // Use a closure to capture reconcileErr by reference, so it reflects any updates made during reconciliation. + defer func() { emitUpdateRunStatusMetric(updateRun, reconcileErr) }() state := updateRun.GetUpdateRunSpec().State @@ -126,14 +130,13 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim } // Initialize the updateRun. - var initErr error - if toBeUpdatedBindings, toBeDeletedBindings, initErr = r.initialize(ctx, updateRun); initErr != nil { - klog.ErrorS(initErr, "Failed to initialize the updateRun", "updateRun", runObjRef) + if toBeUpdatedBindings, toBeDeletedBindings, reconcileErr = r.initialize(ctx, updateRun); reconcileErr != nil { + klog.ErrorS(reconcileErr, "Failed to initialize the updateRun", "updateRun", runObjRef) // errStagedUpdatedAborted cannot be retried. - if errors.Is(initErr, errStagedUpdatedAborted) { - return runtime.Result{}, r.recordInitializationFailed(ctx, updateRun, initErr.Error()) + if errors.Is(reconcileErr, errStagedUpdatedAborted) { + return runtime.Result{}, r.recordInitializationFailed(ctx, updateRun, reconcileErr.Error()) } - return runtime.Result{}, initErr + return runtime.Result{}, reconcileErr } updatingStageIndex = 0 // start from the first stage (typically for Initialize or Run states). klog.V(2).InfoS("Initialized the updateRun", "state", state, "updateRun", runObjRef) @@ -145,14 +148,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim klog.V(2).InfoS("The updateRun is finished", "finishedSuccessfully", finishedCond.Status, "updateRun", runObjRef) return runtime.Result{}, nil } - var validateErr error // Validate the updateRun status to ensure the update can be continued and get the updating stage index and cluster indices. - if updatingStageIndex, toBeUpdatedBindings, toBeDeletedBindings, validateErr = r.validate(ctx, updateRun); validateErr != nil { + if updatingStageIndex, toBeUpdatedBindings, toBeDeletedBindings, reconcileErr = r.validate(ctx, updateRun); reconcileErr != nil { + klog.ErrorS(reconcileErr, "Failed to validate the updateRun", "updateRun", runObjRef) // errStagedUpdatedAborted cannot be retried. - if errors.Is(validateErr, errStagedUpdatedAborted) { - return runtime.Result{}, r.recordUpdateRunFailed(ctx, updateRun, validateErr.Error()) + if errors.Is(reconcileErr, errStagedUpdatedAborted) { + return runtime.Result{}, r.recordUpdateRunFailed(ctx, updateRun, reconcileErr.Error()) } - return runtime.Result{}, validateErr + return runtime.Result{}, reconcileErr } klog.V(2).InfoS("The updateRun is validated", "updateRun", runObjRef) } @@ -163,16 +166,18 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim return runtime.Result{}, r.recordUpdateRunSucceeded(ctx, updateRun) } + var finished bool + var waitTime time.Duration switch state { case placementv1beta1.StateInitialize: klog.V(2).InfoS("The updateRun is initialized but not executed, waiting to execute", "state", state, "updateRun", runObjRef) case placementv1beta1.StateRun: // Execute the updateRun. klog.V(2).InfoS("Continue to execute the updateRun", "updatingStageIndex", updatingStageIndex, "updateRun", runObjRef) - finished, waitTime, execErr := r.execute(ctx, updateRun, updatingStageIndex, toBeUpdatedBindings, toBeDeletedBindings) - if errors.Is(execErr, errStagedUpdatedAborted) { + finished, waitTime, reconcileErr = r.execute(ctx, updateRun, updatingStageIndex, toBeUpdatedBindings, toBeDeletedBindings) + if errors.Is(reconcileErr, errStagedUpdatedAborted) { // errStagedUpdatedAborted cannot be retried. - return runtime.Result{}, r.recordUpdateRunFailed(ctx, updateRun, execErr.Error()) + return runtime.Result{}, r.recordUpdateRunFailed(ctx, updateRun, reconcileErr.Error()) } if finished { @@ -180,14 +185,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim return runtime.Result{}, r.recordUpdateRunSucceeded(ctx, updateRun) } - return r.handleIncompleteUpdateRun(ctx, updateRun, waitTime, execErr, state, runObjRef) + return r.handleIncompleteUpdateRun(ctx, updateRun, waitTime, reconcileErr, state, runObjRef) case placementv1beta1.StateStop: // Stop the updateRun. klog.V(2).InfoS("Stopping the updateRun", "state", state, "updatingStageIndex", updatingStageIndex, "updateRun", runObjRef) - finished, waitTime, stopErr := r.stop(updateRun, updatingStageIndex, toBeUpdatedBindings, toBeDeletedBindings) - if errors.Is(stopErr, errStagedUpdatedAborted) { + finished, waitTime, reconcileErr = r.stop(updateRun, updatingStageIndex, toBeUpdatedBindings, toBeDeletedBindings) + if errors.Is(reconcileErr, errStagedUpdatedAborted) { // errStagedUpdatedAborted cannot be retried. - return runtime.Result{}, r.recordUpdateRunFailed(ctx, updateRun, stopErr.Error()) + return runtime.Result{}, r.recordUpdateRunFailed(ctx, updateRun, reconcileErr.Error()) } if finished { @@ -195,13 +200,14 @@ func (r *Reconciler) Reconcile(ctx context.Context, req runtime.Request) (runtim return runtime.Result{}, r.recordUpdateRunStopped(ctx, updateRun) } - return r.handleIncompleteUpdateRun(ctx, updateRun, waitTime, stopErr, state, runObjRef) + return r.handleIncompleteUpdateRun(ctx, updateRun, waitTime, reconcileErr, state, runObjRef) default: // Initialize, Run, or Stop are the only supported states. - unexpectedErr := controller.NewUnexpectedBehaviorError(fmt.Errorf("found unsupported updateRun state: %s", state)) - klog.ErrorS(unexpectedErr, "Invalid updateRun state", "state", state, "updateRun", runObjRef) - return runtime.Result{}, r.recordUpdateRunFailed(ctx, updateRun, unexpectedErr.Error()) + reconcileErr = controller.NewUnexpectedBehaviorError(fmt.Errorf("found unsupported updateRun state: %s", state)) + klog.ErrorS(reconcileErr, "Invalid updateRun state", "state", state, "updateRun", runObjRef) + // This is an internal error - unsupported state should not happen + return runtime.Result{}, r.recordUpdateRunFailed(ctx, updateRun, reconcileErr.Error()) } return runtime.Result{}, nil } diff --git a/pkg/controllers/updaterun/controller_integration_test.go b/pkg/controllers/updaterun/controller_integration_test.go index 92b050610..0c2b80553 100644 --- a/pkg/controllers/updaterun/controller_integration_test.go +++ b/pkg/controllers/updaterun/controller_integration_test.go @@ -377,7 +377,7 @@ func generateApprovalStageTaskMetric( // the current updateRun state if the updateRun has transitioned since then. func generateMetricsLabels( updateRun *placementv1beta1.ClusterStagedUpdateRun, - state, condition, status, reason string, + state, condition, status, reason, failureType string, ) []*prometheusclientmodel.LabelPair { return []*prometheusclientmodel.LabelPair{ {Name: ptr.To("namespace"), Value: &updateRun.Namespace}, @@ -386,23 +386,24 @@ func generateMetricsLabels( {Name: ptr.To("condition"), Value: ptr.To(condition)}, {Name: ptr.To("status"), Value: ptr.To(status)}, {Name: ptr.To("reason"), Value: ptr.To(reason)}, + {Name: ptr.To("failureType"), Value: ptr.To(failureType)}, } } func generateInitializationSucceededMetric(state placementv1beta1.State, updateRun *placementv1beta1.ClusterStagedUpdateRun) *prometheusclientmodel.Metric { return &prometheusclientmodel.Metric{ Label: generateMetricsLabels(updateRun, string(state), string(placementv1beta1.StagedUpdateRunConditionInitialized), - string(metav1.ConditionTrue), condition.UpdateRunInitializeSucceededReason), + string(metav1.ConditionTrue), condition.UpdateRunInitializeSucceededReason, string(hubmetrics.UpdateRunFailureTypeNone)), Gauge: &prometheusclientmodel.Gauge{ Value: ptr.To(float64(time.Now().UnixNano()) / 1e9), }, } } -func generateInitializationFailedMetric(state placementv1beta1.State, updateRun *placementv1beta1.ClusterStagedUpdateRun) *prometheusclientmodel.Metric { +func generateInitializationFailedMetric(state placementv1beta1.State, updateRun *placementv1beta1.ClusterStagedUpdateRun, failureType string) *prometheusclientmodel.Metric { return &prometheusclientmodel.Metric{ Label: generateMetricsLabels(updateRun, string(state), string(placementv1beta1.StagedUpdateRunConditionInitialized), - string(metav1.ConditionFalse), condition.UpdateRunInitializeFailedReason), + string(metav1.ConditionFalse), condition.UpdateRunInitializeFailedReason, failureType), Gauge: &prometheusclientmodel.Gauge{ Value: ptr.To(float64(time.Now().UnixNano()) / 1e9), }, @@ -412,7 +413,7 @@ func generateInitializationFailedMetric(state placementv1beta1.State, updateRun func generateProgressingMetric(state placementv1beta1.State, updateRun *placementv1beta1.ClusterStagedUpdateRun) *prometheusclientmodel.Metric { return &prometheusclientmodel.Metric{ Label: generateMetricsLabels(updateRun, string(state), string(placementv1beta1.StagedUpdateRunConditionProgressing), - string(metav1.ConditionTrue), condition.UpdateRunProgressingReason), + string(metav1.ConditionTrue), condition.UpdateRunProgressingReason, string(hubmetrics.UpdateRunFailureTypeNone)), Gauge: &prometheusclientmodel.Gauge{ Value: ptr.To(float64(time.Now().UnixNano()) / 1e9), }, @@ -422,7 +423,7 @@ func generateProgressingMetric(state placementv1beta1.State, updateRun *placemen func generateWaitingMetric(state placementv1beta1.State, updateRun *placementv1beta1.ClusterStagedUpdateRun) *prometheusclientmodel.Metric { return &prometheusclientmodel.Metric{ Label: generateMetricsLabels(updateRun, string(state), string(placementv1beta1.StagedUpdateRunConditionProgressing), - string(metav1.ConditionFalse), condition.UpdateRunWaitingReason), + string(metav1.ConditionFalse), condition.UpdateRunWaitingReason, string(hubmetrics.UpdateRunFailureTypeNone)), Gauge: &prometheusclientmodel.Gauge{ Value: ptr.To(float64(time.Now().UnixNano()) / 1e9), }, @@ -432,17 +433,17 @@ func generateWaitingMetric(state placementv1beta1.State, updateRun *placementv1b func generateStuckMetric(state placementv1beta1.State, updateRun *placementv1beta1.ClusterStagedUpdateRun) *prometheusclientmodel.Metric { return &prometheusclientmodel.Metric{ Label: generateMetricsLabels(updateRun, string(state), string(placementv1beta1.StagedUpdateRunConditionProgressing), - string(metav1.ConditionFalse), condition.UpdateRunStuckReason), + string(metav1.ConditionFalse), condition.UpdateRunStuckReason, string(hubmetrics.UpdateRunFailureTypeInternalError)), Gauge: &prometheusclientmodel.Gauge{ Value: ptr.To(float64(time.Now().UnixNano()) / 1e9), }, } } -func generateFailedMetric(state placementv1beta1.State, updateRun *placementv1beta1.ClusterStagedUpdateRun) *prometheusclientmodel.Metric { +func generateFailedMetric(state placementv1beta1.State, updateRun *placementv1beta1.ClusterStagedUpdateRun, failureType string) *prometheusclientmodel.Metric { return &prometheusclientmodel.Metric{ Label: generateMetricsLabels(updateRun, string(state), string(placementv1beta1.StagedUpdateRunConditionSucceeded), - string(metav1.ConditionFalse), condition.UpdateRunFailedReason), + string(metav1.ConditionFalse), condition.UpdateRunFailedReason, failureType), Gauge: &prometheusclientmodel.Gauge{ Value: ptr.To(float64(time.Now().UnixNano()) / 1e9), }, @@ -452,7 +453,7 @@ func generateFailedMetric(state placementv1beta1.State, updateRun *placementv1be func generateStoppingMetric(state placementv1beta1.State, updateRun *placementv1beta1.ClusterStagedUpdateRun) *prometheusclientmodel.Metric { return &prometheusclientmodel.Metric{ Label: generateMetricsLabels(updateRun, string(state), string(placementv1beta1.StagedUpdateRunConditionProgressing), - string(metav1.ConditionUnknown), condition.UpdateRunStoppingReason), + string(metav1.ConditionUnknown), condition.UpdateRunStoppingReason, string(hubmetrics.UpdateRunFailureTypeNone)), Gauge: &prometheusclientmodel.Gauge{ Value: ptr.To(float64(time.Now().UnixNano()) / 1e9), }, @@ -462,7 +463,7 @@ func generateStoppingMetric(state placementv1beta1.State, updateRun *placementv1 func generateStoppedMetric(state placementv1beta1.State, updateRun *placementv1beta1.ClusterStagedUpdateRun) *prometheusclientmodel.Metric { return &prometheusclientmodel.Metric{ Label: generateMetricsLabels(updateRun, string(state), string(placementv1beta1.StagedUpdateRunConditionProgressing), - string(metav1.ConditionFalse), condition.UpdateRunStoppedReason), + string(metav1.ConditionFalse), condition.UpdateRunStoppedReason, string(hubmetrics.UpdateRunFailureTypeNone)), Gauge: &prometheusclientmodel.Gauge{ Value: ptr.To(float64(time.Now().UnixNano()) / 1e9), }, @@ -472,7 +473,7 @@ func generateStoppedMetric(state placementv1beta1.State, updateRun *placementv1b func generateSucceededMetric(state placementv1beta1.State, updateRun *placementv1beta1.ClusterStagedUpdateRun) *prometheusclientmodel.Metric { return &prometheusclientmodel.Metric{ Label: generateMetricsLabels(updateRun, string(state), string(placementv1beta1.StagedUpdateRunConditionSucceeded), - string(metav1.ConditionTrue), condition.UpdateRunSucceededReason), + string(metav1.ConditionTrue), condition.UpdateRunSucceededReason, string(hubmetrics.UpdateRunFailureTypeNone)), Gauge: &prometheusclientmodel.Gauge{ Value: ptr.To(float64(time.Now().UnixNano()) / 1e9), }, @@ -963,16 +964,16 @@ func generateFalseCondition(obj client.Object, condType any) metav1.Condition { } } -func generateFalseProgressingCondition(obj client.Object, condType any, reason string) metav1.Condition { +func generateFalseConditionWithReason(obj client.Object, condType any, reason string) metav1.Condition { falseCond := generateFalseCondition(obj, condType) falseCond.Reason = reason return falseCond } -func generateFalseConditionWithReason(obj client.Object, condType any, reason string) metav1.Condition { - falseCond := generateFalseCondition(obj, condType) - falseCond.Reason = reason - return falseCond +func generateTrueConditionWithReason(obj client.Object, condType any, reason string) metav1.Condition { + trueCond := generateTrueCondition(obj, condType) + trueCond.Reason = reason + return trueCond } func generateProgressingUnknownConditionWithReason(obj client.Object, reason string) metav1.Condition { diff --git a/pkg/controllers/updaterun/execution.go b/pkg/controllers/updaterun/execution.go index 3db55c3e6..164c6b6e0 100644 --- a/pkg/controllers/updaterun/execution.go +++ b/pkg/controllers/updaterun/execution.go @@ -76,6 +76,13 @@ func (r *Reconciler) execute( markUpdateRunProgressingIfNotWaitingOrStuck(updateRun) if updatingStageIndex < len(updateRunStatus.StagesStatus) { updatingStageStatus = &updateRunStatus.StagesStatus[updatingStageIndex] + // Skip the entire stage when there are 0 clusters. + if len(updatingStageStatus.Clusters) == 0 { + klog.V(2).InfoS("The stage has 0 clusters, skipping the entire stage", "stage", updatingStageStatus.StageName, "updateRun", klog.KObj(updateRun)) + markStageUpdatingSkippedNoClusters(updatingStageStatus, updateRun.GetGeneration(), "Stage skipped because it has no clusters") + // No need to wait to get to the next stage. + return false, 0, nil + } approved, err := r.checkBeforeStageTasksStatus(ctx, updatingStageIndex, updateRun) if err != nil { return false, 0, err @@ -249,7 +256,7 @@ func (r *Reconciler) executeUpdatingStage( "bindingSpecInSync", inSync, "bindingState", bindingSpec.State, "bindingRolloutStarted", rolloutStarted, "binding", klog.KObj(binding), "updateRun", updateRunRef) markClusterUpdatingFailed(clusterStatus, updateRun.GetGeneration(), preemptedErr.Error()) - clusterUpdateErrors = append(clusterUpdateErrors, fmt.Errorf("%w: %s", errStagedUpdatedAborted, preemptedErr.Error())) + clusterUpdateErrors = append(clusterUpdateErrors, fmt.Errorf("%w: %w", errStagedUpdatedAborted, preemptedErr)) continue } @@ -773,6 +780,30 @@ func markStageUpdatingSucceeded(stageUpdatingStatus *placementv1beta1.StageUpdat }) } +// markStageUpdatingSkippedNoClusters marks the stage updating status as skipped due to no clusters in memory. +func markStageUpdatingSkippedNoClusters(stageUpdatingStatus *placementv1beta1.StageUpdatingStatus, generation int64, message string) { + if stageUpdatingStatus.StartTime == nil { + stageUpdatingStatus.StartTime = &metav1.Time{Time: time.Now()} + } + if stageUpdatingStatus.EndTime == nil { + stageUpdatingStatus.EndTime = &metav1.Time{Time: time.Now()} + } + meta.SetStatusCondition(&stageUpdatingStatus.Conditions, metav1.Condition{ + Type: string(placementv1beta1.StageUpdatingConditionProgressing), + Status: metav1.ConditionFalse, + ObservedGeneration: generation, + Reason: condition.StageUpdatingSkippedNoClustersReason, + Message: message, + }) + meta.SetStatusCondition(&stageUpdatingStatus.Conditions, metav1.Condition{ + Type: string(placementv1beta1.StageUpdatingConditionSucceeded), + Status: metav1.ConditionTrue, + ObservedGeneration: generation, + Reason: condition.StageUpdatingSkippedNoClustersReason, + Message: message, + }) +} + // markStageUpdatingFailed marks the stage updating status as failed in memory. func markStageUpdatingFailed(stageUpdatingStatus *placementv1beta1.StageUpdatingStatus, generation int64, message string) { if stageUpdatingStatus.EndTime == nil { diff --git a/pkg/controllers/updaterun/execution_integration_test.go b/pkg/controllers/updaterun/execution_integration_test.go index a68c90deb..5bdbe9bf2 100644 --- a/pkg/controllers/updaterun/execution_integration_test.go +++ b/pkg/controllers/updaterun/execution_integration_test.go @@ -31,6 +31,7 @@ import ( clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1" placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" + hubmetrics "go.goms.io/fleet/pkg/metrics/hub" "go.goms.io/fleet/pkg/utils" "go.goms.io/fleet/pkg/utils/condition" ) @@ -161,7 +162,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization succeeded and the execution has not started") - initialized := generateSucceededInitializationStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, clusterResourceOverride) + initialized := generateInitializedStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 10, generateTenClusterStagesStatus(clusterResourceOverride), generateTenClusterDeletionStageStatus()) wantStatus = generateExecutionNotStartedStatus(updateRun, initialized) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") @@ -220,7 +221,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { 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) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[0]) By("Updating the 1st clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -245,7 +246,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { 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) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[1]) By("Updating the 2nd clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -264,7 +265,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { 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) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[2]) By("Updating the 3rd clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -283,7 +284,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { 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) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[3]) By("Updating the 4th clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -302,7 +303,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { 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) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[4]) By("Updating the 5th clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -352,7 +353,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { wantStatus.StagesStatus[0].AfterStageTaskStatus[1].Conditions = append(wantStatus.StagesStatus[0].AfterStageTaskStatus[1].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageTaskConditionApprovalRequestApproved)) // 1st stage completed, mark progressing condition reason as succeeded and add succeeded condition. - wantStatus.StagesStatus[0].Conditions[0] = generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) + wantStatus.StagesStatus[0].Conditions[0] = generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) wantStatus.StagesStatus[0].Conditions = append(wantStatus.StagesStatus[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // 2nd stage waiting for before stage tasks. wantStatus.StagesStatus[1].Conditions = append(wantStatus.StagesStatus[1].Conditions, generateFalseCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing)) @@ -435,7 +436,7 @@ var _ = Describe("UpdateRun execution tests - double stages", 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) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[1].Clusters[0]) By("Updating the 1st clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -462,7 +463,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { 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) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[1].Clusters[1]) By("Updating the 2nd clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -481,7 +482,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { 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) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[1].Clusters[2]) By("Updating the 3rd clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -500,7 +501,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { 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) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[1].Clusters[3]) By("Updating the 4th clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -519,7 +520,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { 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) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[1].Clusters[4]) By("Updating the 5th clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -565,7 +566,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { generateTrueCondition(updateRun, placementv1beta1.StageTaskConditionApprovalRequestApproved)) wantStatus.StagesStatus[1].AfterStageTaskStatus[1].Conditions = append(wantStatus.StagesStatus[1].AfterStageTaskStatus[1].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageTaskConditionWaitTimeElapsed)) - wantStatus.StagesStatus[1].Conditions[0] = generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) + wantStatus.StagesStatus[1].Conditions[0] = generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) wantStatus.StagesStatus[1].Conditions = append(wantStatus.StagesStatus[1].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) meta.SetStatusCondition(&wantStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing)) @@ -623,10 +624,10 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { wantStatus.DeletionStageStatus.Clusters[i].Conditions = append(wantStatus.DeletionStageStatus.Clusters[i].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) } // Mark the stage progressing condition as false with succeeded reason and add succeeded condition. - wantStatus.DeletionStageStatus.Conditions[0] = generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) + wantStatus.DeletionStageStatus.Conditions[0] = generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // Mark updateRun progressing condition as false with succeeded reason and add succeeded condition. - meta.SetStatusCondition(&wantStatus.Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason)) + meta.SetStatusCondition(&wantStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason)) wantStatus.Conditions = append(wantStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionSucceeded)) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") @@ -648,7 +649,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization succeeded and the execution has not started") - initialized := generateSucceededInitializationStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, clusterResourceOverride) + initialized := generateInitializedStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 10, generateTenClusterStagesStatus(clusterResourceOverride), generateTenClusterDeletionStageStatus()) wantStatus = generateExecutionNotStartedStatus(updateRun, initialized) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") @@ -682,7 +683,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { By("Validating the 1st clusterResourceBinding is updated to Bound") binding := resourceBindings[numTargetClusters-1] // cluster-9 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[0]) By("Updating the 1st clusterResourceBinding to ApplyFailed") meta.SetStatusCondition(&binding.Status.Conditions, generateFalseCondition(binding, placementv1beta1.ResourceBindingApplied)) @@ -703,7 +704,7 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { It("Should abort the execution if the binding has unexpected state", func() { By("Validating the 1st clusterResourceBinding is updated to Bound") binding := resourceBindings[numTargetClusters-1] // cluster-9 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[0]) By("Updating the 1st clusterResourceBinding's state to Scheduled (from Bound)") binding.Spec.State = placementv1beta1.BindingStateScheduled @@ -711,16 +712,174 @@ var _ = Describe("UpdateRun execution tests - double stages", func() { By("Validating the updateRun has failed") wantStatus.StagesStatus[0].Clusters[0].Conditions = append(wantStatus.StagesStatus[0].Clusters[0].Conditions, generateFalseCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) - wantStatus.StagesStatus[0].Conditions[0] = generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingFailedReason) + wantStatus.StagesStatus[0].Conditions[0] = generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingFailedReason) wantStatus.StagesStatus[0].Conditions = append(wantStatus.StagesStatus[0].Conditions, generateFalseCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) - meta.SetStatusCondition(&wantStatus.Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunFailedReason)) + meta.SetStatusCondition(&wantStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunFailedReason)) wantStatus.Conditions = append(wantStatus.Conditions, generateFalseCondition(updateRun, placementv1beta1.StagedUpdateRunConditionSucceeded)) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateWaitingMetric(placementv1beta1.StateRun, updateRun), generateProgressingMetric(placementv1beta1.StateRun, updateRun), generateStuckMetric(placementv1beta1.StateRun, updateRun), generateFailedMetric(placementv1beta1.StateRun, updateRun)) + validateUpdateRunMetricsEmitted(generateWaitingMetric(placementv1beta1.StateRun, updateRun), generateProgressingMetric(placementv1beta1.StateRun, updateRun), generateStuckMetric(placementv1beta1.StateRun, updateRun), generateFailedMetric(placementv1beta1.StateRun, updateRun, string(hubmetrics.UpdateRunFailureTypeUserError))) }) }) + + Context("Cluster staged update run should skip entire stage with zero clusters", Ordered, func() { + var wantApprovalRequest *placementv1beta1.ClusterApprovalRequest + BeforeAll(func() { + By("Reassigning the strategy to have 2 stages: one selecting all clusters and one empty") + sortingKey := "index" + updateStrategy.Spec.Stages = []placementv1beta1.StageConfig{ + { + Name: "stage1", + LabelSelector: &metav1.LabelSelector{}, // Empty label selector selects all clusters. + SortingLabelKey: &sortingKey, + AfterStageTasks: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + }, + }, + { + Name: "stage2", + LabelSelector: nil, + BeforeStageTasks: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + }, + AfterStageTasks: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeTimedWait, + WaitTime: &metav1.Duration{ + Duration: time.Second * 10, + }, + }, + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + }, + }, + } + Expect(k8sClient.Update(ctx, updateStrategy)).To(Succeed()) + + By("Creating a new clusterStagedUpdateRun") + Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) + + By("Validating the initialization succeeded") + initialized := generateInitializedStatus( + crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 10, + generateTenClusterSingleStageStatus(clusterResourceOverride), generateTenClusterDeletionStageStatus(), + ) + wantStatus = generateExecutionStartedStatus(updateRun, initialized) + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + + // Build wantStatus based on the current updateRun status after initialization. + wantStatus = updateRun.Status.DeepCopy() + + By("Checking update run status metrics are emitted") + validateUpdateRunMetricsEmitted(generateProgressingMetric(placementv1beta1.StateRun, updateRun)) + }) + + It("Should complete executing the clusters in the first stage and wait for after-stage tasks", func() { + By("Validating the 1st stage has startTime set") + Expect(updateRun.Status.StagesStatus[0].StartTime).ShouldNot(BeNil()) + + By("Marking all bindings in the first stage as available and validating all clusters have completed") + for i := 0; i < numTargetClusters; i++ { + binding := resourceBindings[numTargetClusters-1-i] + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[i]) + meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) + Expect(k8sClient.Status().Update(ctx, binding)).Should(Succeed(), "failed to update the binding status") + + meta.SetStatusCondition(&wantStatus.StagesStatus[0].Clusters[i].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionStarted)) + meta.SetStatusCondition(&wantStatus.StagesStatus[0].Clusters[i].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[0].Conditions = append(wantStatus.StagesStatus[0].AfterStageTaskStatus[0].Conditions, + generateTrueCondition(updateRun, placementv1beta1.StageTaskConditionApprovalRequestCreated)) + meta.SetStatusCondition(&wantStatus.Conditions, generateFalseCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing)) + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + + By("Validating the approvalRequest has been created") + wantApprovalRequest = &placementv1beta1.ClusterApprovalRequest{ + ObjectMeta: metav1.ObjectMeta{ + Name: updateRun.Status.StagesStatus[0].AfterStageTaskStatus[0].ApprovalRequestName, + Labels: map[string]string{ + placementv1beta1.TargetUpdatingStageNameLabel: updateRun.Status.StagesStatus[0].StageName, + placementv1beta1.TargetUpdateRunLabel: updateRun.Name, + placementv1beta1.TaskTypeLabel: placementv1beta1.AfterStageTaskLabelValue, + 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(placementv1beta1.StateRun, updateRun), generateWaitingMetric(placementv1beta1.StateRun, updateRun)) + }) + + It("Should accept the approval request", 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].AfterStageTaskStatus[0].Conditions = append(wantStatus.StagesStatus[0].AfterStageTaskStatus[0].Conditions, + generateTrueCondition(updateRun, placementv1beta1.StageTaskConditionApprovalRequestApproved)) + + By("Checking update run status metrics are emitted") + validateUpdateRunApprovalStageTaskMetric(generateApprovalStageTaskMetric(updateRun, placementv1beta1.AfterStageTaskLabelValue, 1)) + }) + + It("Should skip the entire empty stage (stage2) and complete update run", func() { + // 1st stage completed, mark progressing condition reason as succeeded and add succeeded condition. + wantStatus.StagesStatus[0].Conditions[0] = generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) + wantStatus.StagesStatus[0].Conditions = append(wantStatus.StagesStatus[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) + + By("Validating the 2nd stage has been skipped and the update run completed") + // 2nd stage will not have before-stage or after-stage conditions. + // 2nd stage will not have any clusters. + // 2nd stage conditions should populate as skipped. + wantStatus.StagesStatus[1].Conditions = append(wantStatus.StagesStatus[1].Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSkippedNoClustersReason)) + wantStatus.StagesStatus[1].Conditions = append(wantStatus.StagesStatus[1].Conditions, generateTrueConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionSucceeded, condition.StageUpdatingSkippedNoClustersReason)) + meta.SetStatusCondition(&wantStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing)) + + for i := range wantStatus.DeletionStageStatus.Clusters { + wantStatus.DeletionStageStatus.Clusters[i].Conditions = append(wantStatus.DeletionStageStatus.Clusters[i].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionStarted)) + wantStatus.DeletionStageStatus.Clusters[i].Conditions = append(wantStatus.DeletionStageStatus.Clusters[i].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) + } + wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason)) + wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) + // Complete update run. + meta.SetStatusCondition(&wantStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason)) + meta.SetStatusCondition(&wantStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionSucceeded)) + validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + + By("Validating the 1st stage has endTime set") + Expect(updateRun.Status.StagesStatus[0].EndTime).ShouldNot(BeNil()) + + By("Validating the 2nd stage has no startTime or endTime set") + Expect(updateRun.Status.StagesStatus[1].StartTime).ShouldNot(BeNil()) + Expect(updateRun.Status.StagesStatus[1].EndTime).ShouldNot(BeNil()) + + By("Checking update run status metrics are emitted") + validateUpdateRunMetricsEmitted(generateWaitingMetric(placementv1beta1.StateRun, updateRun), generateProgressingMetric(placementv1beta1.StateRun, updateRun), generateSucceededMetric(placementv1beta1.StateRun, updateRun)) + validateUpdateRunApprovalStageTaskMetric(generateApprovalStageTaskMetric(updateRun, placementv1beta1.AfterStageTaskLabelValue, 1)) + }) + + }) }) var _ = Describe("UpdateRun execution tests - single stage", func() { @@ -825,7 +984,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization succeeded and the execution started") - initialized := generateSucceededInitializationStatusForSmallClusters(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 0) + initialized := generateInitializedStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 3, generateThreeClusterStagesStatus(), generateDeletionStageStatus(0)) wantStatus = generateExecutionStartedStatus(updateRun, initialized) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") @@ -836,7 +995,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { 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[0] // cluster-0 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[0]) By("Updating the 1st clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -857,7 +1016,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { 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[1] // cluster-1 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[1]) By("Updating the 2nd clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -875,7 +1034,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { It("Should mark the 3rd cluster in the 1st stage as succeeded after marking the binding available and complete the updateRun", func() { By("Validating the 3rd clusterResourceBinding is updated to Bound") binding := resourceBindings[2] // cluster-2 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[2]) By("Updating the 3rd clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -884,13 +1043,13 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { By("Validating the 3rd cluster has succeeded and stage waiting for AfterStageTasks") wantStatus.StagesStatus[0].Clusters[2].Conditions = append(wantStatus.StagesStatus[0].Clusters[2].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) // 1st stage completed. - wantStatus.StagesStatus[0].Conditions[0] = generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) + wantStatus.StagesStatus[0].Conditions[0] = generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) wantStatus.StagesStatus[0].Conditions = append(wantStatus.StagesStatus[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // Mark the deletion stage progressing condition as false with succeeded reason and add succeeded condition. - wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason)) + wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason)) wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // Mark updateRun progressing condition as false with succeeded reason and add succeeded condition. - meta.SetStatusCondition(&wantStatus.Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason)) + meta.SetStatusCondition(&wantStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason)) wantStatus.Conditions = append(wantStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionSucceeded)) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") @@ -919,7 +1078,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization succeeded and the execution started") - initialized := generateSucceededInitializationStatusForSmallClusters(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 0) + initialized := generateInitializedStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 3, generateThreeClusterStagesStatus(), generateDeletionStageStatus(0)) wantStatus = generateExecutionNotStartedStatus(updateRun, initialized) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") @@ -978,7 +1137,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { 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[0] // cluster-0 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[0]) By("Updating the 1st clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -999,7 +1158,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { 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[1] // cluster-1 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[1]) By("Updating the 2nd clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -1017,7 +1176,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { It("Should Should complete the 1st stage and complete the updateRun after the 3rd cluster succeeds", func() { By("Validating the 3rd clusterResourceBinding is updated to Bound") binding := resourceBindings[2] // cluster-2 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[2]) By("Updating the 3rd clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -1025,17 +1184,17 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { By("Validating the 3rd cluster has succeeded") wantStatus.StagesStatus[0].Clusters[2].Conditions = append(wantStatus.StagesStatus[0].Clusters[2].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) - meta.SetStatusCondition(&wantStatus.StagesStatus[0].Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason)) + meta.SetStatusCondition(&wantStatus.StagesStatus[0].Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason)) By("Validating the 1st stage has completed and the updateRun has completed") // 1st stage completed. - wantStatus.StagesStatus[0].Conditions[0] = generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) + wantStatus.StagesStatus[0].Conditions[0] = generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) wantStatus.StagesStatus[0].Conditions = append(wantStatus.StagesStatus[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // Mark the deletion stage progressing condition as false with succeeded reason and add succeeded condition. - wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason)) + wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason)) wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // Mark updateRun progressing condition as false with succeeded reason and add succeeded condition. - meta.SetStatusCondition(&wantStatus.Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason)) + meta.SetStatusCondition(&wantStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason)) wantStatus.Conditions = append(wantStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionSucceeded)) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") @@ -1066,7 +1225,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization succeeded and the execution started") - initialized := generateSucceededInitializationStatusForSmallClusters(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 0) + initialized := generateInitializedStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 3, generateThreeClusterStagesStatus(), generateDeletionStageStatus(0)) wantStatus = generateExecutionStartedStatus(updateRun, initialized) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") @@ -1077,7 +1236,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { 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[0] // cluster-0 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[0]) By("Updating the 1st clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -1098,7 +1257,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { 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[1] // cluster-1 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[1]) By("Updating the 2nd clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -1116,7 +1275,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { 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[2] // cluster-2 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[2]) By("Updating the 3rd clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -1137,13 +1296,13 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { wantStatus.StagesStatus[0].AfterStageTaskStatus[0].Conditions = append(wantStatus.StagesStatus[0].AfterStageTaskStatus[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageTaskConditionWaitTimeElapsed)) // 1st stage completed. - wantStatus.StagesStatus[0].Conditions[0] = generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) + wantStatus.StagesStatus[0].Conditions[0] = generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) wantStatus.StagesStatus[0].Conditions = append(wantStatus.StagesStatus[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // Mark the deletion stage progressing condition as false with succeeded reason and add succeeded condition. - wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason)) + wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason)) wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // Mark updateRun progressing condition as false with succeeded reason and add succeeded condition. - meta.SetStatusCondition(&wantStatus.Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason)) + meta.SetStatusCondition(&wantStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason)) wantStatus.Conditions = append(wantStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionSucceeded)) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") @@ -1176,7 +1335,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization succeeded and the execution started") - initialized := generateSucceededInitializationStatusForSmallClusters(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 0) + initialized := generateInitializedStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 3, generateThreeClusterStagesStatus(), generateDeletionStageStatus(0)) wantStatus = generateExecutionStartedStatus(updateRun, initialized) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") @@ -1187,7 +1346,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { 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[0] // cluster-0 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[0]) By("Updating the 1st clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -1208,7 +1367,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { 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[1] // cluster-1 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[1]) By("Updating the 2nd clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -1226,7 +1385,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { 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[2] // cluster-2 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[2]) By("Updating the 3rd clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -1271,13 +1430,13 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { wantStatus.StagesStatus[0].AfterStageTaskStatus[0].Conditions = append(wantStatus.StagesStatus[0].AfterStageTaskStatus[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageTaskConditionApprovalRequestApproved)) // 1st stage completed. - wantStatus.StagesStatus[0].Conditions[0] = generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) + wantStatus.StagesStatus[0].Conditions[0] = generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) wantStatus.StagesStatus[0].Conditions = append(wantStatus.StagesStatus[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // Mark the deletion stage progressing condition as false with succeeded reason and add succeeded condition. - wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason)) + wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason)) wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // Mark updateRun progressing condition as false with succeeded reason and add succeeded condition. - meta.SetStatusCondition(&wantStatus.Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason)) + meta.SetStatusCondition(&wantStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason)) wantStatus.Conditions = append(wantStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionSucceeded)) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") @@ -1310,7 +1469,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization succeeded and the execution started") - initialized := generateSucceededInitializationStatusForSmallClusters(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 0) + initialized := generateInitializedStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 3, generateThreeClusterStagesStatus(), generateDeletionStageStatus(0)) wantStatus = generateExecutionStartedStatus(updateRun, initialized) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") @@ -1318,10 +1477,10 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { validateUpdateRunMetricsEmitted(generateProgressingMetric(placementv1beta1.StateRun, updateRun)) }) - It("Should mark the 1st cluster in the 1st stage as succeeded after marking the binding diff reported", func() { + 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[0] // cluster-0 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[0]) By("Updating the 1st clusterResourceBinding to Diff Reported") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingDiffReported)) @@ -1342,7 +1501,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { It("Should mark the 2nd cluster in the 1st stage as succeeded after marking the binding diff reported", func() { By("Validating the 2nd clusterResourceBinding is updated to Bound") binding := resourceBindings[1] // cluster-1 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[1]) By("Updating the 2nd clusterResourceBinding to Diff Reported") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingDiffReported)) @@ -1360,7 +1519,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { It("Should mark the 3rd cluster in the 1st stage as succeeded after marking the binding diff reported and complete the updateRun", func() { By("Validating the 3rd clusterResourceBinding is updated to Bound") binding := resourceBindings[2] // cluster-2 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[2]) By("Updating the 3rd clusterResourceBinding to Diff Reported") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingDiffReported)) @@ -1369,13 +1528,13 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { By("Validating the 3rd cluster has succeeded and stage waiting for AfterStageTasks") wantStatus.StagesStatus[0].Clusters[2].Conditions = append(wantStatus.StagesStatus[0].Clusters[2].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) // 1st stage completed. - wantStatus.StagesStatus[0].Conditions[0] = generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) + wantStatus.StagesStatus[0].Conditions[0] = generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) wantStatus.StagesStatus[0].Conditions = append(wantStatus.StagesStatus[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // Mark the deletion stage progressing condition as false with succeeded reason and add succeeded condition. - wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason)) + wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason)) wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // Mark updateRun progressing condition as false with succeeded reason and add succeeded condition. - meta.SetStatusCondition(&wantStatus.Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason)) + meta.SetStatusCondition(&wantStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason)) wantStatus.Conditions = append(wantStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionSucceeded)) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") @@ -1403,7 +1562,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization succeeded and the execution started") - initialized := generateSucceededInitializationStatusForSmallClusters(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 0) + initialized := generateInitializedStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 3, generateThreeClusterStagesStatus(), generateDeletionStageStatus(0)) wantStatus = generateExecutionStartedStatus(updateRun, initialized) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") @@ -1419,7 +1578,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { It("Should become stuck if the binding diff reporting fails", func() { By("Validating the 1st clusterResourceBinding is updated to Bound") binding := resourceBindings[0] // cluster-0 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[0]) By("Updating the 1st clusterResourceBinding to diff reported failed") meta.SetStatusCondition(&binding.Status.Conditions, generateFalseCondition(binding, placementv1beta1.ResourceBindingDiffReported)) @@ -1466,7 +1625,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization succeeded and the execution has not started") - initialized := generateSucceededInitializationStatusForSmallClusters(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 0) + initialized := generateInitializedStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 3, generateThreeClusterStagesStatus(), generateDeletionStageStatus(0)) wantStatus = generateExecutionNotStartedStatus(updateRun, initialized) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") @@ -1557,7 +1716,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { 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[0] // cluster-0 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[0]) By("Updating the 1st clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -1575,7 +1734,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { 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[1] // cluster-1 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[1]) By("Updating the 2nd clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -1590,7 +1749,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { 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[2] // cluster-3 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[2]) By("Updating the 3rd clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -1658,13 +1817,13 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { By("Validating the 1st stage has completed") wantStatus.StagesStatus[0].AfterStageTaskStatus[1].Conditions = append(wantStatus.StagesStatus[0].AfterStageTaskStatus[1].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageTaskConditionWaitTimeElapsed)) - wantStatus.StagesStatus[0].Conditions[0] = generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) + wantStatus.StagesStatus[0].Conditions[0] = generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) wantStatus.StagesStatus[0].Conditions = append(wantStatus.StagesStatus[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // Mark the deletion stage progressing condition as false with succeeded reason and add succeeded condition. - wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason)) + wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason)) wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // Mark updateRun progressing condition as false with succeeded reason and add succeeded condition. - meta.SetStatusCondition(&wantStatus.Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason)) + meta.SetStatusCondition(&wantStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason)) wantStatus.Conditions = append(wantStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionSucceeded)) // Need to have a longer wait time for the test to pass, because of the long wait time specified in the update strategy. timeout = time.Second * 90 @@ -1699,7 +1858,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization succeeded and but not execution started") - wantStatus = generateSucceededInitializationStatusForSmallClusters(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 0) + wantStatus = generateInitializedStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 3, generateThreeClusterStagesStatus(), generateDeletionStageStatus(0)) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") By("Checking update run status metrics are emitted") @@ -1739,7 +1898,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { 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[0] // cluster-0 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[0]) By("Updating the 1st clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -1760,7 +1919,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { 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[1] // cluster-1 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[1]) By("Updating the 2nd clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -1778,7 +1937,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { It("Should mark the 3rd cluster in the 1st stage as succeeded after marking the binding available and complete the updateRun", func() { By("Validating the 3rd clusterResourceBinding is updated to Bound") binding := resourceBindings[2] // cluster-2 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[2]) By("Updating the 3rd clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -1787,13 +1946,13 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { By("Validating the 3rd cluster has succeeded and stage waiting for AfterStageTasks") wantStatus.StagesStatus[0].Clusters[2].Conditions = append(wantStatus.StagesStatus[0].Clusters[2].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) // 1st stage completed. - wantStatus.StagesStatus[0].Conditions[0] = generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) + wantStatus.StagesStatus[0].Conditions[0] = generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) wantStatus.StagesStatus[0].Conditions = append(wantStatus.StagesStatus[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // Mark the deletion stage progressing condition as false with succeeded reason and add succeeded condition. - wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason)) + wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason)) wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // Mark updateRun progressing condition as false with succeeded reason and add succeeded condition. - wantStatus.Conditions[1] = generateFalseProgressingCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason) + wantStatus.Conditions[1] = generateFalseConditionWithReason(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason) wantStatus.Conditions = append(wantStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionSucceeded)) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") @@ -1807,7 +1966,7 @@ var _ = Describe("UpdateRun execution tests - single stage", func() { }) }) -func validateBindingState(ctx context.Context, binding *placementv1beta1.ClusterResourceBinding, resourceSnapshotName string, updateRun *placementv1beta1.ClusterStagedUpdateRun, stage int) { +func validateBindingState(ctx context.Context, binding *placementv1beta1.ClusterResourceBinding, resourceSnapshotName string, updateRun *placementv1beta1.ClusterStagedUpdateRun, clusterStatus *placementv1beta1.ClusterUpdatingStatus) { Eventually(func() error { if err := k8sClient.Get(ctx, types.NamespacedName{Name: binding.Name}, binding); err != nil { return err @@ -1819,10 +1978,10 @@ func validateBindingState(ctx context.Context, binding *placementv1beta1.Cluster if binding.Spec.ResourceSnapshotName != resourceSnapshotName { return fmt.Errorf("binding %s has different resourceSnapshot name, got %s, want %s", binding.Name, binding.Spec.ResourceSnapshotName, resourceSnapshotName) } - if diff := cmp.Diff(binding.Spec.ResourceOverrideSnapshots, updateRun.Status.StagesStatus[stage].Clusters[0].ResourceOverrideSnapshots); diff != "" { + if diff := cmp.Diff(binding.Spec.ResourceOverrideSnapshots, clusterStatus.ResourceOverrideSnapshots); diff != "" { return fmt.Errorf("binding %s has different resourceOverrideSnapshots (-want +got):\n%s", binding.Name, diff) } - if diff := cmp.Diff(binding.Spec.ClusterResourceOverrideSnapshots, updateRun.Status.StagesStatus[stage].Clusters[0].ClusterResourceOverrideSnapshots); diff != "" { + if diff := cmp.Diff(binding.Spec.ClusterResourceOverrideSnapshots, clusterStatus.ClusterResourceOverrideSnapshots); diff != "" { return fmt.Errorf("binding %s has different clusterResourceOverrideSnapshots(-want +got):\n%s", binding.Name, diff) } if diff := cmp.Diff(binding.Spec.ApplyStrategy, updateRun.Status.ApplyStrategy); diff != "" { @@ -1834,7 +1993,7 @@ func validateBindingState(ctx context.Context, binding *placementv1beta1.Cluster return fmt.Errorf("binding %s does not have RolloutStarted condition", binding.Name) } return nil - }, timeout, interval).Should(Succeed(), "failed to validate the binding state") + }, timeout*3, interval).Should(Succeed(), "failed to validate the binding state") } func validateNotBoundBindingState(ctx context.Context, binding *placementv1beta1.ClusterResourceBinding) { diff --git a/pkg/controllers/updaterun/execution_test.go b/pkg/controllers/updaterun/execution_test.go index cde44d330..4b9dc09aa 100644 --- a/pkg/controllers/updaterun/execution_test.go +++ b/pkg/controllers/updaterun/execution_test.go @@ -25,6 +25,7 @@ import ( "time" "github.com/google/go-cmp/cmp" + "github.com/google/go-cmp/cmp/cmpopts" "k8s.io/apimachinery/pkg/api/meta" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/runtime" @@ -1286,3 +1287,159 @@ func TestGenerateStuckClustersString(t *testing.T) { }) } } + +func TestExecute_ZeroClustersSkipsEntireStage(t *testing.T) { + tests := []struct { + name string + updateRun *placementv1beta1.ClusterStagedUpdateRun + stageIndex int + wantWaitTime time.Duration + wantErr bool + wantStageStatus placementv1beta1.StageUpdatingStatus + }{ + { + name: "zero clusters should skip entire stage including before-stage tasks", + updateRun: &placementv1beta1.ClusterStagedUpdateRun{ + ObjectMeta: metav1.ObjectMeta{ + Name: "test-update-run", + Generation: 1, + }, + Spec: placementv1beta1.UpdateRunSpec{ + PlacementName: "test-placement", + ResourceSnapshotIndex: "1", + State: placementv1beta1.StateRun, + }, + Status: placementv1beta1.UpdateRunStatus{ + ResourceSnapshotIndexUsed: "1", + StagesStatus: []placementv1beta1.StageUpdatingStatus{ + { + StageName: "empty-stage", + Clusters: []placementv1beta1.ClusterUpdatingStatus{}, // Zero clusters. + BeforeStageTaskStatus: []placementv1beta1.StageTaskStatus{ + { + Type: placementv1beta1.StageTaskTypeApproval, + ApprovalRequestName: "test-update-run-empty-before-stage", + }, + }, + AfterStageTaskStatus: []placementv1beta1.StageTaskStatus{ + { + Type: placementv1beta1.StageTaskTypeTimedWait, + }, + { + Type: placementv1beta1.StageTaskTypeApproval, + ApprovalRequestName: "test-update-run-after-empty-stage", + }, + }, + }, + }, + UpdateStrategySnapshot: &placementv1beta1.UpdateStrategySpec{ + Stages: []placementv1beta1.StageConfig{ + { + Name: "empty-stage", + MaxConcurrency: &intstr.IntOrString{Type: intstr.Int, IntVal: 1}, + BeforeStageTasks: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + }, + AfterStageTasks: []placementv1beta1.StageTask{ + { + Type: placementv1beta1.StageTaskTypeTimedWait, + WaitTime: &metav1.Duration{Duration: 5 * time.Minute}, + }, + { + Type: placementv1beta1.StageTaskTypeApproval, + }, + }, + }, + }, + }, + }, + }, + stageIndex: 0, + wantWaitTime: 0, // No wait time, stage is skipped. + wantErr: false, + wantStageStatus: placementv1beta1.StageUpdatingStatus{ + StageName: "empty-stage", + Clusters: []placementv1beta1.ClusterUpdatingStatus{}, // Zero clusters. + BeforeStageTaskStatus: []placementv1beta1.StageTaskStatus{ + { + Type: placementv1beta1.StageTaskTypeApproval, + ApprovalRequestName: "test-update-run-empty-before-stage", + }, + }, + AfterStageTaskStatus: []placementv1beta1.StageTaskStatus{ + { + Type: placementv1beta1.StageTaskTypeTimedWait, + }, + { + Type: placementv1beta1.StageTaskTypeApproval, + ApprovalRequestName: "test-update-run-after-empty-stage", + }, + }, + StartTime: &metav1.Time{Time: time.Now()}, + EndTime: &metav1.Time{Time: time.Now()}, + Conditions: []metav1.Condition{ + { + Type: string(placementv1beta1.StageUpdatingConditionProgressing), + Status: metav1.ConditionFalse, + ObservedGeneration: 1, + Reason: condition.StageUpdatingSkippedNoClustersReason, + Message: "Stage skipped because it has no clusters", + }, + { + Type: string(placementv1beta1.StageUpdatingConditionSucceeded), + Status: metav1.ConditionTrue, + ObservedGeneration: 1, + Reason: condition.StageUpdatingSkippedNoClustersReason, + Message: "Stage skipped because it has no clusters", + }, + }, + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + scheme := runtime.NewScheme() + _ = placementv1beta1.AddToScheme(scheme) + fakeClient := fake.NewClientBuilder(). + WithScheme(scheme). + WithObjects(tt.updateRun). + WithStatusSubresource(tt.updateRun). + Build() + r := Reconciler{ + Client: fakeClient, + } + ctx := context.Background() + + _, waitTime, gotErr := r.execute(ctx, tt.updateRun, tt.stageIndex, nil, nil) + + if (gotErr != nil) != tt.wantErr { + t.Fatalf("execute() error = %v, wantErr %v", gotErr, tt.wantErr) + } + + if waitTime != tt.wantWaitTime { + t.Fatalf("execute() waitTime = %v, want %v", waitTime, tt.wantWaitTime) + } + + gotStageStatus := tt.updateRun.Status.StagesStatus[tt.stageIndex] + + // Verify StartTime and EndTime are set for skipped stages. + if gotStageStatus.StartTime == nil { + t.Fatal("execute() StartTime should be set for skipped stage") + } + if gotStageStatus.EndTime == nil { + t.Fatal("execute() EndTime should be set for skipped stage") + } + + // Compare stage status using cmp.Diff, ignoring time fields. + if diff := cmp.Diff(tt.wantStageStatus, gotStageStatus, + cmpopts.IgnoreFields(metav1.Condition{}, "LastTransitionTime"), + cmpopts.IgnoreFields(placementv1beta1.StageUpdatingStatus{}, "StartTime", "EndTime"), + ); diff != "" { + t.Fatalf("execute() stage status mismatch (-want +got):\n%s", diff) + } + }) + } +} diff --git a/pkg/controllers/updaterun/initialization.go b/pkg/controllers/updaterun/initialization.go index 0bcbe661b..1cdf8bcff 100644 --- a/pkg/controllers/updaterun/initialization.go +++ b/pkg/controllers/updaterun/initialization.go @@ -91,7 +91,7 @@ func (r *Reconciler) validatePlacement(ctx context.Context, updateRun placementv if apierrors.IsNotFound(err) { placementNotFoundErr := controller.NewUserError(fmt.Errorf("parent placement not found")) klog.ErrorS(err, "Failed to get placement", "placement", placementKey, "updateRun", updateRunRef) - return nil, types.NamespacedName{}, fmt.Errorf("%w: %s", errValidationFailed, placementNotFoundErr.Error()) + return nil, types.NamespacedName{}, fmt.Errorf("%w: %w", errValidationFailed, placementNotFoundErr) } klog.ErrorS(err, "Failed to get placement", "placement", placementKey, "updateRun", updateRunRef) return nil, types.NamespacedName{}, controller.NewAPIServerError(true, err) @@ -106,7 +106,7 @@ func (r *Reconciler) validatePlacement(ctx context.Context, updateRun placementv if placementSpec.Strategy.Type != placementv1beta1.ExternalRolloutStrategyType { klog.V(2).InfoS("The placement does not have an external rollout strategy", "placement", placementKey, "updateRun", updateRunRef) wrongRolloutTypeErr := controller.NewUserError(errors.New("parent placement does not have an external rollout strategy, current strategy: " + string(placementSpec.Strategy.Type))) - return nil, types.NamespacedName{}, fmt.Errorf("%w: %s", errValidationFailed, wrongRolloutTypeErr.Error()) + return nil, types.NamespacedName{}, fmt.Errorf("%w: %w", errValidationFailed, wrongRolloutTypeErr) } updateRunStatus := updateRun.GetUpdateRunStatus() @@ -281,7 +281,7 @@ func (r *Reconciler) generateStagesByStrategy( if apierrors.IsNotFound(err) { // we won't continue or retry the initialization if the UpdateStrategy is not found. strategyNotFoundErr := controller.NewUserError(fmt.Errorf("referenced updateStrategy not found: `%s`", strategyKey)) - return fmt.Errorf("%w: %s", errValidationFailed, strategyNotFoundErr.Error()) + return fmt.Errorf("%w: %w", errValidationFailed, strategyNotFoundErr) } // other err can be retried. return controller.NewAPIServerError(true, err) @@ -348,13 +348,13 @@ func (r *Reconciler) computeRunStageStatus( 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", errValidationFailed, invalidBeforeStageErr.Error()) + return fmt.Errorf("%w: %w", errValidationFailed, invalidBeforeStageErr) } 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. invalidAfterStageErr := controller.NewUserError(fmt.Errorf("the after stage tasks are invalid, updateStrategy: `%s`, stage: %s, err: %s", strategyKey, stage.Name, err.Error())) - return fmt.Errorf("%w: %s", errValidationFailed, invalidAfterStageErr.Error()) + return fmt.Errorf("%w: %w", errValidationFailed, invalidAfterStageErr) } curStageUpdatingStatus := placementv1beta1.StageUpdatingStatus{StageName: stage.Name} @@ -364,7 +364,7 @@ func (r *Reconciler) computeRunStageStatus( klog.ErrorS(err, "Failed to convert label selector", "updateStrategy", strategyKey, "stageName", stage.Name, "labelSelector", stage.LabelSelector, "updateRun", updateRunRef) // no more retries here. invalidLabelErr := controller.NewUserError(fmt.Errorf("the stage label selector is invalid, updateStrategy: `%s`, stage: %s, err: %s", strategyKey, stage.Name, err.Error())) - return fmt.Errorf("%w: %s", errValidationFailed, invalidLabelErr.Error()) + return fmt.Errorf("%w: %w", errValidationFailed, invalidLabelErr) } // List all the clusters that match the label selector. var clusterList clusterv1beta1.MemberClusterList @@ -382,7 +382,7 @@ func (r *Reconciler) computeRunStageStatus( dupErr := controller.NewUserError(fmt.Errorf("cluster `%s` appears in more than one stages", cluster.Name)) klog.ErrorS(dupErr, "Failed to compute the stage", "updateStrategy", strategyKey, "stageName", stage.Name, "updateRun", updateRunRef) // no more retries here. - return fmt.Errorf("%w: %s", errValidationFailed, dupErr.Error()) + return fmt.Errorf("%w: %w", errValidationFailed, dupErr) } if stage.SortingLabelKey != nil { // interpret the label values as integers. @@ -390,7 +390,7 @@ func (r *Reconciler) computeRunStageStatus( keyErr := controller.NewUserError(fmt.Errorf("the sorting label `%s:%s` on cluster `%s` is not valid: %s", *stage.SortingLabelKey, cluster.Labels[*stage.SortingLabelKey], cluster.Name, err.Error())) klog.ErrorS(keyErr, "Failed to sort clusters in the stage", "updateStrategy", strategyKey, "stageName", stage.Name, "updateRun", updateRunRef) // no more retries here. - return fmt.Errorf("%w: %s", errValidationFailed, keyErr.Error()) + return fmt.Errorf("%w: %w", errValidationFailed, keyErr) } } curStageClusters = append(curStageClusters, cluster) @@ -457,7 +457,7 @@ func (r *Reconciler) computeRunStageStatus( sort.Strings(missingClusters) klog.ErrorS(missingErr, "Clusters are missing in any stage", "clusters", strings.Join(missingClusters, ", "), "updateStrategy", strategyKey, "updateRun", updateRunRef) // no more retries here, only show the first 10 missing clusters in the placement status. - return fmt.Errorf("%w: %s, total %d, showing up to 10: %s", errValidationFailed, missingErr.Error(), len(missingClusters), strings.Join(missingClusters[:min(10, len(missingClusters))], ", ")) + return fmt.Errorf("%w: %w, total %d, showing up to 10: %s", errValidationFailed, missingErr, len(missingClusters), strings.Join(missingClusters[:min(10, len(missingClusters))], ", ")) } return nil } @@ -571,10 +571,10 @@ func (r *Reconciler) getResourceSnapshotObjs(ctx context.Context, placement plac if updateRunSpec.ResourceSnapshotIndex != "" { snapshotIndex, err := strconv.Atoi(updateRunSpec.ResourceSnapshotIndex) if err != nil || snapshotIndex < 0 { - err := controller.NewUserError(fmt.Errorf("invalid resource snapshot index `%s` provided, expected an integer >= 0", updateRunSpec.ResourceSnapshotIndex)) - klog.ErrorS(err, "Failed to parse the resource snapshot index", "updateRun", updateRunRef) + userErr := controller.NewUserError(fmt.Errorf("invalid resource snapshot index `%s` provided, expected an integer >= 0", updateRunSpec.ResourceSnapshotIndex)) + klog.ErrorS(userErr, "Failed to parse the resource snapshot index", "updateRun", updateRunRef) // no more retries here. - return nil, fmt.Errorf("%w: %s", errValidationFailed, err.Error()) + return nil, fmt.Errorf("%w: %w", errValidationFailed, userErr) } resourceSnapshotList, err := controller.ListAllResourceSnapshotWithAnIndex(ctx, r.Client, updateRunSpec.ResourceSnapshotIndex, placementKey.Name, placementKey.Namespace) @@ -587,10 +587,10 @@ func (r *Reconciler) getResourceSnapshotObjs(ctx context.Context, placement plac resourceSnapshotObjs = resourceSnapshotList.GetResourceSnapshotObjs() if len(resourceSnapshotObjs) == 0 { - err := controller.NewUserError(fmt.Errorf("no resourceSnapshots with index `%d` found for placement `%s`", snapshotIndex, placementKey)) - klog.ErrorS(err, "No specified resourceSnapshots found", "updateRun", updateRunRef) + userErr := controller.NewUserError(fmt.Errorf("no resourceSnapshots with index `%d` found for placement `%s`", snapshotIndex, placementKey)) + klog.ErrorS(userErr, "No specified resourceSnapshots found", "updateRun", updateRunRef) // no more retries here. - return resourceSnapshotObjs, fmt.Errorf("%w: %s", errValidationFailed, err.Error()) + return resourceSnapshotObjs, fmt.Errorf("%w: %w", errValidationFailed, userErr) } return resourceSnapshotObjs, nil } @@ -602,7 +602,7 @@ func (r *Reconciler) getResourceSnapshotObjs(ctx context.Context, placement plac if err != nil { klog.ErrorS(err, "Failed to select resources for placement", "placement", placementKey, "updateRun", updateRunRef) if errors.Is(err, controller.ErrUserError) { - return nil, fmt.Errorf("%w: %s", errValidationFailed, err.Error()) + return nil, fmt.Errorf("%w: %w", errValidationFailed, err) } return nil, err } diff --git a/pkg/controllers/updaterun/initialization_integration_test.go b/pkg/controllers/updaterun/initialization_integration_test.go index 33d624bbf..9fa92d4d5 100644 --- a/pkg/controllers/updaterun/initialization_integration_test.go +++ b/pkg/controllers/updaterun/initialization_integration_test.go @@ -34,6 +34,7 @@ import ( clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1" placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" + hubmetrics "go.goms.io/fleet/pkg/metrics/hub" "go.goms.io/fleet/pkg/utils" "go.goms.io/fleet/pkg/utils/condition" "go.goms.io/fleet/pkg/utils/resource" @@ -63,6 +64,7 @@ var _ = Describe("Updaterun initialization tests", func() { var unscheduledClusters []*clusterv1beta1.MemberCluster var resourceSnapshot, resourceSnapshot2, resourceSnapshot3 *placementv1beta1.ClusterResourceSnapshot var clusterResourceOverride *placementv1beta1.ClusterResourceOverrideSnapshot + var failureType hubmetrics.UpdateRunFailureType BeforeEach(func() { testUpdateRunName = "updaterun-" + utils.RandStr() @@ -87,6 +89,10 @@ var _ = Describe("Updaterun initialization tests", func() { stageUpdatingWaitTime = time.Second * 3 clusterUpdatingWaitTime = time.Second * 2 + // Tests that fail below will report internal errors, if not user error, because only the updaterun controller + // is running in this test environment. Other controllers (e.g., rollout, work generator, + // binding controllers) are not set up, causing operations that depend on them to fail. + failureType = hubmetrics.UpdateRunFailureTypeInternalError }) AfterEach(func() { @@ -142,7 +148,7 @@ var _ = Describe("Updaterun initialization tests", func() { Context("Test validateCRP", func() { AfterEach(func() { By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun)) + validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun, string(failureType))) }) It("Should fail to initialize if CRP is not found", func() { @@ -150,6 +156,7 @@ var _ = Describe("Updaterun initialization tests", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization failed") + failureType = hubmetrics.UpdateRunFailureTypeUserError validateFailedInitCondition(ctx, updateRun, "parent placement not found") }) @@ -162,6 +169,7 @@ var _ = Describe("Updaterun initialization tests", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization failed") + failureType = hubmetrics.UpdateRunFailureTypeUserError validateFailedInitCondition(ctx, updateRun, "parent placement does not have an external rollout strategy") }) @@ -194,7 +202,7 @@ var _ = Describe("Updaterun initialization tests", func() { AfterEach(func() { By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun)) + validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun, string(failureType))) }) It("Should fail to initialize if the latest policy snapshot is not found", func() { @@ -304,7 +312,7 @@ var _ = Describe("Updaterun initialization tests", func() { AfterEach(func() { By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun)) + validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun, string(failureType))) }) It("Should copy the latest policy snapshot details to the updateRun status -- pickFixed policy", func() { By("Creating scheduling policy snapshot with pickFixed policy") @@ -352,7 +360,7 @@ var _ = Describe("Updaterun initialization tests", func() { }) AfterEach(func() { By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun)) + validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun, string(failureType))) }) It("Should copy the latest policy snapshot details to the updateRun status -- pickAll policy", func() { @@ -410,7 +418,7 @@ var _ = Describe("Updaterun initialization tests", func() { AfterEach(func() { By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun)) + validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun, string(failureType))) }) It("Should fail to initialize if there is no selected or to-be-deleted cluster", func() { By("Creating a new clusterStagedUpdateRun") @@ -504,7 +512,7 @@ var _ = Describe("Updaterun initialization tests", func() { AfterEach(func() { By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun)) + validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun, string(failureType))) }) It("Should not report error if there are only to-be-deleted clusters", func() { @@ -515,8 +523,8 @@ var _ = Describe("Updaterun initialization tests", func() { By("Creating a new clusterStagedUpdateRun") Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) - By("Validating the initialization not failed due to no selected cluster") - // it should fail due to strategy not found + By("Validating initialization proceeds past cluster selection (fails later due to missing strategy)") + failureType = hubmetrics.UpdateRunFailureTypeUserError validateFailedInitCondition(ctx, updateRun, "referenced updateStrategy not found") }) @@ -527,8 +535,8 @@ var _ = Describe("Updaterun initialization tests", func() { By("Creating a new clusterStagedUpdateRun") Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) - By("Validating the initialization not failed due to no selected cluster") - // it should fail due to strategy not found + By("Validating initialization proceeds past cluster selection (fails later due to missing strategy)") + failureType = hubmetrics.UpdateRunFailureTypeUserError validateFailedInitCondition(ctx, updateRun, "referenced updateStrategy not found") By("Validating the ObservedClusterCount is updated") @@ -569,7 +577,7 @@ var _ = Describe("Updaterun initialization tests", func() { AfterEach(func() { By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun)) + validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun, string(failureType))) }) It("fail the initialization if the clusterStagedUpdateStrategy is not found", func() { @@ -577,6 +585,7 @@ var _ = Describe("Updaterun initialization tests", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization failed") + failureType = hubmetrics.UpdateRunFailureTypeUserError validateFailedInitCondition(ctx, updateRun, "referenced updateStrategy not found") }) @@ -594,6 +603,7 @@ var _ = Describe("Updaterun initialization tests", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization failed") + failureType = hubmetrics.UpdateRunFailureTypeUserError validateFailedInitCondition(ctx, updateRun, "afterStageTasks cannot have two tasks of the same type") }) @@ -608,6 +618,7 @@ var _ = Describe("Updaterun initialization tests", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization failed") + failureType = hubmetrics.UpdateRunFailureTypeUserError validateFailedInitCondition(ctx, updateRun, "has wait duration <= 0") }) }) @@ -622,6 +633,7 @@ var _ = Describe("Updaterun initialization tests", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization failed") + failureType = hubmetrics.UpdateRunFailureTypeUserError validateFailedInitCondition(ctx, updateRun, "the sorting label `not-exist-label:`") }) @@ -636,6 +648,7 @@ var _ = Describe("Updaterun initialization tests", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization failed") + failureType = hubmetrics.UpdateRunFailureTypeUserError validateFailedInitCondition(ctx, updateRun, "appears in more than one stages") }) @@ -648,13 +661,14 @@ var _ = Describe("Updaterun initialization tests", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization failed") + failureType = hubmetrics.UpdateRunFailureTypeUserError validateFailedInitCondition(ctx, updateRun, "some clusters are not placed in any stage, total 5, showing up to 10: cluster-0, cluster-2, cluster-4, cluster-6, cluster-8") }) It("Should select all scheduled clusters if labelSelector is empty and select no clusters if labelSelector is nil", func() { By("Creating a clusterStagedUpdateStrategy with two stages, using empty labelSelector and nil labelSelector respectively") - updateStrategy.Spec.Stages[0].LabelSelector = nil // no clusters selected - updateStrategy.Spec.Stages[1].LabelSelector = &metav1.LabelSelector{} // all clusters selected + updateStrategy.Spec.Stages[0].LabelSelector = &metav1.LabelSelector{} // all clusters selected + updateStrategy.Spec.Stages[1].LabelSelector = nil // no clusters selected Expect(k8sClient.Create(ctx, updateStrategy)).To(Succeed()) By("Creating a new clusterStagedUpdateRun") @@ -667,23 +681,9 @@ var _ = Describe("Updaterun initialization tests", func() { } // no resource snapshot created in this test - want := generateSucceededInitializationStatus(crp, updateRun, "", policySnapshot, updateStrategy, clusterResourceOverride) - // No clusters should be selected in the first stage. - want.StagesStatus[0].Clusters = []placementv1beta1.ClusterUpdatingStatus{} - // All clusters should be selected in the second stage and sorted by name. - want.StagesStatus[1].Clusters = []placementv1beta1.ClusterUpdatingStatus{ - {ClusterName: "cluster-0"}, - {ClusterName: "cluster-1"}, - {ClusterName: "cluster-2"}, - {ClusterName: "cluster-3"}, - {ClusterName: "cluster-4"}, - {ClusterName: "cluster-5"}, - {ClusterName: "cluster-6"}, - {ClusterName: "cluster-7"}, - {ClusterName: "cluster-8"}, - {ClusterName: "cluster-9"}, - } - // initialization should fail due to resourceSnapshot not found. + want := generateInitializedStatus(crp, updateRun, "", policySnapshot, updateStrategy, 10, generateTenClusterSingleStageStatus(nil), generateTenClusterDeletionStageStatus()) + // Initialization should fail due to resourceSnapshot not found. + failureType = hubmetrics.UpdateRunFailureTypeUserError want.Conditions = []metav1.Condition{ generateFalseCondition(updateRun, placementv1beta1.StagedUpdateRunConditionInitialized), } @@ -710,12 +710,10 @@ var _ = Describe("Updaterun initialization tests", func() { } // no resource snapshot created in this test - want := generateSucceededInitializationStatus(crp, updateRun, "", policySnapshot, updateStrategy, clusterResourceOverride) - for i := range want.StagesStatus[0].Clusters { - // Remove the CROs, as they are not added in this test. - want.StagesStatus[0].Clusters[i].ClusterResourceOverrideSnapshots = nil - } - // initialization should fail due to resourceSnapshot not found. + stagesStatus := generateTenClusterStagesStatus(nil) + want := generateInitializedStatus(crp, updateRun, "", policySnapshot, updateStrategy, 10, stagesStatus, generateTenClusterDeletionStageStatus()) + // Initialization should fail due to resourceSnapshot not found. + failureType = hubmetrics.UpdateRunFailureTypeUserError want.Conditions = []metav1.Condition{ generateFalseCondition(updateRun, placementv1beta1.StagedUpdateRunConditionInitialized), } @@ -776,7 +774,8 @@ var _ = Describe("Updaterun initialization tests", func() { validateFailedInitCondition(ctx, updateRun, "invalid resource snapshot index `invalid-index` provided") By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun)) + failureType = hubmetrics.UpdateRunFailureTypeUserError + validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun, string(failureType))) }) It("Should fail to initialize if the specified resource snapshot index is invalid - negative integer", func() { @@ -788,7 +787,8 @@ var _ = Describe("Updaterun initialization tests", func() { validateFailedInitCondition(ctx, updateRun, "invalid resource snapshot index `-1` provided") By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun)) + failureType = hubmetrics.UpdateRunFailureTypeUserError + validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun, string(failureType))) }) It("Should fail to initialize if the specified resource snapshot is not found - no resourceSnapshots at all", func() { @@ -799,7 +799,8 @@ var _ = Describe("Updaterun initialization tests", func() { validateFailedInitCondition(ctx, updateRun, "no resourceSnapshots with index `0` found") By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun)) + failureType = hubmetrics.UpdateRunFailureTypeUserError + validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun, string(failureType))) }) It("Should create a new resource snapshot and succeed initialization when no resource index specified - no resourceSnapshots at all", func() { @@ -841,7 +842,8 @@ var _ = Describe("Updaterun initialization tests", func() { validateFailedInitCondition(ctx, updateRun, "no resourceSnapshots with index `0` found") By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun)) + failureType = hubmetrics.UpdateRunFailureTypeUserError + validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun, string(failureType))) }) It("Should fail to initialize if the specified resource snapshot is not found - no resource index label found", func() { @@ -856,7 +858,8 @@ var _ = Describe("Updaterun initialization tests", func() { validateFailedInitCondition(ctx, updateRun, "no resourceSnapshots with index `0` found") By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun)) + failureType = hubmetrics.UpdateRunFailureTypeUserError + validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun, string(failureType))) }) It("Should fail to initialize if the specified resource snapshot is not master snapshot", func() { @@ -871,7 +874,8 @@ var _ = Describe("Updaterun initialization tests", func() { validateFailedInitCondition(ctx, updateRun, "no master resourceSnapshot found for placement") By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun)) + failureType = hubmetrics.UpdateRunFailureTypeInternalError + validateUpdateRunMetricsEmitted(generateInitializationFailedMetric(placementv1beta1.StateInitialize, updateRun, string(failureType))) }) It("Should create a new resource snapshot when no resource index defined, previous snapshots hash does not match", func() { @@ -936,7 +940,7 @@ var _ = Describe("Updaterun initialization tests", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the clusterStagedUpdateRun stats") - initialized := generateSucceededInitializationStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, clusterResourceOverride) + initialized := generateInitializedStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 10, generateTenClusterStagesStatus(clusterResourceOverride), generateTenClusterDeletionStageStatus()) validateClusterStagedUpdateRunStatus(ctx, updateRun, initialized, "") By("Validating the clusterStagedUpdateRun initialized consistently") @@ -1097,137 +1101,174 @@ func validateFailedInitCondition(ctx context.Context, updateRun *placementv1beta }, timeout, interval).Should(Succeed(), "failed to validate the failed initialization condition") } -func generateSucceededInitializationStatus( +// populateStageTaskStatuses populates the BeforeStageTaskStatus and AfterStageTaskStatus +// for all stages in the status based on the strategy configuration. +func populateStageTaskStatuses( + status *placementv1beta1.UpdateRunStatus, + updateRunName string, + stages []placementv1beta1.StageConfig, +) { + for i := range status.StagesStatus { + status.StagesStatus[i].BeforeStageTaskStatus = buildTaskStatuses( + stages[i].BeforeStageTasks, + placementv1beta1.BeforeStageApprovalTaskNameFmt, + updateRunName, + status.StagesStatus[i].StageName, + ) + status.StagesStatus[i].AfterStageTaskStatus = buildTaskStatuses( + stages[i].AfterStageTasks, + placementv1beta1.AfterStageApprovalTaskNameFmt, + updateRunName, + status.StagesStatus[i].StageName, + ) + } +} + +// buildTaskStatuses creates StageTaskStatus slice from StageTask configuration. +func buildTaskStatuses( + tasks []placementv1beta1.StageTask, + approvalNameFmt string, + updateRunName string, + stageName string, +) []placementv1beta1.StageTaskStatus { + if len(tasks) == 0 { + return nil + } + taskStatuses := make([]placementv1beta1.StageTaskStatus, 0, len(tasks)) + for _, task := range tasks { + taskStatus := placementv1beta1.StageTaskStatus{Type: task.Type} + if task.Type == placementv1beta1.StageTaskTypeApproval { + taskStatus.ApprovalRequestName = fmt.Sprintf(approvalNameFmt, updateRunName, stageName) + } + taskStatuses = append(taskStatuses, taskStatus) + } + return taskStatuses +} + +// generateInitializedStatus creates an initialization status with custom stage configurations. +// stagesStatus should contain the StageName and Clusters for each stage. +// The BeforeStageTaskStatus and AfterStageTaskStatus are populated based on the updateStrategy stages. +func generateInitializedStatus( crp *placementv1beta1.ClusterResourcePlacement, updateRun *placementv1beta1.ClusterStagedUpdateRun, resourceSnapshotIndex string, policySnapshot *placementv1beta1.ClusterSchedulingPolicySnapshot, updateStrategy *placementv1beta1.ClusterStagedUpdateStrategy, - clusterResourceOverride *placementv1beta1.ClusterResourceOverrideSnapshot, + policyObservedClusterCount int, + stagesStatus []placementv1beta1.StageUpdatingStatus, + deletionStageStatus *placementv1beta1.StageUpdatingStatus, ) *placementv1beta1.UpdateRunStatus { status := &placementv1beta1.UpdateRunStatus{ PolicySnapshotIndexUsed: policySnapshot.Labels[placementv1beta1.PolicyIndexLabel], - PolicyObservedClusterCount: 10, + PolicyObservedClusterCount: policyObservedClusterCount, ResourceSnapshotIndexUsed: resourceSnapshotIndex, ApplyStrategy: crp.Spec.Strategy.ApplyStrategy.DeepCopy(), UpdateStrategySnapshot: &updateStrategy.Spec, - StagesStatus: []placementv1beta1.StageUpdatingStatus{ - { - StageName: "stage1", - Clusters: []placementv1beta1.ClusterUpdatingStatus{ - {ClusterName: "cluster-9", ClusterResourceOverrideSnapshots: []string{clusterResourceOverride.Name}}, - {ClusterName: "cluster-7", ClusterResourceOverrideSnapshots: []string{clusterResourceOverride.Name}}, - {ClusterName: "cluster-5", ClusterResourceOverrideSnapshots: []string{clusterResourceOverride.Name}}, - {ClusterName: "cluster-3", ClusterResourceOverrideSnapshots: []string{clusterResourceOverride.Name}}, - {ClusterName: "cluster-1", ClusterResourceOverrideSnapshots: []string{clusterResourceOverride.Name}}, - }, - }, - { - StageName: "stage2", - Clusters: []placementv1beta1.ClusterUpdatingStatus{ - {ClusterName: "cluster-0"}, - {ClusterName: "cluster-2"}, - {ClusterName: "cluster-4"}, - {ClusterName: "cluster-6"}, - {ClusterName: "cluster-8"}, - }, - }, - }, - DeletionStageStatus: &placementv1beta1.StageUpdatingStatus{ - StageName: "kubernetes-fleet.io/deleteStage", - Clusters: []placementv1beta1.ClusterUpdatingStatus{ - {ClusterName: "unscheduled-cluster-0"}, - {ClusterName: "unscheduled-cluster-1"}, - {ClusterName: "unscheduled-cluster-2"}, - }, - }, + StagesStatus: stagesStatus, + DeletionStageStatus: deletionStageStatus, Conditions: []metav1.Condition{ - // initialization should succeed! generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionInitialized), }, } - for i := range status.StagesStatus { - 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 + populateStageTaskStatuses(status, updateRun.Name, updateStrategy.Spec.Stages) + return status +} - 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 = fmt.Sprintf(placementv1beta1.AfterStageApprovalTaskNameFmt, updateRun.Name, status.StagesStatus[i].StageName) - } - afterTasks = append(afterTasks, taskStatus) +// generateTenClusterStagesStatus returns the stages status for 10 clusters split across 2 stages. +// Stage 1 contains odd-numbered clusters (9,7,5,3,1) in descending order. +// Stage 2 contains even-numbered clusters (0,2,4,6,8) in ascending order. +// If clusterResourceOverride is provided, it is applied to all odd-numbered clusters (stage 1). +func generateTenClusterStagesStatus(clusterResourceOverride *placementv1beta1.ClusterResourceOverrideSnapshot) []placementv1beta1.StageUpdatingStatus { + const numTargetClusters = 10 + + // Stage 1: odd-numbered clusters in descending order (9,7,5,3,1) + var stage1Clusters []placementv1beta1.ClusterUpdatingStatus + for i := numTargetClusters - 1; i >= 1; i -= 2 { + cluster := placementv1beta1.ClusterUpdatingStatus{ClusterName: fmt.Sprintf("cluster-%d", i)} + if clusterResourceOverride != nil { + cluster.ClusterResourceOverrideSnapshots = []string{clusterResourceOverride.Name} } - status.StagesStatus[i].AfterStageTaskStatus = afterTasks + stage1Clusters = append(stage1Clusters, cluster) + } + + // Stage 2: even-numbered clusters in ascending order (0,2,4,6,8) + var stage2Clusters []placementv1beta1.ClusterUpdatingStatus + for i := 0; i < numTargetClusters; i += 2 { + stage2Clusters = append(stage2Clusters, placementv1beta1.ClusterUpdatingStatus{ClusterName: fmt.Sprintf("cluster-%d", i)}) + } + + return []placementv1beta1.StageUpdatingStatus{ + { + StageName: "stage1", + Clusters: stage1Clusters, + }, + { + StageName: "stage2", + Clusters: stage2Clusters, + }, } - return status } -func generateSucceededInitializationStatusForSmallClusters( - crp *placementv1beta1.ClusterResourcePlacement, - updateRun *placementv1beta1.ClusterStagedUpdateRun, - resourceSnapshotIndex string, - policySnapshot *placementv1beta1.ClusterSchedulingPolicySnapshot, - updateStrategy *placementv1beta1.ClusterStagedUpdateStrategy, - numUnscheduledClusters int, -) *placementv1beta1.UpdateRunStatus { - status := &placementv1beta1.UpdateRunStatus{ - PolicySnapshotIndexUsed: policySnapshot.Labels[placementv1beta1.PolicyIndexLabel], - PolicyObservedClusterCount: 3, - ResourceSnapshotIndexUsed: resourceSnapshotIndex, - ApplyStrategy: crp.Spec.Strategy.ApplyStrategy.DeepCopy(), - UpdateStrategySnapshot: &updateStrategy.Spec, - StagesStatus: []placementv1beta1.StageUpdatingStatus{ - { - StageName: "stage1", - Clusters: []placementv1beta1.ClusterUpdatingStatus{ - {ClusterName: "cluster-0"}, - {ClusterName: "cluster-1"}, - {ClusterName: "cluster-2"}, - }, - }, +// generateTenClusterDeletionStageStatus returns the deletion stage status for 3 unscheduled clusters. +func generateTenClusterDeletionStageStatus() *placementv1beta1.StageUpdatingStatus { + return &placementv1beta1.StageUpdatingStatus{ + StageName: "kubernetes-fleet.io/deleteStage", + Clusters: []placementv1beta1.ClusterUpdatingStatus{ + {ClusterName: "unscheduled-cluster-0"}, + {ClusterName: "unscheduled-cluster-1"}, + {ClusterName: "unscheduled-cluster-2"}, + }, + } +} + +// generateTenClusterSingleStageStatus returns a stages status with stage1 containing all 10 clusters and empty. +// Clusters are sorted by name in descending order (9,8,7,...,0). +// This is used for testing label selector scenarios where all clusters end up in one stage. +func generateTenClusterSingleStageStatus(clusterResourceOverride *placementv1beta1.ClusterResourceOverrideSnapshot) []placementv1beta1.StageUpdatingStatus { + var clusters []placementv1beta1.ClusterUpdatingStatus + for i := 9; i >= 0; i-- { + clusterUpdatingStatus := placementv1beta1.ClusterUpdatingStatus{ClusterName: fmt.Sprintf("cluster-%d", i)} + if i%2 != 0 && clusterResourceOverride != nil { + clusterUpdatingStatus.ClusterResourceOverrideSnapshots = []string{clusterResourceOverride.Name} + } + clusters = append(clusters, clusterUpdatingStatus) + } + return []placementv1beta1.StageUpdatingStatus{ + { + StageName: "stage1", + Clusters: clusters, }, - DeletionStageStatus: &placementv1beta1.StageUpdatingStatus{ - StageName: "kubernetes-fleet.io/deleteStage", + { + StageName: "stage2", Clusters: []placementv1beta1.ClusterUpdatingStatus{}, }, - Conditions: []metav1.Condition{ - // initialization should succeed! - generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionInitialized), + } +} + +// generateThreeClusterStagesStatus returns the stages status for 3 clusters in a single stage. +func generateThreeClusterStagesStatus() []placementv1beta1.StageUpdatingStatus { + return []placementv1beta1.StageUpdatingStatus{ + { + StageName: "stage1", + Clusters: []placementv1beta1.ClusterUpdatingStatus{ + {ClusterName: "cluster-0"}, + {ClusterName: "cluster-1"}, + {ClusterName: "cluster-2"}, + }, }, } +} + +// generateDeletionStageStatus returns a deletion stage status with the specified number of unscheduled clusters. +func generateDeletionStageStatus(numUnscheduledClusters int) *placementv1beta1.StageUpdatingStatus { + status := &placementv1beta1.StageUpdatingStatus{ + StageName: "kubernetes-fleet.io/deleteStage", + Clusters: []placementv1beta1.ClusterUpdatingStatus{}, + } for i := range numUnscheduledClusters { - status.DeletionStageStatus.Clusters = append(status.DeletionStageStatus.Clusters, + status.Clusters = append(status.Clusters, placementv1beta1.ClusterUpdatingStatus{ClusterName: fmt.Sprintf("unscheduled-cluster-%d", i)}) } - for i := range status.StagesStatus { - 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 = fmt.Sprintf(placementv1beta1.AfterStageApprovalTaskNameFmt, updateRun.Name, status.StagesStatus[i].StageName) - } - afterTasks = append(afterTasks, taskStatus) - } - status.StagesStatus[i].AfterStageTaskStatus = afterTasks - } return status } diff --git a/pkg/controllers/updaterun/metrics.go b/pkg/controllers/updaterun/metrics.go index a038c515c..24cad3312 100644 --- a/pkg/controllers/updaterun/metrics.go +++ b/pkg/controllers/updaterun/metrics.go @@ -17,6 +17,7 @@ limitations under the License. package updaterun import ( + "errors" "time" "github.com/prometheus/client_golang/prometheus" @@ -26,6 +27,7 @@ import ( placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" hubmetrics "go.goms.io/fleet/pkg/metrics/hub" "go.goms.io/fleet/pkg/utils/condition" + "go.goms.io/fleet/pkg/utils/controller" ) // deleteUpdateRunMetrics deletes the metrics related to the update run when the update run is deleted. @@ -35,30 +37,51 @@ func deleteUpdateRunMetrics(updateRun placementv1beta1.UpdateRunObj) { hubmetrics.FleetUpdateRunApprovalRequestLatencySeconds.DeletePartialMatch(prometheus.Labels{"namespace": updateRun.GetNamespace(), "name": updateRun.GetName()}) } +// determineFailureType determines the type of failure based on the condition status and error. +// It returns: +// - "none" for successful, in-progress, waiting, or stopping conditions +// - "user_error" for known customer configuration errors (when err wraps controller.ErrUserError) +// - "internal_error" for errors that require investigation +func determineFailureType(err error) hubmetrics.UpdateRunFailureType { + if err != nil { + // Check if error for the failed condition is a user error. + if errors.Is(err, controller.ErrUserError) { + return hubmetrics.UpdateRunFailureTypeUserError + } + // Failed condition that is not a user error is an internal error. + return hubmetrics.UpdateRunFailureTypeInternalError + } + + // If there's no error, there's no failure to categorize. + return hubmetrics.UpdateRunFailureTypeNone +} + // emitUpdateRunStatusMetric emits the update run status metric based on status conditions in the updateRun. -func emitUpdateRunStatusMetric(updateRun placementv1beta1.UpdateRunObj) { +// The err parameter is used to determine the failure type for failed conditions. +func emitUpdateRunStatusMetric(updateRun placementv1beta1.UpdateRunObj, err error) { generation := updateRun.GetGeneration() state := updateRun.GetUpdateRunSpec().State updateRunStatus := updateRun.GetUpdateRunStatus() + failureType := determineFailureType(err) succeedCond := meta.FindStatusCondition(updateRunStatus.Conditions, string(placementv1beta1.StagedUpdateRunConditionSucceeded)) if succeedCond != nil && succeedCond.ObservedGeneration == generation { hubmetrics.FleetUpdateRunStatusLastTimestampSeconds.WithLabelValues(updateRun.GetNamespace(), updateRun.GetName(), string(state), - string(placementv1beta1.StagedUpdateRunConditionSucceeded), string(succeedCond.Status), succeedCond.Reason).SetToCurrentTime() + string(placementv1beta1.StagedUpdateRunConditionSucceeded), string(succeedCond.Status), succeedCond.Reason, string(failureType)).SetToCurrentTime() return } progressingCond := meta.FindStatusCondition(updateRunStatus.Conditions, string(placementv1beta1.StagedUpdateRunConditionProgressing)) if progressingCond != nil && progressingCond.ObservedGeneration == generation { hubmetrics.FleetUpdateRunStatusLastTimestampSeconds.WithLabelValues(updateRun.GetNamespace(), updateRun.GetName(), string(state), - string(placementv1beta1.StagedUpdateRunConditionProgressing), string(progressingCond.Status), progressingCond.Reason).SetToCurrentTime() + string(placementv1beta1.StagedUpdateRunConditionProgressing), string(progressingCond.Status), progressingCond.Reason, string(failureType)).SetToCurrentTime() return } initializedCond := meta.FindStatusCondition(updateRunStatus.Conditions, string(placementv1beta1.StagedUpdateRunConditionInitialized)) if initializedCond != nil && initializedCond.ObservedGeneration == generation { hubmetrics.FleetUpdateRunStatusLastTimestampSeconds.WithLabelValues(updateRun.GetNamespace(), updateRun.GetName(), string(state), - string(placementv1beta1.StagedUpdateRunConditionInitialized), string(initializedCond.Status), initializedCond.Reason).SetToCurrentTime() + string(placementv1beta1.StagedUpdateRunConditionInitialized), string(initializedCond.Status), initializedCond.Reason, string(failureType)).SetToCurrentTime() return } diff --git a/pkg/controllers/updaterun/metrics_test.go b/pkg/controllers/updaterun/metrics_test.go new file mode 100644 index 000000000..7415d002c --- /dev/null +++ b/pkg/controllers/updaterun/metrics_test.go @@ -0,0 +1,66 @@ +/* +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 ( + "errors" + "fmt" + "testing" + + "github.com/google/go-cmp/cmp" + + hubmetrics "go.goms.io/fleet/pkg/metrics/hub" + "go.goms.io/fleet/pkg/utils/controller" +) + +func TestDetermineFailureType(t *testing.T) { + tests := []struct { + name string + err error + want hubmetrics.UpdateRunFailureType + }{ + { + name: "update run no failure", + err: nil, + want: hubmetrics.UpdateRunFailureTypeNone, + }, + { + name: "update run failed with user error", + err: fmt.Errorf("cannot continue the updateRun: failed to validate the updateRun: %w", controller.ErrUserError), + want: hubmetrics.UpdateRunFailureTypeUserError, + }, + { + name: "update run failed with internal error", + err: errors.New("cannot continue the updateRun"), + want: hubmetrics.UpdateRunFailureTypeInternalError, + }, + { + name: "update run is stuck - internal error", + err: errors.New("updateRun is stuck waiting for 1 cluster(s) in stage stage1 to finish updating, please check placement status for potential errors"), + want: hubmetrics.UpdateRunFailureTypeInternalError, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + got := determineFailureType(tc.err) + if diff := cmp.Diff(tc.want, got); diff != "" { + t.Errorf("determineFailureType() = %v, want %v, diff (-want +got):\n%s", got, tc.want, diff) + } + }) + } +} diff --git a/pkg/controllers/updaterun/stop_integration_test.go b/pkg/controllers/updaterun/stop_integration_test.go index cf235b420..acacd8ae4 100644 --- a/pkg/controllers/updaterun/stop_integration_test.go +++ b/pkg/controllers/updaterun/stop_integration_test.go @@ -173,7 +173,7 @@ var _ = Describe("UpdateRun stop tests", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization succeeded and the execution has not started") - initialized := generateSucceededInitializationStatusForSmallClusters(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 3) + initialized := generateInitializedStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 3, generateThreeClusterStagesStatus(), generateDeletionStageStatus(3)) wantStatus = generateExecutionNotStartedStatus(updateRun, initialized) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") @@ -251,7 +251,7 @@ var _ = Describe("UpdateRun stop tests", func() { 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[0] // cluster-0 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[0]) By("Updating the 1st clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -301,7 +301,7 @@ var _ = Describe("UpdateRun stop tests", func() { It("Should have completely stopped after the in-progress cluster has finished updating", func() { By("Validating the 2nd clusterResourceBinding is updated to Bound") binding := resourceBindings[1] // cluster-1 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[1]) By("Updating the 2nd clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -311,9 +311,9 @@ var _ = Describe("UpdateRun stop tests", func() { // Mark 2nd cluster succeeded. meta.SetStatusCondition(&wantStatus.StagesStatus[0].Clusters[1].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) // Mark stage progressing condition as false with stopped reason. - meta.SetStatusCondition(&wantStatus.StagesStatus[0].Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingStoppedReason)) + meta.SetStatusCondition(&wantStatus.StagesStatus[0].Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingStoppedReason)) // Mark updateRun progressing condition as false with stopped reason. - meta.SetStatusCondition(&wantStatus.Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunStoppedReason)) + meta.SetStatusCondition(&wantStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunStoppedReason)) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") By("Checking update run metrics are emitted") @@ -351,7 +351,7 @@ var _ = Describe("UpdateRun stop tests", func() { 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[2] // cluster-2 - validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, 0) + validateBindingState(ctx, binding, resourceSnapshot.Name, updateRun, &updateRun.Status.StagesStatus[0].Clusters[2]) By("Updating the 3rd clusterResourceBinding to Available") meta.SetStatusCondition(&binding.Status.Conditions, generateTrueCondition(binding, placementv1beta1.ResourceBindingAvailable)) @@ -462,7 +462,7 @@ var _ = Describe("UpdateRun stop tests", func() { wantStatus.StagesStatus[0].AfterStageTaskStatus[1].Conditions = append(wantStatus.StagesStatus[0].AfterStageTaskStatus[1].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageTaskConditionWaitTimeElapsed)) // 1st stage completed, mark progressing condition reason as succeeded and add succeeded condition. - wantStatus.StagesStatus[0].Conditions[0] = generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) + wantStatus.StagesStatus[0].Conditions[0] = generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) wantStatus.StagesStatus[0].Conditions = append(wantStatus.StagesStatus[0].Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // Deletion stage started. Mark deletion stage progressing condition as true with progressing reason. meta.SetStatusCondition(&wantStatus.DeletionStageStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing)) @@ -612,10 +612,10 @@ var _ = Describe("UpdateRun stop tests", func() { meta.SetStatusCondition(&wantStatus.DeletionStageStatus.Clusters[i].Conditions, generateTrueCondition(updateRun, placementv1beta1.ClusterUpdatingConditionSucceeded)) } // Mark the stage progressing condition as false with succeeded reason and add succeeded condition. - wantStatus.DeletionStageStatus.Conditions[0] = generateFalseProgressingCondition(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) + wantStatus.DeletionStageStatus.Conditions[0] = generateFalseConditionWithReason(updateRun, placementv1beta1.StageUpdatingConditionProgressing, condition.StageUpdatingSucceededReason) wantStatus.DeletionStageStatus.Conditions = append(wantStatus.DeletionStageStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StageUpdatingConditionSucceeded)) // Mark updateRun progressing condition as false with succeeded reason and add succeeded condition. - meta.SetStatusCondition(&wantStatus.Conditions, generateFalseProgressingCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason)) + meta.SetStatusCondition(&wantStatus.Conditions, generateFalseConditionWithReason(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunSucceededReason)) wantStatus.Conditions = append(wantStatus.Conditions, generateTrueCondition(updateRun, placementv1beta1.StagedUpdateRunConditionSucceeded)) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") diff --git a/pkg/controllers/updaterun/validation.go b/pkg/controllers/updaterun/validation.go index 2c72bedcb..c54aa26d8 100644 --- a/pkg/controllers/updaterun/validation.go +++ b/pkg/controllers/updaterun/validation.go @@ -54,7 +54,7 @@ func (r *Reconciler) validate( if !reflect.DeepEqual(updateRunStatus.ApplyStrategy, updateRunCopyStatus.ApplyStrategy) { mismatchErr := controller.NewUserError(fmt.Errorf("the applyStrategy in the updateRun is outdated, latest: %v, recorded: %v", updateRunCopyStatus.ApplyStrategy, updateRunStatus.ApplyStrategy)) klog.ErrorS(mismatchErr, "the applyStrategy in the placement has changed", "placement", placementNamespacedName, "updateRun", updateRunRef) - return -1, nil, nil, fmt.Errorf("%w: %s", errStagedUpdatedAborted, mismatchErr.Error()) + return -1, nil, nil, fmt.Errorf("%w: %w", errStagedUpdatedAborted, mismatchErr) } // Retrieve the latest policy snapshot. diff --git a/pkg/controllers/updaterun/validation_integration_test.go b/pkg/controllers/updaterun/validation_integration_test.go index 464449210..54a34369d 100644 --- a/pkg/controllers/updaterun/validation_integration_test.go +++ b/pkg/controllers/updaterun/validation_integration_test.go @@ -33,6 +33,7 @@ import ( clusterv1beta1 "go.goms.io/fleet/apis/cluster/v1beta1" placementv1beta1 "go.goms.io/fleet/apis/placement/v1beta1" + hubmetrics "go.goms.io/fleet/pkg/metrics/hub" "go.goms.io/fleet/pkg/utils" "go.goms.io/fleet/pkg/utils/condition" ) @@ -49,6 +50,7 @@ var _ = Describe("UpdateRun validation tests", func() { var resourceSnapshot *placementv1beta1.ClusterResourceSnapshot var clusterResourceOverride *placementv1beta1.ClusterResourceOverrideSnapshot var wantStatus *placementv1beta1.UpdateRunStatus + var failureType hubmetrics.UpdateRunFailureType BeforeEach(func() { testUpdateRunName = "updaterun-" + utils.RandStr() @@ -111,9 +113,14 @@ var _ = Describe("UpdateRun validation tests", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization succeeded") - initialized := generateSucceededInitializationStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, clusterResourceOverride) + initialized := generateInitializedStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 10, generateTenClusterStagesStatus(clusterResourceOverride), generateTenClusterDeletionStageStatus()) wantStatus = generateExecutionNotStartedStatus(updateRun, initialized) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") + + // Tests that fail below will report internal errors, if not user error, because only the updaterun controller + // is running in this test environment. Other controllers (e.g., rollout, work generator, + // binding controllers) are not set up, causing operations that depend on them to fail. + failureType = hubmetrics.UpdateRunFailureTypeInternalError }) AfterEach(func() { @@ -165,7 +172,7 @@ var _ = Describe("UpdateRun validation tests", func() { Context("Test validateCRP", func() { AfterEach(func() { By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateWaitingMetric(placementv1beta1.StateRun, updateRun), generateFailedMetric(placementv1beta1.StateRun, updateRun)) + validateUpdateRunMetricsEmitted(generateWaitingMetric(placementv1beta1.StateRun, updateRun), generateFailedMetric(placementv1beta1.StateRun, updateRun, string(failureType))) }) It("Should fail to validate if the CRP is not found", func() { @@ -173,6 +180,7 @@ var _ = Describe("UpdateRun validation tests", func() { Expect(k8sClient.Delete(ctx, crp)).Should(Succeed()) By("Validating the validation failed") + failureType = hubmetrics.UpdateRunFailureTypeUserError wantStatus = generateFailedValidationStatus(updateRun, wantStatus) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "parent placement not found") }) @@ -186,6 +194,7 @@ var _ = Describe("UpdateRun validation tests", func() { Expect(k8sClient.Create(ctx, crp)).To(Succeed()) By("Validating the validation failed") + failureType = hubmetrics.UpdateRunFailureTypeUserError wantStatus = generateFailedValidationStatus(updateRun, wantStatus) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "parent placement does not have an external rollout strategy") @@ -197,6 +206,7 @@ var _ = Describe("UpdateRun validation tests", func() { Expect(k8sClient.Update(ctx, crp)).To(Succeed()) By("Validating the validation failed") + failureType = hubmetrics.UpdateRunFailureTypeUserError wantStatus = generateFailedValidationStatus(updateRun, wantStatus) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "the applyStrategy in the updateRun is outdated") }) @@ -212,7 +222,7 @@ var _ = Describe("UpdateRun validation tests", func() { validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "no latest policy snapshot associated") By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateWaitingMetric(placementv1beta1.StateRun, updateRun), generateFailedMetric(placementv1beta1.StateRun, updateRun)) + validateUpdateRunMetricsEmitted(generateWaitingMetric(placementv1beta1.StateRun, updateRun), generateFailedMetric(placementv1beta1.StateRun, updateRun, string(failureType))) }) It("Should fail to validate if the latest policySnapshot has changed", func() { @@ -241,7 +251,7 @@ var _ = Describe("UpdateRun validation tests", func() { Expect(k8sClient.Delete(ctx, newPolicySnapshot)).Should(Succeed()) By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateWaitingMetric(placementv1beta1.StateRun, updateRun), generateFailedMetric(placementv1beta1.StateRun, updateRun)) + validateUpdateRunMetricsEmitted(generateWaitingMetric(placementv1beta1.StateRun, updateRun), generateFailedMetric(placementv1beta1.StateRun, updateRun, string(failureType))) }) It("Should fail to validate if the cluster count has changed", func() { @@ -255,14 +265,14 @@ var _ = Describe("UpdateRun validation tests", func() { "the cluster count initialized in the updateRun is outdated") By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateWaitingMetric(placementv1beta1.StateRun, updateRun), generateFailedMetric(placementv1beta1.StateRun, updateRun)) + validateUpdateRunMetricsEmitted(generateWaitingMetric(placementv1beta1.StateRun, updateRun), generateFailedMetric(placementv1beta1.StateRun, updateRun, string(failureType))) }) }) Context("Test validateStagesStatus", func() { AfterEach(func() { By("Checking update run status metrics are emitted") - validateUpdateRunMetricsEmitted(generateWaitingMetric(placementv1beta1.StateRun, updateRun), generateFailedMetric(placementv1beta1.StateRun, updateRun)) + validateUpdateRunMetricsEmitted(generateWaitingMetric(placementv1beta1.StateRun, updateRun), generateFailedMetric(placementv1beta1.StateRun, updateRun, string(failureType))) }) It("Should fail to validate if the UpdateStrategySnapshot is nil", func() { @@ -379,6 +389,7 @@ var _ = Describe("UpdateRun validation tests", func() { var resourceSnapshot *placementv1beta1.ClusterResourceSnapshot var clusterResourceOverride *placementv1beta1.ClusterResourceOverrideSnapshot var wantStatus *placementv1beta1.UpdateRunStatus + BeforeEach(func() { testUpdateRunName = "updaterun-" + utils.RandStr() testCRPName = "crp-" + utils.RandStr() @@ -446,7 +457,7 @@ var _ = Describe("UpdateRun validation tests", func() { Expect(k8sClient.Create(ctx, updateRun)).To(Succeed()) By("Validating the initialization succeeded") - initialized := generateSucceededInitializationStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, clusterResourceOverride) + initialized := generateInitializedStatus(crp, updateRun, testResourceSnapshotIndex, policySnapshot, updateStrategy, 10, generateTenClusterStagesStatus(clusterResourceOverride), generateTenClusterDeletionStageStatus()) wantStatus = generateExecutionNotStartedStatus(updateRun, initialized) validateClusterStagedUpdateRunStatus(ctx, updateRun, wantStatus, "") }) @@ -568,7 +579,7 @@ func generateFailedValidationStatus( updateRun *placementv1beta1.ClusterStagedUpdateRun, started *placementv1beta1.UpdateRunStatus, ) *placementv1beta1.UpdateRunStatus { - started.Conditions[1] = generateFalseProgressingCondition(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunFailedReason) + started.Conditions[1] = generateFalseConditionWithReason(updateRun, placementv1beta1.StagedUpdateRunConditionProgressing, condition.UpdateRunFailedReason) started.Conditions = append(started.Conditions, generateFalseCondition(updateRun, placementv1beta1.StagedUpdateRunConditionSucceeded)) return started } diff --git a/pkg/metrics/hub/metrics.go b/pkg/metrics/hub/metrics.go index e2be20782..82ff70946 100644 --- a/pkg/metrics/hub/metrics.go +++ b/pkg/metrics/hub/metrics.go @@ -22,6 +22,23 @@ import ( "sigs.k8s.io/controller-runtime/pkg/metrics" ) +// UpdateRunFailureType represents the type of failure for an update run. +type UpdateRunFailureType string + +const ( + // UpdateRunFailureTypeNone indicates no failure (success or in-progress states). + UpdateRunFailureTypeNone UpdateRunFailureType = "none" + + // UpdateRunFailureTypeUserError indicates a failure caused by user configuration errors + // such as validation failures, missing resources, or invalid settings. + // These are known customer errors that do not require investigation. + UpdateRunFailureTypeUserError UpdateRunFailureType = "user_error" + + // UpdateRunFailureTypeInternalError indicates a failure caused by internal system errors + // that may require investigation, such as unexpected behaviors or API server errors. + UpdateRunFailureTypeInternalError UpdateRunFailureType = "internal_error" +) + var ( // FleetPlacementStatusLastTimeStampSeconds is a prometheus metric which keeps track of the last placement status. FleetPlacementStatusLastTimeStampSeconds = prometheus.NewGaugeVec(prometheus.GaugeOpts{ @@ -38,10 +55,12 @@ var ( // FleetUpdateRunStatusLastTimestampSeconds is a prometheus metric which holds the // last update timestamp of update run status in seconds. + // The failure_type label indicates whether a failure is a user_error (customer configuration issue), + // internal_error (requires investigation), or none (no failure). FleetUpdateRunStatusLastTimestampSeconds = prometheus.NewGaugeVec(prometheus.GaugeOpts{ Name: "fleet_workload_update_run_status_last_timestamp_seconds", Help: "Last update timestamp of update run status in seconds", - }, []string{"namespace", "name", "state", "condition", "status", "reason"}) + }, []string{"namespace", "name", "state", "condition", "status", "reason", "failureType"}) // FleetUpdateRunApprovalRequestLatencySeconds tracks how long users take to approve approval requests. FleetUpdateRunApprovalRequestLatencySeconds = prometheus.NewHistogramVec(prometheus.HistogramOpts{ diff --git a/pkg/resourcewatcher/change_dector.go b/pkg/resourcewatcher/change_detector.go similarity index 96% rename from pkg/resourcewatcher/change_dector.go rename to pkg/resourcewatcher/change_detector.go index 0f445878a..d7d2622cb 100644 --- a/pkg/resourcewatcher/change_dector.go +++ b/pkg/resourcewatcher/change_detector.go @@ -152,9 +152,8 @@ func (d *ChangeDetector) discoverResources(dynamicResourceEventHandler cache.Res klog.V(2).InfoS("Change detector: discovered resources", "count", len(resourcesToWatch)) } -// dynamicResourceFilter filters out resources that we don't want to watch -// TODO: add UTs for this -func (d *ChangeDetector) dynamicResourceFilter(obj interface{}) bool { +// dynamicResourceFilter filters out resources that we don't want to watch. +func (d *ChangeDetector) dynamicResourceFilter(obj any) bool { key, err := controller.ClusterWideKeyFunc(obj) if err != nil { return false @@ -191,8 +190,8 @@ func (d *ChangeDetector) NeedLeaderElection() bool { // Note: An object that starts passing the filter after an update is considered an add, and // an object that stops passing the filter after an update is considered a delete. // Like the handlers, the filter MUST NOT modify the objects it is given. -func newFilteringHandlerOnAllEvents(filterFunc func(obj interface{}) bool, addFunc func(obj interface{}), - updateFunc func(oldObj, newObj interface{}), deleteFunc func(obj interface{})) cache.ResourceEventHandler { +func newFilteringHandlerOnAllEvents(filterFunc func(obj any) bool, addFunc func(obj any), + updateFunc func(oldObj, newObj any), deleteFunc func(obj any)) cache.ResourceEventHandler { return &cache.FilteringResourceEventHandler{ FilterFunc: filterFunc, Handler: cache.ResourceEventHandlerFuncs{ diff --git a/pkg/resourcewatcher/change_detector_test.go b/pkg/resourcewatcher/change_detector_test.go index c478a4966..cfccf63e2 100644 --- a/pkg/resourcewatcher/change_detector_test.go +++ b/pkg/resourcewatcher/change_detector_test.go @@ -19,7 +19,9 @@ package resourcewatcher import ( "testing" + corev1 "k8s.io/api/core/v1" metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" + "k8s.io/apimachinery/pkg/apis/meta/v1/unstructured" "k8s.io/apimachinery/pkg/runtime/schema" fakediscovery "k8s.io/client-go/discovery/fake" "k8s.io/client-go/kubernetes/fake" @@ -137,7 +139,7 @@ func TestChangeDetector_discoverResources(t *testing.T) { // Track handler additions testHandler := cache.ResourceEventHandlerFuncs{ - AddFunc: func(obj interface{}) {}, + AddFunc: func(obj any) {}, } // Create ChangeDetector with the interface type @@ -164,3 +166,171 @@ func TestChangeDetector_NeedLeaderElection(t *testing.T) { t.Error("ChangeDetector should need leader election") } } + +func TestChangeDetector_dynamicResourceFilter(t *testing.T) { + unstructuredConfigMap := func(namespace, name string) *unstructured.Unstructured { + u := &unstructured.Unstructured{} + u.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind(utils.ConfigMapKind)) + u.SetNamespace(namespace) + u.SetName(name) + return u + } + unstructuredClusterRole := func(name string) *unstructured.Unstructured { + u := &unstructured.Unstructured{} + u.SetGroupVersionKind(schema.GroupVersionKind{Group: "rbac.authorization.k8s.io", Version: "v1", Kind: "ClusterRole"}) + u.SetName(name) + return u + } + typedSecret := func(namespace, name string) *corev1.Secret { + return &corev1.Secret{ + TypeMeta: metav1.TypeMeta{ + Kind: "Secret", + APIVersion: "v1", + }, + ObjectMeta: metav1.ObjectMeta{ + Namespace: namespace, + Name: name, + }, + } + } + + tests := []struct { + name string + obj any + skippedNamespaces map[string]bool + want bool + }{ + { + name: "non-runtime object is filtered out", + obj: "not-a-runtime-object", + want: false, + }, + { + name: "nil object is filtered out", + obj: nil, + want: false, + }, + { + // Tombstones from informer cache deletions are not unwrapped by ClusterWideKeyFunc, + // so the filter rejects them. The downstream delete handler unwraps tombstones separately. + name: "tombstone object is filtered out", + obj: cache.DeletedFinalStateUnknown{Key: "default/cm", Obj: unstructuredConfigMap("default", "cm")}, + want: false, + }, + { + name: "object in fleet-prefixed namespace is filtered out", + obj: unstructuredConfigMap("fleet-system", "cm"), + want: false, + }, + { + name: "object in kube-prefixed namespace is filtered out", + obj: unstructuredConfigMap("kube-system", "cm"), + want: false, + }, + { + name: "object in user-skipped namespace is filtered out", + obj: unstructuredConfigMap("skip-me", "cm"), + skippedNamespaces: map[string]bool{"skip-me": true}, + want: false, + }, + { + name: "unstructured ConfigMap kube-root-ca.crt is filtered out by ShouldPropagateObj", + obj: unstructuredConfigMap("default", "kube-root-ca.crt"), + want: false, + }, + { + name: "unstructured ConfigMap with regular name is allowed", + obj: unstructuredConfigMap("default", "user-config"), + want: true, + }, + { + // Cluster-scoped objects have an empty namespace; ShouldPropagateNamespace returns true + // for "" (no reserved prefix match, not in skip-list). + name: "cluster-scoped unstructured object is allowed", + obj: unstructuredClusterRole("admin"), + want: true, + }, + { + // Typed objects bypass the ShouldPropagateObj check because the type assertion to + // *unstructured.Unstructured fails. In production the dynamic informer only emits + // unstructured objects, but this case documents the bypass. + name: "typed object bypasses ShouldPropagateObj and is allowed", + obj: typedSecret("default", "my-secret"), + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + fakeInformerManager := &testinformer.FakeManager{ + APIResources: make(map[schema.GroupVersionKind]bool), + } + detector := &ChangeDetector{ + InformerManager: fakeInformerManager, + SkippedNamespaces: tt.skippedNamespaces, + } + + got := detector.dynamicResourceFilter(tt.obj) + if got != tt.want { + t.Errorf("dynamicResourceFilter(%v) = %v, want %v", tt.obj, got, tt.want) + } + }) + } +} + +// TestNewFilteringHandlerOnAllEvents verifies that the helper composes a +// FilteringResourceEventHandler that gates each event type on the supplied filter: +// callbacks fire only when the filter accepts the object. +func TestNewFilteringHandlerOnAllEvents(t *testing.T) { + cm := func(name string) *corev1.ConfigMap { + return &corev1.ConfigMap{ObjectMeta: metav1.ObjectMeta{Name: name}} + } + + tests := []struct { + name string + event string // "add", "update", or "delete" + obj *corev1.ConfigMap + wantHit bool + }{ + {name: "add fires when filter accepts", event: "add", obj: cm("keep"), wantHit: true}, + {name: "add skipped when filter rejects", event: "add", obj: cm("drop"), wantHit: false}, + {name: "update fires when filter accepts", event: "update", obj: cm("keep"), wantHit: true}, + {name: "update skipped when filter rejects", event: "update", obj: cm("drop"), wantHit: false}, + {name: "delete fires when filter accepts", event: "delete", obj: cm("keep"), wantHit: true}, + {name: "delete skipped when filter rejects", event: "delete", obj: cm("drop"), wantHit: false}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + var addCalls, updateCalls, deleteCalls int + filter := func(obj any) bool { + c, ok := obj.(*corev1.ConfigMap) + return ok && c.Name == "keep" + } + handler := newFilteringHandlerOnAllEvents(filter, + func(_ any) { addCalls++ }, + func(_, _ any) { updateCalls++ }, + func(_ any) { deleteCalls++ }, + ) + + if _, ok := handler.(*cache.FilteringResourceEventHandler); !ok { + t.Fatalf("newFilteringHandlerOnAllEvents() = %T, want *cache.FilteringResourceEventHandler", handler) + } + + switch tt.event { + case "add": + handler.OnAdd(tt.obj, false) + case "update": + handler.OnUpdate(tt.obj, tt.obj) + case "delete": + handler.OnDelete(tt.obj) + } + + gotHit := addCalls+updateCalls+deleteCalls == 1 + if gotHit != tt.wantHit { + t.Errorf("newFilteringHandlerOnAllEvents() %s callback fired = %v, want %v (counts add=%d update=%d delete=%d)", + tt.event, gotHit, tt.wantHit, addCalls, updateCalls, deleteCalls) + } + }) + } +} diff --git a/pkg/resourcewatcher/event_handlers_test.go b/pkg/resourcewatcher/event_handlers_test.go index 8618ed679..c6f852d3b 100644 --- a/pkg/resourcewatcher/event_handlers_test.go +++ b/pkg/resourcewatcher/event_handlers_test.go @@ -133,7 +133,7 @@ func (t *fakeController) Enqueue(_ interface{}) { t.Enqueued = true } +// Run is a no-op; the fake is only used to verify that Enqueue is called. func (t *fakeController) Run(_ context.Context, _ int) error { - //TODO implement me - panic("implement me") + return nil } diff --git a/pkg/utils/apiresources.go b/pkg/utils/apiresources.go index 2a6aa080e..f44e003bd 100644 --- a/pkg/utils/apiresources.go +++ b/pkg/utils/apiresources.go @@ -121,6 +121,21 @@ var ( Kind: placementv1beta1.ClusterApprovalRequestKind, } + StagedUpdateRunGK = schema.GroupKind{ + Group: placementv1beta1.GroupVersion.Group, + Kind: placementv1beta1.StagedUpdateRunKind, + } + + StagedUpdateStrategyGK = schema.GroupKind{ + Group: placementv1beta1.GroupVersion.Group, + Kind: placementv1beta1.StagedUpdateStrategyKind, + } + + ApprovalRequestGK = schema.GroupKind{ + Group: placementv1beta1.GroupVersion.Group, + Kind: placementv1beta1.ApprovalRequestKind, + } + ClusterResourcePlacementEvictionGK = schema.GroupKind{ Group: placementv1beta1.GroupVersion.Group, Kind: placementv1beta1.ClusterResourcePlacementEvictionKind, @@ -210,6 +225,9 @@ func NewResourceConfig(isAllowList bool) *ResourceConfig { r.AddGroupKind(ClusterStagedUpdateRunGK) r.AddGroupKind(ClusterStagedUpdateStrategyGK) r.AddGroupKind(ClusterApprovalRequestGK) + r.AddGroupKind(StagedUpdateRunGK) + r.AddGroupKind(StagedUpdateStrategyGK) + r.AddGroupKind(ApprovalRequestGK) r.AddGroupKind(ClusterResourcePlacementEvictionGK) r.AddGroupKind(ClusterResourcePlacementDisruptionBudgetGK) r.AddGroupKind(ClusterResourceOverrideGK) diff --git a/pkg/utils/apiresources_test.go b/pkg/utils/apiresources_test.go index de249791f..7e8b2c2b2 100644 --- a/pkg/utils/apiresources_test.go +++ b/pkg/utils/apiresources_test.go @@ -529,6 +529,47 @@ func TestDefaultResourceConfigGroupVersionKindParse(t *testing.T) { Version: "v1", Kind: "ResourceOverrideSnapshot", }, + // Namespaced staged update resources (should also be disabled) + { + Group: "placement.kubernetes-fleet.io", + Version: "v1beta1", + Kind: "StagedUpdateRun", + }, + { + Group: "placement.kubernetes-fleet.io", + Version: "v1", + Kind: "StagedUpdateRun", + }, + { + Group: "placement.kubernetes-fleet.io", + Version: "v1beta1", + Kind: "StagedUpdateStrategy", + }, + { + Group: "placement.kubernetes-fleet.io", + Version: "v1", + Kind: "StagedUpdateStrategy", + }, + { + Group: "placement.kubernetes-fleet.io", + Version: "v1beta1", + Kind: "ApprovalRequest", + }, + { + Group: "placement.kubernetes-fleet.io", + Version: "v1", + Kind: "ApprovalRequest", + }, + { + Group: "placement.kubernetes-fleet.io", + Version: "v1beta1", + Kind: "ClusterResourcePlacementStatus", + }, + { + Group: "placement.kubernetes-fleet.io", + Version: "v1", + Kind: "ClusterResourcePlacementStatus", + }, } resourcesNotInDefaultResourcesList := []schema.GroupVersionKind{ diff --git a/pkg/utils/condition/reason.go b/pkg/utils/condition/reason.go index 29d9291a2..0e8df1412 100644 --- a/pkg/utils/condition/reason.go +++ b/pkg/utils/condition/reason.go @@ -197,6 +197,9 @@ const ( // StageUpdatingSucceededReason is the reason string of condition if the stage updating succeeded. StageUpdatingSucceededReason = "StageUpdatingSucceeded" + // StageUpdatingSkippedNoClustersReason is the reason string of condition if the stage was skipped because it has no clusters. + StageUpdatingSkippedNoClustersReason = "StageUpdatingSkippedNoClusters" + // ClusterUpdatingStartedReason is the reason string of condition if the cluster updating has started. ClusterUpdatingStartedReason = "ClusterUpdatingStarted" diff --git a/pkg/webhook/webhook.go b/pkg/webhook/webhook.go index 7b6457ad9..cd59764bc 100644 --- a/pkg/webhook/webhook.go +++ b/pkg/webhook/webhook.go @@ -641,12 +641,9 @@ func (w *Config) buildFleetGuardRailValidatingWebhooks() []admv1.ValidatingWebho // Build core v1 resources list, conditionally including pods if workload is enabled coreV1Resources := []string{bindingResourceName, configMapResourceName, endPointResourceName, - limitRangeResourceName, persistentVolumeClaimsName, persistentVolumeClaimsName + "/status", podTemplateResourceName, + limitRangeResourceName, persistentVolumeClaimsName, persistentVolumeClaimsName + "/status", podTemplateResourceName, podResourceName, podResourceName + "/status", replicationControllerResourceName, replicationControllerResourceName + "/status", resourceQuotaResourceName, resourceQuotaResourceName + "/status", secretResourceName, serviceAccountResourceName, servicesResourceName, servicesResourceName + "/status"} - if w.enableWorkload { - coreV1Resources = append(coreV1Resources, podResourceName, podResourceName+"/status") - } namespacedResourcesRules = append(namespacedResourcesRules, admv1.RuleWithOperations{ Operations: cuOperations, @@ -654,11 +651,8 @@ func (w *Config) buildFleetGuardRailValidatingWebhooks() []admv1.ValidatingWebho }) // Build apps/v1 resources list, conditionally including replicasets if workload is enabled - appsV1Resources := []string{controllerRevisionResourceName, daemonSetResourceName, daemonSetResourceName + "/status", + appsV1Resources := []string{controllerRevisionResourceName, daemonSetResourceName, daemonSetResourceName + "/status", replicaSetResourceName, replicaSetResourceName + "/status", deploymentResourceName, deploymentResourceName + "/status", statefulSetResourceName, statefulSetResourceName + "/status"} - if w.enableWorkload { - appsV1Resources = append(appsV1Resources, replicaSetResourceName, replicaSetResourceName+"/status") - } namespacedResourcesRules = append(namespacedResourcesRules, admv1.RuleWithOperations{ Operations: cuOperations, diff --git a/test/e2e/cluster_staged_updaterun_test.go b/test/e2e/cluster_staged_updaterun_test.go index ea03305c6..6f108dbca 100644 --- a/test/e2e/cluster_staged_updaterun_test.go +++ b/test/e2e/cluster_staged_updaterun_test.go @@ -664,20 +664,21 @@ var _ = Describe("test CRP rollout with staged update run", func() { createClusterStagedUpdateRunSucceed(updateRunNames[2], crpName, resourceSnapshotIndex1st, strategyName, placementv1beta1.StateRun) }) - It("Should still have resources on all member clusters and complete stage canary", func() { + It("Should still have resources on all member clusters and skip stage canary", func() { checkIfPlacedWorkResourcesOnMemberClustersConsistently(allMemberClusters) By("Validating crp status keeping as rollout pending with member-cluster-3 only") 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, placementv1beta1.AfterStageApprovalTaskNameFmt, placementv1beta1.AfterStageTaskLabelValue) + // Completes stage by skipping since there are 0 clusters. }) - It("Should remove resources on member-cluster-1 and member-cluster-2 after approval and complete the cluster staged update run successfully", func() { + It("Should remove resources on member-cluster-1 and member-cluster-2 after before-stage task approval and complete the cluster staged update run successfully", func() { validateAndApproveClusterApprovalRequests(updateRunNames[2], envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt, placementv1beta1.BeforeStageTaskLabelValue) - // need to go through two stages + // Need to go through two stages. The second stage takes care of member-cluster-3 and ensure the resource is rolled out to there. + // The deletion stage will remove the resources from member-cluster-1 and member-cluster-2. csurSucceededActual := testutilsupdaterun.ClusterStagedUpdateRunStatusSucceededActual(ctx, hubClient, updateRunNames[2], resourceSnapshotIndex1st, policySnapshotIndex3rd, 1, defaultApplyStrategy, &strategy.Spec, [][]string{{}, {allMemberClusterNames[2]}}, []string{allMemberClusterNames[0], allMemberClusterNames[1]}, nil, nil, true) Eventually(csurSucceededActual, 2*updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s succeeded", updateRunNames[2]) checkIfRemovedWorkResourcesFromMemberClusters([]*framework.Cluster{allMemberClusters[0], allMemberClusters[1]}) @@ -763,21 +764,21 @@ var _ = Describe("test CRP rollout with staged update run", func() { validateLatestClusterResourceSnapshot(crpName, resourceSnapshotIndex1st) }) - It("Should not rollout any resources to member clusters and complete stage canary", func() { + It("Should not rollout any resources to member clusters and skip stage canary", func() { checkIfRemovedWorkResourcesFromMemberClustersConsistently(allMemberClusters) By("Validating crp status as pending rollout still") 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, placementv1beta1.AfterStageApprovalTaskNameFmt, placementv1beta1.AfterStageTaskLabelValue) + // Completes stage by skipping since there are 0 clusters. }) 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 after approval and complete the cluster staged update run successfully", func() { + It("Should rollout resources to member-cluster-3 after before-stage task approval and complete the cluster staged update run successfully", func() { validateAndApproveClusterApprovalRequests(updateRunNames[0], envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt, placementv1beta1.BeforeStageTaskLabelValue) csurSucceededActual := testutilsupdaterun.ClusterStagedUpdateRunStatusSucceededActual(ctx, hubClient, updateRunNames[0], resourceSnapshotIndex1st, policySnapshotIndex1st, 1, defaultApplyStrategy, &strategy.Spec, [][]string{{}, {allMemberClusterNames[2]}}, nil, nil, nil, true) diff --git a/test/e2e/staged_updaterun_test.go b/test/e2e/staged_updaterun_test.go index a2f8dcaaa..58028c421 100644 --- a/test/e2e/staged_updaterun_test.go +++ b/test/e2e/staged_updaterun_test.go @@ -660,20 +660,21 @@ var _ = Describe("test RP rollout with staged update run", Label("resourceplacem createStagedUpdateRunSucceed(updateRunNames[2], testNamespace, rpName, resourceSnapshotIndex1st, strategyName, placementv1beta1.StateRun) }) - It("Should still have resources on all member clusters and complete stage canary", func() { + It("Should still have resources on all member clusters and skip stage canary", func() { checkIfPlacedWorkResourcesOnMemberClustersConsistently(allMemberClusters) By("Validating rp status keeping as rollout pending with member-cluster-3 only") 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, placementv1beta1.AfterStageApprovalTaskNameFmt, placementv1beta1.AfterStageTaskLabelValue) + // Completes stage by skipping since there are 0 clusters. }) - It("Should remove resources on member-cluster-1 and member-cluster-2 after approval and complete the staged update run successfully", func() { + It("Should remove resources on member-cluster-1 and member-cluster-2 after before-stage task approval and complete the staged update run successfully", func() { validateAndApproveNamespacedApprovalRequests(updateRunNames[2], testNamespace, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt, placementv1beta1.BeforeStageTaskLabelValue) - // need to go through two stages + // Need to go through two stages. The second stage takes care of member-cluster-3 and ensure the resource is rolled out to there. + // The deletion stage will remove the resources from member-cluster-1 and member-cluster-2. surSucceededActual := testutilsupdaterun.StagedUpdateRunStatusSucceededActual(ctx, hubClient, updateRunNames[2], testNamespace, resourceSnapshotIndex1st, policySnapshotIndex3rd, 1, defaultApplyStrategy, &strategy.Spec, [][]string{{}, {allMemberClusterNames[2]}}, []string{allMemberClusterNames[0], allMemberClusterNames[1]}, nil, nil, true) Eventually(surSucceededActual, 2*updateRunEventuallyDuration, eventuallyInterval).Should(Succeed(), "Failed to validate updateRun %s/%s succeeded", testNamespace, updateRunNames[2]) checkIfRemovedConfigMapFromMemberClusters([]*framework.Cluster{allMemberClusters[0], allMemberClusters[1]}) @@ -763,21 +764,21 @@ var _ = Describe("test RP rollout with staged update run", Label("resourceplacem validateLatestResourceSnapshot(rpName, testNamespace, resourceSnapshotIndex1st) }) - It("Should not rollout any resources to member clusters and complete stage canary", func() { + It("Should not rollout any resources to member clusters and skip stage canary", func() { checkIfRemovedConfigMapFromMemberClustersConsistently(allMemberClusters) By("Validating rp status as pending rollout still") 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, placementv1beta1.AfterStageApprovalTaskNameFmt, placementv1beta1.AfterStageTaskLabelValue) + // Completes stage by skipping since there are 0 clusters. }) 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() { + It("Should rollout resources to member-cluster-3 after before-stage task approval and complete the cluster staged update run successfully", func() { validateAndApproveNamespacedApprovalRequests(updateRunNames[0], testNamespace, envProd, placementv1beta1.BeforeStageApprovalTaskNameFmt, placementv1beta1.BeforeStageTaskLabelValue) surSucceededActual := testutilsupdaterun.StagedUpdateRunStatusSucceededActual(ctx, hubClient, updateRunNames[0], testNamespace, resourceSnapshotIndex1st, policySnapshotIndex1st, 1, defaultApplyStrategy, &strategy.Spec, [][]string{{}, {allMemberClusterNames[2]}}, nil, nil, nil, true) diff --git a/test/utils/updaterun/updaterun_status.go b/test/utils/updaterun/updaterun_status.go index a952b7284..d3c01a0e5 100644 --- a/test/utils/updaterun/updaterun_status.go +++ b/test/utils/updaterun/updaterun_status.go @@ -191,7 +191,10 @@ func buildStageUpdatingStatuses( 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) + // Skip populating task conditions if the stage has 0 clusters (stage is skipped). + if len(wantSelectedClusters[i]) > 0 { + stagesStatus[i].BeforeStageTaskStatus[j].Conditions = updateRunStageTaskSucceedConditions(updateRun.GetGeneration(), task.Type) + } } stagesStatus[i].AfterStageTaskStatus = make([]placementv1beta1.StageTaskStatus, len(stage.AfterStageTasks)) for j, task := range stage.AfterStageTasks { @@ -199,9 +202,17 @@ func buildStageUpdatingStatuses( if task.Type == placementv1beta1.StageTaskTypeApproval { stagesStatus[i].AfterStageTaskStatus[j].ApprovalRequestName = fmt.Sprintf(placementv1beta1.AfterStageApprovalTaskNameFmt, updateRun.GetName(), stage.Name) } - stagesStatus[i].AfterStageTaskStatus[j].Conditions = updateRunStageTaskSucceedConditions(updateRun.GetGeneration(), task.Type) + // Skip populating task conditions if the stage has 0 clusters (stage is skipped). + if len(wantSelectedClusters[i]) > 0 { + stagesStatus[i].AfterStageTaskStatus[j].Conditions = updateRunStageTaskSucceedConditions(updateRun.GetGeneration(), task.Type) + } + } + // Use skipped conditions if the stage has 0 clusters. + if len(wantSelectedClusters[i]) == 0 { + stagesStatus[i].Conditions = updateRunStageSkippedNoClustersConditions(updateRun.GetGeneration()) + } else { + stagesStatus[i].Conditions = updateRunStageRolloutSucceedConditions(updateRun.GetGeneration()) } - stagesStatus[i].Conditions = updateRunStageRolloutSucceedConditions(updateRun.GetGeneration()) } return stagesStatus } @@ -264,6 +275,23 @@ func updateRunStageRolloutSucceedConditions(generation int64) []metav1.Condition } } +func updateRunStageSkippedNoClustersConditions(generation int64) []metav1.Condition { + return []metav1.Condition{ + { + Type: string(placementv1beta1.StageUpdatingConditionProgressing), + Status: metav1.ConditionFalse, + Reason: condition.StageUpdatingSkippedNoClustersReason, + ObservedGeneration: generation, + }, + { + Type: string(placementv1beta1.StageUpdatingConditionSucceeded), + Status: metav1.ConditionTrue, + Reason: condition.StageUpdatingSkippedNoClustersReason, + ObservedGeneration: generation, + }, + } +} + func updateRunStageTaskSucceedConditions(generation int64, taskType placementv1beta1.StageTaskType) []metav1.Condition { if taskType == placementv1beta1.StageTaskTypeApproval { return []metav1.Condition{