When apply conditions.SetSummary on Unknown condition, result is true

[youngbo89]

Description

With a single condition Unknown, conditions.SetSummary sets ReadyCondition "False" but
With two conditions with Unknown and True then it sets "True".

So here is my question; Is it intended? or Do I use conditions.SetSummary in wrong way?

Test

Env

mod:

	sigs.k8s.io/cluster-api v1.3.2
	sigs.k8s.io/controller-runtime v0.13.1

golang compiler:

$ go version
go version go1.20.6 darwin/arm64

Code

func TestSummary(t *testing.T) {
	tests := []struct {
		name              string
		summaryConditions []v1beta1.ConditionType
		conditions        []*v1beta1.Condition
		want              bool
	}{
		{
			name:              "Given Conditions=<Unknown> then expect summary false",
			summaryConditions: []v1beta1.ConditionType{"c1"},
			conditions:        []*v1beta1.Condition{conditions.UnknownCondition("c1", "reason", "")},
			want:              false,
		},
		{
			name:              "Given Conditions=<Unknown, True> then expect summary false",
			summaryConditions: []v1beta1.ConditionType{"c1", "c2"},
			conditions:        []*v1beta1.Condition{conditions.UnknownCondition("c1", "reason", ""), conditions.TrueCondition("c2")},
			want:              false,
		},
	}
	for _, tt := range tests {
		t.Run(tt.name, func(t *testing.T) {

			cluster := &v1beta1.Cluster{}

			for _, c := range tt.conditions {
				conditions.Set(cluster, c)
			}

			conditions.SetSummary(cluster, conditions.WithConditions(tt.summaryConditions...))

			assert.Equal(t, tt.want, conditions.IsTrue(cluster, v1beta1.ReadyCondition))
		})
	}
}

Output:

--- FAIL: TestSummary (0.00s)
    --- PASS: TestSummary/Given_Conditions=<Unknown>_then_expect_summary_false (0.00s)
    --- FAIL: TestSummary/Given_Conditions=<Unknown,_True>_then_expect_summary_false (0.00s)


Expected :false
Actual   :true

[youngbo89]

I found the answer in

cluster-api/util/conditions/merge.go

Lines 34 to 46 in 2061cab

	// merge a list of condition into a single one.
	// This operation is designed to ensure visibility of the most relevant conditions for defining the
	// operational state of a component. E.g. If there is one error in the condition list, this one takes
	// priority over the other conditions and it is should be reflected in the target condition.
	//
	// More specifically:
	// 1. Conditions are grouped by status, severity
	// 2. The resulting condition groups are sorted according to the following priority:
	// - P0 - Status=False, Severity=Error
	// - P1 - Status=False, Severity=Warning
	// - P2 - Status=False, Severity=Info
	// - P3 - Status=True
	// - P4 - Status=Unknown

.

Closed