diff --git a/pkg/apis/serving/v1alpha1/service_types.go b/pkg/apis/serving/v1alpha1/service_types.go index 753f10ad5f1c..94e75a41d376 100644 --- a/pkg/apis/serving/v1alpha1/service_types.go +++ b/pkg/apis/serving/v1alpha1/service_types.go @@ -144,7 +144,7 @@ func (ss *ServiceStatus) GetCondition(t ServiceConditionType) *ServiceCondition return nil } -func (ss *ServiceStatus) SetCondition(new *ServiceCondition) { +func (ss *ServiceStatus) setCondition(new *ServiceCondition) { if new == nil { return } @@ -169,3 +169,15 @@ func (ss *ServiceStatus) RemoveCondition(t ServiceConditionType) { } ss.Conditions = conditions } + +func (ss *ServiceStatus) InitializeConditions() { + sct := []ServiceConditionType{ServiceConditionReady} + for _, cond := range sct { + if rc := ss.GetCondition(cond); rc == nil { + ss.setCondition(&ServiceCondition{ + Type: cond, + Status: corev1.ConditionUnknown, + }) + } + } +} diff --git a/pkg/apis/serving/v1alpha1/service_types_test.go b/pkg/apis/serving/v1alpha1/service_types_test.go index d712aeec09c7..9e2e6a69b7b1 100644 --- a/pkg/apis/serving/v1alpha1/service_types_test.go +++ b/pkg/apis/serving/v1alpha1/service_types_test.go @@ -13,173 +13,208 @@ limitations under the License. package v1alpha1 import ( - "testing" + "testing" - corev1 "k8s.io/api/core/v1" + corev1 "k8s.io/api/core/v1" ) func TestServiceGeneration(t *testing.T) { - service := Service{} - if got, want := service.GetGeneration(), int64(0); got != want { - t.Errorf("Empty Service generation should be %d, was %d", want, got) - } - - answer := int64(42) - service.SetGeneration(answer) - if got := service.GetGeneration(); got != answer { - t.Errorf("GetGeneration mismatch; got %d, want %d", got, answer) - } + service := Service{} + if got, want := service.GetGeneration(), int64(0); got != want { + t.Errorf("Empty Service generation should be %d, was %d", want, got) + } + + answer := int64(42) + service.SetGeneration(answer) + if got := service.GetGeneration(); got != answer { + t.Errorf("GetGeneration mismatch; got %d, want %d", got, answer) + } } func TestServiceIsReady(t *testing.T) { - cases := []struct { - name string - status ServiceStatus - isReady bool - }{ - { - name: "empty status should not be ready", - status: ServiceStatus{}, - isReady: false, - }, - { - name: "Different condition type should not be ready", - status: ServiceStatus{ - Conditions: []ServiceCondition{ - { - Type: "Foo", - Status: corev1.ConditionTrue, - }, - }, - }, - isReady: false, - }, - { - name: "False condition status should not be ready", - status: ServiceStatus{ - Conditions: []ServiceCondition{ - { - Type: ServiceConditionReady, - Status: corev1.ConditionFalse, - }, - }, - }, - isReady: false, - }, - { - name: "Unknown condition status should not be ready", - status: ServiceStatus{ - Conditions: []ServiceCondition{ - { - Type: ServiceConditionReady, - Status: corev1.ConditionUnknown, - }, - }, - }, - isReady: false, - }, - { - name: "Missing condition status should not be ready", - status: ServiceStatus{ - Conditions: []ServiceCondition{ - { - Type: ServiceConditionReady, - }, - }, - }, - isReady: false, - }, - { - name: "True condition status should be ready", - status: ServiceStatus{ - Conditions: []ServiceCondition{ - { - Type: ServiceConditionReady, - Status: corev1.ConditionTrue, - }, - }, - }, - isReady: true, - }, - { - name: "Multiple conditions with ready status should be ready", - status: ServiceStatus{ - Conditions: []ServiceCondition{ - { - Type: "Foo", - Status: corev1.ConditionTrue, - }, - { - Type: ServiceConditionReady, - Status: corev1.ConditionTrue, - }, - }, - }, - isReady: true, - }, - { - name: "Multiple conditions with ready status false should not be ready", - status: ServiceStatus{ - Conditions: []ServiceCondition{ - { - Type: "Foo", - Status: corev1.ConditionTrue, - }, - { - Type: ServiceConditionReady, - Status: corev1.ConditionFalse, - }, - }, - }, - isReady: false, - }, - } - - for _, tc := range cases { - if e, a := tc.isReady, tc.status.IsReady(); e != a { - t.Errorf("%q expected: %v got: %v", tc.name, e, a) - } - } + cases := []struct { + name string + status ServiceStatus + isReady bool + }{ + { + name: "empty status should not be ready", + status: ServiceStatus{}, + isReady: false, + }, + { + name: "Different condition type should not be ready", + status: ServiceStatus{ + Conditions: []ServiceCondition{ + { + Type: "Foo", + Status: corev1.ConditionTrue, + }, + }, + }, + isReady: false, + }, + { + name: "False condition status should not be ready", + status: ServiceStatus{ + Conditions: []ServiceCondition{ + { + Type: ServiceConditionReady, + Status: corev1.ConditionFalse, + }, + }, + }, + isReady: false, + }, + { + name: "Unknown condition status should not be ready", + status: ServiceStatus{ + Conditions: []ServiceCondition{ + { + Type: ServiceConditionReady, + Status: corev1.ConditionUnknown, + }, + }, + }, + isReady: false, + }, + { + name: "Missing condition status should not be ready", + status: ServiceStatus{ + Conditions: []ServiceCondition{ + { + Type: ServiceConditionReady, + }, + }, + }, + isReady: false, + }, + { + name: "True condition status should be ready", + status: ServiceStatus{ + Conditions: []ServiceCondition{ + { + Type: ServiceConditionReady, + Status: corev1.ConditionTrue, + }, + }, + }, + isReady: true, + }, + { + name: "Multiple conditions with ready status should be ready", + status: ServiceStatus{ + Conditions: []ServiceCondition{ + { + Type: "Foo", + Status: corev1.ConditionTrue, + }, + { + Type: ServiceConditionReady, + Status: corev1.ConditionTrue, + }, + }, + }, + isReady: true, + }, + { + name: "Multiple conditions with ready status false should not be ready", + status: ServiceStatus{ + Conditions: []ServiceCondition{ + { + Type: "Foo", + Status: corev1.ConditionTrue, + }, + { + Type: ServiceConditionReady, + Status: corev1.ConditionFalse, + }, + }, + }, + isReady: false, + }, + } + + for _, tc := range cases { + if e, a := tc.isReady, tc.status.IsReady(); e != a { + t.Errorf("%q expected: %v got: %v", tc.name, e, a) + } + } } func TestServiceConditions(t *testing.T) { - svc := &Service{} - foo := &ServiceCondition { - Type: "Foo", - Status: "True", - } - bar := &ServiceCondition { - Type: "Bar", - Status: "True", - } - - // Add a single condition. - svc.Status.SetCondition(foo) - if got, want := len(svc.Status.Conditions), 1; got != want { - t.Fatalf("Unexpected Condition length; got %d, want %d", got, want) - } - - // Remove non-existent condition. - svc.Status.RemoveCondition(bar.Type) - if got, want := len(svc.Status.Conditions), 1; got != want { - t.Fatalf("Unexpected Condition length; got %d, want %d", got, want) - } - - // Add a second Condition. - svc.Status.SetCondition(bar) - if got, want := len(svc.Status.Conditions), 2; got != want { - t.Fatalf("Unexpected Condition length; got %d, want %d", got, want) - } - - // Remove the first Condition. - svc.Status.RemoveCondition(foo.Type) - if got, want := len(svc.Status.Conditions), 1; got != want { - t.Fatalf("Unexpected condition length; got %d, want %d", got, want) - } - - // Test Add nil condition. - svc.Status.SetCondition(nil) - if got, want := len(svc.Status.Conditions), 1; got != want { - t.Fatal("Error, nil condition was allowed to be added.") - } + svc := &Service{} + foo := &ServiceCondition{ + Type: "Foo", + Status: "True", + } + bar := &ServiceCondition{ + Type: "Bar", + Status: "True", + } + + // Add a single condition. + svc.Status.setCondition(foo) + if got, want := len(svc.Status.Conditions), 1; got != want { + t.Fatalf("Unexpected Condition length; got %d, want %d", got, want) + } + + // Remove non-existent condition. + svc.Status.RemoveCondition(bar.Type) + if got, want := len(svc.Status.Conditions), 1; got != want { + t.Fatalf("Unexpected Condition length; got %d, want %d", got, want) + } + + // Add a second Condition. + svc.Status.setCondition(bar) + if got, want := len(svc.Status.Conditions), 2; got != want { + t.Fatalf("Unexpected Condition length; got %d, want %d", got, want) + } + + // Remove the first Condition. + svc.Status.RemoveCondition(foo.Type) + if got, want := len(svc.Status.Conditions), 1; got != want { + t.Fatalf("Unexpected condition length; got %d, want %d", got, want) + } + + // Test Add nil condition. + svc.Status.setCondition(nil) + if got, want := len(svc.Status.Conditions), 1; got != want { + t.Fatal("Error, nil condition was allowed to be added.") + } +} + +func TestTypicalServiceFlow(t *testing.T) { + r := &Service{} + r.Status.InitializeConditions() + checkConditionOngoingService(r.Status, ServiceConditionReady, t) + + // TODO(#1134): Service needs proper conditions. +} + +func checkConditionSucceededService(rs ServiceStatus, rct ServiceConditionType, t *testing.T) *ServiceCondition { + t.Helper() + return checkConditionService(rs, rct, corev1.ConditionTrue, t) +} + +func checkConditionFailedService(rs ServiceStatus, rct ServiceConditionType, t *testing.T) *ServiceCondition { + t.Helper() + return checkConditionService(rs, rct, corev1.ConditionFalse, t) +} + +func checkConditionOngoingService(rs ServiceStatus, rct ServiceConditionType, t *testing.T) *ServiceCondition { + t.Helper() + return checkConditionService(rs, rct, corev1.ConditionUnknown, t) +} + +func checkConditionService(rs ServiceStatus, rct ServiceConditionType, cs corev1.ConditionStatus, t *testing.T) *ServiceCondition { + t.Helper() + r := rs.GetCondition(rct) + if r == nil { + t.Fatalf("Get(%v) = nil, wanted %v=%v", rct, rct, cs) + } + if r.Status != cs { + t.Fatalf("Get(%v) = %v, wanted %v", rct, r.Status, cs) + } + return r } diff --git a/pkg/controller/service/service.go b/pkg/controller/service/service.go index 69acffeb845f..3199f161f8d4 100644 --- a/pkg/controller/service/service.go +++ b/pkg/controller/service/service.go @@ -135,9 +135,9 @@ func (c *Controller) updateServiceEvent(key string) error { return err } - // Don't modify the informers copy service = service.DeepCopy() + service.Status.InitializeConditions() // We added the Generation to avoid fighting the Configuration controller, // which adds a Generation to avoid fighting the Revision controller. We