Adds Degraded and Progressing status conditions

pkg/operator/controller/controller.go

@@ -11,6 +11,7 @@ import (
 	operatorclient "github.com/openshift/cluster-ingress-operator/pkg/operator/client"
 	"github.com/openshift/cluster-ingress-operator/pkg/util/slice"
 
+	appsv1 "k8s.io/api/apps/v1"
 	corev1 "k8s.io/api/core/v1"
 	"k8s.io/client-go/tools/record"
 
@@ -194,16 +195,7 @@ func (r *reconciler) enforceEffectiveIngressDomain(ic *operatorv1.IngressControl
 	}
 	if !unique {
 		log.Info("domain not unique, not setting status domain for IngressController", "namespace", ic.Namespace, "name", ic.Name)
-		availableCondition := operatorv1.OperatorCondition{
-			Type:    operatorv1.IngressControllerAvailableConditionType,
-			Status:  operatorv1.ConditionFalse,
-			Reason:  "InvalidDomain",
-			Message: fmt.Sprintf("domain %q is already in use by another IngressController", domain),
-		}
-		oldAvailableCondition := getIngressAvailableCondition(updated.Status.Conditions)
-		setIngressLastTransitionTime(&availableCondition, oldAvailableCondition)
-		// TODO: refactor when we deal with multiple ingress conditions
-		updated.Status.Conditions = []operatorv1.OperatorCondition{availableCondition}
+		r.syncIngressControllerStatus(&appsv1.Deployment{}, ic, false)
 	} else {
 		updated.Status.Domain = domain
 	}
@@ -395,7 +387,7 @@ func (r *reconciler) ensureIngressController(ci *operatorv1.IngressController, d
 			errs = append(errs, fmt.Errorf("failed to integrate metrics with openshift-monitoring for ingresscontroller %s: %v", ci.Name, err))
 		}
 
-		if err := r.syncIngressControllerStatus(deployment, ci); err != nil {
+		if err := r.syncIngressControllerStatus(deployment, ci, true); err != nil {
 			errs = append(errs, fmt.Errorf("failed to sync ingresscontroller status: %v", err))
 		}
 	}

pkg/operator/controller/ingress_status.go

@@ -15,7 +15,8 @@ import (
 
 // syncIngressControllerStatus computes the current status of ic and
 // updates status upon any changes since last sync.
-func (r *reconciler) syncIngressControllerStatus(deployment *appsv1.Deployment, ic *operatorv1.IngressController) error {
+func (r *reconciler) syncIngressControllerStatus(deployment *appsv1.Deployment, ic *operatorv1.IngressController,
+	uniqueDomain bool) error {
 	selector, err := metav1.LabelSelectorAsSelector(deployment.Spec.Selector)
 	if err != nil {
 		return fmt.Errorf("deployment has invalid spec.selector: %v", err)
@@ -24,7 +25,7 @@ func (r *reconciler) syncIngressControllerStatus(deployment *appsv1.Deployment,
 	updated := ic.DeepCopy()
 	updated.Status.AvailableReplicas = deployment.Status.AvailableReplicas
 	updated.Status.Selector = selector.String()
-	updated.Status.Conditions = computeIngressStatusConditions(updated.Status.Conditions, deployment)
+	updated.Status.Conditions = computeIngressStatusConditions(updated.Status.Conditions, deployment, uniqueDomain)
 	if !ingressStatusesEqual(updated.Status, ic.Status) {
 		if err := r.client.Status().Update(context.TODO(), updated); err != nil {
 			return fmt.Errorf("failed to update ingresscontroller status: %v", err)
@@ -35,26 +36,123 @@ func (r *reconciler) syncIngressControllerStatus(deployment *appsv1.Deployment,
 }
 
 // computeIngressStatusConditions computes the ingress controller's current state.
-func computeIngressStatusConditions(oldConditions []operatorv1.OperatorCondition, deployment *appsv1.Deployment) []operatorv1.OperatorCondition {
+func computeIngressStatusConditions(oldConditions []operatorv1.OperatorCondition, deployment *appsv1.Deployment,
+	uniqueDomain bool) []operatorv1.OperatorCondition {
+	oldDegradedCondition := getIngressDegradedCondition(oldConditions)
+	oldProgressingCondition := getIngressProgressingCondition(oldConditions)
 	oldAvailableCondition := getIngressAvailableCondition(oldConditions)
 
 	return []operatorv1.OperatorCondition{
-		computeIngressAvailableCondition(oldAvailableCondition, deployment),
+		computeIngressDegradedCondition(oldDegradedCondition, deployment, uniqueDomain),
+		computeIngressProgressingCondition(oldProgressingCondition, deployment, uniqueDomain),
+		computeIngressAvailableCondition(oldAvailableCondition, deployment, uniqueDomain),
 	}
 }
 
+// computeIngressDegradedCondition computes the ingress controller's current Degraded status state.
+func computeIngressDegradedCondition(oldDegradedCondition *operatorv1.OperatorCondition, deployment *appsv1.Deployment,
+	uniqueDomain bool) operatorv1.OperatorCondition {
+	degradedCondition := operatorv1.OperatorCondition{
+		Type: operatorv1.OperatorStatusTypeDegraded,
+	}
+
+	switch {
+	case !uniqueDomain:
+		degradedCondition.Status = operatorv1.ConditionTrue
+		degradedCondition.Reason = "DomainNotSet"
+		degradedCondition.Message = "IngressController status domain not set"
+	case deployment.Status.AvailableReplicas == 0:
+		degradedCondition.Status = operatorv1.ConditionTrue
+		degradedCondition.Reason = "DeploymentDegraded"
+		degradedCondition.Message = "No Deployment replicas available"
+	case deployment.Status.UnavailableReplicas != 0:
+		degradedCondition.Status = operatorv1.ConditionTrue
+		degradedCondition.Reason = "DeploymentUnavailable"
+		degradedCondition.Message = "Deployment contains an unavailable replica"
+	default:
+		degradedCondition.Status = operatorv1.ConditionFalse
+		degradedCondition.Reason = "AsExpected"
+		degradedCondition.Message = "All Deployment replicas available"
+	}
+
+	setIngressLastTransitionTime(&degradedCondition, oldDegradedCondition)
+	return degradedCondition
+}
+
+// getIngressDegradedCondition fetches ingress controller's degraded condition from the given conditions.
+func getIngressDegradedCondition(conditions []operatorv1.OperatorCondition) *operatorv1.OperatorCondition {
+	var degradedCondition *operatorv1.OperatorCondition
+	for i := range conditions {
+		switch conditions[i].Type {
+		case operatorv1.OperatorStatusTypeDegraded:
+			degradedCondition = &conditions[i]
+			break
+		}
+	}
+
+	return degradedCondition
+}
+
+// computeIngressProgressingCondition computes the ingress controller's current Progressing status state.
+func computeIngressProgressingCondition(oldProgressingCondition *operatorv1.OperatorCondition, deployment *appsv1.Deployment,
+	uniqueDomain bool) operatorv1.OperatorCondition {
+	progressingCondition := operatorv1.OperatorCondition{
+		Type: operatorv1.OperatorStatusTypeProgressing,
+	}
+
+	switch {
+	case !uniqueDomain:
+		progressingCondition.Status = operatorv1.ConditionFalse
+		progressingCondition.Reason = "DomainNotSet"
+		progressingCondition.Message = "IngressController status domain not set"
+	case deployment.Status.UnavailableReplicas != 0:
+		progressingCondition.Status = operatorv1.ConditionTrue
+		progressingCondition.Reason = "DeploymentUnavailable"
+		progressingCondition.Message = "Deployment contains 1 or more unavailable replicas"
+	default:
+		progressingCondition.Status = operatorv1.ConditionFalse
+		progressingCondition.Reason = "AsExpected"
+		progressingCondition.Message = "All expected Deployment replicas are available"
+	}
+
+	setIngressLastTransitionTime(&progressingCondition, oldProgressingCondition)
+	return progressingCondition
+}
+
+// getIngressProgressingCondition fetches ingress controller's progressing condition from the given conditions.
+func getIngressProgressingCondition(conditions []operatorv1.OperatorCondition) *operatorv1.OperatorCondition {
+	var progressingCondition *operatorv1.OperatorCondition
+	for i := range conditions {
+		switch conditions[i].Type {
+		case operatorv1.OperatorStatusTypeProgressing:
+			progressingCondition = &conditions[i]
+			break
+		}
+	}
+
+	return progressingCondition
+}
+
 // computeIngressAvailableCondition computes the ingress controller's current Available status state.
-func computeIngressAvailableCondition(oldAvailableCondition *operatorv1.OperatorCondition, deployment *appsv1.Deployment) operatorv1.OperatorCondition {
+func computeIngressAvailableCondition(oldAvailableCondition *operatorv1.OperatorCondition, deployment *appsv1.Deployment,
+	uniqueDomain bool) operatorv1.OperatorCondition {
 	availableCondition := operatorv1.OperatorCondition{
 		Type: operatorv1.IngressControllerAvailableConditionType,
 	}
 
-	if deployment.Status.AvailableReplicas > 0 {
-		availableCondition.Status = operatorv1.ConditionTrue
-	} else {
+	switch {
+	case !uniqueDomain:
+		availableCondition.Status = operatorv1.ConditionFalse
+		availableCondition.Reason = "DomainNotSet"
+		availableCondition.Message = "IngressController status domain not set"
+	case deployment.Status.AvailableReplicas == 0:
 		availableCondition.Status = operatorv1.ConditionFalse
 		availableCondition.Reason = "DeploymentUnavailable"
-		availableCondition.Message = "no Deployment replicas available"
+		availableCondition.Message = "No Deployment replicas available"
+	default:
+		availableCondition.Status = operatorv1.ConditionTrue
+		availableCondition.Reason = "AsExpected"
+		availableCondition.Message = "At least 1 Deployment replica is available"
 	}
 
 	setIngressLastTransitionTime(&availableCondition, oldAvailableCondition)

pkg/operator/controller/ingress_status_test.go

@@ -14,35 +14,86 @@ import (
 )
 
 func TestComputeIngressStatusConditions(t *testing.T) {
+	type testInputs struct {
+		domainSet              bool
+		unavailRepl, availRepl int32
+	}
+	type testOutputs struct {
+		degraded, progressing, available bool
+	}
 	testCases := []struct {
-		description     string
-		availRepl, repl int32
-		condType        string
-		condStatus      operatorv1.ConditionStatus
+		description string
+		inputs      testInputs
+		outputs     testOutputs
 	}{
-		{"0/2 deployment replicas available", 0, 2, operatorv1.OperatorStatusTypeAvailable, operatorv1.ConditionFalse},
-		{"1/2 deployment replicas available", 1, 2, operatorv1.OperatorStatusTypeAvailable, operatorv1.ConditionTrue},
-		{"2/2 deployment replicas available", 2, 2, operatorv1.OperatorStatusTypeAvailable, operatorv1.ConditionTrue},
+		{
+			description: "0/0 deployment replicas available",
+			inputs:      testInputs{true, 0, 0},
+			outputs:     testOutputs{true, false, false},
+		},
+		{
+			description: "no status domain set",
+			inputs:      testInputs{false, 0, 0},
+			outputs:     testOutputs{true, false, false},
+		},
+		{
+			description: "0/2 deployment replicas available",
+			inputs:      testInputs{true, 2, 0},
+			outputs:     testOutputs{true, true, false},
+		},
+		{
+			description: "1/2 deployment replicas available",
+			inputs:      testInputs{true, 1, 1},
+			outputs:     testOutputs{true, true, true},
+		},
+		{
+			description: "2/2 deployment replicas available",
+			inputs:      testInputs{true, 0, 2},
+			outputs:     testOutputs{false, false, true},
+		},
 	}
 
 	for i, tc := range testCases {
+		var degraded, progressing, available operatorv1.ConditionStatus
 		deploy := &appsv1.Deployment{
 			ObjectMeta: metav1.ObjectMeta{
 				Name: fmt.Sprintf("ingress-controller-%d", i+1),
 			},
 			Status: appsv1.DeploymentStatus{
-				Replicas:          tc.repl,
-				AvailableReplicas: tc.availRepl,
+				UnavailableReplicas: tc.inputs.unavailRepl,
+				AvailableReplicas:   tc.inputs.availRepl,
 			},
 		}
-
+		if tc.outputs.degraded {
+			degraded = operatorv1.ConditionTrue
+		} else {
+			degraded = operatorv1.ConditionFalse
+		}
+		if tc.outputs.progressing {
+			progressing = operatorv1.ConditionTrue
+		} else {
+			progressing = operatorv1.ConditionFalse
+		}
+		if tc.outputs.available {
+			available = operatorv1.ConditionTrue
+		} else {
+			available = operatorv1.ConditionFalse
+		}
 		expected := []operatorv1.OperatorCondition{
 			{
-				Type:   tc.condType,
-				Status: tc.condStatus,
+				Type:   operatorv1.OperatorStatusTypeDegraded,
+				Status: degraded,
+			},
+			{
+				Type:   operatorv1.OperatorStatusTypeProgressing,
+				Status: progressing,
+			},
+			{
+				Type:   operatorv1.OperatorStatusTypeAvailable,
+				Status: available,
 			},
 		}
-		actual := computeIngressStatusConditions([]operatorv1.OperatorCondition{}, deploy)
+		actual := computeIngressStatusConditions([]operatorv1.OperatorCondition{}, deploy, tc.inputs.domainSet)
 		conditionsCmpOpts := []cmp.Option{
 			cmpopts.IgnoreFields(operatorv1.OperatorCondition{}, "LastTransitionTime", "Reason", "Message"),
 			cmpopts.EquateEmpty(),
@@ -54,6 +105,76 @@ func TestComputeIngressStatusConditions(t *testing.T) {
 	}
 }
 
+func TestSetIngressLastTransitionTime(t *testing.T) {
+	type testInputs struct {
+		statusChange, reasonChange, msgChange bool
+	}
+	type testOutputs struct {
+		transition bool
+	}
+	testCases := []struct {
+		description string
+		inputs      testInputs
+		outputs     testOutputs
+	}{
+		{
+			description: "no condition changes",
+			inputs:      testInputs{false, false, false},
+			outputs:     testOutputs{false},
+		},
+		{
+			description: "condition status changed",
+			inputs:      testInputs{true, false, false},
+			outputs:     testOutputs{true},
+		},
+		{
+			description: "condition reason changed",
+			inputs:      testInputs{false, true, false},
+			outputs:     testOutputs{true},
+		},
+		{
+			description: "condition message changed",
+			inputs:      testInputs{false, false, true},
+			outputs:     testOutputs{true},
+		},
+	}
+
+	for _, tc := range testCases {
+		var (
+			reason                            = "old reason"
+			msg                               = "old msg"
+			status operatorv1.ConditionStatus = "old status"
+		)
+		oldCondition := &operatorv1.OperatorCondition{
+			Type:               operatorv1.OperatorStatusTypeAvailable,
+			Status:             status,
+			Reason:             reason,
+			Message:            msg,
+			LastTransitionTime: metav1.Unix(0, 0),
+		}
+		if tc.inputs.msgChange {
+			msg = "new message"
+		}
+		if tc.inputs.reasonChange {
+			reason = "new reason"
+		}
+		if tc.inputs.statusChange {
+			status = "new status"
+		}
+		expectCondition := &operatorv1.OperatorCondition{
+			Type:    operatorv1.OperatorStatusTypeAvailable,
+			Status:  status,
+			Reason:  reason,
+			Message: msg,
+		}
+		setIngressLastTransitionTime(expectCondition, oldCondition)
+		if tc.outputs.transition != (expectCondition.LastTransitionTime != oldCondition.LastTransitionTime) {
+			t.Fatalf(fmt.Sprintf("%q: expected LastTransitionTime %v, got %v", tc.description,
+				oldCondition.LastTransitionTime, expectCondition.LastTransitionTime))
+		}
+	}
+}
+
 func TestIngressStatusesEqual(t *testing.T) {
 	testCases := []struct {
 		description string

pkg/operator/controller/status.go

@@ -210,9 +210,6 @@ func computeOperatorDegradedCondition(oldCondition *configv1.ClusterOperatorStat
 // computeOperatorProgressingCondition computes the operator's current Progressing status state.
 func (r *reconciler) computeOperatorProgressingCondition(oldCondition *configv1.ClusterOperatorStatusCondition,
 	allIngressesAvailable bool, oldVersions, curVersions []configv1.OperandVersion) configv1.ClusterOperatorStatusCondition {
-	// TODO: Update progressingCondition when an ingresscontroller
-	//       progressing condition is created. The Operator's condition
-	//       should be derived from the ingresscontroller's condition.
 	progressingCondition := configv1.ClusterOperatorStatusCondition{
 		Type: configv1.OperatorProgressing,
 	}