diff --git a/apis/condition_set.go b/apis/condition_set.go index 7148352ca..d4c70098e 100644 --- a/apis/condition_set.go +++ b/apis/condition_set.go @@ -74,9 +74,6 @@ type ConditionManager interface { // InitializeConditions updates all Conditions in the ConditionSet to Unknown // if not set. InitializeConditions() - - // InitializeCondition updates a Condition to Unknown if not set. - InitializeCondition(t ConditionType) } // NewLivingConditionSet returns a ConditionSet to hold the conditions for the @@ -302,19 +299,37 @@ func (r conditionsImpl) MarkFalse(t ConditionType, reason, messageFormat string, // InitializeConditions updates all Conditions in the ConditionSet to Unknown // if not set. func (r conditionsImpl) InitializeConditions() { - for _, t := range r.dependents { - r.InitializeCondition(t) + happy := r.GetCondition(r.happy) + if happy == nil { + happy = &Condition{ + Type: r.happy, + Status: corev1.ConditionUnknown, + Severity: ConditionSeverityError, + } + r.SetCondition(*happy) + } + // If the happy state is true, it implies that all of the terminal + // subconditions must be true, so initialize any unset conditions to + // true if our happy condition is true, otherwise unknown. + status := corev1.ConditionUnknown + if happy.Status == corev1.ConditionTrue { + status = corev1.ConditionTrue + } + for _, t := range r.dependents { + r.initializeTerminalCondition(t, status) } - r.InitializeCondition(r.happy) } -// InitializeCondition updates a Condition to Unknown if not set. -func (r conditionsImpl) InitializeCondition(t ConditionType) { - if c := r.GetCondition(t); c == nil { - r.SetCondition(Condition{ - Type: t, - Status: corev1.ConditionUnknown, - Severity: r.severity(t), - }) +// initializeTerminalCondition initializes a Condition to the given status if unset. +func (r conditionsImpl) initializeTerminalCondition(t ConditionType, status corev1.ConditionStatus) *Condition { + if c := r.GetCondition(t); c != nil { + return c } + c := Condition{ + Type: t, + Status: status, + Severity: ConditionSeverityError, + } + r.SetCondition(c) + return &c } diff --git a/apis/condition_set_impl_test.go b/apis/condition_set_impl_test.go index 735270c61..645a748b3 100644 --- a/apis/condition_set_impl_test.go +++ b/apis/condition_set_impl_test.go @@ -805,3 +805,38 @@ func TestInitializeConditions(t *testing.T) { }) } } + +func TestTerminalInitialization(t *testing.T) { + set := NewLivingConditionSet("Foo") + status := &TestStatus{} + + manager := set.Manage(status) + manager.InitializeConditions() + + if got, want := len(status.c), 2; got != want { + t.Errorf("InitializeConditions() = %v, wanted %v", got, want) + } + + manager.MarkTrue("Foo") + if !manager.IsHappy() { + t.Error("IsHappy() = false, wanted true") + } + + // Add a new condition "Bar" to simulate the addition of conditions. + set = NewLivingConditionSet("Foo", "Bar") + + // Create a new manager for the new set and re-initialize to simulate + // Reconcile() with the new conditions. + manager = set.Manage(status) + manager.InitializeConditions() + + if got, want := len(status.c), 3; got != want { + t.Errorf("InitializeConditions() = %v, wanted %v", got, want) + } + + if c := manager.GetCondition("Bar"); c == nil { + t.Error("GetCondition(Bar) = nil, wanted True") + } else if got, want := c.Status, corev1.ConditionTrue; got != want { + t.Errorf("GetCondition(Bar) = %s, wanted %s", got, want) + } +} diff --git a/apis/duck/v1beta1/status_types.go b/apis/duck/v1beta1/status_types.go index d487991ca..a82490b9a 100644 --- a/apis/duck/v1beta1/status_types.go +++ b/apis/duck/v1beta1/status_types.go @@ -17,6 +17,7 @@ limitations under the License. package v1beta1 import ( + "context" "time" corev1 "k8s.io/api/core/v1" @@ -96,6 +97,19 @@ func (s *Status) GetCondition(t apis.ConditionType) *apis.Condition { return nil } +// ConvertTo helps implement apis.Convertible for types embedding this Status. +func (source *Status) ConvertTo(ctx context.Context, sink *Status) { + for _, c := range source.Conditions { + switch c.Type { + // Copy over the "happy" condition, which is the only condition that + // we can reliably transfer. + case apis.ConditionReady, apis.ConditionSucceeded: + sink.SetConditions(apis.Conditions{c}) + return + } + } +} + // Populate implements duck.Populatable func (t *KResource) Populate() { t.Status.ObservedGeneration = 42 diff --git a/apis/duck/v1beta1/status_types_test.go b/apis/duck/v1beta1/status_types_test.go index 8c644ae15..35408214b 100644 --- a/apis/duck/v1beta1/status_types_test.go +++ b/apis/duck/v1beta1/status_types_test.go @@ -17,6 +17,7 @@ limitations under the License. package v1beta1 import ( + "context" "testing" "github.com/google/go-cmp/cmp" @@ -85,6 +86,15 @@ func TestConditionSet(t *testing.T) { } } + s2 := &Status{} + s.ConvertTo(context.Background(), s2) + if condSet.Manage(s2).IsHappy() { + t.Error("s2.IsHappy() = true, wanted false") + } + if got, want := len(s2.Conditions), 1; got != want { + t.Errorf("len(s2.Conditions) = %d, wanted %d", got, want) + } + for _, c := range []apis.ConditionType{"Foo"} { mgr.MarkFalse(c, "bad", "for business") } @@ -97,6 +107,15 @@ func TestConditionSet(t *testing.T) { } } + s2 = &Status{} + s.ConvertTo(context.Background(), s2) + if condSet.Manage(s2).IsHappy() { + t.Error("s2.IsHappy() = true, wanted false") + } + if got, want := len(s2.Conditions), 1; got != want { + t.Errorf("len(s2.Conditions) = %d, wanted %d", got, want) + } + for _, c := range []apis.ConditionType{"Foo"} { mgr.MarkTrue(c) } @@ -108,4 +127,13 @@ func TestConditionSet(t *testing.T) { t.Errorf("GetCondition(%q) = %v, wanted %v", c, got, want) } } + + s2 = &Status{} + s.ConvertTo(context.Background(), s2) + if !condSet.Manage(s2).IsHappy() { + t.Error("s2.IsHappy() = false, wanted true") + } + if got, want := len(s2.Conditions), 1; got != want { + t.Errorf("len(s2.Conditions) = %d, wanted %d", got, want) + } } diff --git a/apis/interfaces.go b/apis/interfaces.go index aa1e4d29d..601d083dd 100644 --- a/apis/interfaces.go +++ b/apis/interfaces.go @@ -38,10 +38,10 @@ type Validatable interface { // "higher" versions of the same type. type Convertible interface { // ConvertUp up-converts the receiver into `to`. - ConvertUp(to Convertible) error + ConvertUp(ctx context.Context, to Convertible) error // ConvertDown down-converts from `from` into the receiver. - ConvertDown(from Convertible) error + ConvertDown(ctx context.Context, from Convertible) error } // Immutable indicates that a particular type has fields that should