From 7e80f2feabaf2ed78c007cb63a75e338d71f85c2 Mon Sep 17 00:00:00 2001 From: Matt Moore Date: Mon, 11 Jun 2018 17:04:25 +0000 Subject: [PATCH] This is the Configuration equivalent of #1113, #1118 --- .../serving/v1alpha1/configuration_types.go | 70 +++++++++-- .../v1alpha1/configuration_types_test.go | 116 +++++++++++++++++- pkg/controller/configuration/configuration.go | 92 +++----------- .../configuration/configuration_test.go | 103 ++++++++-------- 4 files changed, 244 insertions(+), 137 deletions(-) diff --git a/pkg/apis/serving/v1alpha1/configuration_types.go b/pkg/apis/serving/v1alpha1/configuration_types.go index d0a1e4dc0976..801d2e54f951 100644 --- a/pkg/apis/serving/v1alpha1/configuration_types.go +++ b/pkg/apis/serving/v1alpha1/configuration_types.go @@ -18,6 +18,7 @@ package v1alpha1 import ( "encoding/json" + "fmt" build "github.com/knative/build/pkg/apis/build/v1alpha1" @@ -146,8 +147,8 @@ func (r *Configuration) GetSpecJSON() ([]byte, error) { // IsReady looks at the conditions on the ConfigurationStatus. // ConfigurationConditionReady returns true if ConditionStatus is True -func (configStatus *ConfigurationStatus) IsReady() bool { - if c := configStatus.GetCondition(ConfigurationConditionReady); c != nil { +func (cs *ConfigurationStatus) IsReady() bool { + if c := cs.GetCondition(ConfigurationConditionReady); c != nil { return c.Status == corev1.ConditionTrue } return false @@ -156,9 +157,9 @@ func (configStatus *ConfigurationStatus) IsReady() bool { // IsLatestReadyRevisionNameUpToDate returns true if the Configuration is ready // and LatestCreateRevisionName is equal to LatestReadyRevisionName. Otherwise // it returns false. -func (configStatus *ConfigurationStatus) IsLatestReadyRevisionNameUpToDate() bool { - return configStatus.IsReady() && - configStatus.LatestCreatedRevisionName == configStatus.LatestReadyRevisionName +func (cs *ConfigurationStatus) IsLatestReadyRevisionNameUpToDate() bool { + return cs.IsReady() && + cs.LatestCreatedRevisionName == cs.LatestReadyRevisionName } func (config *ConfigurationStatus) GetCondition(t ConfigurationConditionType) *ConfigurationCondition { @@ -170,28 +171,75 @@ func (config *ConfigurationStatus) GetCondition(t ConfigurationConditionType) *C return nil } -func (configStatus *ConfigurationStatus) SetCondition(new *ConfigurationCondition) { +func (cs *ConfigurationStatus) setCondition(new *ConfigurationCondition) { if new == nil { return } t := new.Type var conditions []ConfigurationCondition - for _, cond := range configStatus.Conditions { + for _, cond := range cs.Conditions { if cond.Type != t { conditions = append(conditions, cond) } } conditions = append(conditions, *new) - configStatus.Conditions = conditions + cs.Conditions = conditions } -func (configStatus *ConfigurationStatus) RemoveCondition(t ConfigurationConditionType) { +func (cs *ConfigurationStatus) RemoveCondition(t ConfigurationConditionType) { var conditions []ConfigurationCondition - for _, cond := range configStatus.Conditions { + for _, cond := range cs.Conditions { if cond.Type != t { conditions = append(conditions, cond) } } - configStatus.Conditions = conditions + cs.Conditions = conditions +} + +func (cs *ConfigurationStatus) InitializeConditions() { + rct := []ConfigurationConditionType{ + ConfigurationConditionLatestRevisionReady, ConfigurationConditionReady} + for _, cond := range rct { + if rc := cs.GetCondition(cond); rc == nil { + cs.setCondition(&ConfigurationCondition{ + Type: cond, + Status: corev1.ConditionUnknown, + }) + } + } +} + +func (cs *ConfigurationStatus) SetLatestCreatedRevisionName(name string) { + cs.LatestCreatedRevisionName = name + cs.setCondition(&ConfigurationCondition{ + Type: ConfigurationConditionLatestRevisionReady, + Status: corev1.ConditionUnknown, + }) +} + +func (cs *ConfigurationStatus) SetLatestReadyRevisionName(name string) { + cs.LatestReadyRevisionName = name + for _, cond := range []ConfigurationConditionType{ + ConfigurationConditionLatestRevisionReady, ConfigurationConditionReady} { + cs.setCondition(&ConfigurationCondition{ + Type: cond, + Status: corev1.ConditionTrue, + }) + } +} + +func (cs *ConfigurationStatus) MarkLatestCreatedFailed(name, message string) { + cct := []ConfigurationConditionType{ConfigurationConditionLatestRevisionReady} + if cs.LatestReadyRevisionName == "" { + cct = append(cct, ConfigurationConditionReady) + } + for _, cond := range cct { + cs.setCondition(&ConfigurationCondition{ + Type: cond, + Status: corev1.ConditionFalse, + Reason: "RevisionFailed", + Message: fmt.Sprintf("revision %q failed with message: %s", name, message), + }) + } } diff --git a/pkg/apis/serving/v1alpha1/configuration_types_test.go b/pkg/apis/serving/v1alpha1/configuration_types_test.go index 4d97a1cdb7d3..7dad990ffdd3 100644 --- a/pkg/apis/serving/v1alpha1/configuration_types_test.go +++ b/pkg/apis/serving/v1alpha1/configuration_types_test.go @@ -13,6 +13,7 @@ limitations under the License. package v1alpha1 import ( + "strings" "testing" corev1 "k8s.io/api/core/v1" @@ -153,7 +154,7 @@ func TestConfigurationConditions(t *testing.T) { } // Add a new condition. - config.Status.SetCondition(foo) + config.Status.setCondition(foo) if got, want := len(config.Status.Conditions), 1; got != want { t.Fatalf("Unexpected Condition length; got %d, want %d", got, want) @@ -167,7 +168,7 @@ func TestConfigurationConditions(t *testing.T) { } // Add a second condition. - config.Status.SetCondition(bar) + config.Status.setCondition(bar) if got, want := len(config.Status.Conditions), 2; got != want { t.Fatalf("Unexpected Condition length; got %d, want %d", got, want) @@ -181,7 +182,7 @@ func TestConfigurationConditions(t *testing.T) { } // Add nil condition. - config.Status.SetCondition(nil) + config.Status.setCondition(nil) if got, want := len(config.Status.Conditions), 1; got != want { t.Fatalf("Unexpected Condition length; got %d, want %d", got, want) @@ -255,3 +256,112 @@ func TestLatestReadyRevisionNameUpToDate(t *testing.T) { } } } + +func TestTypicalFlow(t *testing.T) { + r := &Configuration{} + r.Status.InitializeConditions() + checkConditionOngoingConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t) + checkConditionOngoingConfiguration(r.Status, ConfigurationConditionReady, t) + + r.Status.SetLatestCreatedRevisionName("foo") + checkConditionOngoingConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t) + checkConditionOngoingConfiguration(r.Status, ConfigurationConditionReady, t) + + r.Status.SetLatestReadyRevisionName("foo") + checkConditionSucceededConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t) + checkConditionSucceededConfiguration(r.Status, ConfigurationConditionReady, t) + + r.Status.SetLatestCreatedRevisionName("bar") + checkConditionOngoingConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t) + checkConditionSucceededConfiguration(r.Status, ConfigurationConditionReady, t) + + r.Status.SetLatestReadyRevisionName("bar") + checkConditionSucceededConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t) + checkConditionSucceededConfiguration(r.Status, ConfigurationConditionReady, t) +} + +func TestFailingFirstRevisionWithRecovery(t *testing.T) { + r := &Configuration{} + r.Status.InitializeConditions() + checkConditionOngoingConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t) + checkConditionOngoingConfiguration(r.Status, ConfigurationConditionReady, t) + + r.Status.SetLatestCreatedRevisionName("foo") + checkConditionOngoingConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t) + checkConditionOngoingConfiguration(r.Status, ConfigurationConditionReady, t) + + want := "the message" + r.Status.MarkLatestCreatedFailed("foo", want) + if c := checkConditionFailedConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t); !strings.Contains(c.Message, want) { + t.Errorf("MarkLatestCreatedFailed = %v, want substring %v", c.Message, want) + } + if c := checkConditionFailedConfiguration(r.Status, ConfigurationConditionReady, t); !strings.Contains(c.Message, want) { + t.Errorf("MarkLatestCreatedFailed = %v, want substring %v", c.Message, want) + } + + // When a new revision comes along the LatestRevisionReady condition becomes Unknown, + // but Ready is still false (we've never seen a Ready Revision). + r.Status.SetLatestCreatedRevisionName("bar") + checkConditionOngoingConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t) + checkConditionFailedConfiguration(r.Status, ConfigurationConditionReady, t) + + // When the new revision becomes ready, then Ready becomes true as well. + r.Status.SetLatestReadyRevisionName("bar") + checkConditionSucceededConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t) + checkConditionSucceededConfiguration(r.Status, ConfigurationConditionReady, t) +} + +func TestFailingSecondRevision(t *testing.T) { + r := &Configuration{} + r.Status.InitializeConditions() + checkConditionOngoingConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t) + checkConditionOngoingConfiguration(r.Status, ConfigurationConditionReady, t) + + r.Status.SetLatestCreatedRevisionName("foo") + checkConditionOngoingConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t) + checkConditionOngoingConfiguration(r.Status, ConfigurationConditionReady, t) + + r.Status.SetLatestReadyRevisionName("foo") + checkConditionSucceededConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t) + checkConditionSucceededConfiguration(r.Status, ConfigurationConditionReady, t) + + r.Status.SetLatestCreatedRevisionName("bar") + checkConditionOngoingConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t) + checkConditionSucceededConfiguration(r.Status, ConfigurationConditionReady, t) + + // When the second revision fails, the Configuration remains "Ready" because there is a + // revision suitable for serving traffic. + want := "the message" + r.Status.MarkLatestCreatedFailed("bar", want) + if c := checkConditionFailedConfiguration(r.Status, ConfigurationConditionLatestRevisionReady, t); !strings.Contains(c.Message, want) { + t.Errorf("MarkLatestCreatedFailed = %v, want substring %v", c.Message, want) + } + checkConditionSucceededConfiguration(r.Status, ConfigurationConditionReady, t) +} + +func checkConditionSucceededConfiguration(rs ConfigurationStatus, rct ConfigurationConditionType, t *testing.T) *ConfigurationCondition { + t.Helper() + return checkConditionConfiguration(rs, rct, corev1.ConditionTrue, t) +} + +func checkConditionFailedConfiguration(rs ConfigurationStatus, rct ConfigurationConditionType, t *testing.T) *ConfigurationCondition { + t.Helper() + return checkConditionConfiguration(rs, rct, corev1.ConditionFalse, t) +} + +func checkConditionOngoingConfiguration(rs ConfigurationStatus, rct ConfigurationConditionType, t *testing.T) *ConfigurationCondition { + t.Helper() + return checkConditionConfiguration(rs, rct, corev1.ConditionUnknown, t) +} + +func checkConditionConfiguration(rs ConfigurationStatus, rct ConfigurationConditionType, cs corev1.ConditionStatus, t *testing.T) *ConfigurationCondition { + 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/configuration/configuration.go b/pkg/controller/configuration/configuration.go index 8b1ed10642a1..a35ef07f2bb2 100644 --- a/pkg/controller/configuration/configuration.go +++ b/pkg/controller/configuration/configuration.go @@ -17,7 +17,6 @@ limitations under the License. package configuration import ( - "context" "fmt" buildv1alpha1 "github.com/knative/build/pkg/apis/build/v1alpha1" @@ -28,7 +27,6 @@ import ( informers "github.com/knative/serving/pkg/client/informers/externalversions" listers "github.com/knative/serving/pkg/client/listers/serving/v1alpha1" "github.com/knative/serving/pkg/controller" - "github.com/knative/serving/pkg/logging" "github.com/knative/serving/pkg/logging/logkey" "go.uber.org/zap" corev1 "k8s.io/api/core/v1" @@ -130,6 +128,7 @@ func (c *Controller) syncHandler(key string) error { } // Don't modify the informer's copy. config = config.DeepCopy() + config.Status.InitializeConditions() // Configuration business logic if config.GetGeneration() == config.Status.ObservedGeneration { @@ -217,7 +216,7 @@ func (c *Controller) syncHandler(key string) error { // we just reconciled against so we don't keep generating revisions. // Also update the LatestCreatedRevisionName so that we'll know revision to check // for ready state so that when ready, we can make it Latest. - config.Status.LatestCreatedRevisionName = created.ObjectMeta.Name + config.Status.SetLatestCreatedRevisionName(created.ObjectMeta.Name) config.Status.ObservedGeneration = config.Spec.Generation logger.Infof("Updating the configuration status:\n%+v", config) @@ -264,7 +263,6 @@ func (c *Controller) addRevisionEvent(obj interface{}) { } logger := loggerWithConfigInfo(c.Logger, namespace, configName).With(zap.String(logkey.Revision, revisionName)) - ctx := logging.WithLogger(context.TODO(), logger) config, err := c.lister.Configurations(namespace).Get(configName) if err != nil { @@ -281,89 +279,39 @@ func (c *Controller) addRevisionEvent(obj interface{}) { // Don't modify the informer's copy. config = config.DeepCopy() - if !revision.Status.IsReady() { - logger.Infof("Revision %q of configuration %q is not ready", revisionName, config.Name) - - //add LatestRevision condition to be false with the status from the revision - c.markConfigurationLatestRevisionStatus(ctx, config, revision) + // Track whether the configuration was already ready in order to + // present an initial readiness event. + alreadyReady := config.Status.IsReady() - if _, err := c.updateStatus(config); err != nil { - logger.Error("Error updating configuration", zap.Error(err)) - } + rc := revision.Status.GetCondition(v1alpha1.RevisionConditionReady) + if rc == nil || rc.Status == corev1.ConditionUnknown { + logger.Infof("Revision %q of configuration %q is not ready", revisionName, config.Name) c.Recorder.Eventf(config, corev1.EventTypeNormal, "LatestRevisionUpdate", "Latest revision of configuration is not ready") - - } else { - logger.Info("Revision is ready") - - alreadyReady := config.Status.IsReady() - if !alreadyReady { - c.markConfigurationReady(ctx, config, revision) - } - logger.Infof("Setting LatestReadyRevisionName of Configuration %q to revision %q", - config.Name, revision.Name) - config.Status.LatestReadyRevisionName = revision.Name - + } else if rc.Status == corev1.ConditionTrue { + logger.Infof("Revision %q of configuration %q is ready", revisionName, config.Name) + config.Status.SetLatestReadyRevisionName(revision.Name) if _, err := c.updateStatus(config); err != nil { logger.Error("Error updating configuration", zap.Error(err)) } + if !alreadyReady { c.Recorder.Eventf(config, corev1.EventTypeNormal, "ConfigurationReady", "Configuration becomes ready") } c.Recorder.Eventf(config, corev1.EventTypeNormal, "LatestReadyUpdate", "LatestReadyRevisionName updated to %q", revision.Name) - + } else { + logger.Infof("Revision %q of configuration %q has failed", revisionName, config.Name) + config.Status.MarkLatestCreatedFailed(revision.Name, rc.Message) + if _, err := c.updateStatus(config); err != nil { + logger.Error("Error updating configuration", zap.Error(err)) + } + c.Recorder.Eventf(config, corev1.EventTypeWarning, "LatestCreatedFailed", + "Latest created revision %q has failed", revision.Name) } - return } func (c *Controller) updateRevisionEvent(old, new interface{}) { c.addRevisionEvent(new) } - -func getLatestRevisionStatusCondition(revision *v1alpha1.Revision) *v1alpha1.RevisionCondition { - for _, cond := range revision.Status.Conditions { - if !(cond.Type == v1alpha1.RevisionConditionReady && cond.Status == corev1.ConditionTrue) { - return &cond - } - } - return nil -} - -// Mark ConfigurationConditionReady of Configuration ready as the given latest -// created revision is ready. -func (c *Controller) markConfigurationReady( - ctx context.Context, - config *v1alpha1.Configuration, revision *v1alpha1.Revision) { - logger := logging.FromContext(ctx) - logger.Info("Marking Configuration ready") - config.Status.RemoveCondition(v1alpha1.ConfigurationConditionLatestRevisionReady) - config.Status.SetCondition( - &v1alpha1.ConfigurationCondition{ - Type: v1alpha1.ConfigurationConditionReady, - Status: corev1.ConditionTrue, - Reason: "LatestRevisionReady", - }) -} - -// Mark ConfigurationConditionLatestRevisionReady of Configuration to false with the status -// from the revision -func (c *Controller) markConfigurationLatestRevisionStatus( - ctx context.Context, - config *v1alpha1.Configuration, revision *v1alpha1.Revision) { - logger := logging.FromContext(ctx) - config.Status.RemoveCondition(v1alpha1.ConfigurationConditionReady) - cond := getLatestRevisionStatusCondition(revision) - if cond == nil { - logger.Info("Revision status is not updated yet") - return - } - config.Status.SetCondition( - &v1alpha1.ConfigurationCondition{ - Type: v1alpha1.ConfigurationConditionLatestRevisionReady, - Status: corev1.ConditionFalse, - Reason: cond.Reason, - Message: cond.Message, - }) -} diff --git a/pkg/controller/configuration/configuration_test.go b/pkg/controller/configuration/configuration_test.go index c729c9925f90..373deca93589 100644 --- a/pkg/controller/configuration/configuration_test.go +++ b/pkg/controller/configuration/configuration_test.go @@ -305,8 +305,8 @@ func TestMarkConfigurationReadyWhenLatestRevisionReady(t *testing.T) { t.Fatalf("Couldn't get config: %v", err) } - // Config should not have any conditions after reconcile - if got, want := len(reconciledConfig.Status.Conditions), 0; got != want { + // Config should be initialized with its conditions as Unknown. + if got, want := len(reconciledConfig.Status.Conditions), 2; got != want { t.Errorf("Conditions length diff; got %v, want %v", got, want) } // Config should not have a latest ready revision @@ -340,16 +340,18 @@ func TestMarkConfigurationReadyWhenLatestRevisionReady(t *testing.T) { t.Fatalf("Couldn't get config: %v", err) } - expectedConfigConditions := []v1alpha1.ConfigurationCondition{ - v1alpha1.ConfigurationCondition{ - Type: v1alpha1.ConfigurationConditionReady, + for _, ct := range []v1alpha1.ConfigurationConditionType{"Ready"} { + got := readyConfig.Status.GetCondition(ct) + want := &v1alpha1.ConfigurationCondition{ + Type: ct, Status: corev1.ConditionTrue, - Reason: "LatestRevisionReady", - }, - } - if diff := cmp.Diff(expectedConfigConditions, readyConfig.Status.Conditions); diff != "" { - t.Errorf("Unexpected config conditions diff (-want +got): %v", diff) + // TODO(mattmoor): LastTransitionTime: got.LastTransitionTime, + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("Unexpected config conditions diff (-want +got): %v", diff) + } } + if got, want := readyConfig.Status.LatestReadyRevisionName, revision.Name; got != want { t.Errorf("Latest in Status diff; got %v, want %v", got, want) } @@ -443,13 +445,11 @@ func TestDoNotUpdateConfigurationWhenLatestReadyRevisionNameIsUpToDate(t *testin config := getTestConfiguration() config.Status = v1alpha1.ConfigurationStatus{ - Conditions: []v1alpha1.ConfigurationCondition{ - v1alpha1.ConfigurationCondition{ - Type: v1alpha1.ConfigurationConditionReady, - Status: corev1.ConditionTrue, - Reason: "LatestRevisionReady", - }, - }, + Conditions: []v1alpha1.ConfigurationCondition{{ + Type: v1alpha1.ConfigurationConditionReady, + Status: corev1.ConditionTrue, + Reason: "LatestRevisionReady", + }}, LatestCreatedRevisionName: revName, LatestReadyRevisionName: revName, } @@ -482,7 +482,7 @@ func TestMarkConfigurationStatusWhenLatestRevisionIsNotReady(t *testing.T) { // Events are delivered asynchronously so we need to use hooks here. Each hook // tests for a specific event. h := NewHooks() - h.OnCreate(&kubeClient.Fake, "events", ExpectNormalEventDelivery(t, "Latest revision of configuration is not ready")) + h.OnCreate(&kubeClient.Fake, "events", ExpectWarningEventDelivery(t, `Latest created revision "test-config-00001" has failed`)) configClient.Create(config) // Since syncHandler looks in the lister, we need to add it to the informer @@ -503,14 +503,13 @@ func TestMarkConfigurationStatusWhenLatestRevisionIsNotReady(t *testing.T) { revision := revList.Items[0] // mark the revision not ready with the status - revision.Status = v1alpha1.RevisionStatus{ - Conditions: []v1alpha1.RevisionCondition{{ - Type: v1alpha1.RevisionConditionReady, - Status: corev1.ConditionFalse, - Reason: "BuildFailed", - Message: "Build step failed with error", - }}, - } + revision.Status.MarkBuildFailed(&buildv1alpha1.BuildCondition{ + Type: buildv1alpha1.BuildSucceeded, + Status: corev1.ConditionFalse, + Reason: "StepFailed", + Message: "Build step failed with error", + }) + // Since addRevisionEvent looks in the lister, we need to add it to the informer elaInformer.Serving().V1alpha1().Configurations().Informer().GetIndexer().Add(reconciledConfig) controller.addRevisionEvent(&revision) @@ -520,16 +519,18 @@ func TestMarkConfigurationStatusWhenLatestRevisionIsNotReady(t *testing.T) { t.Fatalf("Couldn't get config: %v", err) } - expectedConfigConditions := []v1alpha1.ConfigurationCondition{ - v1alpha1.ConfigurationCondition{ - Type: v1alpha1.ConfigurationConditionLatestRevisionReady, + for _, ct := range []v1alpha1.ConfigurationConditionType{"LatestRevisionReady"} { + got := readyConfig.Status.GetCondition(ct) + want := &v1alpha1.ConfigurationCondition{ + Type: ct, Status: corev1.ConditionFalse, - Reason: "BuildFailed", - Message: "Build step failed with error", - }, - } - if diff := cmp.Diff(expectedConfigConditions, readyConfig.Status.Conditions); diff != "" { - t.Errorf("Unexpected config conditions diff (-want +got): %v", diff) + Reason: "RevisionFailed", + Message: `revision "test-config-00001" failed with message: Build step failed with error`, + // TODO(mattmoor): LastTransitionTime: got.LastTransitionTime, + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("Unexpected config conditions diff (-want +got): %v", diff) + } } if got, want := readyConfig.Status.LatestCreatedRevisionName, revision.Name; got != want { @@ -553,14 +554,12 @@ func TestMarkConfigurationReadyWhenLatestRevisionRecovers(t *testing.T) { config := getTestConfiguration() config.Status.LatestCreatedRevisionName = revName - config.Status.Conditions = []v1alpha1.ConfigurationCondition{ - v1alpha1.ConfigurationCondition{ - Type: v1alpha1.ConfigurationConditionLatestRevisionReady, - Status: corev1.ConditionFalse, - Reason: "BuildFailed", - Message: "Build step failed with error", - }, - } + config.Status.Conditions = []v1alpha1.ConfigurationCondition{{ + Type: v1alpha1.ConfigurationConditionLatestRevisionReady, + Status: corev1.ConditionFalse, + Reason: "BuildFailed", + Message: "Build step failed with error", + }} // Events are delivered asynchronously so we need to use hooks here. Each hook // tests for a specific event. h := NewHooks() @@ -588,16 +587,18 @@ func TestMarkConfigurationReadyWhenLatestRevisionRecovers(t *testing.T) { t.Fatalf("Couldn't get config: %v", err) } - expectedConfigConditions := []v1alpha1.ConfigurationCondition{ - v1alpha1.ConfigurationCondition{ - Type: v1alpha1.ConfigurationConditionReady, + for _, ct := range []v1alpha1.ConfigurationConditionType{"Ready"} { + got := readyConfig.Status.GetCondition(ct) + want := &v1alpha1.ConfigurationCondition{ + Type: ct, Status: corev1.ConditionTrue, - Reason: "LatestRevisionReady", - }, - } - if diff := cmp.Diff(expectedConfigConditions, readyConfig.Status.Conditions); diff != "" { - t.Errorf("Unexpected config conditions diff (-want +got): %v", diff) + // TODO(mattmoor): LastTransitionTime: got.LastTransitionTime, + } + if diff := cmp.Diff(want, got); diff != "" { + t.Errorf("Unexpected config conditions diff (-want +got): %v", diff) + } } + if got, want := readyConfig.Status.LatestReadyRevisionName, revision.Name; got != want { t.Errorf("LatestReadyRevision do not match; got %v, want %v", got, want) }