Expand SetAggregate API and features

NB: needs more tests

Signed-off-by: Hidde Beydals <hello@hidde.co>

runtime/conditions/getter.go

@@ -124,7 +124,7 @@ func summary(from Getter, t string, options ...MergeOption) *metav1.Condition {
 		o(mergeOpt)
 	}
 
-	// Identifies the conditions in scope for the Summary by taking all the existing conditions except Ready,
+	// Identifies the conditions in scope for the Summary by taking all the existing conditions except t,
 	// or, if a list of conditions types is specified, only the conditions the condition in that list.
 	conditionsInScope := make([]localizedCondition, 0, len(conditions))
 	for i := range conditions {
@@ -231,26 +231,77 @@ func mirror(from Getter, targetCondition string, options ...MirrorOptions) *meta
 	return condition
 }
 
-// Aggregates all the the Ready condition from a list of dependent objects into the target object;
-// if the Ready condition does not exists in one of the source object, the object is excluded from
-// the aggregation; if none of the source object have ready condition, no target conditions is generated.
+// aggregate the conditions from a list of depending objects into the target object; the condition
+// scope can be set using WithConditions; if none of the source objects have the conditions within
+// the scope, no target condition is generated.
 func aggregate(from []Getter, targetCondition string, options ...MergeOption) *metav1.Condition {
+	mergeOpt := &mergeOptions{
+		stepCounter:    len(from),
+	}
+	for _, o := range options {
+		o(mergeOpt)
+	}
+
 	conditionsInScope := make([]localizedCondition, 0, len(from))
 	for i := range from {
-		condition := Get(from[i], meta.ReadyCondition)
+		conditions := from[i].GetConditions()
+		for i, _ := range conditions {
+			c := conditions[i]
+			if mergeOpt.conditionTypes != nil {
+				found := false
+				for _, tt := range mergeOpt.conditionTypes {
+					if c.Type == tt {
+						found = true
+						break
+					}
+				}
+				if !found {
+					continue
+				}
+			}
 
-		conditionsInScope = append(conditionsInScope, localizedCondition{
-			Condition: condition,
-			Getter:    from[i],
-		})
+			conditionsInScope = append(conditionsInScope, localizedCondition{
+				Condition: &c,
+				Getter:    from[i],
+			})
+		}
 	}
 
-	mergeOpt := &mergeOptions{
-		addStepCounter: true,
-		stepCounter:    len(from),
+	// If it is required to add a counter only if a subset of condition exists, check if the conditions
+	// in scope are included in this subset or not.
+	if mergeOpt.addCounterOnlyIfConditionTypes != nil {
+		for _, c := range conditionsInScope {
+			found := false
+			for _, tt := range mergeOpt.addCounterOnlyIfConditionTypes {
+				if c.Type == tt {
+					found = true
+					break
+				}
+			}
+			if !found {
+				mergeOpt.addCounter = false
+				break
+			}
+		}
 	}
-	for _, o := range options {
-		o(mergeOpt)
+
+	// If it is required to add a source ref only if a condition type exists, check if the conditions
+	// in scope are included in this subset or not.
+	if mergeOpt.addSourceRefIfConditionTypes != nil {
+		for _, c := range conditionsInScope {
+			found := false
+			for _, tt := range mergeOpt.addSourceRefIfConditionTypes {
+				if c.Type == tt {
+					found = true
+					break
+				}
+			}
+			if found {
+				mergeOpt.addSourceRef = true
+				break
+			}
+		}
 	}
+
 	return merge(conditionsInScope, targetCondition, mergeOpt)
 }

runtime/conditions/getter_test.go

@@ -247,6 +247,7 @@ func TestAggregate(t *testing.T) {
 		name string
 		from []Getter
 		t    string
+		opts []MergeOption
 		want *metav1.Condition
 	}{
 		{
@@ -255,7 +256,7 @@ func TestAggregate(t *testing.T) {
 			want: nil,
 		},
 		{
-			name: "Returns foo condition with the aggregation of object's ready conditions",
+			name: "Returns foo condition with an aggregation of the object's top group conditions",
 			from: []Getter{
 				getterWithConditions(ready1),
 				getterWithConditions(ready1),
@@ -264,15 +265,75 @@ func TestAggregate(t *testing.T) {
 				getterWithConditions(bar),
 			},
 			t:    "foo",
-			want: FalseCondition("foo", "reason false1", "2 of 5 completed"),
+			want: FalseCondition("foo", "reason false1", "message false1"),
+		},
+		{
+			name: "Returns foo condition with the aggregation of object's subset conditions",
+			from: []Getter{
+				getterWithConditions(ready1),
+				getterWithConditions(ready1),
+				getterWithConditions(ready2, bar),
+				getterWithConditions(),
+				getterWithConditions(bar),
+			},
+			opts: []MergeOption{
+				WithConditions("bar"),
+			},
+			t:    "foo",
+			want: FalseCondition("foo", "reason falseBar1", "message falseBar1"),
+		},
+		{
+			name: "Returns foo condition with the aggregation of object's subset priority conditions",
+			from: []Getter{
+				getterWithConditions(ready1),
+				getterWithConditions(ready1),
+				getterWithConditions(ready2, bar),
+				getterWithConditions(),
+				getterWithConditions(bar),
+			},
+			opts: []MergeOption{
+				WithConditions("bar", meta.ReadyCondition),
+			},
+			t:    "foo",
+			want: FalseCondition("foo", "reason falseBar1", "message falseBar1"),
+		},
+		{
+			name: "Returns foo condition with the aggregation of object's subset priority conditions (inverse)",
+			from: []Getter{
+				getterWithConditions(ready1),
+				getterWithConditions(ready1),
+				getterWithConditions(ready2, bar),
+				getterWithConditions(),
+				getterWithConditions(bar),
+			},
+			opts: []MergeOption{
+				WithConditions(meta.ReadyCondition, "bar"),
+			},
+			t:    "foo",
+			want: FalseCondition("foo", "reason false1", "message false1"),
+		},
+		{
+			name: "Returns foo condition with source ref",
+			from: []Getter{
+				getterWithConditions(ready1),
+				getterWithConditions(ready1),
+				getterWithConditions(ready2, bar),
+				getterWithConditions(),
+				getterWithConditions(bar),
+			},
+			opts: []MergeOption{
+				WithSourceRefIf(meta.ReadyCondition),
+			},
+			t:    "foo",
+			want: FalseCondition("foo", "reason false1 @ /", "message false1"),
 		},
 	}
 
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			g := NewWithT(t)
 
-			got := aggregate(tt.from, tt.t)
+			got := aggregate(tt.from, tt.t, tt.opts...)
 			if tt.want == nil {
 				g.Expect(got).To(BeNil())
 				return

runtime/conditions/merge_strategies.go

@@ -27,9 +27,14 @@ import (
 type mergeOptions struct {
 	conditionTypes                     []string
 	negativePolarityConditionTypes     []string
+
 	addSourceRef                       bool
+	addSourceRefIfConditionTypes       []string
+	addCounter                         bool
+	addCounterOnlyIfConditionTypes     []string
 	addStepCounter                     bool
 	addStepCounterIfOnlyConditionTypes []string
+
 	stepCounter                        int
 }
 
@@ -44,7 +49,7 @@ type MergeOption func(*mergeOptions)
 //
 // NOTE: The order of conditions types defines the priority for determining the Reason and Message for the
 // target condition.
-// IMPORTANT: This options works only while generating the Summary condition.
+// IMPORTANT: This options works only while generating the Summaryor Aggregated condition.
 func WithConditions(t ...string) MergeOption {
 	return func(c *mergeOptions) {
 		c.conditionTypes = t
@@ -56,15 +61,38 @@ func WithConditions(t ...string) MergeOption {
 // happens.
 //
 // NOTE: https://github.com/kubernetes/community/blob/master/contributors/devel/sig-architecture/api-conventions.md#typical-status-properties
-// IMPORTANT: This option works only while generating the Summary condition.
+// IMPORTANT: This option works only while generating the Summary or Aggregated condition.
 func WithNegativePolarityConditions(t ...string) MergeOption {
 	return func(c *mergeOptions) {
 		c.negativePolarityConditionTypes = t
 	}
 }
 
+// WithCounter instructs merge to add a "x of y Type" string to the message,
+// where x is the number of conditions in the top group, y is the number of objects in scope,
+// and Type is the top group condition type.
+func WithCounter() MergeOption {
+	return func(c *mergeOptions) {
+		c.addCounter = true
+	}
+}
+
+// WithCounterIfOnly ensures a counter is show only if a subset of condition exists.
+// This may apply when you want to use a step counter while reconciling the resource, but then
+// want to move away from this notation as soon as the resource has been reconciled, and e.g. a
+// health check condition is generated
+//
+// IMPORTANT: This options requires WithStepCounter or WithStepCounterIf to be set.
+// IMPORTANT: This option works only while generating the Aggregated condition.
+func WithCounterIfOnly(t ...string) MergeOption {
+	return func(c *mergeOptions) {
+		c.addCounterOnlyIfConditionTypes = t
+	}
+}
+
 // WithStepCounter instructs merge to add a "x of y completed" string to the message,
 // where x is the number of conditions with Status=true and y is the number of conditions in scope.
+// IMPORTANT: This options works only while generating the Summary or Aggregated condition.
 func WithStepCounter() MergeOption {
 	return func(c *mergeOptions) {
 		c.addStepCounter = true
@@ -74,33 +102,41 @@ func WithStepCounter() MergeOption {
 // WithStepCounterIf adds a step counter if the value is true.
 // This can be used e.g. to add a step counter only if the object is not being deleted.
 //
-// IMPORTANT: This options works only while generating the Summary condition.
+// IMPORTANT: This option works only while generating the Summary or Aggregated condition.
 func WithStepCounterIf(value bool) MergeOption {
 	return func(c *mergeOptions) {
 		c.addStepCounter = value
 	}
 }
 
-// WithStepCounterIfOnly ensure a step counter is show only if a subset of condition exists.
+// WithStepCounterIfOnly ensures a step counter is show only if a subset of condition exists.
 // This may apply when you want to use a step counter while reconciling the resource, but then
 // want to move away from this notation as soon as the resource has been reconciled, and e.g. a
 // health check condition is generated
 //
 // IMPORTANT: This options requires WithStepCounter or WithStepCounterIf to be set.
-// IMPORTANT: This options works only while generating the Summary condition.
+// IMPORTANT: This option works only while generating the Summary or Aggregated condition.
 func WithStepCounterIfOnly(t ...string) MergeOption {
 	return func(c *mergeOptions) {
 		c.addStepCounterIfOnlyConditionTypes = t
 	}
 }
 
-// AddSourceRef instructs merge to add info about the originating object to the target Reason.
-func AddSourceRef() MergeOption {
+// WithSourceRef instructs merge to add info about the originating object to the target Reason and
+// in summaries.
+func WithSourceRef() MergeOption {
 	return func(c *mergeOptions) {
 		c.addSourceRef = true
 	}
 }
 
+// WithSourceRefIf ensures a source ref is show only if one of the types in the set exists.
+func WithSourceRefIf(t ...string) MergeOption {
+	return func(c *mergeOptions) {
+		c.addSourceRefIfConditionTypes = t
+	}
+}
+
 // getReason returns the reason to be applied to the condition resulting by merging a set of condition groups.
 // The reason is computed according to the given mergeOptions.
 func getReason(groups conditionGroups, options *mergeOptions) string {
@@ -135,10 +171,24 @@ func getMessage(groups conditionGroups, options *mergeOptions) string {
 	if options.addStepCounter {
 		return getStepCounterMessage(groups, options.stepCounter)
 	}
-
+	if options.addCounter {
+		return getCounterMessage(groups, options.stepCounter)
+	}
 	return getFirstMessage(groups, options.conditionTypes)
 }
 
+// getCounterMessage returns a "x of y <Type>", where x is the number of conditions in the top
+// group, y is the number passed to this method and <Type> is the condition type of the top
+// group.
+func getCounterMessage(groups conditionGroups, to int) string {
+	topGroup := groups.TopGroup()
+	if topGroup == nil {
+		return fmt.Sprintf("%d of %d",0, to)
+	}
+	ct := len(topGroup.conditions)
+	return fmt.Sprintf("%d of %d %s", ct, to, topGroup.conditions[0].Type)
+}
+
 // getStepCounterMessage returns a message "x of y completed", where x is the number of conditions
 // with Status=True and Polarity=Positive and y is the number passed to this method.
 func getStepCounterMessage(groups conditionGroups, to int) string {

runtime/conditions/setter.go

@@ -136,10 +136,9 @@ func SetMirror(to Setter, targetCondition string, from Getter, options ...Mirror
 	Set(to, mirror(from, targetCondition, options...))
 }
 
-// SetAggregate creates a new condition with the aggregation of all the the Ready condition
-// from a list of dependent objects; if the Ready condition does not exists in one of the source object,
-// the object is excluded from the aggregation; if none of the source object have ready condition,
-// no target conditions is generated.
+// SetAggregate creates a new condition with the aggregation of all the conditions from a
+// list of dependency objects, or a subset using WithConditions; if none of the source objects
+// have a condition within the scope of the merge operation, no target condition is generated.
 func SetAggregate(to Setter, targetCondition string, from []Getter, options ...MergeOption) {
 	Set(to, aggregate(from, targetCondition, options...))
 }