Merge pull request #11990 from fabriziopandini/set-merge-operation

🌱 Set merge operation

Author: k8s-ci-robot

internal/controllers/cluster/cluster_controller_status.go

@@ -1025,7 +1025,7 @@ type clusterConditionCustomMergeStrategy struct {
 	negativePolarityConditionTypes []string
 }
 
-func (c clusterConditionCustomMergeStrategy) Merge(conditions []v1beta2conditions.ConditionWithOwnerInfo, conditionTypes []string) (status metav1.ConditionStatus, reason, message string, err error) {
+func (c clusterConditionCustomMergeStrategy) Merge(operation v1beta2conditions.MergeOperation, conditions []v1beta2conditions.ConditionWithOwnerInfo, conditionTypes []string) (status metav1.ConditionStatus, reason, message string, err error) {
 	return v1beta2conditions.DefaultMergeStrategy(v1beta2conditions.GetPriorityFunc(
 		func(condition metav1.Condition) v1beta2conditions.MergePriority {
 			// While cluster is deleting, treat unknown conditions from external objects as info (it is ok that those objects have been deleted at this stage).
@@ -1050,7 +1050,7 @@ func (c clusterConditionCustomMergeStrategy) Merge(conditions []v1beta2condition
 			clusterv1.ClusterAvailableUnknownV1Beta2Reason,
 			clusterv1.ClusterAvailableV1Beta2Reason,
 		)),
-	).Merge(conditions, conditionTypes)
+	).Merge(operation, conditions, conditionTypes)
 }
 
 func setAvailableCondition(ctx context.Context, cluster *clusterv1.Cluster, clusterClass *clusterv1.ClusterClass) {

internal/controllers/machine/machine_controller_status.go

@@ -496,7 +496,7 @@ type machineConditionCustomMergeStrategy struct {
 	negativePolarityConditionTypes []string
 }
 
-func (c machineConditionCustomMergeStrategy) Merge(conditions []v1beta2conditions.ConditionWithOwnerInfo, conditionTypes []string) (status metav1.ConditionStatus, reason, message string, err error) {
+func (c machineConditionCustomMergeStrategy) Merge(operation v1beta2conditions.MergeOperation, conditions []v1beta2conditions.ConditionWithOwnerInfo, conditionTypes []string) (status metav1.ConditionStatus, reason, message string, err error) {
 	return v1beta2conditions.DefaultMergeStrategy(
 		// While machine is deleting, treat unknown conditions from external objects as info (it is ok that those objects have been deleted at this stage).
 		v1beta2conditions.GetPriorityFunc(func(condition metav1.Condition) v1beta2conditions.MergePriority {
@@ -522,7 +522,7 @@ func (c machineConditionCustomMergeStrategy) Merge(conditions []v1beta2condition
 			clusterv1.MachineReadyUnknownV1Beta2Reason,
 			clusterv1.MachineReadyV1Beta2Reason,
 		)),
-	).Merge(conditions, conditionTypes)
+	).Merge(operation, conditions, conditionTypes)
 }
 
 // transformControlPlaneAndEtcdConditions Group readiness gates for control plane conditions when they have the same messages.

util/conditions/v1beta2/aggregate.go

@@ -110,7 +110,7 @@ func NewAggregateCondition[T Getter](sourceObjs []T, sourceConditionType string,
 		}
 	}
 
-	status, reason, message, err := aggregateOpt.mergeStrategy.Merge(conditionsInScope, []string{sourceConditionType})
+	status, reason, message, err := aggregateOpt.mergeStrategy.Merge(AggregateMergeOperation, conditionsInScope, []string{sourceConditionType})
 	if err != nil {
 		return nil, err
 	}

util/conditions/v1beta2/merge_strategies.go

@@ -51,6 +51,19 @@ func (o ConditionOwnerInfo) String() string {
 	return fmt.Sprintf("%s %s", o.Kind, o.Name)
 }
 
+// MergeOperation defines merge operations.
+type MergeOperation string
+
+const (
+	// SummaryMergeOperation defines a merge operation of type Summary.
+	// Summary should merge different conditions from the same object.
+	SummaryMergeOperation MergeOperation = "Summary"
+
+	// AggregateMergeOperation defines a merge operation of type Aggregate.
+	// Aggregate should merge the same condition across many objects.
+	AggregateMergeOperation MergeOperation = "Aggregate"
+)
+
 // MergeStrategy defines a strategy used to merge conditions during the aggregate or summary operation.
 type MergeStrategy interface {
 	// Merge passed in conditions.
@@ -59,7 +72,7 @@ type MergeStrategy interface {
 	// Conditions passed in must be of the given conditionTypes (other condition types must be discarded).
 	//
 	// The list of conditionTypes has an implicit order; it is up to the implementation of merge to use this info or not.
-	Merge(conditions []ConditionWithOwnerInfo, conditionTypes []string) (status metav1.ConditionStatus, reason, message string, err error)
+	Merge(operation MergeOperation, conditions []ConditionWithOwnerInfo, conditionTypes []string) (status metav1.ConditionStatus, reason, message string, err error)
 }
 
 // DefaultMergeStrategyOption is some configuration that modifies the DefaultMergeStrategy behaviour.
@@ -199,7 +212,7 @@ type defaultMergeStrategy struct {
 // - issues: conditions with positive polarity (normal True) and status False or conditions with negative polarity (normal False) and status True.
 // - unknown: conditions with status unknown.
 // - info: conditions with positive polarity (normal True) and status True or conditions with negative polarity (normal False) and status False.
-func (d *defaultMergeStrategy) Merge(conditions []ConditionWithOwnerInfo, conditionTypes []string) (status metav1.ConditionStatus, reason, message string, err error) {
+func (d *defaultMergeStrategy) Merge(operation MergeOperation, conditions []ConditionWithOwnerInfo, conditionTypes []string) (status metav1.ConditionStatus, reason, message string, err error) {
 	if len(conditions) == 0 {
 		return "", "", "", errors.New("can't merge an empty list of conditions")
 	}
@@ -208,15 +221,6 @@ func (d *defaultMergeStrategy) Merge(conditions []ConditionWithOwnerInfo, condit
 		return "", "", "", errors.New("can't merge without a getPriority func")
 	}
 
-	// Infer which operation is calling this func, so it is possible to use different strategies for computing the message for the target condition.
-	// - When merge should consider a single condition type, we can assume this func is called within an aggregate operation
-	//   (Aggregate should merge the same condition across many objects)
-	isAggregateOperation := len(conditionTypes) == 1
-
-	// - Otherwise we can assume this func is called within a summary operation
-	//   (Summary should merge different conditions from the same object)
-	isSummaryOperation := !isAggregateOperation
-
 	// sortConditions the relevance defined by the users (the order of condition types), LastTransition time (older first).
 	sortConditions(conditions, conditionTypes)
 
@@ -265,7 +269,7 @@ func (d *defaultMergeStrategy) Merge(conditions []ConditionWithOwnerInfo, condit
 	//
 	// When including messages from conditions, they are sorted by issue/unknown and by the implicit order of condition types
 	// provided by the user (it is considered as order of relevance).
-	if isSummaryOperation {
+	if operation == SummaryMergeOperation {
 		message = summaryMessage(conditions, d, status)
 	}
 
@@ -291,7 +295,7 @@ func (d *defaultMergeStrategy) Merge(conditions []ConditionWithOwnerInfo, condit
 	//
 	// e.g. ...; 2 more Objects with issues; 1 more Objects with unknown status
 	//
-	if isAggregateOperation {
+	if operation == AggregateMergeOperation {
 		n := 3
 		messages := []string{}
 

util/conditions/v1beta2/summary.go

@@ -137,7 +137,7 @@ func NewSummaryCondition(sourceObj Getter, targetConditionType string, opts ...S
 		return nil, errors.New("summary can't be performed when the list of conditions to be summarized is empty")
 	}
 
-	status, reason, message, err := summarizeOpt.mergeStrategy.Merge(conditionsInScope, summarizeOpt.conditionTypes)
+	status, reason, message, err := summarizeOpt.mergeStrategy.Merge(SummaryMergeOperation, conditionsInScope, summarizeOpt.conditionTypes)
 	if err != nil {
 		return nil, err
 	}