diff --git a/pkg/apis/ela/v1alpha1/configuration_types.go b/pkg/apis/ela/v1alpha1/configuration_types.go index 0fca27df7414..b034376ab5c6 100644 --- a/pkg/apis/ela/v1alpha1/configuration_types.go +++ b/pkg/apis/ela/v1alpha1/configuration_types.go @@ -55,6 +55,9 @@ const ( // ConfigurationConditionReady is set when the configuration is starting to materialize // runtime resources, and becomes true when those resources are ready. ConfigurationConditionReady ConfigurationConditionType = "Ready" + // ConfigurationConditionLatestRevisionReady is set to indicate the status of the latest + // revision of the configuration when it has not become ready yet + ConfigurationConditionLatestRevisionReady ConfigurationConditionType = "LatestRevisionReady" ) // ConfigurationCondition defines a readiness condition for a Configuration. diff --git a/pkg/controller/configuration/configuration.go b/pkg/controller/configuration/configuration.go index f8c33f613296..011c4722990a 100644 --- a/pkg/controller/configuration/configuration.go +++ b/pkg/controller/configuration/configuration.go @@ -380,13 +380,6 @@ func (c *Controller) addRevisionEvent(obj interface{}) { return } - if !revision.Status.IsReady() { - // The revision isn't ready, so ignore this event. - glog.Infof("Revision %q is not ready", revisionName) - return - } - glog.Infof("Revision %q is ready", revisionName) - config, err := c.lister.Configurations(namespace).Get(configName) if err != nil { glog.Errorf("Error fetching configuration '%s/%s' upon revision becoming ready: %v", @@ -410,27 +403,42 @@ func (c *Controller) addRevisionEvent(obj interface{}) { // Don't modify the informer's copy. config = config.DeepCopy() - alreadyReady := config.Status.IsReady() - if !alreadyReady { - c.markConfigurationReady(config, revision) - } + if !revision.Status.IsReady() { + glog.Infof("Revision %q of configuration %q is not ready", revisionName, config.Name) - glog.Infof("Setting LatestReadyRevisionName of Configuration %q to revision %q", - config.Name, revision.Name) - config.Status.LatestReadyRevisionName = revision.Name + //add LatestRevision condition to be false with the status from the revision + c.markConfigurationLatestRevisionStatus(config, revision) - if _, err := c.updateStatus(config); err != nil { - glog.Errorf("Error updating configuration '%s/%s': %v", - namespace, configName, err) - } + if _, err := c.updateStatus(config); err != nil { + glog.Errorf("Error updating configuration '%s/%s': %v", + namespace, configName, err) + } + c.recorder.Eventf(config, corev1.EventTypeNormal, "LatestRevisionUpdate", + "Latest revision of configuration is not ready") - 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 { + glog.Infof("Revision %q is ready", revisionName) + + alreadyReady := config.Status.IsReady() + if !alreadyReady { + c.markConfigurationReady(config, revision) + } + glog.Infof("Setting LatestReadyRevisionName of Configuration %q to revision %q", + config.Name, revision.Name) + config.Status.LatestReadyRevisionName = revision.Name + + if _, err := c.updateStatus(config); err != nil { + glog.Errorf("Error updating configuration '%s/%s': %v", + namespace, configName, 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) + } return } @@ -449,11 +457,21 @@ func lookupRevisionOwner(revision *v1alpha1.Revision) string { return "" } +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( config *v1alpha1.Configuration, revision *v1alpha1.Revision) { glog.Infof("Marking Configuration %q ready", config.Name) + config.Status.RemoveCondition(v1alpha1.ConfigurationConditionLatestRevisionReady) config.Status.SetCondition( &v1alpha1.ConfigurationCondition{ Type: v1alpha1.ConfigurationConditionReady, @@ -461,3 +479,22 @@ func (c *Controller) markConfigurationReady( Reason: "LatestRevisionReady", }) } + +// Mark ConfigurationConditionLatestRevisionReady of Configuration to false with the status +// from the revision +func (c *Controller) markConfigurationLatestRevisionStatus( + config *v1alpha1.Configuration, revision *v1alpha1.Revision) { + config.Status.RemoveCondition(v1alpha1.ConfigurationConditionReady) + cond := getLatestRevisionStatusCondition(revision) + if cond == nil { + glog.Infof("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 015917656644..f1e03d7e2bc2 100644 --- a/pkg/controller/configuration/configuration_test.go +++ b/pkg/controller/configuration/configuration_test.go @@ -453,3 +453,139 @@ func TestDoNotUpdateConfigurationWhenLatestReadyRevisionNameIsUpToDate(t *testin controller.addRevisionEvent(revision) } + +func TestMarkConfigurationStatusWhenLatestRevisionIsNotReady(t *testing.T) { + kubeClient, elaClient, controller, _, elaInformer := newTestController(t) + configClient := elaClient.ElafrosV1alpha1().Configurations(testNamespace) + + config := getTestConfiguration() + config.Status.LatestCreatedRevisionName = revName + + // 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")) + + configClient.Create(config) + // Since syncHandler looks in the lister, we need to add it to the informer + elaInformer.Elafros().V1alpha1().Configurations().Informer().GetIndexer().Add(config) + controller.syncHandler(keyOrDie(config)) + + reconciledConfig, err := configClient.Get(config.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Couldn't get config: %v", err) + } + + // Get the revision created + revList, err := elaClient.ElafrosV1alpha1().Revisions(config.Namespace).List(metav1.ListOptions{}) + if err != nil { + t.Fatalf("error listing revisions: %v", err) + } + + 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", + }}, + } + // Since addRevisionEvent looks in the lister, we need to add it to the informer + elaInformer.Elafros().V1alpha1().Configurations().Informer().GetIndexer().Add(reconciledConfig) + controller.addRevisionEvent(&revision) + + readyConfig, err := configClient.Get(config.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Couldn't get config: %v", err) + } + + expectedConfigConditions := []v1alpha1.ConfigurationCondition{ + v1alpha1.ConfigurationCondition{ + Type: v1alpha1.ConfigurationConditionLatestRevisionReady, + 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) + } + + if got, want := readyConfig.Status.LatestCreatedRevisionName, revision.Name; got != want { + t.Errorf("LatestCreatedRevision do not match; got %v, want %v", got, want) + } + + if got, want := readyConfig.Status.LatestReadyRevisionName, ""; got != want { + t.Errorf("LatestReadyRevision should be empty; got %v, want %v", got, want) + } + + // wait for events to be created + if err := h.WaitForHooks(time.Second * 3); err != nil { + t.Error(err) + } +} + +func TestMarkConfigurationReadyWhenLatestRevisionRecovers(t *testing.T) { + kubeClient, elaClient, controller, _, elaInformer := newTestController(t) + configClient := elaClient.ElafrosV1alpha1().Configurations(testNamespace) + + 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", + }, + } + // 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, "Configuration becomes ready")) + h.OnCreate(&kubeClient.Fake, "events", ExpectNormalEventDelivery(t, "LatestReadyRevisionName updated to .+")) + + configClient.Create(config) + + controllerRef := metav1.NewControllerRef(config, controllerKind) + revision := getTestRevision() + revision.OwnerReferences = append(revision.OwnerReferences, *controllerRef) + // mark the revision as Ready + revision.Status = v1alpha1.RevisionStatus{ + Conditions: []v1alpha1.RevisionCondition{{ + Type: v1alpha1.RevisionConditionReady, + Status: corev1.ConditionTrue, + }}, + } + // Since addRevisionEvent looks in the lister, we need to add it to the informer + elaInformer.Elafros().V1alpha1().Configurations().Informer().GetIndexer().Add(config) + controller.addRevisionEvent(revision) + + readyConfig, err := configClient.Get(config.Name, metav1.GetOptions{}) + if err != nil { + t.Fatalf("Couldn't get config: %v", err) + } + + expectedConfigConditions := []v1alpha1.ConfigurationCondition{ + v1alpha1.ConfigurationCondition{ + Type: v1alpha1.ConfigurationConditionReady, + Status: corev1.ConditionTrue, + Reason: "LatestRevisionReady", + }, + } + if diff := cmp.Diff(expectedConfigConditions, readyConfig.Status.Conditions); 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) + } + + // wait for events to be created + if err := h.WaitForHooks(time.Second * 3); err != nil { + t.Error(err) + } +}