✨ Replace Timeout with RolloutConfig (#281)

* 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>

* Add more unit test for Rollout ProgressivePerGroup case

Signed-off-by: melserngawy <melserng@redhat.com>

* 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>

---------

Signed-off-by: Dale Haiducek <19750917+dhaiducek@users.noreply.github.com>
Signed-off-by: melserngawy <melserng@redhat.com>
Co-authored-by: melserngawy <melserng@redhat.com>

cluster/v1alpha1/types_rolloutstrategy.go

@@ -1,103 +1,154 @@
 package v1alpha1
 
 import (
+	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/util/intstr"
 )
 
 // +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
 
 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. 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 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"`
 }
 
 // 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"`
 }
 
 // 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"`
 }