diff --git a/modules/common/condition/funcs.go b/modules/common/condition/funcs.go index 71db2fe1..13f34f2b 100644 --- a/modules/common/condition/funcs.go +++ b/modules/common/condition/funcs.go @@ -295,9 +295,15 @@ func GetHigherPrioCondition(cond1, cond2 *Condition) *Condition { } // Mirror - mirrors Status, Message, Reason and Severity from the latest condition -// of a sorted conditionGroup list into a target condition of type t. -// The conditionGroup entries are split by Status with the order False, True, Unknown. +// of a sorted conditionGroup list into a target condition of type t. If the +// top level ReadyCondition is True then it is assumed that there are no False +// or important Uknown conditions present in the list as ReadyCondition is expected +// to be an aggregated status condition. +// If ReadyCondition is not True then the conditionGroup entries are split by +// Status with the order False, Unknown, True. // If Status=False its again split into Severity with the order Error, Warning, Info. +// So Mirror either reflects the ReadyCondition=True or reflects the latest most +// sever False or Uknown condition. func (conditions *Conditions) Mirror(t Type) *Condition { if conditions == nil || len(*conditions) == 0 { @@ -344,8 +350,17 @@ func (conditions *Conditions) Mirror(t Type) *Condition { break } - mirrorCondition = UnknownCondition(t, c.Reason, c.Message) - mirrorCondition.LastTransitionTime = c.LastTransitionTime + if c.Status == corev1.ConditionUnknown { + mirrorCondition = UnknownCondition(t, c.Reason, c.Message) + mirrorCondition.LastTransitionTime = c.LastTransitionTime + break + } + + // The only valid values for Status is True, False, Unknown are handled + // above so if we reach here then we have an invalid status condition. + // This should never happen. + panic(fmt.Sprintf("Condition %v has invalid status value '%s'. The only valid values are True, False, Unknown", c, c.Status)) + } return mirrorCondition @@ -392,9 +407,9 @@ func groupOrder(c Condition) int { case SeverityInfo: return 2 } - case corev1.ConditionTrue: - return 3 case corev1.ConditionUnknown: + return 3 + case corev1.ConditionTrue: return 4 } diff --git a/modules/common/condition/funcs_test.go b/modules/common/condition/funcs_test.go index 2512c406..de4095b7 100644 --- a/modules/common/condition/funcs_test.go +++ b/modules/common/condition/funcs_test.go @@ -387,10 +387,10 @@ func TestMirror(t *testing.T) { g.Expect(conditions).To(haveSameConditionsOf(CreateList(unknownReady, trueA))) targetCondition = conditions.Mirror("targetConditon") // expect to be mirrored trueA - g.Expect(targetCondition.Status).To(BeIdenticalTo(trueA.Status)) - g.Expect(targetCondition.Severity).To(BeIdenticalTo(trueA.Severity)) - g.Expect(targetCondition.Reason).To(BeIdenticalTo(trueA.Reason)) - g.Expect(targetCondition.Message).To(BeIdenticalTo(trueA.Message)) + g.Expect(targetCondition.Status).To(BeIdenticalTo(unknownReady.Status)) + g.Expect(targetCondition.Severity).To(BeIdenticalTo(unknownReady.Severity)) + g.Expect(targetCondition.Reason).To(BeIdenticalTo(unknownReady.Reason)) + g.Expect(targetCondition.Message).To(BeIdenticalTo(unknownReady.Message)) conditions.Set(falseB) g.Expect(conditions).To(haveSameConditionsOf(CreateList(unknownReady, trueA, falseB))) @@ -422,6 +422,45 @@ func TestMirror(t *testing.T) { g.Expect(targetCondition.Message).To(BeIdenticalTo(trueReady.Message)) } +func TestMirrorReadyTrueAndFalse(t *testing.T) { + g := NewWithT(t) + + conditions := Conditions{} + // We expect that ReadyCondition True means the overall status of the + // resource is ready that condition is mirrored even if there are other + // conditions in non True state. + conditions.Init(&Conditions{*trueReady, *falseB, *unknownA}) + + targetCondition := conditions.Mirror("targetConditon") + + g.Expect(targetCondition.Status).To(BeIdenticalTo(trueReady.Status)) + g.Expect(targetCondition.Severity).To(BeIdenticalTo(trueReady.Severity)) + g.Expect(targetCondition.Reason).To(BeIdenticalTo(trueReady.Reason)) + g.Expect(targetCondition.Message).To(BeIdenticalTo(trueReady.Message)) +} + +func TestMirrorInvalidStatus(t *testing.T) { + g := NewWithT(t) + + conditions := Conditions{} + invalidStatusA := Condition{ + Type: "a", + Status: "FooBar", + Reason: "", + Severity: SeverityNone, + Message: "", + } + conditions.Init(&Conditions{invalidStatusA}) + // NOTE(gibi): we always have a ReadyCondition added automatically so it + // is picked up by Mirror before reaching to the condition with FooBar + // status. So to trigger the error case we need to remove the ReadyCondition + // explicitly. + conditions.Remove(ReadyCondition) + + g.Expect(func() { conditions.Mirror("targetConditon") }).To( + PanicWith(MatchRegexp(`Condition \{a FooBar .*\} has invalid status value 'FooBar'.`))) +} + func TestIsError(t *testing.T) { g := NewWithT(t) @@ -444,11 +483,15 @@ func TestGetHigherPrioCondition(t *testing.T) { c = GetHigherPrioCondition(nil, unknownA) g.Expect(hasSameState(c, unknownA)).To(BeTrue()) - // Status True has higher prio then Status Unknown + // Status Unknown has higher prio then Status True c = GetHigherPrioCondition(unknownA, trueA) - g.Expect(hasSameState(c, trueA)).To(BeTrue()) + g.Expect(hasSameState(c, unknownA)).To(BeTrue()) // Status False has higher prio then Status Unknown + c = GetHigherPrioCondition(falseA, unknownA) + g.Expect(hasSameState(c, falseA)).To(BeTrue()) + + // Status False has higher prio then Status True c = GetHigherPrioCondition(falseA, trueA) g.Expect(hasSameState(c, falseA)).To(BeTrue())