Add LatestRevisionReady condition in Configuration

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.

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,15 +457,44 @@ 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,
 			Status: corev1.ConditionTrue,
 			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,
+		})
+}

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)
+	}
+}