From 1141ad415631b278c7528420ca8f8f047c558f87 Mon Sep 17 00:00:00 2001 From: Dale Haiducek <19750917+dhaiducek@users.noreply.github.com> Date: Thu, 2 Nov 2023 16:17:09 -0400 Subject: [PATCH 1/3] Replace Timeout with RolloutConfig Uses `MinSuccessTime`, `ProgressDeadline`, and `MaxFailures` for a more fine-tuned configuration. Also passes `Timeout` over to `ProgressDeadline` and marks it as deprecated. Signed-off-by: Dale Haiducek <19750917+dhaiducek@users.noreply.github.com> --- ...gement.io_clustermanagementaddons.crd.yaml | 195 +++- cluster/v1alpha1/helpers.go | 235 ++++- cluster/v1alpha1/helpers_test.go | 967 +++++++++++------- cluster/v1alpha1/types_rolloutstrategy.go | 79 +- cluster/v1alpha1/zz_generated.deepcopy.go | 38 +- .../zz_generated.swagger_doc_generated.go | 29 +- ...gement.io_manifestworkreplicasets.crd.yaml | 184 +++- work/v1alpha1/types_manifestworkreplicaset.go | 2 +- 8 files changed, 1219 insertions(+), 510 deletions(-) diff --git a/addon/v1alpha1/0000_00_addon.open-cluster-management.io_clustermanagementaddons.crd.yaml b/addon/v1alpha1/0000_00_addon.open-cluster-management.io_clustermanagementaddons.crd.yaml index a42b5f57c..75178916b 100644 --- a/addon/v1alpha1/0000_00_addon.open-cluster-management.io_clustermanagementaddons.crd.yaml +++ b/addon/v1alpha1/0000_00_addon.open-cluster-management.io_clustermanagementaddons.crd.yaml @@ -145,24 +145,67 @@ spec: defined in ClusterManagementAddOn. properties: all: - description: All define required fields for RolloutStrategy + description: All defines required fields for RolloutStrategy type All properties: + maxFailures: + anyOf: + - type: integer + - type: string + default: "0" + description: MaxFailures is a percentage or number + of clusters in the current rollout that can fail + before proceeding to the next rollout. MaxFailures + is only considered for rollout types Progressive + and ProgressivePerGroup. For Progressive, this + is considered over the total number of clusters. + For ProgressivePerGroup, this is considered according + to the size of the current group. Default is that + no failures are tolerated. + pattern: ^((100|[0-9]{1,2})%|[0-9]+)$ + x-kubernetes-int-or-string: true + minSuccessTime: + default: "0" + description: MinSuccessTime is a "soak" time. In + other words, the minimum amount of time the workload + applier controller will wait from the start of + each rollout before proceeding (assuming a successful + state has been reached and MaxFailures wasn't + breached). MinSuccessTime is only considered for + rollout types Progressive and ProgressivePerGroup. + The default value is 0 meaning the workload applier + proceeds immediately after a successful state + is reached. MinSuccessTime must be defined in + [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m + , 360s + type: string + progressDeadline: + default: None + description: ProgressDeadline defines how long workload + applier controller will wait for the workload + to reach a successful state in the cluster. ProgressDeadline + default value is "None", meaning the workload + applier will wait for a successful state indefinitely. + ProgressDeadline must be defined in [0-9h]|[0-9m]|[0-9s] + format examples; 2h , 90m , 360s + pattern: ^(([0-9])+[h|m|s])|None$ + type: string timeout: default: None - description: Timeout define how long workload applier - controller will wait till workload reach successful - state in the cluster. Timeout default value is - None meaning the workload applier will not proceed - apply workload to other clusters if did not reach - the successful state. Timeout must be defined - in [0-9h]|[0-9m]|[0-9s] format examples; 2h , - 90m , 360s + description: "Timeout defines how long the workload + applier controller will wait until the workload + reaches a successful state in the cluster. Timeout + default value is None meaning the workload applier + will not proceed apply workload to other clusters + if did not reach the successful state. Timeout + must be defined in [0-9h]|[0-9m]|[0-9s] format + examples; 2h , 90m , 360s \n Deprecated: Use ProgressDeadline + instead." pattern: ^(([0-9])+[h|m|s])|None$ type: string type: object progressive: - description: Progressive define required fields for + description: Progressive defines required fields for RolloutStrategy type Progressive properties: mandatoryDecisionGroups: @@ -200,21 +243,64 @@ spec: placement->DecisionStrategy. pattern: ^((100|[0-9]{1,2})%|[0-9]+)$ x-kubernetes-int-or-string: true + maxFailures: + anyOf: + - type: integer + - type: string + default: "0" + description: MaxFailures is a percentage or number + of clusters in the current rollout that can fail + before proceeding to the next rollout. MaxFailures + is only considered for rollout types Progressive + and ProgressivePerGroup. For Progressive, this + is considered over the total number of clusters. + For ProgressivePerGroup, this is considered according + to the size of the current group. Default is that + no failures are tolerated. + pattern: ^((100|[0-9]{1,2})%|[0-9]+)$ + x-kubernetes-int-or-string: true + minSuccessTime: + default: "0" + description: MinSuccessTime is a "soak" time. In + other words, the minimum amount of time the workload + applier controller will wait from the start of + each rollout before proceeding (assuming a successful + state has been reached and MaxFailures wasn't + breached). MinSuccessTime is only considered for + rollout types Progressive and ProgressivePerGroup. + The default value is 0 meaning the workload applier + proceeds immediately after a successful state + is reached. MinSuccessTime must be defined in + [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m + , 360s + type: string + progressDeadline: + default: None + description: ProgressDeadline defines how long workload + applier controller will wait for the workload + to reach a successful state in the cluster. ProgressDeadline + default value is "None", meaning the workload + applier will wait for a successful state indefinitely. + ProgressDeadline must be defined in [0-9h]|[0-9m]|[0-9s] + format examples; 2h , 90m , 360s + pattern: ^(([0-9])+[h|m|s])|None$ + type: string timeout: default: None - description: Timeout define how long workload applier - controller will wait till workload reach successful - state in the cluster. Timeout default value is - None meaning the workload applier will not proceed - apply workload to other clusters if did not reach - the successful state. Timeout must be defined - in [0-9h]|[0-9m]|[0-9s] format examples; 2h , - 90m , 360s + description: "Timeout defines how long the workload + applier controller will wait until the workload + reaches a successful state in the cluster. Timeout + default value is None meaning the workload applier + will not proceed apply workload to other clusters + if did not reach the successful state. Timeout + must be defined in [0-9h]|[0-9m]|[0-9s] format + examples; 2h , 90m , 360s \n Deprecated: Use ProgressDeadline + instead." pattern: ^(([0-9])+[h|m|s])|None$ type: string type: object progressivePerGroup: - description: ProgressivePerGroup define required fields + description: ProgressivePerGroup defines required fields for RolloutStrategy type ProgressivePerGroup properties: mandatoryDecisionGroups: @@ -241,16 +327,59 @@ spec: type: string type: object type: array + maxFailures: + anyOf: + - type: integer + - type: string + default: "0" + description: MaxFailures is a percentage or number + of clusters in the current rollout that can fail + before proceeding to the next rollout. MaxFailures + is only considered for rollout types Progressive + and ProgressivePerGroup. For Progressive, this + is considered over the total number of clusters. + For ProgressivePerGroup, this is considered according + to the size of the current group. Default is that + no failures are tolerated. + pattern: ^((100|[0-9]{1,2})%|[0-9]+)$ + x-kubernetes-int-or-string: true + minSuccessTime: + default: "0" + description: MinSuccessTime is a "soak" time. In + other words, the minimum amount of time the workload + applier controller will wait from the start of + each rollout before proceeding (assuming a successful + state has been reached and MaxFailures wasn't + breached). MinSuccessTime is only considered for + rollout types Progressive and ProgressivePerGroup. + The default value is 0 meaning the workload applier + proceeds immediately after a successful state + is reached. MinSuccessTime must be defined in + [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m + , 360s + type: string + progressDeadline: + default: None + description: ProgressDeadline defines how long workload + applier controller will wait for the workload + to reach a successful state in the cluster. ProgressDeadline + default value is "None", meaning the workload + applier will wait for a successful state indefinitely. + ProgressDeadline must be defined in [0-9h]|[0-9m]|[0-9s] + format examples; 2h , 90m , 360s + pattern: ^(([0-9])+[h|m|s])|None$ + type: string timeout: default: None - description: Timeout define how long workload applier - controller will wait till workload reach successful - state in the cluster. Timeout default value is - None meaning the workload applier will not proceed - apply workload to other clusters if did not reach - the successful state. Timeout must be defined - in [0-9h]|[0-9m]|[0-9s] format examples; 2h , - 90m , 360s + description: "Timeout defines how long the workload + applier controller will wait until the workload + reaches a successful state in the cluster. Timeout + default value is None meaning the workload applier + will not proceed apply workload to other clusters + if did not reach the successful state. Timeout + must be defined in [0-9h]|[0-9m]|[0-9s] format + examples; 2h , 90m , 360s \n Deprecated: Use ProgressDeadline + instead." pattern: ^(([0-9])+[h|m|s])|None$ type: string type: object @@ -263,11 +392,13 @@ spec: clusters progressively per cluster. The workload will not be applied to the next cluster unless one of the current applied clusters reach the successful state - or timeout. 3) ProgressivePerGroup means apply the - workload to decisionGroup clusters progressively per - group. The workload will not be applied to the next - decisionGroup unless all clusters in the current group - reach the successful state or timeout. + and haven't breached the MaxFailures configuration. + 3) ProgressivePerGroup means apply the workload to + decisionGroup clusters progressively per group. The + workload will not be applied to the next decisionGroup + unless all clusters in the current group reach the + successful state and haven't breached the MaxFailures + configuration. enum: - All - Progressive diff --git a/cluster/v1alpha1/helpers.go b/cluster/v1alpha1/helpers.go index 7af8bd1b7..ae142ebe2 100644 --- a/cluster/v1alpha1/helpers.go +++ b/cluster/v1alpha1/helpers.go @@ -46,13 +46,16 @@ type ClusterRolloutStatus struct { // Status is the required field indicating the rollout status. Status RolloutStatus // LastTransitionTime is the last transition time of the rollout status (optional field). - // Used to calculate timeout for progressing and failed status. + // Used to calculate timeout for progressing and failed status and minimum success time (i.e. soak + // time) for succeeded status. LastTransitionTime *metav1.Time // TimeOutTime is the timeout time when the status is progressing or failed (optional field). TimeOutTime *metav1.Time } -// RolloutResult contains list of clusters that are timeOut, removed and required to rollOut +// RolloutResult contains list of clusters that are timeOut, removed and required to rollOut. A +// boolean is also provided signaling that the rollout may be shortened due to the number of failed +// clusters exceeding the MaxFailure threshold. type RolloutResult struct { // ClustersToRollout is a slice of ClusterRolloutStatus that will be rolled out. ClustersToRollout []ClusterRolloutStatus @@ -60,6 +63,8 @@ type RolloutResult struct { ClustersTimeOut []ClusterRolloutStatus // ClustersRemoved is a slice of ClusterRolloutStatus that are removed. ClustersRemoved []ClusterRolloutStatus + // MaxFailureBreach is a boolean signaling whether the rollout was cut short because of failed clusters. + MaxFailureBreach bool } // ClusterRolloutStatusFunc defines a function that return the rollout status for a given workload. @@ -75,7 +80,7 @@ type RolloutHandler[T any] struct { statusFunc ClusterRolloutStatusFunc[T] } -// NewRolloutHandler creates a new RolloutHandler with the give workload type. +// NewRolloutHandler creates a new RolloutHandler with the given workload type. func NewRolloutHandler[T any](pdTracker *clusterv1beta1.PlacementDecisionClustersTracker, statusFunc ClusterRolloutStatusFunc[T]) (*RolloutHandler[T], error) { if pdTracker == nil { return nil, fmt.Errorf("invalid placement decision tracker %v", pdTracker) @@ -84,7 +89,7 @@ func NewRolloutHandler[T any](pdTracker *clusterv1beta1.PlacementDecisionCluster return &RolloutHandler[T]{pdTracker: pdTracker, statusFunc: statusFunc}, nil } -// The input are a RolloutStrategy and existingClusterRolloutStatus list. +// The inputs are a RolloutStrategy and existingClusterRolloutStatus list. // The existing ClusterRolloutStatus list should be created using the ClusterRolloutStatusFunc to determine the current workload rollout status. // The existing ClusterRolloutStatus list should contain all the current workloads rollout status such as ToApply, Progressing, Succeeded, // Failed, TimeOut and Skip in order to determine the added, removed, timeout clusters and next clusters to rollout. @@ -112,7 +117,7 @@ func (r *RolloutHandler[T]) getRolloutAllClusters(rolloutStrategy RolloutStrateg } // Parse timeout for the rollout - failureTimeout, err := parseTimeout(strategy.All.Timeout.Timeout) + failureTimeout, err := parseTimeout(strategy.All.ProgressDeadline) if err != nil { return &strategy, RolloutResult{}, err } @@ -122,7 +127,7 @@ func (r *RolloutHandler[T]) getRolloutAllClusters(rolloutStrategy RolloutStrateg // Check for removed Clusters currentClusterStatus, removedClusterStatus := r.getRemovedClusters(allClusterGroups, existingClusterStatus) - rolloutResult := progressivePerCluster(allClusterGroups, len(allClusters), failureTimeout, currentClusterStatus) + rolloutResult := progressivePerCluster(allClusterGroups, len(allClusters), len(allClusters), time.Duration(0), failureTimeout, currentClusterStatus) rolloutResult.ClustersRemoved = removedClusterStatus return &strategy, rolloutResult, nil @@ -135,9 +140,10 @@ func (r *RolloutHandler[T]) getProgressiveClusters(rolloutStrategy RolloutStrate if strategy.Progressive == nil { strategy.Progressive = &RolloutProgressive{} } + minSuccessTime := strategy.Progressive.MinSuccessTime.Duration // Parse timeout for non-mandatory decision groups - failureTimeout, err := parseTimeout(strategy.Progressive.Timeout.Timeout) + failureTimeout, err := parseTimeout(strategy.Progressive.ProgressDeadline) if err != nil { return &strategy, RolloutResult{}, err } @@ -146,13 +152,21 @@ func (r *RolloutHandler[T]) getProgressiveClusters(rolloutStrategy RolloutStrate clusterGroups := r.pdTracker.ExistingClusterGroupsBesides() currentClusterStatus, removedClusterStatus := r.getRemovedClusters(clusterGroups, existingClusterStatus) + // Parse maximum failure threshold for continuing the rollout, defaulting to zero + maxFailures, err := calculateRolloutSize(strategy.Progressive.MaxFailures, len(clusterGroups.GetClusters()), 0) + if err != nil { + return &strategy, RolloutResult{}, fmt.Errorf("failed to parse the provided maxFailures: %w", err) + } + // Upgrade mandatory decision groups first groupKeys := decisionGroupsToGroupKeys(strategy.Progressive.MandatoryDecisionGroups.MandatoryDecisionGroups) clusterGroups = r.pdTracker.ExistingClusterGroups(groupKeys...) // Perform progressive rollOut for mandatory decision groups first. if len(clusterGroups) > 0 { - rolloutResult := progressivePerGroup(clusterGroups, failureTimeout, currentClusterStatus) + rolloutResult := progressivePerGroup( + clusterGroups, intstr.FromInt(maxFailures), minSuccessTime, failureTimeout, currentClusterStatus, true, + ) if len(rolloutResult.ClustersToRollout) > 0 || len(rolloutResult.ClustersTimeOut) > 0 { rolloutResult.ClustersRemoved = removedClusterStatus return &strategy, rolloutResult, nil @@ -162,13 +176,13 @@ func (r *RolloutHandler[T]) getProgressiveClusters(rolloutStrategy RolloutStrate // Calculate the size of progressive rollOut // If the MaxConcurrency not defined, total clusters length is considered as maxConcurrency. clusterGroups = r.pdTracker.ExistingClusterGroupsBesides(groupKeys...) - length, err := calculateRolloutSize(strategy.Progressive.MaxConcurrency, len(clusterGroups.GetClusters())) + rolloutSize, err := calculateRolloutSize(strategy.Progressive.MaxConcurrency, len(clusterGroups.GetClusters()), len(clusterGroups.GetClusters())) if err != nil { - return &strategy, RolloutResult{}, err + return &strategy, RolloutResult{}, fmt.Errorf("failed to parse the provided maxConcurrency: %w", err) } // Rollout the remaining clusters - rolloutResult := progressivePerCluster(clusterGroups, length, failureTimeout, currentClusterStatus) + rolloutResult := progressivePerCluster(clusterGroups, rolloutSize, maxFailures, minSuccessTime, failureTimeout, currentClusterStatus) rolloutResult.ClustersRemoved = removedClusterStatus return &strategy, rolloutResult, nil @@ -181,13 +195,21 @@ func (r *RolloutHandler[T]) getProgressivePerGroupClusters(rolloutStrategy Rollo if strategy.ProgressivePerGroup == nil { strategy.ProgressivePerGroup = &RolloutProgressivePerGroup{} } + minSuccessTime := rolloutStrategy.ProgressivePerGroup.MinSuccessTime.Duration + maxFailures := rolloutStrategy.ProgressivePerGroup.MaxFailures // Parse timeout for non-mandatory decision groups - failureTimeout, err := parseTimeout(strategy.ProgressivePerGroup.Timeout.Timeout) + failureTimeout, err := parseTimeout(strategy.ProgressivePerGroup.ProgressDeadline) if err != nil { return &strategy, RolloutResult{}, err } + // Check format of MaxFailures--this value will be re-parsed and used in progressivePerGroup() + err = parseRolloutSize(maxFailures) + if err != nil { + return &strategy, RolloutResult{}, fmt.Errorf("failed to parse the provided maxFailures: %w", err) + } + // Check for removed Clusters clusterGroups := r.pdTracker.ExistingClusterGroupsBesides() currentClusterStatus, removedClusterStatus := r.getRemovedClusters(clusterGroups, existingClusterStatus) @@ -199,7 +221,7 @@ func (r *RolloutHandler[T]) getProgressivePerGroupClusters(rolloutStrategy Rollo // Perform progressive rollout per group for mandatory decision groups first if len(clusterGroups) > 0 { - rolloutResult := progressivePerGroup(clusterGroups, failureTimeout, currentClusterStatus) + rolloutResult := progressivePerGroup(clusterGroups, maxFailures, minSuccessTime, failureTimeout, currentClusterStatus, false) if len(rolloutResult.ClustersToRollout) > 0 || len(rolloutResult.ClustersTimeOut) > 0 { rolloutResult.ClustersRemoved = removedClusterStatus @@ -211,7 +233,7 @@ func (r *RolloutHandler[T]) getProgressivePerGroupClusters(rolloutStrategy Rollo restClusterGroups := r.pdTracker.ExistingClusterGroupsBesides(groupKeys...) // Perform progressive rollout per group for the remaining decision groups - rolloutResult := progressivePerGroup(restClusterGroups, failureTimeout, currentClusterStatus) + rolloutResult := progressivePerGroup(restClusterGroups, maxFailures, minSuccessTime, failureTimeout, currentClusterStatus, false) rolloutResult.ClustersRemoved = removedClusterStatus return &strategy, rolloutResult, nil @@ -238,35 +260,78 @@ func (r *RolloutHandler[T]) getRemovedClusters(clusterGroupsMap clusterv1beta1.C return currentClusterStatus, removedClusterStatus } -func progressivePerCluster(clusterGroupsMap clusterv1beta1.ClusterGroupsMap, length int, timeout time.Duration, existingClusterStatus []ClusterRolloutStatus) RolloutResult { +// progressivePerCluster parses the rollout status for the given clusters and returns the rollout +// result. It sorts the clusters alphabetically in order to determine the rollout groupings and the +// rollout group size is determined by the MaxConcurrency setting. +func progressivePerCluster( + clusterGroupsMap clusterv1beta1.ClusterGroupsMap, + rolloutSize int, + maxFailures int, + minSuccessTime time.Duration, + timeout time.Duration, + existingClusterStatus []ClusterRolloutStatus, +) RolloutResult { var rolloutClusters, timeoutClusters []ClusterRolloutStatus existingClusters := make(map[string]bool) + failureCount := 0 + failureBreach := false + + // Sort existing cluster status for consistency in case ToApply was determined by the workload applier + sort.Slice(existingClusterStatus, func(i, j int) bool { + return existingClusterStatus[i].ClusterName < existingClusterStatus[j].ClusterName + }) + // Collect current cluster status and determine any TimeOut statuses for _, status := range existingClusterStatus { if status.ClusterName == "" { continue } existingClusters[status.ClusterName] = true - rolloutClusters, timeoutClusters = determineRolloutStatus(status, timeout, rolloutClusters, timeoutClusters) - if len(rolloutClusters) >= length { + // If there was a breach of MaxFailures, only handle clusters that have already had workload applied + if !failureBreach || failureBreach && status.Status != ToApply { + rolloutClusters, timeoutClusters = determineRolloutStatus(status, minSuccessTime, timeout, rolloutClusters, timeoutClusters) + } + + // Keep track of TimeOut or Failed clusters and check total against MaxFailures + if status.Status == TimeOut || status.Status == Failed { + failureCount++ + + failureBreach = failureCount > maxFailures + } + + // Return if the list of rollout clusters has reached the target rollout size + if len(rolloutClusters) >= rolloutSize { return RolloutResult{ ClustersToRollout: rolloutClusters, ClustersTimeOut: timeoutClusters, + MaxFailureBreach: failureBreach, } } } + if failureBreach { + return RolloutResult{ + ClustersToRollout: rolloutClusters, + ClustersTimeOut: timeoutClusters, + MaxFailureBreach: failureBreach, + } + } + clusters := clusterGroupsMap.GetClusters().UnsortedList() clusterToGroupKey := clusterGroupsMap.ClusterToGroupKey() + // Sort the clusters in alphabetical order to ensure consistency. sort.Strings(clusters) + + // Amend clusters to the rollout up to the rollout size for _, cluster := range clusters { if existingClusters[cluster] { continue } + // For clusters without a rollout status, set the status to ToApply status := ClusterRolloutStatus{ ClusterName: cluster, Status: ToApply, @@ -274,7 +339,8 @@ func progressivePerCluster(clusterGroupsMap clusterv1beta1.ClusterGroupsMap, len } rolloutClusters = append(rolloutClusters, status) - if len(rolloutClusters) >= length { + // Return if the list of rollout clusters has reached the target rollout size + if len(rolloutClusters) >= rolloutSize { return RolloutResult{ ClustersToRollout: rolloutClusters, ClustersTimeOut: timeoutClusters, @@ -288,7 +354,14 @@ func progressivePerCluster(clusterGroupsMap clusterv1beta1.ClusterGroupsMap, len } } -func progressivePerGroup(clusterGroupsMap clusterv1beta1.ClusterGroupsMap, timeout time.Duration, existingClusterStatus []ClusterRolloutStatus) RolloutResult { +func progressivePerGroup( + clusterGroupsMap clusterv1beta1.ClusterGroupsMap, + maxFailures intstr.IntOrString, + minSuccessTime time.Duration, + timeout time.Duration, + existingClusterStatus []ClusterRolloutStatus, + accumulateFailures bool, +) RolloutResult { var rolloutClusters, timeoutClusters []ClusterRolloutStatus existingClusters := make(map[string]bool) @@ -302,13 +375,24 @@ func progressivePerGroup(clusterGroupsMap clusterv1beta1.ClusterGroupsMap, timeo existingClusters[status.ClusterName] = false } else { existingClusters[status.ClusterName] = true - rolloutClusters, timeoutClusters = determineRolloutStatus(status, timeout, rolloutClusters, timeoutClusters) + rolloutClusters, timeoutClusters = determineRolloutStatus(status, minSuccessTime, timeout, rolloutClusters, timeoutClusters) } } + var failureCount int + failureBreach := false clusterGroupKeys := clusterGroupsMap.GetOrderedGroupKeys() for _, key := range clusterGroupKeys { if subclusters, ok := clusterGroupsMap[key]; ok { + // Only reset the failure count for ProgressivePerGroup + // since Progressive is over the total number of clusters + if !accumulateFailures { + failureCount = 0 + } + failureBreach = false + // Calculate the max failure threshold for the group--the returned error was checked + // previously, so it's ignored here + maxGroupFailures, _ := calculateRolloutSize(maxFailures, subclusters.Len(), 0) // Iterate through clusters in the group clusters := subclusters.UnsortedList() sort.Strings(clusters) @@ -323,13 +407,22 @@ func progressivePerGroup(clusterGroupsMap clusterv1beta1.ClusterGroupsMap, timeo GroupKey: key, } rolloutClusters = append(rolloutClusters, status) + + // Keep track of TimeOut or Failed clusters and check total against MaxFailures + if status.Status == TimeOut || status.Status == Failed { + failureCount++ + + failureBreach = failureCount > maxGroupFailures + } } - // As it is perGroup Return if there are clusters to rollOut - if len(rolloutClusters) > 0 { + // As it is perGroup Return if there are clusters to rollOut, + // or there was a breach of the MaxFailure configuration + if len(rolloutClusters) > 0 || failureBreach { return RolloutResult{ ClustersToRollout: rolloutClusters, ClustersTimeOut: timeoutClusters, + MaxFailureBreach: failureBreach, } } } @@ -338,26 +431,47 @@ func progressivePerGroup(clusterGroupsMap clusterv1beta1.ClusterGroupsMap, timeo return RolloutResult{ ClustersToRollout: rolloutClusters, ClustersTimeOut: timeoutClusters, + MaxFailureBreach: failureBreach, } } -// determineRolloutStatus checks whether a cluster should continue its rollout based on its current status and timeout. -// The function update the cluster status and append it to the expected slice. +// determineRolloutStatus checks whether a cluster should continue its rollout based on its current +// status and timeout. The function updates the cluster status and appends it to the expected slice. +// Nothing is done for TimeOut or Skip statuses. +// +// The minSuccessTime parameter is utilized for handling succeeded clusters that are still within +// the configured soak time, in which case the cluster will be returned as a rolloutCluster. // -// The timeout parameter is utilized for handling progressing and failed statuses and any other unknown status: -// 1. If timeout is set to None (maxTimeDuration), the function will append the clusterStatus to the rollOut Clusters. +// The timeout parameter is utilized for handling progressing and failed statuses and any other +// unknown status: +// 1. If timeout is set to None (maxTimeDuration), the function will append the clusterStatus to +// the rollOut Clusters. // 2. If timeout is set to 0, the function append the clusterStatus to the timeOut clusters. -func determineRolloutStatus(status ClusterRolloutStatus, timeout time.Duration, rolloutClusters []ClusterRolloutStatus, timeoutClusters []ClusterRolloutStatus) ([]ClusterRolloutStatus, []ClusterRolloutStatus) { +func determineRolloutStatus( + status ClusterRolloutStatus, + minSuccessTime time.Duration, + timeout time.Duration, + rolloutClusters []ClusterRolloutStatus, + timeoutClusters []ClusterRolloutStatus, +) ([]ClusterRolloutStatus, []ClusterRolloutStatus) { switch status.Status { case ToApply: rolloutClusters = append(rolloutClusters, status) - case TimeOut, Succeeded, Skip: + case Succeeded: + // If the cluster succeeded but is still within the MinSuccessTime (i.e. "soak" time), + // still add it to the list of rolloutClusters + minSuccessTimeTime := getTimeOutTime(status.LastTransitionTime, minSuccessTime) + if RolloutClock.Now().Before(minSuccessTimeTime.Time) { + rolloutClusters = append(rolloutClusters, status) + } + return rolloutClusters, timeoutClusters - default: // For progressing, failed status and any other unknown status. + case TimeOut, Skip: + return rolloutClusters, timeoutClusters + default: // For progressing, failed, or unknown status. timeOutTime := getTimeOutTime(status.LastTransitionTime, timeout) status.TimeOutTime = timeOutTime - // check if current time is before the timeout time if RolloutClock.Now().Before(timeOutTime.Time) { rolloutClusters = append(rolloutClusters, status) @@ -370,7 +484,8 @@ func determineRolloutStatus(status ClusterRolloutStatus, timeout time.Duration, return rolloutClusters, timeoutClusters } -// get the timeout time +// getTimeOutTime calculates the timeout time given a start time and duration, instantiating the +// RolloutClock if a start time isn't provided. func getTimeOutTime(startTime *metav1.Time, timeout time.Duration) *metav1.Time { var timeoutTime time.Time if startTime == nil { @@ -381,34 +496,62 @@ func getTimeOutTime(startTime *metav1.Time, timeout time.Duration) *metav1.Time return &metav1.Time{Time: timeoutTime} } -func calculateRolloutSize(maxConcurrency intstr.IntOrString, total int) (int, error) { - length := total +// calculateRolloutSize calculates the maximum portion from a total number of clusters by parsing a +// maximum threshold value that can be either a quantity or a percent, returning an error if the +// threshold can't be parsed to either of those. +func calculateRolloutSize(maxThreshold intstr.IntOrString, total int, defaultThreshold int) (int, error) { + length := defaultThreshold + + // Verify the format of the IntOrString value + err := parseRolloutSize(maxThreshold) + if err != nil { + return length, err + } - switch maxConcurrency.Type { + // Calculate the rollout size--errors are ignored because + // they were handled in parseRolloutSize() previously + switch maxThreshold.Type { case intstr.Int: - length = maxConcurrency.IntValue() + length = maxThreshold.IntValue() case intstr.String: - str := maxConcurrency.StrVal + str := maxThreshold.StrVal + f, _ := strconv.ParseFloat(str[:len(str)-1], 64) + length = int(math.Ceil(f / 100 * float64(total))) + } + + if length <= 0 || length > total { + length = defaultThreshold + } + + return length, nil +} + +// parseRolloutSize parses a maximum threshold value that can be either a quantity or a percent, +// returning an error if the threshold can't be parsed to either of those. +func parseRolloutSize(maxThreshold intstr.IntOrString) error { + + switch maxThreshold.Type { + case intstr.Int: + break + case intstr.String: + str := maxThreshold.StrVal if strings.HasSuffix(str, "%") { - f, err := strconv.ParseFloat(str[:len(str)-1], 64) + _, err := strconv.ParseFloat(str[:len(str)-1], 64) if err != nil { - return length, err + return err } - length = int(math.Ceil(f / 100 * float64(total))) } else { - return length, fmt.Errorf("%v invalid type: string is not a percentage", maxConcurrency) + return fmt.Errorf("'%s' is an invalid maximum threshold value: string is not a percentage", str) } default: - return length, fmt.Errorf("incorrect MaxConcurrency type %v", maxConcurrency.Type) + return fmt.Errorf("invalid maximum threshold type %+v", maxThreshold.Type) } - if length <= 0 || length > total { - length = total - } - - return length, nil + return nil } +// ParseTimeout will return the maximum possible duration given "None", an empty string, or an +// invalid duration, otherwise parsing and returning the duration provided. func parseTimeout(timeoutStr string) (time.Duration, error) { // Define the regex pattern to match the timeout string pattern := "^(([0-9])+[h|m|s])|None$" diff --git a/cluster/v1alpha1/helpers_test.go b/cluster/v1alpha1/helpers_test.go index 396e3bd9e..9241cb9e9 100644 --- a/cluster/v1alpha1/helpers_test.go +++ b/cluster/v1alpha1/helpers_test.go @@ -19,6 +19,7 @@ var fakeTime = metav1.NewTime(time.Date(2022, time.January, 01, 0, 0, 0, 0, time var fakeTimeMax_60s = metav1.NewTime(fakeTime.Add(maxTimeDuration - time.Minute)) var fakeTimeMax_120s = metav1.NewTime(fakeTime.Add(maxTimeDuration - 2*time.Minute)) var fakeTime30s = metav1.NewTime(fakeTime.Add(30 * time.Second)) +var fakeTime60s = metav1.NewTime(fakeTime.Add(time.Minute)) var fakeTime_30s = metav1.NewTime(fakeTime.Add(-30 * time.Second)) var fakeTime_60s = metav1.NewTime(fakeTime.Add(-time.Minute)) var fakeTime_120s = metav1.NewTime(fakeTime.Add(-2 * time.Minute)) @@ -37,23 +38,23 @@ type dummyWorkload struct { // Dummy Workload status const ( - valid = "valid" + waiting = "waiting" applying = "applying" done = "done" - missing = "missing" + failed = "failed" ) // Dummy ClusterRolloutStatusFunc implementation that will be used to create a RolloutHandler. func dummyWorkloadClusterRolloutStatusFunc(clusterName string, workload dummyWorkload) (ClusterRolloutStatus, error) { // workload obj should be used to determine the clusterRolloutStatus. switch workload.State { - case valid: + case waiting: return ClusterRolloutStatus{GroupKey: workload.ClusterGroup, ClusterName: clusterName, Status: ToApply, LastTransitionTime: workload.LastTransitionTime}, nil case applying: return ClusterRolloutStatus{GroupKey: workload.ClusterGroup, ClusterName: clusterName, Status: Progressing, LastTransitionTime: workload.LastTransitionTime}, nil case done: return ClusterRolloutStatus{GroupKey: workload.ClusterGroup, ClusterName: clusterName, Status: Succeeded, LastTransitionTime: workload.LastTransitionTime}, nil - case missing: + case failed: return ClusterRolloutStatus{GroupKey: workload.ClusterGroup, ClusterName: clusterName, Status: Failed, LastTransitionTime: workload.LastTransitionTime}, nil default: return ClusterRolloutStatus{GroupKey: workload.ClusterGroup, ClusterName: clusterName, Status: ToApply, LastTransitionTime: workload.LastTransitionTime}, nil @@ -67,9 +68,7 @@ type testCase struct { clusterRolloutStatusFunc ClusterRolloutStatusFunc[dummyWorkload] // Using type dummy workload obj expectRolloutStrategy *RolloutStrategy existingWorkloads []dummyWorkload - expectRolloutClusters []ClusterRolloutStatus - expectTimeOutClusters []ClusterRolloutStatus - expectRemovedClusters []ClusterRolloutStatus + expectRolloutResult RolloutResult } func (f *FakePlacementDecisionGetter) List(selector labels.Selector, namespace string) (ret []*clusterv1beta1.PlacementDecision, err error) { @@ -79,27 +78,29 @@ func (f *FakePlacementDecisionGetter) List(selector labels.Selector, namespace s func TestGetRolloutCluster_All(t *testing.T) { tests := []testCase{ { - name: "test rollout all with timeout 90s witout workload created", - rolloutStrategy: RolloutStrategy{Type: All, All: &RolloutAll{Timeout: Timeout{"90s"}}}, + name: "test rollout all with timeout 90s without workload created", + rolloutStrategy: RolloutStrategy{Type: All, All: &RolloutAll{RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}}}, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ {GroupName: "group1", GroupIndex: 0}: sets.New[string]("cluster1", "cluster2"), {GroupName: "", GroupIndex: 1}: sets.New[string]("cluster3", "cluster4", "cluster5", "cluster6"), }, clusterRolloutStatusFunc: dummyWorkloadClusterRolloutStatusFunc, existingWorkloads: []dummyWorkload{}, - expectRolloutStrategy: &RolloutStrategy{Type: All, All: &RolloutAll{Timeout: Timeout{"90s"}}}, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster6", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, + expectRolloutStrategy: &RolloutStrategy{Type: All, All: &RolloutAll{RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}}}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster6", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, + }, }, }, { name: "test rollout all with timeout 90s", - rolloutStrategy: RolloutStrategy{Type: All, All: &RolloutAll{Timeout: Timeout{"90s"}}}, + rolloutStrategy: RolloutStrategy{Type: All, All: &RolloutAll{RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}}}, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ {GroupName: "group1", GroupIndex: 0}: sets.New[string]("cluster1", "cluster2"), {GroupName: "", GroupIndex: 1}: sets.New[string]("cluster3", "cluster4", "cluster5", "cluster6"), @@ -115,7 +116,7 @@ func TestGetRolloutCluster_All(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: missing, + State: failed, LastTransitionTime: &fakeTime_120s, }, { @@ -127,7 +128,7 @@ func TestGetRolloutCluster_All(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, ClusterName: "cluster4", - State: missing, + State: failed, LastTransitionTime: &fakeTime_60s, }, { @@ -137,15 +138,17 @@ func TestGetRolloutCluster_All(t *testing.T) { LastTransitionTime: &fakeTime_60s, }, }, - expectRolloutStrategy: &RolloutStrategy{Type: All, All: &RolloutAll{Timeout: Timeout{"90s"}}}, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: Failed, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, - {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, - {ClusterName: "cluster6", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, - }, - expectTimeOutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + expectRolloutStrategy: &RolloutStrategy{Type: All, All: &RolloutAll{RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}}}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: Failed, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + {ClusterName: "cluster6", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, + }, + ClustersTimeOut: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + }, }, }, } @@ -166,20 +169,23 @@ func TestGetRolloutCluster_All(t *testing.T) { actualRolloutStrategy, actualRolloutResult, _ := rolloutHandler.GetRolloutCluster(test.rolloutStrategy, existingRolloutClusters) if !reflect.DeepEqual(actualRolloutStrategy.All, test.expectRolloutStrategy.All) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect strategy : %v, actual : %v", test.name, test.expectRolloutStrategy, actualRolloutStrategy) - return + t.Errorf("Case: %v, Failed to run NewRolloutHandler.\nExpect strategy: %+v\nActual strategy: %+v", test.name, test.expectRolloutStrategy, actualRolloutStrategy) } - if !reflect.DeepEqual(actualRolloutResult.ClustersToRollout, test.expectRolloutClusters) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect rollout clusters: %v, actual : %v", test.name, test.expectRolloutClusters, actualRolloutResult.ClustersToRollout) - return + if !reflect.DeepEqual(actualRolloutResult.ClustersToRollout, test.expectRolloutResult.ClustersToRollout) { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect rollout clusters: %+v\nActual rollout clusters: %+v", + test.name, test.expectRolloutResult.ClustersToRollout, actualRolloutResult.ClustersToRollout) } - if !reflect.DeepEqual(actualRolloutResult.ClustersTimeOut, test.expectTimeOutClusters) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect timeout clusters: %v, actual : %v", test.name, test.expectTimeOutClusters, actualRolloutResult.ClustersTimeOut) - return + if !reflect.DeepEqual(actualRolloutResult.ClustersTimeOut, test.expectRolloutResult.ClustersTimeOut) { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect timeout clusters: %+v\nActual timeout clusters: %+v", + test.name, test.expectRolloutResult.ClustersTimeOut, actualRolloutResult.ClustersTimeOut) } - if !reflect.DeepEqual(actualRolloutResult.ClustersRemoved, test.expectRemovedClusters) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect removed clusters: %v, actual : %v", test.name, test.expectRemovedClusters, actualRolloutResult.ClustersRemoved) - return + if !reflect.DeepEqual(actualRolloutResult.ClustersRemoved, test.expectRolloutResult.ClustersRemoved) { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect removed clusters: %+v\nActual removed clusters: %+v", + test.name, test.expectRolloutResult.ClustersRemoved, actualRolloutResult.ClustersRemoved) + } + if actualRolloutResult.MaxFailureBreach != test.expectRolloutResult.MaxFailureBreach { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect failure breach: %+v\nActual failure breach: %+v", + test.name, test.expectRolloutResult.MaxFailureBreach, actualRolloutResult.MaxFailureBreach) } } } @@ -187,11 +193,42 @@ func TestGetRolloutCluster_All(t *testing.T) { func TestGetRolloutCluster_Progressive(t *testing.T) { tests := []testCase{ { - name: "test progressive rollout with timeout 90s witout workload created", + name: "test progressive rollout deprecated timeout", + rolloutStrategy: RolloutStrategy{ + Type: Progressive, + Progressive: &RolloutProgressive{ + RolloutConfig: RolloutConfig{Timeout: "90s"}, + MaxConcurrency: intstr.FromInt(2), + }, + }, + existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ + {GroupName: "group1", GroupIndex: 0}: sets.New[string]("cluster1", "cluster2", "cluster3"), + {GroupName: "", GroupIndex: 1}: sets.New[string]("cluster4", "cluster5", "cluster6"), + }, + clusterRolloutStatusFunc: dummyWorkloadClusterRolloutStatusFunc, + expectRolloutStrategy: &RolloutStrategy{ + Type: Progressive, + Progressive: &RolloutProgressive{ + RolloutConfig: RolloutConfig{ + Timeout: "90s", + }, + MaxConcurrency: intstr.FromInt(2), + }, + }, + existingWorkloads: []dummyWorkload{}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + }, + }, + }, + { + name: "test progressive rollout with timeout 90s without workload created", rolloutStrategy: RolloutStrategy{ Type: Progressive, Progressive: &RolloutProgressive{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, MaxConcurrency: intstr.FromInt(2), }, }, @@ -203,14 +240,16 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: Progressive, Progressive: &RolloutProgressive{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, MaxConcurrency: intstr.FromInt(2), }, }, existingWorkloads: []dummyWorkload{}, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + }, }, }, { @@ -219,7 +258,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { Type: Progressive, Progressive: &RolloutProgressive{ MaxConcurrency: intstr.FromInt(4), - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -232,7 +271,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { Type: Progressive, Progressive: &RolloutProgressive{ MaxConcurrency: intstr.FromInt(4), - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingWorkloads: []dummyWorkload{ @@ -245,57 +284,59 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster3", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupIndex: 1}, ClusterName: "cluster4", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupIndex: 1}, ClusterName: "cluster5", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupIndex: 1}, ClusterName: "cluster6", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupIndex: 2}, ClusterName: "cluster7", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupIndex: 2}, ClusterName: "cluster8", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupIndex: 2}, ClusterName: "cluster9", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupIndex: 2}, ClusterName: "cluster10", - State: valid, + State: waiting, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, - {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, - }, - expectRemovedClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster10", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 2}, Status: ToApply}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + }, + ClustersRemoved: []ClusterRolloutStatus{ + {ClusterName: "cluster10", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 2}, Status: ToApply}, + }, }, }, { @@ -303,7 +344,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { rolloutStrategy: RolloutStrategy{ Type: Progressive, Progressive: &RolloutProgressive{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -315,7 +356,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: Progressive, Progressive: &RolloutProgressive{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingWorkloads: []dummyWorkload{ @@ -328,70 +369,72 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster3", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupIndex: 1}, ClusterName: "cluster4", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupIndex: 1}, ClusterName: "cluster5", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupIndex: 1}, ClusterName: "cluster6", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupIndex: 2}, ClusterName: "cluster7", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupIndex: 2}, ClusterName: "cluster8", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupIndex: 2}, ClusterName: "cluster9", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupIndex: 2}, ClusterName: "cluster10", - State: valid, + State: waiting, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, - {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster6", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster7", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 2}, Status: ToApply}, - {ClusterName: "cluster8", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 2}, Status: ToApply}, - {ClusterName: "cluster9", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 2}, Status: ToApply}, - }, - expectRemovedClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster10", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 2}, Status: ToApply}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster6", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster7", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 2}, Status: ToApply}, + {ClusterName: "cluster8", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 2}, Status: ToApply}, + {ClusterName: "cluster9", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 2}, Status: ToApply}, + }, + ClustersRemoved: []ClusterRolloutStatus{ + {ClusterName: "cluster10", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 2}, Status: ToApply}, + }, }, }, { - name: "test progressive rollout with timeout 90s", + name: "test progressive rollout with timeout 90s and no maxFailures set", rolloutStrategy: RolloutStrategy{ Type: Progressive, Progressive: &RolloutProgressive{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, MaxConcurrency: intstr.FromInt(2), }, }, @@ -403,7 +446,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: Progressive, Progressive: &RolloutProgressive{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, MaxConcurrency: intstr.FromInt(2), }, }, @@ -417,7 +460,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: missing, + State: failed, LastTransitionTime: &fakeTime_120s, }, { @@ -427,12 +470,125 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { LastTransitionTime: &fakeTime_60s, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, - {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + }, + ClustersTimeOut: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + }, + MaxFailureBreach: true, + }, + }, + { + name: "test progressive rollout with timeout 90s, 100% maxFailures, minSuccessTime 60s", + rolloutStrategy: RolloutStrategy{ + Type: Progressive, + Progressive: &RolloutProgressive{ + RolloutConfig: RolloutConfig{ + ProgressDeadline: "90s", + MaxFailures: intstr.FromString("100%"), + MinSuccessTime: metav1.Duration{Duration: time.Minute}, + }, + MaxConcurrency: intstr.FromInt(2), + }, + }, + existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ + {GroupName: "group1", GroupIndex: 0}: sets.New[string]("cluster1", "cluster2", "cluster3"), + {GroupName: "", GroupIndex: 1}: sets.New[string]("cluster4", "cluster5", "cluster6"), + }, + clusterRolloutStatusFunc: dummyWorkloadClusterRolloutStatusFunc, + expectRolloutStrategy: &RolloutStrategy{ + Type: Progressive, + Progressive: &RolloutProgressive{ + RolloutConfig: RolloutConfig{ + ProgressDeadline: "90s", + MaxFailures: intstr.FromString("100%"), + MinSuccessTime: metav1.Duration{Duration: time.Minute}, + }, + MaxConcurrency: intstr.FromInt(2), + }, + }, + existingWorkloads: []dummyWorkload{ + { + ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, + ClusterName: "cluster1", + State: done, + LastTransitionTime: &fakeTime_30s, + }, + { + ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, + ClusterName: "cluster2", + State: failed, + LastTransitionTime: &fakeTime_120s, + }, + { + ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, + ClusterName: "cluster3", + State: applying, + LastTransitionTime: &fakeTime_30s, + }, + }, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Succeeded, LastTransitionTime: &fakeTime_30s}, + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Progressing, LastTransitionTime: &fakeTime_30s, TimeOutTime: &fakeTime60s}, + }, + ClustersTimeOut: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + }, + MaxFailureBreach: false, + }, + }, + { + name: "test progressive rollout with timeout 90s and maxFailures 1", + rolloutStrategy: RolloutStrategy{ + Type: Progressive, + Progressive: &RolloutProgressive{ + RolloutConfig: RolloutConfig{ProgressDeadline: "90s", MaxFailures: intstr.FromInt(1)}, + MaxConcurrency: intstr.FromInt(2), + }, + }, + existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ + {GroupName: "group1", GroupIndex: 0}: sets.New[string]("cluster1", "cluster2", "cluster3"), + {GroupName: "", GroupIndex: 1}: sets.New[string]("cluster4", "cluster5", "cluster6"), + }, + clusterRolloutStatusFunc: dummyWorkloadClusterRolloutStatusFunc, + expectRolloutStrategy: &RolloutStrategy{ + Type: Progressive, + Progressive: &RolloutProgressive{ + RolloutConfig: RolloutConfig{ProgressDeadline: "90s", MaxFailures: intstr.FromInt(1)}, + MaxConcurrency: intstr.FromInt(2), + }, + }, + existingWorkloads: []dummyWorkload{ + { + ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, + ClusterName: "cluster1", + State: done, + LastTransitionTime: &fakeTime_60s, + }, + { + ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, + ClusterName: "cluster2", + State: failed, + LastTransitionTime: &fakeTime_120s, + }, + { + ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, + ClusterName: "cluster3", + State: applying, + LastTransitionTime: &fakeTime_60s, + }, }, - expectTimeOutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, + }, + ClustersTimeOut: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + }, }, }, { @@ -440,7 +596,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { rolloutStrategy: RolloutStrategy{ Type: Progressive, Progressive: &RolloutProgressive{ - Timeout: Timeout{"0s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "0s"}, MaxConcurrency: intstr.FromInt(2), }, }, @@ -452,7 +608,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: Progressive, Progressive: &RolloutProgressive{ - Timeout: Timeout{"0s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "0s"}, MaxConcurrency: intstr.FromInt(2), }, }, @@ -476,17 +632,19 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { LastTransitionTime: &fakeTime_30s, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, - }, - expectTimeOutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_30s, TimeOutTime: &fakeTime_30s}, - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_30s, TimeOutTime: &fakeTime_30s}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, + }, + ClustersTimeOut: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_30s, TimeOutTime: &fakeTime_30s}, + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_30s, TimeOutTime: &fakeTime_30s}, + }, }, }, { - name: "test progressive rollout with mandatroyDecisionGroup and timeout 90s ", + name: "test progressive rollout with mandatoryDecisionGroup and timeout 90s ", rolloutStrategy: RolloutStrategy{ Type: Progressive, Progressive: &RolloutProgressive{ @@ -495,7 +653,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { {GroupName: "group1"}, }, }, - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, MaxConcurrency: intstr.FromInt(3), }, }, @@ -512,7 +670,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { {GroupName: "group1"}, }, }, - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, MaxConcurrency: intstr.FromInt(3), }, }, @@ -526,7 +684,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: missing, + State: failed, LastTransitionTime: &fakeTime_120s, }, { @@ -536,19 +694,70 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { LastTransitionTime: &fakeTime_60s, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + }, + ClustersTimeOut: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + }, + }, + }, + { + name: "test progressive rollout with timeout None, MaxConcurrency 50%, MaxFailures 100%", + rolloutStrategy: RolloutStrategy{ + Type: Progressive, + Progressive: &RolloutProgressive{ + RolloutConfig: RolloutConfig{ProgressDeadline: "None", MaxFailures: intstr.FromString("100%")}, + MaxConcurrency: intstr.FromString("50%"), // 50% of total clusters + }, + }, + existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ + {GroupName: "group1", GroupIndex: 0}: sets.New[string]("cluster1", "cluster2"), + {GroupName: "", GroupIndex: 1}: sets.New[string]("cluster3", "cluster4", "cluster5", "cluster6"), + }, + clusterRolloutStatusFunc: dummyWorkloadClusterRolloutStatusFunc, + expectRolloutStrategy: &RolloutStrategy{ + Type: Progressive, + Progressive: &RolloutProgressive{ + RolloutConfig: RolloutConfig{ProgressDeadline: "None", MaxFailures: intstr.FromString("100%")}, + MaxConcurrency: intstr.FromString("50%"), // 50% of total clusters + }, + }, + existingWorkloads: []dummyWorkload{ + { + ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, + ClusterName: "cluster1", + State: done, + LastTransitionTime: &fakeTime_60s, + }, + { + ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, + ClusterName: "cluster2", + State: failed, + LastTransitionTime: &fakeTime_120s, + }, + { + ClusterGroup: clusterv1beta1.GroupKey{GroupIndex: 1}, + ClusterName: "cluster3", + State: applying, + LastTransitionTime: &fakeTime_60s, + }, }, - expectTimeOutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTimeMax_120s}, + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTimeMax_60s}, + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, + }, }, }, { - name: "test progressive rollout with timeout None and MaxConcurrency 50%", + name: "test progressive rollout with timeout None, MaxConcurrency 50%, MaxFailures 0%", rolloutStrategy: RolloutStrategy{ Type: Progressive, Progressive: &RolloutProgressive{ - Timeout: Timeout{"None"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "None", MaxFailures: intstr.FromString("0%")}, MaxConcurrency: intstr.FromString("50%"), // 50% of total clusters }, }, @@ -560,7 +769,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: Progressive, Progressive: &RolloutProgressive{ - Timeout: Timeout{"None"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "None", MaxFailures: intstr.FromString("0%")}, MaxConcurrency: intstr.FromString("50%"), // 50% of total clusters }, }, @@ -574,7 +783,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: missing, + State: failed, LastTransitionTime: &fakeTime_120s, }, { @@ -584,10 +793,12 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { LastTransitionTime: &fakeTime_60s, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTimeMax_120s}, - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTimeMax_60s}, - {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTimeMax_120s}, + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTimeMax_60s}, + }, + MaxFailureBreach: true, }, }, { @@ -601,7 +812,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { }, }, MaxConcurrency: intstr.FromInt(3), - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -619,28 +830,30 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { }, }, MaxConcurrency: intstr.FromInt(3), - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingWorkloads: []dummyWorkload{ { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster1", - State: missing, + State: failed, LastTransitionTime: &fakeTime_120s, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: missing, + State: failed, LastTransitionTime: &fakeTime_60s, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, - }, - expectTimeOutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + }, + ClustersTimeOut: []ClusterRolloutStatus{ + {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + }, }, }, { @@ -671,7 +884,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { }, }, MaxConcurrency: intstr.FromInt(3), - Timeout: Timeout{""}, + RolloutConfig: RolloutConfig{ProgressDeadline: ""}, }, }, existingWorkloads: []dummyWorkload{ @@ -688,10 +901,12 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { LastTransitionTime: &fakeTime_120s, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 2}, Status: ToApply}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 2}, Status: ToApply}, + }, }, }, } @@ -714,20 +929,23 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { actualRolloutStrategy, actualRolloutResult, _ := rolloutHandler.GetRolloutCluster(test.rolloutStrategy, existingRolloutClusters) if !reflect.DeepEqual(actualRolloutStrategy.Progressive, test.expectRolloutStrategy.Progressive) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect strategy : %v, actual : %v", test.name, test.expectRolloutStrategy, actualRolloutStrategy) - return + t.Errorf("Case: %v, Failed to run NewRolloutHandler.\nExpect strategy: %+v\nActual strategy: %+v", test.name, test.expectRolloutStrategy, actualRolloutStrategy) } - if !reflect.DeepEqual(actualRolloutResult.ClustersToRollout, test.expectRolloutClusters) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect rollout clusters: %v, actual : %v", test.name, test.expectRolloutClusters, actualRolloutResult.ClustersToRollout) - return + if !reflect.DeepEqual(actualRolloutResult.ClustersToRollout, test.expectRolloutResult.ClustersToRollout) { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect rollout clusters: %+v\nActual rollout clusters: %+v", + test.name, test.expectRolloutResult.ClustersToRollout, actualRolloutResult.ClustersToRollout) } - if !reflect.DeepEqual(actualRolloutResult.ClustersTimeOut, test.expectTimeOutClusters) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect timeout clusters: %v, actual : %v", test.name, test.expectTimeOutClusters, actualRolloutResult.ClustersTimeOut) - return + if !reflect.DeepEqual(actualRolloutResult.ClustersTimeOut, test.expectRolloutResult.ClustersTimeOut) { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect timeout clusters: %+v\nActual timeout clusters: %+v", + test.name, test.expectRolloutResult.ClustersTimeOut, actualRolloutResult.ClustersTimeOut) } - if !reflect.DeepEqual(actualRolloutResult.ClustersRemoved, test.expectRemovedClusters) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect removed clusters: %v, actual : %v", test.name, test.expectRemovedClusters, actualRolloutResult.ClustersRemoved) - return + if !reflect.DeepEqual(actualRolloutResult.ClustersRemoved, test.expectRolloutResult.ClustersRemoved) { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect removed clusters: %+v\nActual removed clusters: %+v", + test.name, test.expectRolloutResult.ClustersRemoved, actualRolloutResult.ClustersRemoved) + } + if actualRolloutResult.MaxFailureBreach != test.expectRolloutResult.MaxFailureBreach { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect failure breach: %+v\nActual failure breach: %+v", + test.name, test.expectRolloutResult.MaxFailureBreach, actualRolloutResult.MaxFailureBreach) } } } @@ -735,11 +953,11 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { tests := []testCase{ { - name: "test progressivePerGroup rollout with timeout 90s witout workload created", + name: "test progressivePerGroup rollout with timeout 90s without workload created", rolloutStrategy: RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -750,14 +968,16 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingWorkloads: []dummyWorkload{}, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + }, }, }, { @@ -765,7 +985,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { rolloutStrategy: RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -776,7 +996,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingWorkloads: []dummyWorkload{ @@ -789,33 +1009,35 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster3", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupIndex: 1}, ClusterName: "cluster4", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupIndex: 1}, ClusterName: "cluster5", - State: valid, + State: waiting, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupIndex: 1}, ClusterName: "cluster6", - State: valid, + State: waiting, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + }, }, }, { @@ -823,7 +1045,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { rolloutStrategy: RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -834,7 +1056,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingWorkloads: []dummyWorkload{ @@ -847,7 +1069,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: missing, + State: failed, LastTransitionTime: &fakeTime_120s, }, { @@ -857,11 +1079,13 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { LastTransitionTime: &fakeTime_60s, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, - }, - expectTimeOutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + }, + ClustersTimeOut: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + }, }, }, { @@ -869,7 +1093,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { rolloutStrategy: RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -881,7 +1105,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingWorkloads: []dummyWorkload{ @@ -894,7 +1118,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: missing, + State: failed, LastTransitionTime: &fakeTime_120s, }, { @@ -904,21 +1128,23 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { LastTransitionTime: &fakeTime_60s, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster6", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, - }, - expectTimeOutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster6", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + }, + ClustersTimeOut: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + }, }, }, { - name: "test progressivePerGroup rollout with timeout 90s and first group timeOut, second group successed", + name: "test progressivePerGroup rollout with timeout 90s and first group timeOut, second group succeeded", rolloutStrategy: RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -930,7 +1156,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingWorkloads: []dummyWorkload{ @@ -943,7 +1169,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: missing, + State: failed, LastTransitionTime: &fakeTime_120s, }, { @@ -971,13 +1197,15 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { LastTransitionTime: &fakeTime_60s, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster7", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 2}, Status: ToApply}, - {ClusterName: "cluster8", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 2}, Status: ToApply}, - {ClusterName: "cluster9", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 2}, Status: ToApply}, - }, - expectTimeOutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster7", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 2}, Status: ToApply}, + {ClusterName: "cluster8", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 2}, Status: ToApply}, + {ClusterName: "cluster9", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 2}, Status: ToApply}, + }, + ClustersTimeOut: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + }, }, }, { @@ -985,7 +1213,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { rolloutStrategy: RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - Timeout: Timeout{"None"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "None"}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -997,7 +1225,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - Timeout: Timeout{"None"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "None"}, }, }, existingWorkloads: []dummyWorkload{ @@ -1010,7 +1238,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: missing, + State: failed, LastTransitionTime: &fakeTime_120s, }, { @@ -1020,12 +1248,14 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { LastTransitionTime: &fakeTime_60s, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTimeMax_120s}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTimeMax_120s}, + }, }, }, { - name: "test ProgressivePerGroup rollout with mandatroyDecisionGroup failing and timeout 90s ", + name: "test ProgressivePerGroup rollout with mandatoryDecisionGroup failing and timeout 90s ", rolloutStrategy: RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ @@ -1034,7 +1264,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { {GroupName: "group1"}, }, }, - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -1050,7 +1280,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { {GroupName: "group1"}, }, }, - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingWorkloads: []dummyWorkload{ @@ -1063,23 +1293,25 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: missing, + State: failed, LastTransitionTime: &fakeTime_120s, }, { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster3", - State: missing, + State: failed, LastTransitionTime: &fakeTime_120s, }, }, - expectTimeOutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + expectRolloutResult: RolloutResult{ + ClustersTimeOut: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + }, }, }, { - name: "test ProgressivePerGroup rollout with mandatroyDecisionGroup Succeeded and timeout 90s ", + name: "test ProgressivePerGroup rollout with mandatoryDecisionGroup Succeeded and timeout 90s ", rolloutStrategy: RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ @@ -1088,7 +1320,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { {GroupName: "group1"}, }, }, - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -1105,7 +1337,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { {GroupName: "group1"}, }, }, - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingWorkloads: []dummyWorkload{ @@ -1128,10 +1360,12 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { LastTransitionTime: &fakeTime_60s, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster6", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster6", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + }, }, }, } @@ -1154,20 +1388,23 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { actualRolloutStrategy, actualRolloutResult, _ := rolloutHandler.GetRolloutCluster(test.rolloutStrategy, existingRolloutClusters) if !reflect.DeepEqual(actualRolloutStrategy.ProgressivePerGroup, test.expectRolloutStrategy.ProgressivePerGroup) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect strategy : %v, actual : %v", test.name, test.expectRolloutStrategy, actualRolloutStrategy) - return + t.Errorf("Case: %v, Failed to run NewRolloutHandler.\nExpect strategy: %+v\nActual strategy: %+v", test.name, test.expectRolloutStrategy, actualRolloutStrategy) } - if !reflect.DeepEqual(actualRolloutResult.ClustersToRollout, test.expectRolloutClusters) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect rollout clusters: %v, actual : %v", test.name, test.expectRolloutClusters, actualRolloutResult.ClustersToRollout) - return + if !reflect.DeepEqual(actualRolloutResult.ClustersToRollout, test.expectRolloutResult.ClustersToRollout) { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect rollout clusters: %+v\nActual rollout clusters: %+v", + test.name, test.expectRolloutResult.ClustersToRollout, actualRolloutResult.ClustersToRollout) } - if !reflect.DeepEqual(actualRolloutResult.ClustersTimeOut, test.expectTimeOutClusters) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect timeout clusters: %v, actual : %v", test.name, test.expectTimeOutClusters, actualRolloutResult.ClustersTimeOut) - return + if !reflect.DeepEqual(actualRolloutResult.ClustersTimeOut, test.expectRolloutResult.ClustersTimeOut) { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect timeout clusters: %+v\nActual timeout clusters: %+v", + test.name, test.expectRolloutResult.ClustersTimeOut, actualRolloutResult.ClustersTimeOut) } - if !reflect.DeepEqual(actualRolloutResult.ClustersRemoved, test.expectRemovedClusters) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect removed clusters: %v, actual : %v", test.name, test.expectRemovedClusters, actualRolloutResult.ClustersRemoved) - return + if !reflect.DeepEqual(actualRolloutResult.ClustersRemoved, test.expectRolloutResult.ClustersRemoved) { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect removed clusters: %+v\nActual removed clusters: %+v", + test.name, test.expectRolloutResult.ClustersRemoved, actualRolloutResult.ClustersRemoved) + } + if actualRolloutResult.MaxFailureBreach != test.expectRolloutResult.MaxFailureBreach { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect failure breach: %+v\nActual failure breach: %+v", + test.name, test.expectRolloutResult.MaxFailureBreach, actualRolloutResult.MaxFailureBreach) } } } @@ -1176,7 +1413,7 @@ func TestGetRolloutCluster_ClusterAdded(t *testing.T) { tests := []testCase{ { name: "test rollout all with timeout 90s and cluster added", - rolloutStrategy: RolloutStrategy{Type: All, All: &RolloutAll{Timeout: Timeout{"90s"}}}, + rolloutStrategy: RolloutStrategy{Type: All, All: &RolloutAll{RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}}}, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ {GroupName: "group1", GroupIndex: 0}: sets.New[string]("cluster1", "cluster2", "cluster7"), {GroupName: "", GroupIndex: 1}: sets.New[string]("cluster3", "cluster4", "cluster5", "cluster6"), @@ -1192,7 +1429,7 @@ func TestGetRolloutCluster_ClusterAdded(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: missing, + State: failed, LastTransitionTime: &fakeTime_120s, }, { @@ -1204,7 +1441,7 @@ func TestGetRolloutCluster_ClusterAdded(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, ClusterName: "cluster4", - State: missing, + State: failed, LastTransitionTime: &fakeTime_60s, }, { @@ -1214,16 +1451,18 @@ func TestGetRolloutCluster_ClusterAdded(t *testing.T) { LastTransitionTime: &fakeTime_60s, }, }, - expectRolloutStrategy: &RolloutStrategy{Type: All, All: &RolloutAll{Timeout: Timeout{"90s"}}}, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: Failed, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, - {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, - {ClusterName: "cluster6", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster7", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, - }, - expectTimeOutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + expectRolloutStrategy: &RolloutStrategy{Type: All, All: &RolloutAll{RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}}}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: Failed, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + {ClusterName: "cluster6", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster7", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + }, + ClustersTimeOut: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + }, }, }, { @@ -1237,7 +1476,7 @@ func TestGetRolloutCluster_ClusterAdded(t *testing.T) { }, }, MaxConcurrency: intstr.FromInt(3), - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -1255,7 +1494,7 @@ func TestGetRolloutCluster_ClusterAdded(t *testing.T) { }, }, MaxConcurrency: intstr.FromInt(3), - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingWorkloads: []dummyWorkload{ @@ -1290,11 +1529,12 @@ func TestGetRolloutCluster_ClusterAdded(t *testing.T) { LastTransitionTime: &fakeTime_60s, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - //{ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, - {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, - {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 2}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, - {ClusterName: "cluster7", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 2}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + {ClusterName: "cluster7", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + }, }, }, { @@ -1302,7 +1542,7 @@ func TestGetRolloutCluster_ClusterAdded(t *testing.T) { rolloutStrategy: RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -1313,7 +1553,7 @@ func TestGetRolloutCluster_ClusterAdded(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingWorkloads: []dummyWorkload{ @@ -1342,10 +1582,12 @@ func TestGetRolloutCluster_ClusterAdded(t *testing.T) { LastTransitionTime: &fakeTime_60s, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, - {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: ToApply}, + }, }, }, } @@ -1367,21 +1609,24 @@ func TestGetRolloutCluster_ClusterAdded(t *testing.T) { actualRolloutStrategy, actualRolloutResult, _ := rolloutHandler.GetRolloutCluster(test.rolloutStrategy, existingRolloutClusters) - if !reflect.DeepEqual(actualRolloutStrategy.Type, test.expectRolloutStrategy.Type) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect strategy : %v, actual : %v", test.name, test.expectRolloutStrategy, actualRolloutStrategy) - return + if !reflect.DeepEqual(actualRolloutStrategy.All, test.expectRolloutStrategy.All) { + t.Errorf("Case: %v, Failed to run NewRolloutHandler.\nExpect strategy: %+v\nActual strategy: %+v", test.name, test.expectRolloutStrategy, actualRolloutStrategy) } - if !reflect.DeepEqual(actualRolloutResult.ClustersToRollout, test.expectRolloutClusters) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect rollout clusters: %v, actual : %v", test.name, test.expectRolloutClusters, actualRolloutResult.ClustersToRollout) - return + if !reflect.DeepEqual(actualRolloutResult.ClustersToRollout, test.expectRolloutResult.ClustersToRollout) { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect rollout clusters: %+v\nActual rollout clusters: %+v", + test.name, test.expectRolloutResult.ClustersToRollout, actualRolloutResult.ClustersToRollout) } - if !reflect.DeepEqual(actualRolloutResult.ClustersTimeOut, test.expectTimeOutClusters) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect timeout clusters: %v, actual : %v", test.name, test.expectTimeOutClusters, actualRolloutResult.ClustersTimeOut) - return + if !reflect.DeepEqual(actualRolloutResult.ClustersTimeOut, test.expectRolloutResult.ClustersTimeOut) { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect timeout clusters: %+v\nActual timeout clusters: %+v", + test.name, test.expectRolloutResult.ClustersTimeOut, actualRolloutResult.ClustersTimeOut) } - if !reflect.DeepEqual(actualRolloutResult.ClustersRemoved, test.expectRemovedClusters) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect removed clusters: %v, actual : %v", test.name, test.expectRemovedClusters, actualRolloutResult.ClustersRemoved) - return + if !reflect.DeepEqual(actualRolloutResult.ClustersRemoved, test.expectRolloutResult.ClustersRemoved) { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect removed clusters: %+v\nActual removed clusters: %+v", + test.name, test.expectRolloutResult.ClustersRemoved, actualRolloutResult.ClustersRemoved) + } + if actualRolloutResult.MaxFailureBreach != test.expectRolloutResult.MaxFailureBreach { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect failure breach: %+v\nActual failure breach: %+v", + test.name, test.expectRolloutResult.MaxFailureBreach, actualRolloutResult.MaxFailureBreach) } } } @@ -1390,7 +1635,7 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { tests := []testCase{ { name: "test rollout all with timeout 90s and clusters removed", - rolloutStrategy: RolloutStrategy{Type: All, All: &RolloutAll{Timeout: Timeout{"90s"}}}, + rolloutStrategy: RolloutStrategy{Type: All, All: &RolloutAll{RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}}}, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ {GroupName: "group1", GroupIndex: 0}: sets.New[string]("cluster1"), {GroupName: "", GroupIndex: 1}: sets.New[string]("cluster3", "cluster5"), @@ -1406,7 +1651,7 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: missing, + State: failed, LastTransitionTime: &fakeTime_120s, }, { @@ -1418,7 +1663,7 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, ClusterName: "cluster4", - State: missing, + State: failed, LastTransitionTime: &fakeTime_60s, }, { @@ -1428,24 +1673,26 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { LastTransitionTime: &fakeTime_60s, }, }, - expectRolloutStrategy: &RolloutStrategy{Type: All, All: &RolloutAll{Timeout: Timeout{"90s"}}}, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, - }, - expectTimeOutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, - }, - expectRemovedClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_120s}, - {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: Failed, LastTransitionTime: &fakeTime_60s}, + expectRolloutStrategy: &RolloutStrategy{Type: All, All: &RolloutAll{RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}}}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + }, + ClustersTimeOut: []ClusterRolloutStatus{ + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + }, + ClustersRemoved: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_120s}, + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: Failed, LastTransitionTime: &fakeTime_60s}, + }, }, }, { - name: "test progressive rollout with timeout 90s and cluster removed", + name: "test progressive rollout with timeout 90s, MaxFailures 100%, and cluster removed", rolloutStrategy: RolloutStrategy{ Type: Progressive, Progressive: &RolloutProgressive{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s", MaxFailures: intstr.FromString("100%")}, MaxConcurrency: intstr.FromInt(2), }, }, @@ -1457,7 +1704,7 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: Progressive, Progressive: &RolloutProgressive{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, MaxConcurrency: intstr.FromInt(2), }, }, @@ -1471,7 +1718,7 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: missing, + State: failed, LastTransitionTime: &fakeTime_120s, }, { @@ -1481,19 +1728,21 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { LastTransitionTime: &fakeTime_60s, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, - {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, - }, - expectTimeOutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, - }, - expectRemovedClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Succeeded, LastTransitionTime: &fakeTime_60s}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupName: "", GroupIndex: 1}, Status: ToApply}, + }, + ClustersTimeOut: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + }, + ClustersRemoved: []ClusterRolloutStatus{ + {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Succeeded, LastTransitionTime: &fakeTime_60s}, + }, }, }, { - name: "test progressive rollout with mandatroyDecisionGroup, timeout 90s and cluster removed from mandatroyDecisionGroup", + name: "test progressive rollout with mandatoryDecisionGroup, timeout 90s and cluster removed from mandatoryDecisionGroup", rolloutStrategy: RolloutStrategy{ Type: Progressive, Progressive: &RolloutProgressive{ @@ -1502,7 +1751,7 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { {GroupName: "group1"}, }, }, - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, MaxConcurrency: intstr.FromInt(2), }, }, @@ -1519,7 +1768,7 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { {GroupName: "group1"}, }, }, - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, MaxConcurrency: intstr.FromInt(2), }, }, @@ -1533,7 +1782,7 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: missing, + State: failed, LastTransitionTime: &fakeTime_120s, }, { @@ -1543,12 +1792,14 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { LastTransitionTime: &fakeTime_60s, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, - }, - expectRemovedClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_120s}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + }, + ClustersRemoved: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_120s}, + }, }, }, { @@ -1556,7 +1807,7 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { rolloutStrategy: RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -1567,7 +1818,7 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingWorkloads: []dummyWorkload{ @@ -1580,7 +1831,7 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: missing, + State: failed, LastTransitionTime: &fakeTime_60s, }, { @@ -1590,11 +1841,13 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { LastTransitionTime: &fakeTime_60s, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, - }, - expectRemovedClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_60s}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Progressing, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime30s}, + }, + ClustersRemoved: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_60s}, + }, }, }, { @@ -1602,7 +1855,7 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { rolloutStrategy: RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -1613,7 +1866,7 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingWorkloads: []dummyWorkload{ @@ -1626,7 +1879,7 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: missing, + State: failed, LastTransitionTime: &fakeTime_60s, }, { @@ -1636,20 +1889,22 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { LastTransitionTime: &fakeTime_120s, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster6", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, - }, - expectTimeOutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, - }, - expectRemovedClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_60s}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster6", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + }, + ClustersTimeOut: []ClusterRolloutStatus{ + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + }, + ClustersRemoved: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_60s}, + }, }, }, { - name: "test ProgressivePerGroup rollout with mandatroyDecisionGroup, timeout 90s and cluster removed from mandatroyDecisionGroup", + name: "test ProgressivePerGroup rollout with mandatoryDecisionGroup, timeout 90s and cluster removed from mandatoryDecisionGroup", rolloutStrategy: RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ @@ -1658,7 +1913,7 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { {GroupName: "group1"}, }, }, - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -1675,7 +1930,7 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { {GroupName: "group1"}, }, }, - Timeout: Timeout{"90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, }, }, existingWorkloads: []dummyWorkload{ @@ -1688,7 +1943,7 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { { ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, ClusterName: "cluster2", - State: missing, + State: failed, LastTransitionTime: &fakeTime_120s, }, { @@ -1698,13 +1953,15 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { LastTransitionTime: &fakeTime_60s, }, }, - expectRolloutClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, - {ClusterName: "cluster6", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, - }, - expectRemovedClusters: []ClusterRolloutStatus{ - {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_120s}, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster6", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + }, + ClustersRemoved: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_120s}, + }, }, }, } @@ -1727,20 +1984,23 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { actualRolloutStrategy, actualRolloutResult, _ := rolloutHandler.GetRolloutCluster(test.rolloutStrategy, existingRolloutClusters) if !reflect.DeepEqual(actualRolloutStrategy.Type, test.expectRolloutStrategy.Type) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect strategy : %v, actual : %v", test.name, test.expectRolloutStrategy, actualRolloutStrategy) - return + t.Errorf("Case: %v, Failed to run NewRolloutHandler.\nExpect strategy: %+v\nActual strategy: %+v", test.name, test.expectRolloutStrategy, actualRolloutStrategy) } - if !reflect.DeepEqual(actualRolloutResult.ClustersToRollout, test.expectRolloutClusters) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect rollout clusters: %v, actual : %v", test.name, test.expectRolloutClusters, actualRolloutResult.ClustersToRollout) - return + if !reflect.DeepEqual(actualRolloutResult.ClustersToRollout, test.expectRolloutResult.ClustersToRollout) { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect rollout clusters: %+v\nActual rollout clusters: %+v", + test.name, test.expectRolloutResult.ClustersToRollout, actualRolloutResult.ClustersToRollout) } - if !reflect.DeepEqual(actualRolloutResult.ClustersTimeOut, test.expectTimeOutClusters) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect timeout clusters: %v, actual : %v", test.name, test.expectTimeOutClusters, actualRolloutResult.ClustersTimeOut) - return + if !reflect.DeepEqual(actualRolloutResult.ClustersTimeOut, test.expectRolloutResult.ClustersTimeOut) { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect timeout clusters: %+v\nActual timeout clusters: %+v", + test.name, test.expectRolloutResult.ClustersTimeOut, actualRolloutResult.ClustersTimeOut) } - if !reflect.DeepEqual(actualRolloutResult.ClustersRemoved, test.expectRemovedClusters) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect removed clusters: %v, actual : %v", test.name, test.expectRemovedClusters, actualRolloutResult.ClustersRemoved) - return + if !reflect.DeepEqual(actualRolloutResult.ClustersRemoved, test.expectRolloutResult.ClustersRemoved) { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect removed clusters: %+v\nActual removed clusters: %+v", + test.name, test.expectRolloutResult.ClustersRemoved, actualRolloutResult.ClustersRemoved) + } + if actualRolloutResult.MaxFailureBreach != test.expectRolloutResult.MaxFailureBreach { + t.Errorf("Case: %v: Failed to run NewRolloutHandler.\nExpect failure breach: %+v\nActual failure breach: %+v", + test.name, test.expectRolloutResult.MaxFailureBreach, actualRolloutResult.MaxFailureBreach) } } @@ -1749,6 +2009,7 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { func TestDetermineRolloutStatus(t *testing.T) { testCases := []struct { name string + minSuccessTime time.Duration timeout time.Duration clusterStatus ClusterRolloutStatus expectRolloutClusters []ClusterRolloutStatus @@ -1770,6 +2031,12 @@ func TestDetermineRolloutStatus(t *testing.T) { clusterStatus: ClusterRolloutStatus{ClusterName: "cluster1", Status: Succeeded}, timeout: time.Minute, }, + { + name: "Succeeded status within the minSuccessTime", + clusterStatus: ClusterRolloutStatus{ClusterName: "cluster1", Status: Succeeded}, + minSuccessTime: time.Minute, + expectRolloutClusters: []ClusterRolloutStatus{{ClusterName: "cluster1", Status: Succeeded}}, + }, { name: "TimeOut status", clusterStatus: ClusterRolloutStatus{ClusterName: "cluster1", Status: TimeOut}, @@ -1782,13 +2049,13 @@ func TestDetermineRolloutStatus(t *testing.T) { expectRolloutClusters: []ClusterRolloutStatus{{ClusterName: "cluster1", Status: Progressing, LastTransitionTime: &fakeTime_30s, TimeOutTime: &fakeTime30s}}, }, { - name: "Failed status out the timeout duration", + name: "Failed status outside of the timeout duration", clusterStatus: ClusterRolloutStatus{ClusterName: "cluster1", Status: Failed, LastTransitionTime: &fakeTime_60s}, timeout: time.Minute, expectTimeOutClusters: []ClusterRolloutStatus{{ClusterName: "cluster1", Status: TimeOut, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime}}, }, { - name: "unknown status out the timeout duration", + name: "unknown status outside of the timeout duration", clusterStatus: ClusterRolloutStatus{ClusterName: "cluster1", Status: 8, LastTransitionTime: &fakeTime_60s}, timeout: time.Minute, expectTimeOutClusters: []ClusterRolloutStatus{{ClusterName: "cluster1", Status: TimeOut, LastTransitionTime: &fakeTime_60s, TimeOutTime: &fakeTime}}, @@ -1804,13 +2071,13 @@ func TestDetermineRolloutStatus(t *testing.T) { RolloutClock = testingclock.NewFakeClock(fakeTime.Time) for _, tc := range testCases { var rolloutClusters, timeoutClusters []ClusterRolloutStatus - rolloutClusters, timeoutClusters = determineRolloutStatus(tc.clusterStatus, tc.timeout, rolloutClusters, timeoutClusters) + rolloutClusters, timeoutClusters = determineRolloutStatus(tc.clusterStatus, tc.minSuccessTime, tc.timeout, rolloutClusters, timeoutClusters) if !reflect.DeepEqual(rolloutClusters, tc.expectRolloutClusters) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect rollout clusters: %v, actual : %v", tc.name, tc.expectRolloutClusters, rolloutClusters) + t.Errorf("Case: %v Failed to run NewRolloutHandler.\nExpect rollout clusters: %+v\nActual rollout clusters: %+v", tc.name, tc.expectRolloutClusters, rolloutClusters) return } if !reflect.DeepEqual(timeoutClusters, tc.expectTimeOutClusters) { - t.Errorf("Case: %v, Failed to run NewRolloutHandler. Expect timeout clusters: %v, actual : %v", tc.name, tc.expectTimeOutClusters, timeoutClusters) + t.Errorf("Case: %v\nFailed to run NewRolloutHandler. Expect timeout clusters: %+v\nActual rollout clusters: %+v", tc.name, tc.expectTimeOutClusters, timeoutClusters) return } } @@ -1827,7 +2094,7 @@ func TestCalculateRolloutSize(t *testing.T) { }{ {"maxConcurrency type is int", intstr.FromInt(50), 50, nil}, {"maxConcurrency type is string with percentage", intstr.FromString("30%"), int(math.Ceil(0.3 * float64(total))), nil}, - {"maxConcurrency type is string without percentage", intstr.FromString("invalid"), total, fmt.Errorf("%v invalid type: string is not a percentage", intstr.FromString("invalid"))}, + {"maxConcurrency type is string without percentage", intstr.FromString("invalid"), total, fmt.Errorf("'%s' is an invalid maximum threshold value: string is not a percentage", intstr.FromString("invalid").StrVal)}, {"maxConcurrency type is int 0", intstr.FromInt(0), total, nil}, {"maxConcurrency type is int but out of range", intstr.FromInt(total + 1), total, nil}, {"maxConcurrency type is string with percentage but out of range", intstr.FromString("200%"), total, nil}, @@ -1835,7 +2102,7 @@ func TestCalculateRolloutSize(t *testing.T) { for _, test := range tests { t.Run(test.name, func(t *testing.T) { - length, err := calculateRolloutSize(test.maxConcurrency, total) + length, err := calculateRolloutSize(test.maxConcurrency, total, total) // Compare the result with the expected result if length != test.expected { diff --git a/cluster/v1alpha1/types_rolloutstrategy.go b/cluster/v1alpha1/types_rolloutstrategy.go index c9fa38155..62e96ccf5 100644 --- a/cluster/v1alpha1/types_rolloutstrategy.go +++ b/cluster/v1alpha1/types_rolloutstrategy.go @@ -1,6 +1,7 @@ package v1alpha1 import ( + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" "k8s.io/apimachinery/pkg/util/intstr" ) @@ -8,46 +9,83 @@ import ( // RolloutStrategy API used by workload applier APIs to define how the workload will be applied to the selected clusters by the Placement and DecisionStrategy. +type RolloutType string + const ( //All means apply the workload to all clusters in the decision groups at once. - All string = "All" + All RolloutType = "All" //Progressive means apply the workload to the selected clusters progressively per cluster. - Progressive string = "Progressive" + Progressive RolloutType = "Progressive" //ProgressivePerGroup means apply the workload to the selected clusters progressively per group. - ProgressivePerGroup string = "ProgressivePerGroup" + ProgressivePerGroup RolloutType = "ProgressivePerGroup" ) // Rollout strategy to apply workload to the selected clusters by Placement and DecisionStrategy. type RolloutStrategy struct { // Rollout strategy Types are All, Progressive and ProgressivePerGroup // 1) All means apply the workload to all clusters in the decision groups at once. - // 2) Progressive means apply the workload to the selected clusters progressively per cluster. The workload will not be applied to the next cluster unless one of the current applied clusters reach the successful state or timeout. - // 3) ProgressivePerGroup means apply the workload to decisionGroup clusters progressively per group. The workload will not be applied to the next decisionGroup unless all clusters in the current group reach the successful state or timeout. + // 2) Progressive means apply the workload to the selected clusters progressively per cluster. The workload will not + // be applied to the next cluster unless one of the current applied clusters reach the successful state and haven't + // breached the MaxFailures configuration. + // 3) ProgressivePerGroup means apply the workload to decisionGroup clusters progressively per group. The workload + // will not be applied to the next decisionGroup unless all clusters in the current group reach the successful + // state and haven't breached the MaxFailures configuration. // +kubebuilder:validation:Enum=All;Progressive;ProgressivePerGroup // +kubebuilder:default:=All // +optional - Type string `json:"type,omitempty"` + Type RolloutType `json:"type,omitempty"` - // All define required fields for RolloutStrategy type All + // All defines required fields for RolloutStrategy type All // +optional All *RolloutAll `json:"all,omitempty"` - // Progressive define required fields for RolloutStrategy type Progressive + // Progressive defines required fields for RolloutStrategy type Progressive // +optional Progressive *RolloutProgressive `json:"progressive,omitempty"` - // ProgressivePerGroup define required fields for RolloutStrategy type ProgressivePerGroup + // ProgressivePerGroup defines required fields for RolloutStrategy type ProgressivePerGroup // +optional ProgressivePerGroup *RolloutProgressivePerGroup `json:"progressivePerGroup,omitempty"` } // Timeout to consider while applying the workload. -type Timeout struct { - // Timeout define how long workload applier controller will wait till workload reach successful state in the cluster. - // Timeout default value is None meaning the workload applier will not proceed apply workload to other clusters if did not reach the successful state. +type RolloutConfig struct { + // MinSuccessTime is a "soak" time. In other words, the minimum amount of time the workload applier controller will + // wait from the start of each rollout before proceeding (assuming a successful state has been reached and MaxFailures + // wasn't breached). + // MinSuccessTime is only considered for rollout types Progressive and ProgressivePerGroup. + // The default value is 0 meaning the workload applier proceeds immediately after a successful state is reached. + // MinSuccessTime must be defined in [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m , 360s + // +kubebuilder:default:="0" + // +optional + MinSuccessTime metav1.Duration `json:"minSuccessTime,omitempty"` + // ProgressDeadline defines how long workload applier controller will wait for the workload to reach a successful state in the cluster. + // ProgressDeadline default value is "None", meaning the workload applier will wait for a successful state indefinitely. + // ProgressDeadline must be defined in [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m , 360s + // +kubebuilder:validation:Pattern="^(([0-9])+[h|m|s])|None$" + // +kubebuilder:default:="None" + // +optional + ProgressDeadline string `json:"progressDeadline,omitempty"` + // MaxFailures is a percentage or number of clusters in the current rollout that can fail before proceeding to the + // next rollout. + // MaxFailures is only considered for rollout types Progressive and ProgressivePerGroup. For Progressive, this is + // considered over the total number of clusters. For ProgressivePerGroup, this is considered according to the size of + // the current group. + // Default is that no failures are tolerated. + // +kubebuilder:validation:Pattern="^((100|[0-9]{1,2})%|[0-9]+)$" + // +kubebuilder:validation:XIntOrString + // +kubebuilder:default="0" + // +optional + MaxFailures intstr.IntOrString `json:"maxFailures,omitempty"` + // Timeout defines how long the workload applier controller will wait until the workload reaches a successful state in + // the cluster. + // Timeout default value is None meaning the workload applier will not proceed apply workload to other clusters if did + // not reach the successful state. // Timeout must be defined in [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m , 360s + // + // Deprecated: Use ProgressDeadline instead. // +kubebuilder:validation:Pattern="^(([0-9])+[h|m|s])|None$" - // +kubebuilder:default:=None + // +kubebuilder:default:="None" // +optional Timeout string `json:"timeout,omitempty"` } @@ -75,29 +113,30 @@ type MandatoryDecisionGroups struct { // RolloutAll is a RolloutStrategy Type type RolloutAll struct { // +optional - Timeout `json:",inline"` + RolloutConfig `json:",inline"` } // RolloutProgressivePerGroup is a RolloutStrategy Type type RolloutProgressivePerGroup struct { // +optional - MandatoryDecisionGroups `json:",inline"` + RolloutConfig `json:",inline"` // +optional - Timeout `json:",inline"` + MandatoryDecisionGroups `json:",inline"` } // RolloutProgressive is a RolloutStrategy Type type RolloutProgressive struct { + // +optional + RolloutConfig `json:",inline"` + // +optional MandatoryDecisionGroups `json:",inline"` - // MaxConcurrency is the max number of clusters to deploy workload concurrently. The default value for MaxConcurrency is determined from the clustersPerDecisionGroup defined in the placement->DecisionStrategy. + // MaxConcurrency is the max number of clusters to deploy workload concurrently. The default value for MaxConcurrency + // is determined from the clustersPerDecisionGroup defined in the placement->DecisionStrategy. // +kubebuilder:validation:Pattern="^((100|[0-9]{1,2})%|[0-9]+)$" // +kubebuilder:validation:XIntOrString // +optional MaxConcurrency intstr.IntOrString `json:"maxConcurrency,omitempty"` - - // +optional - Timeout `json:",inline"` } diff --git a/cluster/v1alpha1/zz_generated.deepcopy.go b/cluster/v1alpha1/zz_generated.deepcopy.go index 2be1d2833..5223c00b1 100644 --- a/cluster/v1alpha1/zz_generated.deepcopy.go +++ b/cluster/v1alpha1/zz_generated.deepcopy.go @@ -249,7 +249,7 @@ func (in *MandatoryDecisionGroups) DeepCopy() *MandatoryDecisionGroups { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RolloutAll) DeepCopyInto(out *RolloutAll) { *out = *in - out.Timeout = in.Timeout + out.RolloutConfig = in.RolloutConfig } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RolloutAll. @@ -262,12 +262,29 @@ func (in *RolloutAll) DeepCopy() *RolloutAll { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *RolloutConfig) DeepCopyInto(out *RolloutConfig) { + *out = *in + out.MinSuccessTime = in.MinSuccessTime + out.MaxFailures = in.MaxFailures +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RolloutConfig. +func (in *RolloutConfig) DeepCopy() *RolloutConfig { + if in == nil { + return nil + } + out := new(RolloutConfig) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RolloutProgressive) DeepCopyInto(out *RolloutProgressive) { *out = *in + out.RolloutConfig = in.RolloutConfig in.MandatoryDecisionGroups.DeepCopyInto(&out.MandatoryDecisionGroups) out.MaxConcurrency = in.MaxConcurrency - out.Timeout = in.Timeout } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RolloutProgressive. @@ -283,8 +300,8 @@ func (in *RolloutProgressive) DeepCopy() *RolloutProgressive { // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *RolloutProgressivePerGroup) DeepCopyInto(out *RolloutProgressivePerGroup) { *out = *in + out.RolloutConfig = in.RolloutConfig in.MandatoryDecisionGroups.DeepCopyInto(&out.MandatoryDecisionGroups) - out.Timeout = in.Timeout } // DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new RolloutProgressivePerGroup. @@ -362,18 +379,3 @@ func (in *RolloutStrategy) DeepCopy() *RolloutStrategy { in.DeepCopyInto(out) return out } - -// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. -func (in *Timeout) DeepCopyInto(out *Timeout) { - *out = *in -} - -// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new Timeout. -func (in *Timeout) DeepCopy() *Timeout { - if in == nil { - return nil - } - out := new(Timeout) - in.DeepCopyInto(out) - return out -} diff --git a/cluster/v1alpha1/zz_generated.swagger_doc_generated.go b/cluster/v1alpha1/zz_generated.swagger_doc_generated.go index 4cadc7a2e..e983fb5a1 100644 --- a/cluster/v1alpha1/zz_generated.swagger_doc_generated.go +++ b/cluster/v1alpha1/zz_generated.swagger_doc_generated.go @@ -105,6 +105,18 @@ func (RolloutAll) SwaggerDoc() map[string]string { return map_RolloutAll } +var map_RolloutConfig = map[string]string{ + "": "Timeout to consider while applying the workload.", + "minSuccessTime": "MinSuccessTime is a \"soak\" time. In other words, the minimum amount of time the workload applier controller will wait from the start of each rollout before proceeding (assuming a successful state has been reached and MaxFailures wasn't breached). MinSuccessTime is only considered for rollout types Progressive and ProgressivePerGroup. The default value is 0 meaning the workload applier proceeds immediately after a successful state is reached. MinSuccessTime must be defined in [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m , 360s", + "progressDeadline": "ProgressDeadline defines how long workload applier controller will wait for the workload to reach a successful state in the cluster. ProgressDeadline default value is \"None\", meaning the workload applier will wait for a successful state indefinitely. ProgressDeadline must be defined in [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m , 360s", + "maxFailures": "MaxFailures is a percentage or number of clusters in the current rollout that can fail before proceeding to the next rollout. MaxFailures is only considered for rollout types Progressive and ProgressivePerGroup. For Progressive, this is considered over the total number of clusters. For ProgressivePerGroup, this is considered according to the size of the current group. Default is that no failures are tolerated.", + "timeout": "Timeout defines how long the workload applier controller will wait until the workload reaches a successful state in the cluster. Timeout default value is None meaning the workload applier will not proceed apply workload to other clusters if did not reach the successful state. Timeout must be defined in [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m , 360s\n\nDeprecated: Use ProgressDeadline instead.", +} + +func (RolloutConfig) SwaggerDoc() map[string]string { + return map_RolloutConfig +} + var map_RolloutProgressive = map[string]string{ "": "RolloutProgressive is a RolloutStrategy Type", "maxConcurrency": "MaxConcurrency is the max number of clusters to deploy workload concurrently. The default value for MaxConcurrency is determined from the clustersPerDecisionGroup defined in the placement->DecisionStrategy.", @@ -124,23 +136,14 @@ func (RolloutProgressivePerGroup) SwaggerDoc() map[string]string { var map_RolloutStrategy = map[string]string{ "": "Rollout strategy to apply workload to the selected clusters by Placement and DecisionStrategy.", - "type": "Rollout strategy Types are All, Progressive and ProgressivePerGroup 1) All means apply the workload to all clusters in the decision groups at once. 2) Progressive means apply the workload to the selected clusters progressively per cluster. The workload will not be applied to the next cluster unless one of the current applied clusters reach the successful state or timeout. 3) ProgressivePerGroup means apply the workload to decisionGroup clusters progressively per group. The workload will not be applied to the next decisionGroup unless all clusters in the current group reach the successful state or timeout.", - "all": "All define required fields for RolloutStrategy type All", - "progressive": "Progressive define required fields for RolloutStrategy type Progressive", - "progressivePerGroup": "ProgressivePerGroup define required fields for RolloutStrategy type ProgressivePerGroup", + "type": "Rollout strategy Types are All, Progressive and ProgressivePerGroup 1) All means apply the workload to all clusters in the decision groups at once. 2) Progressive means apply the workload to the selected clusters progressively per cluster. The workload will not\n be applied to the next cluster unless one of the current applied clusters reach the successful state and haven't\n breached the MaxFailures configuration.\n3) ProgressivePerGroup means apply the workload to decisionGroup clusters progressively per group. The workload\n will not be applied to the next decisionGroup unless all clusters in the current group reach the successful\n state and haven't breached the MaxFailures configuration.", + "all": "All defines required fields for RolloutStrategy type All", + "progressive": "Progressive defines required fields for RolloutStrategy type Progressive", + "progressivePerGroup": "ProgressivePerGroup defines required fields for RolloutStrategy type ProgressivePerGroup", } func (RolloutStrategy) SwaggerDoc() map[string]string { return map_RolloutStrategy } -var map_Timeout = map[string]string{ - "": "Timeout to consider while applying the workload.", - "timeout": "Timeout define how long workload applier controller will wait till workload reach successful state in the cluster. Timeout default value is None meaning the workload applier will not proceed apply workload to other clusters if did not reach the successful state. Timeout must be defined in [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m , 360s", -} - -func (Timeout) SwaggerDoc() map[string]string { - return map_Timeout -} - // AUTO-GENERATED FUNCTIONS END HERE diff --git a/work/v1alpha1/0000_00_work.open-cluster-management.io_manifestworkreplicasets.crd.yaml b/work/v1alpha1/0000_00_work.open-cluster-management.io_manifestworkreplicasets.crd.yaml index ecd82e555..b437dcf6e 100644 --- a/work/v1alpha1/0000_00_work.open-cluster-management.io_manifestworkreplicasets.crd.yaml +++ b/work/v1alpha1/0000_00_work.open-cluster-management.io_manifestworkreplicasets.crd.yaml @@ -328,29 +328,70 @@ spec: rolloutStrategy: default: all: - timeout: None + progressDeadline: None type: All description: Rollout strategy to apply workload to the selected clusters by Placement and DecisionStrategy. properties: all: - description: All define required fields for RolloutStrategy + description: All defines required fields for RolloutStrategy type All properties: + maxFailures: + anyOf: + - type: integer + - type: string + default: "0" + description: MaxFailures is a percentage or number of + clusters in the current rollout that can fail before + proceeding to the next rollout. MaxFailures is only + considered for rollout types Progressive and ProgressivePerGroup. + For Progressive, this is considered over the total + number of clusters. For ProgressivePerGroup, this + is considered according to the size of the current + group. Default is that no failures are tolerated. + pattern: ^((100|[0-9]{1,2})%|[0-9]+)$ + x-kubernetes-int-or-string: true + minSuccessTime: + default: "0" + description: MinSuccessTime is a "soak" time. In other + words, the minimum amount of time the workload applier + controller will wait from the start of each rollout + before proceeding (assuming a successful state has + been reached and MaxFailures wasn't breached). MinSuccessTime + is only considered for rollout types Progressive and + ProgressivePerGroup. The default value is 0 meaning + the workload applier proceeds immediately after a + successful state is reached. MinSuccessTime must be + defined in [0-9h]|[0-9m]|[0-9s] format examples; 2h + , 90m , 360s + type: string + progressDeadline: + default: None + description: ProgressDeadline defines how long workload + applier controller will wait for the workload to reach + a successful state in the cluster. ProgressDeadline + default value is "None", meaning the workload applier + will wait for a successful state indefinitely. ProgressDeadline + must be defined in [0-9h]|[0-9m]|[0-9s] format examples; + 2h , 90m , 360s + pattern: ^(([0-9])+[h|m|s])|None$ + type: string timeout: default: None - description: Timeout define how long workload applier - controller will wait till workload reach successful - state in the cluster. Timeout default value is None - meaning the workload applier will not proceed apply - workload to other clusters if did not reach the successful - state. Timeout must be defined in [0-9h]|[0-9m]|[0-9s] - format examples; 2h , 90m , 360s + description: "Timeout defines how long the workload + applier controller will wait until the workload reaches + a successful state in the cluster. Timeout default + value is None meaning the workload applier will not + proceed apply workload to other clusters if did not + reach the successful state. Timeout must be defined + in [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m + , 360s \n Deprecated: Use ProgressDeadline instead." pattern: ^(([0-9])+[h|m|s])|None$ type: string type: object progressive: - description: Progressive define required fields for RolloutStrategy + description: Progressive defines required fields for RolloutStrategy type Progressive properties: mandatoryDecisionGroups: @@ -387,20 +428,61 @@ spec: defined in the placement->DecisionStrategy. pattern: ^((100|[0-9]{1,2})%|[0-9]+)$ x-kubernetes-int-or-string: true + maxFailures: + anyOf: + - type: integer + - type: string + default: "0" + description: MaxFailures is a percentage or number of + clusters in the current rollout that can fail before + proceeding to the next rollout. MaxFailures is only + considered for rollout types Progressive and ProgressivePerGroup. + For Progressive, this is considered over the total + number of clusters. For ProgressivePerGroup, this + is considered according to the size of the current + group. Default is that no failures are tolerated. + pattern: ^((100|[0-9]{1,2})%|[0-9]+)$ + x-kubernetes-int-or-string: true + minSuccessTime: + default: "0" + description: MinSuccessTime is a "soak" time. In other + words, the minimum amount of time the workload applier + controller will wait from the start of each rollout + before proceeding (assuming a successful state has + been reached and MaxFailures wasn't breached). MinSuccessTime + is only considered for rollout types Progressive and + ProgressivePerGroup. The default value is 0 meaning + the workload applier proceeds immediately after a + successful state is reached. MinSuccessTime must be + defined in [0-9h]|[0-9m]|[0-9s] format examples; 2h + , 90m , 360s + type: string + progressDeadline: + default: None + description: ProgressDeadline defines how long workload + applier controller will wait for the workload to reach + a successful state in the cluster. ProgressDeadline + default value is "None", meaning the workload applier + will wait for a successful state indefinitely. ProgressDeadline + must be defined in [0-9h]|[0-9m]|[0-9s] format examples; + 2h , 90m , 360s + pattern: ^(([0-9])+[h|m|s])|None$ + type: string timeout: default: None - description: Timeout define how long workload applier - controller will wait till workload reach successful - state in the cluster. Timeout default value is None - meaning the workload applier will not proceed apply - workload to other clusters if did not reach the successful - state. Timeout must be defined in [0-9h]|[0-9m]|[0-9s] - format examples; 2h , 90m , 360s + description: "Timeout defines how long the workload + applier controller will wait until the workload reaches + a successful state in the cluster. Timeout default + value is None meaning the workload applier will not + proceed apply workload to other clusters if did not + reach the successful state. Timeout must be defined + in [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m + , 360s \n Deprecated: Use ProgressDeadline instead." pattern: ^(([0-9])+[h|m|s])|None$ type: string type: object progressivePerGroup: - description: ProgressivePerGroup define required fields + description: ProgressivePerGroup defines required fields for RolloutStrategy type ProgressivePerGroup properties: mandatoryDecisionGroups: @@ -427,15 +509,56 @@ spec: type: string type: object type: array + maxFailures: + anyOf: + - type: integer + - type: string + default: "0" + description: MaxFailures is a percentage or number of + clusters in the current rollout that can fail before + proceeding to the next rollout. MaxFailures is only + considered for rollout types Progressive and ProgressivePerGroup. + For Progressive, this is considered over the total + number of clusters. For ProgressivePerGroup, this + is considered according to the size of the current + group. Default is that no failures are tolerated. + pattern: ^((100|[0-9]{1,2})%|[0-9]+)$ + x-kubernetes-int-or-string: true + minSuccessTime: + default: "0" + description: MinSuccessTime is a "soak" time. In other + words, the minimum amount of time the workload applier + controller will wait from the start of each rollout + before proceeding (assuming a successful state has + been reached and MaxFailures wasn't breached). MinSuccessTime + is only considered for rollout types Progressive and + ProgressivePerGroup. The default value is 0 meaning + the workload applier proceeds immediately after a + successful state is reached. MinSuccessTime must be + defined in [0-9h]|[0-9m]|[0-9s] format examples; 2h + , 90m , 360s + type: string + progressDeadline: + default: None + description: ProgressDeadline defines how long workload + applier controller will wait for the workload to reach + a successful state in the cluster. ProgressDeadline + default value is "None", meaning the workload applier + will wait for a successful state indefinitely. ProgressDeadline + must be defined in [0-9h]|[0-9m]|[0-9s] format examples; + 2h , 90m , 360s + pattern: ^(([0-9])+[h|m|s])|None$ + type: string timeout: default: None - description: Timeout define how long workload applier - controller will wait till workload reach successful - state in the cluster. Timeout default value is None - meaning the workload applier will not proceed apply - workload to other clusters if did not reach the successful - state. Timeout must be defined in [0-9h]|[0-9m]|[0-9s] - format examples; 2h , 90m , 360s + description: "Timeout defines how long the workload + applier controller will wait until the workload reaches + a successful state in the cluster. Timeout default + value is None meaning the workload applier will not + proceed apply workload to other clusters if did not + reach the successful state. Timeout must be defined + in [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m + , 360s \n Deprecated: Use ProgressDeadline instead." pattern: ^(([0-9])+[h|m|s])|None$ type: string type: object @@ -447,11 +570,12 @@ spec: means apply the workload to the selected clusters progressively per cluster. The workload will not be applied to the next cluster unless one of the current applied clusters reach - the successful state or timeout. 3) ProgressivePerGroup - means apply the workload to decisionGroup clusters progressively - per group. The workload will not be applied to the next - decisionGroup unless all clusters in the current group - reach the successful state or timeout. + the successful state and haven't breached the MaxFailures + configuration. 3) ProgressivePerGroup means apply the + workload to decisionGroup clusters progressively per group. + The workload will not be applied to the next decisionGroup + unless all clusters in the current group reach the successful + state and haven't breached the MaxFailures configuration. enum: - All - Progressive diff --git a/work/v1alpha1/types_manifestworkreplicaset.go b/work/v1alpha1/types_manifestworkreplicaset.go index b68e00897..7c693d318 100644 --- a/work/v1alpha1/types_manifestworkreplicaset.go +++ b/work/v1alpha1/types_manifestworkreplicaset.go @@ -92,7 +92,7 @@ type LocalPlacementReference struct { Name string `json:"name"` // +optional - // +kubebuilder:default={type: All, all: {timeout: None}} + // +kubebuilder:default={type: All, all: {progressDeadline: None}} RolloutStrategy cluster.RolloutStrategy `json:"rolloutStrategy"` } From 93d5292de2f832fd124c843fa1b3af19da80eab1 Mon Sep 17 00:00:00 2001 From: melserngawy Date: Fri, 3 Nov 2023 16:45:59 -0400 Subject: [PATCH 2/3] Add more unit test for Rollout ProgressivePerGroup case Signed-off-by: melserngawy --- cluster/v1alpha1/helpers.go | 2 +- cluster/v1alpha1/helpers_test.go | 152 +++++++++++++++++++++++++++++++ 2 files changed, 153 insertions(+), 1 deletion(-) diff --git a/cluster/v1alpha1/helpers.go b/cluster/v1alpha1/helpers.go index ae142ebe2..6e1c17928 100644 --- a/cluster/v1alpha1/helpers.go +++ b/cluster/v1alpha1/helpers.go @@ -418,7 +418,7 @@ func progressivePerGroup( // As it is perGroup Return if there are clusters to rollOut, // or there was a breach of the MaxFailure configuration - if len(rolloutClusters) > 0 || failureBreach { + if len(rolloutClusters) > maxGroupFailures || failureBreach { return RolloutResult{ ClustersToRollout: rolloutClusters, ClustersTimeOut: timeoutClusters, diff --git a/cluster/v1alpha1/helpers_test.go b/cluster/v1alpha1/helpers_test.go index 9241cb9e9..91865e6f9 100644 --- a/cluster/v1alpha1/helpers_test.go +++ b/cluster/v1alpha1/helpers_test.go @@ -1254,6 +1254,102 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { }, }, }, + { + name: "test progressivePerGroup rollout with timeout None, first group 1 cluster is failing and maxFailures is 1", + rolloutStrategy: RolloutStrategy{ + Type: ProgressivePerGroup, + ProgressivePerGroup: &RolloutProgressivePerGroup{ + RolloutConfig: RolloutConfig{ProgressDeadline: "None", MaxFailures: intstr.FromInt(1)}, + }, + }, + existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ + {GroupName: "group1", GroupIndex: 0}: sets.New[string]("cluster1", "cluster2", "cluster3"), + {GroupName: "", GroupIndex: 1}: sets.New[string]("cluster4", "cluster5", "cluster6"), + {GroupName: "", GroupIndex: 2}: sets.New[string]("cluster7", "cluster8", "cluster9"), + }, + clusterRolloutStatusFunc: dummyWorkloadClusterRolloutStatusFunc, + expectRolloutStrategy: &RolloutStrategy{ + Type: ProgressivePerGroup, + ProgressivePerGroup: &RolloutProgressivePerGroup{ + RolloutConfig: RolloutConfig{ProgressDeadline: "None", MaxFailures: intstr.FromInt(1)}, + }, + }, + existingWorkloads: []dummyWorkload{ + { + ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, + ClusterName: "cluster1", + State: done, + LastTransitionTime: &fakeTime_60s, + }, + { + ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, + ClusterName: "cluster2", + State: failed, + LastTransitionTime: &fakeTime_120s, + }, + { + ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, + ClusterName: "cluster3", + State: done, + LastTransitionTime: &fakeTime_60s, + }, + }, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTimeMax_120s}, + {ClusterName: "cluster4", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster5", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + {ClusterName: "cluster6", GroupKey: clusterv1beta1.GroupKey{GroupIndex: 1}, Status: ToApply}, + }, + }, + }, + { + name: "test progressivePerGroup rollout with timeout None, first group 2 clusters are failing 2 and maxFailures is 30%", + rolloutStrategy: RolloutStrategy{ + Type: ProgressivePerGroup, + ProgressivePerGroup: &RolloutProgressivePerGroup{ + RolloutConfig: RolloutConfig{ProgressDeadline: "None", MaxFailures: intstr.FromString("30%")}, + }, + }, + existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ + {GroupName: "group1", GroupIndex: 0}: sets.New[string]("cluster1", "cluster2", "cluster3"), + {GroupName: "", GroupIndex: 1}: sets.New[string]("cluster4", "cluster5", "cluster6"), + {GroupName: "", GroupIndex: 2}: sets.New[string]("cluster7", "cluster8", "cluster9"), + }, + clusterRolloutStatusFunc: dummyWorkloadClusterRolloutStatusFunc, + expectRolloutStrategy: &RolloutStrategy{ + Type: ProgressivePerGroup, + ProgressivePerGroup: &RolloutProgressivePerGroup{ + RolloutConfig: RolloutConfig{ProgressDeadline: "None", MaxFailures: intstr.FromString("30%")}, + }, + }, + existingWorkloads: []dummyWorkload{ + { + ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, + ClusterName: "cluster1", + State: done, + LastTransitionTime: &fakeTime_60s, + }, + { + ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, + ClusterName: "cluster2", + State: failed, + LastTransitionTime: &fakeTime_120s, + }, + { + ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, + ClusterName: "cluster3", + State: failed, + LastTransitionTime: &fakeTime_120s, + }, + }, + expectRolloutResult: RolloutResult{ + ClustersToRollout: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTimeMax_120s}, + {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTimeMax_120s}, + }, + }, + }, { name: "test ProgressivePerGroup rollout with mandatoryDecisionGroup failing and timeout 90s ", rolloutStrategy: RolloutStrategy{ @@ -1310,6 +1406,62 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { }, }, }, + { + name: "test progressivePerGroup rollout with mandatoryDecisionGroup failing 1 cluster, timeout 90s and with maxFailures is 1", + rolloutStrategy: RolloutStrategy{ + Type: ProgressivePerGroup, + ProgressivePerGroup: &RolloutProgressivePerGroup{ + MandatoryDecisionGroups: MandatoryDecisionGroups{ + MandatoryDecisionGroups: []MandatoryDecisionGroup{ + {GroupName: "group1"}, + }, + }, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s", MaxFailures: intstr.FromInt(1)}, + }, + }, + existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ + {GroupName: "group1", GroupIndex: 0}: sets.New[string]("cluster1", "cluster2", "cluster3"), + {GroupName: "", GroupIndex: 1}: sets.New[string]("cluster4", "cluster5", "cluster6"), + {GroupName: "", GroupIndex: 2}: sets.New[string]("cluster7", "cluster8", "cluster9"), + }, + clusterRolloutStatusFunc: dummyWorkloadClusterRolloutStatusFunc, + expectRolloutStrategy: &RolloutStrategy{ + Type: ProgressivePerGroup, + ProgressivePerGroup: &RolloutProgressivePerGroup{ + MandatoryDecisionGroups: MandatoryDecisionGroups{ + MandatoryDecisionGroups: []MandatoryDecisionGroup{ + {GroupName: "group1"}, + }, + }, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s", MaxFailures: intstr.FromInt(1)}, + }, + }, + existingWorkloads: []dummyWorkload{ + { + ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, + ClusterName: "cluster1", + State: done, + LastTransitionTime: &fakeTime_60s, + }, + { + ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, + ClusterName: "cluster2", + State: failed, + LastTransitionTime: &fakeTime_120s, + }, + { + ClusterGroup: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, + ClusterName: "cluster3", + State: done, + LastTransitionTime: &fakeTime_60s, + }, + }, + expectRolloutResult: RolloutResult{ + ClustersTimeOut: []ClusterRolloutStatus{ + {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, + }, + }, + }, { name: "test ProgressivePerGroup rollout with mandatoryDecisionGroup Succeeded and timeout 90s ", rolloutStrategy: RolloutStrategy{ From 9b88aa508ea2ec758f4f4c9a94a8e1068ade4d12 Mon Sep 17 00:00:00 2001 From: Dale Haiducek <19750917+dhaiducek@users.noreply.github.com> Date: Fri, 3 Nov 2023 20:14:33 -0400 Subject: [PATCH 3/3] Fix ProgressivePerGroup handling - MandatoryDecisionGroups should tolerate no failures - The logic for ProgressivePerGroup was incorrect Signed-off-by: Dale Haiducek <19750917+dhaiducek@users.noreply.github.com> --- ...gement.io_clustermanagementaddons.crd.yaml | 32 ++++----- cluster/v1alpha1/helpers.go | 70 +++++++++---------- cluster/v1alpha1/helpers_test.go | 43 +++++++----- cluster/v1alpha1/types_rolloutstrategy.go | 68 ++++++++++-------- .../zz_generated.swagger_doc_generated.go | 3 +- ...gement.io_manifestworkreplicasets.crd.yaml | 27 ++++--- 6 files changed, 122 insertions(+), 121 deletions(-) diff --git a/addon/v1alpha1/0000_00_addon.open-cluster-management.io_clustermanagementaddons.crd.yaml b/addon/v1alpha1/0000_00_addon.open-cluster-management.io_clustermanagementaddons.crd.yaml index 75178916b..e08074e70 100644 --- a/addon/v1alpha1/0000_00_addon.open-cluster-management.io_clustermanagementaddons.crd.yaml +++ b/addon/v1alpha1/0000_00_addon.open-cluster-management.io_clustermanagementaddons.crd.yaml @@ -160,8 +160,10 @@ spec: and ProgressivePerGroup. For Progressive, this is considered over the total number of clusters. For ProgressivePerGroup, this is considered according - to the size of the current group. Default is that - no failures are tolerated. + to the size of the current group. For both Progressive + and ProgressivePerGroup, the MaxFailures does + not apply for MandatoryDecisionGroups, which tolerate + no failures. Default is that no failures are tolerated. pattern: ^((100|[0-9]{1,2})%|[0-9]+)$ x-kubernetes-int-or-string: true minSuccessTime: @@ -255,8 +257,10 @@ spec: and ProgressivePerGroup. For Progressive, this is considered over the total number of clusters. For ProgressivePerGroup, this is considered according - to the size of the current group. Default is that - no failures are tolerated. + to the size of the current group. For both Progressive + and ProgressivePerGroup, the MaxFailures does + not apply for MandatoryDecisionGroups, which tolerate + no failures. Default is that no failures are tolerated. pattern: ^((100|[0-9]{1,2})%|[0-9]+)$ x-kubernetes-int-or-string: true minSuccessTime: @@ -339,8 +343,10 @@ spec: and ProgressivePerGroup. For Progressive, this is considered over the total number of clusters. For ProgressivePerGroup, this is considered according - to the size of the current group. Default is that - no failures are tolerated. + to the size of the current group. For both Progressive + and ProgressivePerGroup, the MaxFailures does + not apply for MandatoryDecisionGroups, which tolerate + no failures. Default is that no failures are tolerated. pattern: ^((100|[0-9]{1,2})%|[0-9]+)$ x-kubernetes-int-or-string: true minSuccessTime: @@ -385,20 +391,6 @@ spec: type: object type: default: All - description: Rollout strategy Types are All, Progressive - and ProgressivePerGroup 1) All means apply the workload - to all clusters in the decision groups at once. 2) - Progressive means apply the workload to the selected - clusters progressively per cluster. The workload will - not be applied to the next cluster unless one of the - current applied clusters reach the successful state - and haven't breached the MaxFailures configuration. - 3) ProgressivePerGroup means apply the workload to - decisionGroup clusters progressively per group. The - workload will not be applied to the next decisionGroup - unless all clusters in the current group reach the - successful state and haven't breached the MaxFailures - configuration. enum: - All - Progressive diff --git a/cluster/v1alpha1/helpers.go b/cluster/v1alpha1/helpers.go index 6e1c17928..8aaa6ab31 100644 --- a/cluster/v1alpha1/helpers.go +++ b/cluster/v1alpha1/helpers.go @@ -162,10 +162,10 @@ func (r *RolloutHandler[T]) getProgressiveClusters(rolloutStrategy RolloutStrate groupKeys := decisionGroupsToGroupKeys(strategy.Progressive.MandatoryDecisionGroups.MandatoryDecisionGroups) clusterGroups = r.pdTracker.ExistingClusterGroups(groupKeys...) - // Perform progressive rollOut for mandatory decision groups first. + // Perform progressive rollOut for mandatory decision groups first, tolerating no failures if len(clusterGroups) > 0 { rolloutResult := progressivePerGroup( - clusterGroups, intstr.FromInt(maxFailures), minSuccessTime, failureTimeout, currentClusterStatus, true, + clusterGroups, intstr.FromInt(0), minSuccessTime, failureTimeout, currentClusterStatus, ) if len(rolloutResult.ClustersToRollout) > 0 || len(rolloutResult.ClustersTimeOut) > 0 { rolloutResult.ClustersRemoved = removedClusterStatus @@ -219,9 +219,9 @@ func (r *RolloutHandler[T]) getProgressivePerGroupClusters(rolloutStrategy Rollo groupKeys := decisionGroupsToGroupKeys(mandatoryDecisionGroups) clusterGroups = r.pdTracker.ExistingClusterGroups(groupKeys...) - // Perform progressive rollout per group for mandatory decision groups first + // Perform progressive rollout per group for mandatory decision groups first, tolerating no failures if len(clusterGroups) > 0 { - rolloutResult := progressivePerGroup(clusterGroups, maxFailures, minSuccessTime, failureTimeout, currentClusterStatus, false) + rolloutResult := progressivePerGroup(clusterGroups, intstr.FromInt(0), minSuccessTime, failureTimeout, currentClusterStatus) if len(rolloutResult.ClustersToRollout) > 0 || len(rolloutResult.ClustersTimeOut) > 0 { rolloutResult.ClustersRemoved = removedClusterStatus @@ -233,7 +233,7 @@ func (r *RolloutHandler[T]) getProgressivePerGroupClusters(rolloutStrategy Rollo restClusterGroups := r.pdTracker.ExistingClusterGroupsBesides(groupKeys...) // Perform progressive rollout per group for the remaining decision groups - rolloutResult := progressivePerGroup(restClusterGroups, maxFailures, minSuccessTime, failureTimeout, currentClusterStatus, false) + rolloutResult := progressivePerGroup(restClusterGroups, maxFailures, minSuccessTime, failureTimeout, currentClusterStatus) rolloutResult.ClustersRemoved = removedClusterStatus return &strategy, rolloutResult, nil @@ -291,7 +291,7 @@ func progressivePerCluster( // If there was a breach of MaxFailures, only handle clusters that have already had workload applied if !failureBreach || failureBreach && status.Status != ToApply { - rolloutClusters, timeoutClusters = determineRolloutStatus(status, minSuccessTime, timeout, rolloutClusters, timeoutClusters) + rolloutClusters, timeoutClusters = determineRolloutStatus(&status, minSuccessTime, timeout, rolloutClusters, timeoutClusters) } // Keep track of TimeOut or Failed clusters and check total against MaxFailures @@ -360,44 +360,43 @@ func progressivePerGroup( minSuccessTime time.Duration, timeout time.Duration, existingClusterStatus []ClusterRolloutStatus, - accumulateFailures bool, ) RolloutResult { var rolloutClusters, timeoutClusters []ClusterRolloutStatus - existingClusters := make(map[string]bool) + existingClusters := make(map[string]RolloutStatus) for _, status := range existingClusterStatus { if status.ClusterName == "" { continue } - if status.Status == ToApply { - // Set as false to consider the cluster in the decisionGroups iteration. - existingClusters[status.ClusterName] = false - } else { - existingClusters[status.ClusterName] = true - rolloutClusters, timeoutClusters = determineRolloutStatus(status, minSuccessTime, timeout, rolloutClusters, timeoutClusters) + // ToApply will be reconsidered in the decisionGroups iteration. + if status.Status != ToApply { + rolloutClusters, timeoutClusters = determineRolloutStatus(&status, minSuccessTime, timeout, rolloutClusters, timeoutClusters) + existingClusters[status.ClusterName] = status.Status } } - var failureCount int + totalFailureCount := 0 failureBreach := false clusterGroupKeys := clusterGroupsMap.GetOrderedGroupKeys() for _, key := range clusterGroupKeys { + groupFailureCount := 0 if subclusters, ok := clusterGroupsMap[key]; ok { - // Only reset the failure count for ProgressivePerGroup - // since Progressive is over the total number of clusters - if !accumulateFailures { - failureCount = 0 - } - failureBreach = false // Calculate the max failure threshold for the group--the returned error was checked // previously, so it's ignored here - maxGroupFailures, _ := calculateRolloutSize(maxFailures, subclusters.Len(), 0) + maxGroupFailures, _ := calculateRolloutSize(maxFailures, len(subclusters), 0) // Iterate through clusters in the group clusters := subclusters.UnsortedList() sort.Strings(clusters) for _, cluster := range clusters { - if existingClusters[cluster] { + if status, ok := existingClusters[cluster]; ok { + // Keep track of TimeOut or Failed clusters and check total against MaxFailures + if status == TimeOut || status == Failed { + groupFailureCount++ + + failureBreach = groupFailureCount > maxGroupFailures + } + continue } @@ -407,18 +406,13 @@ func progressivePerGroup( GroupKey: key, } rolloutClusters = append(rolloutClusters, status) - - // Keep track of TimeOut or Failed clusters and check total against MaxFailures - if status.Status == TimeOut || status.Status == Failed { - failureCount++ - - failureBreach = failureCount > maxGroupFailures - } } - // As it is perGroup Return if there are clusters to rollOut, - // or there was a breach of the MaxFailure configuration - if len(rolloutClusters) > maxGroupFailures || failureBreach { + totalFailureCount += groupFailureCount + + // As it is perGroup, return if there are clusters to rollOut that aren't + // Failed/Timeout, or there was a breach of the MaxFailure configuration + if len(rolloutClusters)-totalFailureCount > 0 || failureBreach { return RolloutResult{ ClustersToRollout: rolloutClusters, ClustersTimeOut: timeoutClusters, @@ -448,7 +442,7 @@ func progressivePerGroup( // the rollOut Clusters. // 2. If timeout is set to 0, the function append the clusterStatus to the timeOut clusters. func determineRolloutStatus( - status ClusterRolloutStatus, + status *ClusterRolloutStatus, minSuccessTime time.Duration, timeout time.Duration, rolloutClusters []ClusterRolloutStatus, @@ -457,13 +451,13 @@ func determineRolloutStatus( switch status.Status { case ToApply: - rolloutClusters = append(rolloutClusters, status) + rolloutClusters = append(rolloutClusters, *status) case Succeeded: // If the cluster succeeded but is still within the MinSuccessTime (i.e. "soak" time), // still add it to the list of rolloutClusters minSuccessTimeTime := getTimeOutTime(status.LastTransitionTime, minSuccessTime) if RolloutClock.Now().Before(minSuccessTimeTime.Time) { - rolloutClusters = append(rolloutClusters, status) + rolloutClusters = append(rolloutClusters, *status) } return rolloutClusters, timeoutClusters @@ -474,10 +468,10 @@ func determineRolloutStatus( status.TimeOutTime = timeOutTime // check if current time is before the timeout time if RolloutClock.Now().Before(timeOutTime.Time) { - rolloutClusters = append(rolloutClusters, status) + rolloutClusters = append(rolloutClusters, *status) } else { status.Status = TimeOut - timeoutClusters = append(timeoutClusters, status) + timeoutClusters = append(timeoutClusters, *status) } } diff --git a/cluster/v1alpha1/helpers_test.go b/cluster/v1alpha1/helpers_test.go index 91865e6f9..f8403105a 100644 --- a/cluster/v1alpha1/helpers_test.go +++ b/cluster/v1alpha1/helpers_test.go @@ -592,11 +592,11 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { }, }, { - name: "test progressive rollout with timeout 0s", + name: "test progressive rollout with timeout 0s, maxFailures 3", rolloutStrategy: RolloutStrategy{ Type: Progressive, Progressive: &RolloutProgressive{ - RolloutConfig: RolloutConfig{ProgressDeadline: "0s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "0s", MaxFailures: intstr.FromInt(3)}, MaxConcurrency: intstr.FromInt(2), }, }, @@ -608,7 +608,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: Progressive, Progressive: &RolloutProgressive{ - RolloutConfig: RolloutConfig{ProgressDeadline: "0s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "0s", MaxFailures: intstr.FromInt(3)}, MaxConcurrency: intstr.FromInt(2), }, }, @@ -644,7 +644,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { }, }, { - name: "test progressive rollout with mandatoryDecisionGroup and timeout 90s ", + name: "test progressive rollout with mandatoryDecisionGroup and timeout 90s", rolloutStrategy: RolloutStrategy{ Type: Progressive, Progressive: &RolloutProgressive{ @@ -701,6 +701,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { ClustersTimeOut: []ClusterRolloutStatus{ {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, }, + MaxFailureBreach: true, }, }, { @@ -854,6 +855,7 @@ func TestGetRolloutCluster_Progressive(t *testing.T) { ClustersTimeOut: []ClusterRolloutStatus{ {ClusterName: "cluster1", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, }, + MaxFailureBreach: true, }, }, { @@ -1086,14 +1088,15 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { ClustersTimeOut: []ClusterRolloutStatus{ {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, }, + MaxFailureBreach: true, }, }, { - name: "test progressivePerGroup rollout with timeout 90s and first group timeOut", + name: "test progressivePerGroup rollout with timeout 90s and first group timeOut, maxFailures 2", rolloutStrategy: RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s", MaxFailures: intstr.FromInt(2)}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -1105,7 +1108,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s", MaxFailures: intstr.FromInt(2)}, }, }, existingWorkloads: []dummyWorkload{ @@ -1144,7 +1147,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { rolloutStrategy: RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s", MaxFailures: intstr.FromInt(2)}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -1156,7 +1159,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s", MaxFailures: intstr.FromInt(2)}, }, }, existingWorkloads: []dummyWorkload{ @@ -1252,14 +1255,15 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { ClustersToRollout: []ClusterRolloutStatus{ {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTimeMax_120s}, }, + MaxFailureBreach: true, }, }, { - name: "test progressivePerGroup rollout with timeout None, first group 1 cluster is failing and maxFailures is 1", + name: "test progressivePerGroup rollout with timeout None, first group 1 cluster is failing and maxFailures is 2", rolloutStrategy: RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - RolloutConfig: RolloutConfig{ProgressDeadline: "None", MaxFailures: intstr.FromInt(1)}, + RolloutConfig: RolloutConfig{ProgressDeadline: "None", MaxFailures: intstr.FromInt(2)}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -1271,7 +1275,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { expectRolloutStrategy: &RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - RolloutConfig: RolloutConfig{ProgressDeadline: "None", MaxFailures: intstr.FromInt(1)}, + RolloutConfig: RolloutConfig{ProgressDeadline: "None", MaxFailures: intstr.FromInt(2)}, }, }, existingWorkloads: []dummyWorkload{ @@ -1348,6 +1352,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTimeMax_120s}, {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: Failed, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTimeMax_120s}, }, + MaxFailureBreach: true, }, }, { @@ -1404,10 +1409,11 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, {ClusterName: "cluster3", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, }, + MaxFailureBreach: true, }, }, { - name: "test progressivePerGroup rollout with mandatoryDecisionGroup failing 1 cluster, timeout 90s and with maxFailures is 1", + name: "test progressivePerGroup rollout with mandatoryDecisionGroup failing 1 cluster, timeout 90s and with maxFailures is 2", rolloutStrategy: RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ @@ -1416,7 +1422,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { {GroupName: "group1"}, }, }, - RolloutConfig: RolloutConfig{ProgressDeadline: "90s", MaxFailures: intstr.FromInt(1)}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s", MaxFailures: intstr.FromInt(2)}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -1433,7 +1439,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { {GroupName: "group1"}, }, }, - RolloutConfig: RolloutConfig{ProgressDeadline: "90s", MaxFailures: intstr.FromInt(1)}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s", MaxFailures: intstr.FromInt(2)}, }, }, existingWorkloads: []dummyWorkload{ @@ -1460,6 +1466,7 @@ func TestGetRolloutCluster_ProgressivePerGroup(t *testing.T) { ClustersTimeOut: []ClusterRolloutStatus{ {ClusterName: "cluster2", GroupKey: clusterv1beta1.GroupKey{GroupName: "group1", GroupIndex: 0}, Status: TimeOut, LastTransitionTime: &fakeTime_120s, TimeOutTime: &fakeTime_30s}, }, + MaxFailureBreach: true, }, }, { @@ -2003,11 +2010,11 @@ func TestGetRolloutCluster_ClusterRemoved(t *testing.T) { }, }, { - name: "test progressivePerGroup rollout with timeout 90s and cluster removed after rollout start while the group timeout.", + name: "test progressivePerGroup rollout with timeout 90s and cluster removed after rollout start while the group timeout", rolloutStrategy: RolloutStrategy{ Type: ProgressivePerGroup, ProgressivePerGroup: &RolloutProgressivePerGroup{ - RolloutConfig: RolloutConfig{ProgressDeadline: "90s"}, + RolloutConfig: RolloutConfig{ProgressDeadline: "90s", MaxFailures: intstr.FromInt(2)}, }, }, existingScheduledClusterGroups: map[clusterv1beta1.GroupKey]sets.Set[string]{ @@ -2223,7 +2230,7 @@ func TestDetermineRolloutStatus(t *testing.T) { RolloutClock = testingclock.NewFakeClock(fakeTime.Time) for _, tc := range testCases { var rolloutClusters, timeoutClusters []ClusterRolloutStatus - rolloutClusters, timeoutClusters = determineRolloutStatus(tc.clusterStatus, tc.minSuccessTime, tc.timeout, rolloutClusters, timeoutClusters) + rolloutClusters, timeoutClusters = determineRolloutStatus(&tc.clusterStatus, tc.minSuccessTime, tc.timeout, rolloutClusters, timeoutClusters) if !reflect.DeepEqual(rolloutClusters, tc.expectRolloutClusters) { t.Errorf("Case: %v Failed to run NewRolloutHandler.\nExpect rollout clusters: %+v\nActual rollout clusters: %+v", tc.name, tc.expectRolloutClusters, rolloutClusters) return diff --git a/cluster/v1alpha1/types_rolloutstrategy.go b/cluster/v1alpha1/types_rolloutstrategy.go index 62e96ccf5..4fbe6524a 100644 --- a/cluster/v1alpha1/types_rolloutstrategy.go +++ b/cluster/v1alpha1/types_rolloutstrategy.go @@ -7,7 +7,8 @@ import ( // +k8s:deepcopy-gen=true -// RolloutStrategy API used by workload applier APIs to define how the workload will be applied to the selected clusters by the Placement and DecisionStrategy. +// RolloutStrategy API used by workload applier APIs to define how the workload will be applied to +// the selected clusters by the Placement and DecisionStrategy. type RolloutType string @@ -24,12 +25,13 @@ const ( type RolloutStrategy struct { // Rollout strategy Types are All, Progressive and ProgressivePerGroup // 1) All means apply the workload to all clusters in the decision groups at once. - // 2) Progressive means apply the workload to the selected clusters progressively per cluster. The workload will not - // be applied to the next cluster unless one of the current applied clusters reach the successful state and haven't - // breached the MaxFailures configuration. - // 3) ProgressivePerGroup means apply the workload to decisionGroup clusters progressively per group. The workload - // will not be applied to the next decisionGroup unless all clusters in the current group reach the successful - // state and haven't breached the MaxFailures configuration. + // 2) Progressive means apply the workload to the selected clusters progressively per cluster. The + // workload will not be applied to the next cluster unless one of the current applied clusters + // reach the successful state and haven't breached the MaxFailures configuration. + // 3) ProgressivePerGroup means apply the workload to decisionGroup clusters progressively per + // group. The workload will not be applied to the next decisionGroup unless all clusters in the + // current group reach the successful state and haven't breached the MaxFailures configuration. + // +kubebuilder:validation:Enum=All;Progressive;ProgressivePerGroup // +kubebuilder:default:=All // +optional @@ -50,37 +52,42 @@ type RolloutStrategy struct { // Timeout to consider while applying the workload. type RolloutConfig struct { - // MinSuccessTime is a "soak" time. In other words, the minimum amount of time the workload applier controller will - // wait from the start of each rollout before proceeding (assuming a successful state has been reached and MaxFailures - // wasn't breached). + // MinSuccessTime is a "soak" time. In other words, the minimum amount of time the workload + // applier controller will wait from the start of each rollout before proceeding (assuming a + // successful state has been reached and MaxFailures wasn't breached). // MinSuccessTime is only considered for rollout types Progressive and ProgressivePerGroup. - // The default value is 0 meaning the workload applier proceeds immediately after a successful state is reached. + // The default value is 0 meaning the workload applier proceeds immediately after a successful + // state is reached. // MinSuccessTime must be defined in [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m , 360s // +kubebuilder:default:="0" // +optional MinSuccessTime metav1.Duration `json:"minSuccessTime,omitempty"` - // ProgressDeadline defines how long workload applier controller will wait for the workload to reach a successful state in the cluster. - // ProgressDeadline default value is "None", meaning the workload applier will wait for a successful state indefinitely. + // ProgressDeadline defines how long workload applier controller will wait for the workload to + // reach a successful state in the cluster. + // ProgressDeadline default value is "None", meaning the workload applier will wait for a + // successful state indefinitely. // ProgressDeadline must be defined in [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m , 360s // +kubebuilder:validation:Pattern="^(([0-9])+[h|m|s])|None$" // +kubebuilder:default:="None" // +optional ProgressDeadline string `json:"progressDeadline,omitempty"` - // MaxFailures is a percentage or number of clusters in the current rollout that can fail before proceeding to the - // next rollout. - // MaxFailures is only considered for rollout types Progressive and ProgressivePerGroup. For Progressive, this is - // considered over the total number of clusters. For ProgressivePerGroup, this is considered according to the size of - // the current group. + // MaxFailures is a percentage or number of clusters in the current rollout that can fail before + // proceeding to the next rollout. + // MaxFailures is only considered for rollout types Progressive and ProgressivePerGroup. For + // Progressive, this is considered over the total number of clusters. For ProgressivePerGroup, + // this is considered according to the size of the current group. For both Progressive and + // ProgressivePerGroup, the MaxFailures does not apply for MandatoryDecisionGroups, which tolerate + // no failures. // Default is that no failures are tolerated. // +kubebuilder:validation:Pattern="^((100|[0-9]{1,2})%|[0-9]+)$" // +kubebuilder:validation:XIntOrString // +kubebuilder:default="0" // +optional MaxFailures intstr.IntOrString `json:"maxFailures,omitempty"` - // Timeout defines how long the workload applier controller will wait until the workload reaches a successful state in - // the cluster. - // Timeout default value is None meaning the workload applier will not proceed apply workload to other clusters if did - // not reach the successful state. + // Timeout defines how long the workload applier controller will wait until the workload reaches a + // successful state in the cluster. + // Timeout default value is None meaning the workload applier will not proceed apply workload to + // other clusters if did not reach the successful state. // Timeout must be defined in [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m , 360s // // Deprecated: Use ProgressDeadline instead. @@ -93,19 +100,23 @@ type RolloutConfig struct { // MandatoryDecisionGroup set the decision group name or group index. // GroupName is considered first to select the decisionGroups then GroupIndex. type MandatoryDecisionGroup struct { - // GroupName of the decision group should match the placementDecisions label value with label key cluster.open-cluster-management.io/decision-group-name + // GroupName of the decision group should match the placementDecisions label value with label key + // cluster.open-cluster-management.io/decision-group-name // +optional GroupName string `json:"groupName,omitempty"` - // GroupIndex of the decision group should match the placementDecisions label value with label key cluster.open-cluster-management.io/decision-group-index + // GroupIndex of the decision group should match the placementDecisions label value with label key + // cluster.open-cluster-management.io/decision-group-index // +optional GroupIndex int32 `json:"groupIndex,omitempty"` } // MandatoryDecisionGroups type MandatoryDecisionGroups struct { - // List of the decision groups names or indexes to apply the workload first and fail if workload did not reach successful state. - // GroupName or GroupIndex must match with the decisionGroups defined in the placement's decisionStrategy + // List of the decision groups names or indexes to apply the workload first and fail if workload + // did not reach successful state. + // GroupName or GroupIndex must match with the decisionGroups defined in the placement's + // decisionStrategy // +optional MandatoryDecisionGroups []MandatoryDecisionGroup `json:"mandatoryDecisionGroups,omitempty"` } @@ -133,8 +144,9 @@ type RolloutProgressive struct { // +optional MandatoryDecisionGroups `json:",inline"` - // MaxConcurrency is the max number of clusters to deploy workload concurrently. The default value for MaxConcurrency - // is determined from the clustersPerDecisionGroup defined in the placement->DecisionStrategy. + // MaxConcurrency is the max number of clusters to deploy workload concurrently. The default value + // for MaxConcurrency is determined from the clustersPerDecisionGroup defined in the + // placement->DecisionStrategy. // +kubebuilder:validation:Pattern="^((100|[0-9]{1,2})%|[0-9]+)$" // +kubebuilder:validation:XIntOrString // +optional diff --git a/cluster/v1alpha1/zz_generated.swagger_doc_generated.go b/cluster/v1alpha1/zz_generated.swagger_doc_generated.go index e983fb5a1..79766e058 100644 --- a/cluster/v1alpha1/zz_generated.swagger_doc_generated.go +++ b/cluster/v1alpha1/zz_generated.swagger_doc_generated.go @@ -109,7 +109,7 @@ var map_RolloutConfig = map[string]string{ "": "Timeout to consider while applying the workload.", "minSuccessTime": "MinSuccessTime is a \"soak\" time. In other words, the minimum amount of time the workload applier controller will wait from the start of each rollout before proceeding (assuming a successful state has been reached and MaxFailures wasn't breached). MinSuccessTime is only considered for rollout types Progressive and ProgressivePerGroup. The default value is 0 meaning the workload applier proceeds immediately after a successful state is reached. MinSuccessTime must be defined in [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m , 360s", "progressDeadline": "ProgressDeadline defines how long workload applier controller will wait for the workload to reach a successful state in the cluster. ProgressDeadline default value is \"None\", meaning the workload applier will wait for a successful state indefinitely. ProgressDeadline must be defined in [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m , 360s", - "maxFailures": "MaxFailures is a percentage or number of clusters in the current rollout that can fail before proceeding to the next rollout. MaxFailures is only considered for rollout types Progressive and ProgressivePerGroup. For Progressive, this is considered over the total number of clusters. For ProgressivePerGroup, this is considered according to the size of the current group. Default is that no failures are tolerated.", + "maxFailures": "MaxFailures is a percentage or number of clusters in the current rollout that can fail before proceeding to the next rollout. MaxFailures is only considered for rollout types Progressive and ProgressivePerGroup. For Progressive, this is considered over the total number of clusters. For ProgressivePerGroup, this is considered according to the size of the current group. For both Progressive and ProgressivePerGroup, the MaxFailures does not apply for MandatoryDecisionGroups, which tolerate no failures. Default is that no failures are tolerated.", "timeout": "Timeout defines how long the workload applier controller will wait until the workload reaches a successful state in the cluster. Timeout default value is None meaning the workload applier will not proceed apply workload to other clusters if did not reach the successful state. Timeout must be defined in [0-9h]|[0-9m]|[0-9s] format examples; 2h , 90m , 360s\n\nDeprecated: Use ProgressDeadline instead.", } @@ -136,7 +136,6 @@ func (RolloutProgressivePerGroup) SwaggerDoc() map[string]string { var map_RolloutStrategy = map[string]string{ "": "Rollout strategy to apply workload to the selected clusters by Placement and DecisionStrategy.", - "type": "Rollout strategy Types are All, Progressive and ProgressivePerGroup 1) All means apply the workload to all clusters in the decision groups at once. 2) Progressive means apply the workload to the selected clusters progressively per cluster. The workload will not\n be applied to the next cluster unless one of the current applied clusters reach the successful state and haven't\n breached the MaxFailures configuration.\n3) ProgressivePerGroup means apply the workload to decisionGroup clusters progressively per group. The workload\n will not be applied to the next decisionGroup unless all clusters in the current group reach the successful\n state and haven't breached the MaxFailures configuration.", "all": "All defines required fields for RolloutStrategy type All", "progressive": "Progressive defines required fields for RolloutStrategy type Progressive", "progressivePerGroup": "ProgressivePerGroup defines required fields for RolloutStrategy type ProgressivePerGroup", diff --git a/work/v1alpha1/0000_00_work.open-cluster-management.io_manifestworkreplicasets.crd.yaml b/work/v1alpha1/0000_00_work.open-cluster-management.io_manifestworkreplicasets.crd.yaml index b437dcf6e..c5a083042 100644 --- a/work/v1alpha1/0000_00_work.open-cluster-management.io_manifestworkreplicasets.crd.yaml +++ b/work/v1alpha1/0000_00_work.open-cluster-management.io_manifestworkreplicasets.crd.yaml @@ -349,7 +349,10 @@ spec: For Progressive, this is considered over the total number of clusters. For ProgressivePerGroup, this is considered according to the size of the current - group. Default is that no failures are tolerated. + group. For both Progressive and ProgressivePerGroup, + the MaxFailures does not apply for MandatoryDecisionGroups, + which tolerate no failures. Default is that no failures + are tolerated. pattern: ^((100|[0-9]{1,2})%|[0-9]+)$ x-kubernetes-int-or-string: true minSuccessTime: @@ -440,7 +443,10 @@ spec: For Progressive, this is considered over the total number of clusters. For ProgressivePerGroup, this is considered according to the size of the current - group. Default is that no failures are tolerated. + group. For both Progressive and ProgressivePerGroup, + the MaxFailures does not apply for MandatoryDecisionGroups, + which tolerate no failures. Default is that no failures + are tolerated. pattern: ^((100|[0-9]{1,2})%|[0-9]+)$ x-kubernetes-int-or-string: true minSuccessTime: @@ -521,7 +527,10 @@ spec: For Progressive, this is considered over the total number of clusters. For ProgressivePerGroup, this is considered according to the size of the current - group. Default is that no failures are tolerated. + group. For both Progressive and ProgressivePerGroup, + the MaxFailures does not apply for MandatoryDecisionGroups, + which tolerate no failures. Default is that no failures + are tolerated. pattern: ^((100|[0-9]{1,2})%|[0-9]+)$ x-kubernetes-int-or-string: true minSuccessTime: @@ -564,18 +573,6 @@ spec: type: object type: default: All - description: Rollout strategy Types are All, Progressive - and ProgressivePerGroup 1) All means apply the workload - to all clusters in the decision groups at once. 2) Progressive - means apply the workload to the selected clusters progressively - per cluster. The workload will not be applied to the next - cluster unless one of the current applied clusters reach - the successful state and haven't breached the MaxFailures - configuration. 3) ProgressivePerGroup means apply the - workload to decisionGroup clusters progressively per group. - The workload will not be applied to the next decisionGroup - unless all clusters in the current group reach the successful - state and haven't breached the MaxFailures configuration. enum: - All - Progressive