diff --git a/pkg/operator/controller/controller.go b/pkg/operator/controller/controller.go index cb043b5a26..75ace4a26b 100644 --- a/pkg/operator/controller/controller.go +++ b/pkg/operator/controller/controller.go @@ -131,47 +131,53 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err } if ingress != nil { - dnsConfig := &configv1.DNS{} - if err := r.client.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, dnsConfig); err != nil { - errs = append(errs, fmt.Errorf("failed to get dns 'cluster': %v", err)) - dnsConfig = nil - } - infraConfig := &configv1.Infrastructure{} - if err := r.client.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, infraConfig); err != nil { - errs = append(errs, fmt.Errorf("failed to get infrastructure 'cluster': %v", err)) - infraConfig = nil - } - ingressConfig := &configv1.Ingress{} - if err := r.client.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, ingressConfig); err != nil { - errs = append(errs, fmt.Errorf("failed to get ingress 'cluster': %v", err)) - ingressConfig = nil - } - - // For now, if the cluster configs are unavailable, defer reconciliation - // because weaving conditionals everywhere to deal with various nil states - // is too complicated. It doesn't seem too risky to rely on the invariant - // of the cluster config being available. - if dnsConfig != nil && infraConfig != nil && ingressConfig != nil { - // Ensure we have all the necessary scaffolding on which to place router instances. - if err := r.ensureRouterNamespace(); err != nil { - errs = append(errs, fmt.Errorf("failed to ensure router namespace: %v", err)) + // Delete the ingress if the deletion timestamp is present. + if ingress.DeletionTimestamp != nil { + if err := r.ensureIngressDeleted(ingress); err != nil { + errs = append(errs, fmt.Errorf("failed to ensure ingress deletion: %v", err)) } - - if err := r.enforceEffectiveIngressDomain(ingress, ingressConfig); err != nil { - errs = append(errs, fmt.Errorf("failed to enforce the effective ingress domain for ingresscontroller %s: %v", ingress.Name, err)) - } else if IsStatusDomainSet(ingress) { - if err := r.enforceEffectiveEndpointPublishingStrategy(ingress, infraConfig); err != nil { - errs = append(errs, fmt.Errorf("failed to enforce the effective HA configuration for ingresscontroller %s: %v", ingress.Name, err)) - } else if ingress.DeletionTimestamp != nil { - // Handle deletion. - if err := r.ensureIngressDeleted(ingress); err != nil { - errs = append(errs, fmt.Errorf("failed to ensure ingress deletion: %v", err)) + } else { + dnsConfig := &configv1.DNS{} + if err := r.client.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, dnsConfig); err != nil { + errs = append(errs, fmt.Errorf("failed to get dns 'cluster': %v", err)) + dnsConfig = nil + } + infraConfig := &configv1.Infrastructure{} + if err := r.client.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, infraConfig); err != nil { + errs = append(errs, fmt.Errorf("failed to get infrastructure 'cluster': %v", err)) + infraConfig = nil + } + ingressConfig := &configv1.Ingress{} + if err := r.client.Get(context.TODO(), types.NamespacedName{Name: "cluster"}, ingressConfig); err != nil { + errs = append(errs, fmt.Errorf("failed to get ingress 'cluster': %v", err)) + ingressConfig = nil + } + // For now, if the cluster configs are unavailable, defer reconciliation + // because weaving conditionals everywhere to deal with various nil states + // is too complicated. It doesn't seem too risky to rely on the invariant + // of the cluster config being available. + if dnsConfig != nil && infraConfig != nil && ingressConfig != nil { + // Ensure we have all the necessary scaffolding on which to place router instances. + if err := r.ensureRouterNamespace(); err != nil { + errs = append(errs, fmt.Errorf("failed to ensure router namespace: %v", err)) + // status.domain must be set before creating dependant resources. + } else if !IsStatusDomainSet(ingress) { + deployment := &appsv1.Deployment{} + service := &corev1.Service{} + operandEvents := []corev1.Event{} + wildcardRecord := &iov1.DNSRecord{} + if err := r.syncIngressControllerStatus(ingress, ingressConfig, deployment, service, operandEvents, wildcardRecord, dnsConfig); err != nil { + errs = append(errs, fmt.Errorf("failed to sync ingresscontroller status: %v", err)) } - } else if err := r.enforceIngressFinalizer(ingress); err != nil { - errs = append(errs, fmt.Errorf("failed to enforce ingress finalizer %s/%s: %v", ingress.Namespace, ingress.Name, err)) - } else { + // To avoid another reconciliation loop, check the status.domain again as + // syncIngressControllerStatus may have not set it. + } else if IsStatusDomainSet(ingress) { // Handle everything else. - if err := r.ensureIngressController(ingress, dnsConfig, infraConfig); err != nil { + if err := r.enforceEffectiveEndpointPublishingStrategy(ingress, infraConfig); err != nil { + errs = append(errs, fmt.Errorf("failed to enforce the effective HA configuration for ingresscontroller %s: %v", ingress.Name, err)) + } else if err := r.enforceIngressFinalizer(ingress); err != nil { + errs = append(errs, fmt.Errorf("failed to enforce ingress finalizer %s/%s: %v", ingress.Namespace, ingress.Name, err)) + } else if err := r.ensureIngressController(ingress, ingressConfig, dnsConfig, infraConfig); err != nil { errs = append(errs, fmt.Errorf("failed to ensure ingresscontroller: %v", err)) } } @@ -187,74 +193,6 @@ func (r *reconciler) Reconcile(request reconcile.Request) (reconcile.Result, err return result, utilerrors.NewAggregate(errs) } -// enforceEffectiveIngressDomain determines the effective ingress domain for the -// given ingresscontroller and ingress configuration and publishes it to the -// ingresscontroller's status. -func (r *reconciler) enforceEffectiveIngressDomain(ic *operatorv1.IngressController, ingressConfig *configv1.Ingress) error { - // The ingresscontroller's ingress domain is immutable, so if we have - // published a domain to status, we must continue using it. - if len(ic.Status.Domain) > 0 { - return nil - } - - updated := ic.DeepCopy() - var domain string - switch { - case len(ic.Spec.Domain) > 0: - domain = ic.Spec.Domain - default: - domain = ingressConfig.Spec.Domain - } - unique, err := r.isDomainUnique(domain) - if err != nil { - return err - } - 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} - } else { - updated.Status.Domain = domain - } - - if err := r.client.Status().Update(context.TODO(), updated); err != nil { - return fmt.Errorf("failed to update status of IngressController %s/%s: %v", updated.Namespace, updated.Name, err) - } - if err := r.client.Get(context.TODO(), types.NamespacedName{Namespace: updated.Namespace, Name: updated.Name}, ic); err != nil { - return fmt.Errorf("failed to get IngressController %s/%s: %v", updated.Namespace, updated.Name, err) - } - return nil -} - -// isDomainUnique compares domain with spec.domain of all ingress controllers -// and returns a false if a conflict exists or an error if the -// ingress controller list operation returns an error. -func (r *reconciler) isDomainUnique(domain string) (bool, error) { - ingresses := &operatorv1.IngressControllerList{} - if err := r.cache.List(context.TODO(), ingresses, client.InNamespace(r.Namespace)); err != nil { - return false, fmt.Errorf("failed to list ingresscontrollers: %v", err) - } - - // Compare domain with all ingress controllers for a conflict. - for _, ing := range ingresses.Items { - if domain == ing.Status.Domain { - log.Info("domain conflicts with existing IngressController", "domain", domain, "namespace", - ing.Namespace, "name", ing.Name) - return false, nil - } - } - - return true, nil -} - // publishingStrategyTypeForInfra returns the appropriate endpoint publishing // strategy type for the given infrastructure config. func publishingStrategyTypeForInfra(infraConfig *configv1.Infrastructure) operatorv1.EndpointPublishingStrategyType { @@ -412,7 +350,7 @@ func (r *reconciler) ensureRouterNamespace() error { } // ensureIngressController ensures all necessary router resources exist for a given ingresscontroller. -func (r *reconciler) ensureIngressController(ci *operatorv1.IngressController, dnsConfig *configv1.DNS, infraConfig *configv1.Infrastructure) error { +func (r *reconciler) ensureIngressController(ci *operatorv1.IngressController, ingressConfig *configv1.Ingress, dnsConfig *configv1.DNS, infraConfig *configv1.Infrastructure) error { errs := []error{} if deployment, err := r.ensureRouterDeployment(ci, infraConfig); err != nil { @@ -451,7 +389,7 @@ func (r *reconciler) ensureIngressController(ci *operatorv1.IngressController, d errs = append(errs, fmt.Errorf("failed to list events in namespace %q: %v", "openshift-ingress", err)) } - if err := r.syncIngressControllerStatus(ci, deployment, lbService, operandEvents.Items, wildcardRecord, dnsConfig); err != nil { + if err := r.syncIngressControllerStatus(ci, ingressConfig, deployment, lbService, operandEvents.Items, wildcardRecord, dnsConfig); err != nil { errs = append(errs, fmt.Errorf("failed to sync ingresscontroller status: %v", err)) } } diff --git a/pkg/operator/controller/ingress_status.go b/pkg/operator/controller/ingress_status.go index e5afd75d26..dffdec0462 100644 --- a/pkg/operator/controller/ingress_status.go +++ b/pkg/operator/controller/ingress_status.go @@ -15,26 +15,34 @@ import ( appsv1 "k8s.io/api/apps/v1" corev1 "k8s.io/api/core/v1" + "sigs.k8s.io/controller-runtime/pkg/client" + metav1 "k8s.io/apimachinery/pkg/apis/meta/v1" ) // syncIngressControllerStatus computes the current status of ic and // updates status upon any changes since last sync. -func (r *reconciler) syncIngressControllerStatus(ic *operatorv1.IngressController, deployment *appsv1.Deployment, service *corev1.Service, operandEvents []corev1.Event, wildcardRecord *iov1.DNSRecord, dnsConfig *configv1.DNS) error { - selector, err := metav1.LabelSelectorAsSelector(deployment.Spec.Selector) - if err != nil { - return fmt.Errorf("deployment has invalid spec.selector: %v", err) - } - +func (r *reconciler) syncIngressControllerStatus(ic *operatorv1.IngressController, ingressConfig *configv1.Ingress, deployment *appsv1.Deployment, service *corev1.Service, operandEvents []corev1.Event, wildcardRecord *iov1.DNSRecord, dnsConfig *configv1.DNS) error { updated := ic.DeepCopy() - updated.Status.AvailableReplicas = deployment.Status.AvailableReplicas - updated.Status.Selector = selector.String() - updated.Status.Conditions = []operatorv1.OperatorCondition{} - updated.Status.Conditions = append(updated.Status.Conditions, computeIngressStatusConditions(updated.Status.Conditions, deployment)...) - updated.Status.Conditions = append(updated.Status.Conditions, computeLoadBalancerStatus(ic, service, operandEvents)...) - updated.Status.Conditions = append(updated.Status.Conditions, computeDNSStatus(ic, wildcardRecord, dnsConfig)...) + if IsStatusDomainSet(ic) { + selector, err := metav1.LabelSelectorAsSelector(deployment.Spec.Selector) + if err != nil { + return fmt.Errorf("deployment has invalid spec.selector: %v", err) + } + updated.Status.AvailableReplicas = deployment.Status.AvailableReplicas + updated.Status.Selector = selector.String() + updated.Status.Conditions = append(updated.Status.Conditions, computeDeploymentStatus(deployment)) + updated.Status.Conditions = append(updated.Status.Conditions, computeLoadBalancerStatus(ic, service, operandEvents)...) + updated.Status.Conditions = append(updated.Status.Conditions, computeDNSStatus(ic, wildcardRecord, dnsConfig)...) + } else { + if err := r.enforceEffectiveIngressDomain(updated, ingressConfig); err != nil { + log.Error(err, "failed to enforce the effective ingress domain for ingresscontroller", "name", ic.Name) + } + } + + updated.Status.Conditions = append(updated.Status.Conditions, computeIngressStatusConditions(ic, updated.Status.Conditions)...) for i := range updated.Status.Conditions { newCondition := &updated.Status.Conditions[i] var oldCondition *operatorv1.OperatorCondition @@ -58,43 +66,75 @@ func (r *reconciler) syncIngressControllerStatus(ic *operatorv1.IngressControlle } // computeIngressStatusConditions computes the ingress controller's current state. -func computeIngressStatusConditions(oldConditions []operatorv1.OperatorCondition, deployment *appsv1.Deployment) []operatorv1.OperatorCondition { - oldAvailableCondition := getIngressAvailableCondition(oldConditions) - +func computeIngressStatusConditions(ic *operatorv1.IngressController, conditions []operatorv1.OperatorCondition) []operatorv1.OperatorCondition { return []operatorv1.OperatorCondition{ - computeIngressAvailableCondition(oldAvailableCondition, deployment), + computeIngressDegradedCondition(ic, conditions), + computeIngressAvailableCondition(ic, conditions), } } -// computeIngressAvailableCondition computes the ingress controller's current Available status state. -func computeIngressAvailableCondition(oldAvailableCondition *operatorv1.OperatorCondition, deployment *appsv1.Deployment) operatorv1.OperatorCondition { - availableCondition := operatorv1.OperatorCondition{ - Type: operatorv1.IngressControllerAvailableConditionType, +// computeIngressDegradedCondition computes the ingress controller's current Degraded status state. +func computeIngressDegradedCondition(ic *operatorv1.IngressController, conditions []operatorv1.OperatorCondition) operatorv1.OperatorCondition { + condition := operatorv1.OperatorCondition{ + Type: operatorv1.OperatorStatusTypeDegraded, } - if deployment.Status.AvailableReplicas > 0 { - availableCondition.Status = operatorv1.ConditionTrue - } else { - availableCondition.Status = operatorv1.ConditionFalse - availableCondition.Reason = "DeploymentUnavailable" - availableCondition.Message = "no Deployment replicas available" + matchedCondition := false + for _, c := range conditions { + if c.Status == operatorv1.ConditionFalse { + matchedCondition = true + break + } + } + + switch { + case len(ic.Status.Domain) == 0: + condition.Status = operatorv1.ConditionTrue + condition.Reason = "StatusDomainUnset" + condition.Message = "The IngressController status domain is not set" + case matchedCondition: + condition.Status = operatorv1.ConditionTrue + condition.Reason = "DependantResourceFailure" + condition.Message = "The IngressController contains a failed dependant resource" + default: + condition.Status = operatorv1.ConditionFalse + condition.Reason = "AsExpected" + condition.Message = "All IngressController dependant resources are available" } - return availableCondition + return condition } -// getIngressAvailableCondition fetches ingress controller's available condition from the given conditions. -func getIngressAvailableCondition(conditions []operatorv1.OperatorCondition) *operatorv1.OperatorCondition { - var availableCondition *operatorv1.OperatorCondition - for i := range conditions { - switch conditions[i].Type { - case operatorv1.IngressControllerAvailableConditionType: - availableCondition = &conditions[i] +// computeIngressAvailableCondition computes the ingress controller's current Available status state. +func computeIngressAvailableCondition(ic *operatorv1.IngressController, conditions []operatorv1.OperatorCondition) operatorv1.OperatorCondition { + condition := operatorv1.OperatorCondition{ + Type: operatorv1.OperatorStatusTypeAvailable, + } + + matchedCondition := false + for _, c := range conditions { + if c.Status == operatorv1.ConditionFalse { + matchedCondition = true break } } - return availableCondition + switch { + case len(ic.Status.Domain) == 0: + condition.Status = operatorv1.ConditionFalse + condition.Reason = "StatusDomainUnset" + condition.Message = "The IngressController status domain is not set" + case matchedCondition: + condition.Status = operatorv1.ConditionFalse + condition.Reason = "DependantResourceFailure" + condition.Message = "The IngressController contains a failed dependant resource" + default: + condition.Status = operatorv1.ConditionTrue + condition.Reason = "AsExpected" + condition.Message = "All IngressControllers dependant resources are available" + } + + return condition } // setIngressLastTransitionTime sets LastTransitionTime for the given ingress controller condition. @@ -124,6 +164,31 @@ func ingressStatusesEqual(a, b operatorv1.IngressControllerStatus) bool { return true } +// computeDeploymentStatus returns a DeploymentReady status condition based +// on the status of the given Deployment. +func computeDeploymentStatus(deployment *appsv1.Deployment) operatorv1.OperatorCondition { + condition := operatorv1.OperatorCondition{ + Type: "DeploymentReady", + } + + switch { + case deployment.Spec.Replicas != nil && deployment.Status.AvailableReplicas == 0: + condition.Status = operatorv1.ConditionFalse + condition.Reason = "DeploymentDegraded" + condition.Message = "The Deployment has no available replicas" + case deployment.Status.UnavailableReplicas != 0: + condition.Status = operatorv1.ConditionFalse + condition.Reason = "DeploymentUnavailable" + condition.Message = "The Deployment contains an unavailable replica" + default: + condition.Status = operatorv1.ConditionTrue + condition.Reason = "DeploymentAvailable" + condition.Message = "All Deployment replicas are available" + } + + return condition +} + // computeLoadBalancerStatus returns the complete set of current // LoadBalancer-prefixed conditions for the given ingress controller. func computeLoadBalancerStatus(ic *operatorv1.IngressController, service *corev1.Service, operandEvents []corev1.Event) []operatorv1.OperatorCondition { @@ -286,3 +351,56 @@ func computeDNSStatus(ic *operatorv1.IngressController, wildcardRecord *iov1.DNS return conditions } + +// enforceEffectiveIngressDomain determines the effective ingress domain +// for the given ic and ingressConfig. +func (r *reconciler) enforceEffectiveIngressDomain(ic *operatorv1.IngressController, ingressConfig *configv1.Ingress) error { + var domain string + + switch { + case isSpecDomainSet(ic): + domain = ic.Spec.Domain + default: + domain = ingressConfig.Spec.Domain + } + uniqueDomain, err := r.isDomainUnique(domain) + if err != nil { + return err + } + if uniqueDomain { + ic.Status.Domain = domain + } else { + return fmt.Errorf("non unique domain, status.domain for IngressController %s/%s not set", ic.Namespace, ic.Name) + } + + return nil +} + +// isSpecDomainSet checks whether spec.domain of ingress is set. +func isSpecDomainSet(ingress *operatorv1.IngressController) bool { + if len(ingress.Spec.Domain) == 0 { + return false + } + return true +} + +// isDomainUnique compares domain with status.domain of all ingress controllers +// returning false if a conflict exists or an error if the ingress controller +// list operation returns an error. +func (r *reconciler) isDomainUnique(domain string) (bool, error) { + ingresses := &operatorv1.IngressControllerList{} + if err := r.client.List(context.TODO(), ingresses, client.InNamespace(r.Namespace)); err != nil { + return false, fmt.Errorf("failed to list ingresscontrollers: %v", err) + } + + // Compare domain with all ingress controllers for a conflict. + for _, ing := range ingresses.Items { + if domain == ing.Status.Domain { + log.Info("domain conflicts with existing IngressController", "domain", domain, "namespace", + ing.Namespace, "name", ing.Name) + return false, nil + } + } + + return true, nil +} diff --git a/pkg/operator/controller/ingress_status_test.go b/pkg/operator/controller/ingress_status_test.go index 4106a67069..51c7946aee 100644 --- a/pkg/operator/controller/ingress_status_test.go +++ b/pkg/operator/controller/ingress_status_test.go @@ -190,36 +190,59 @@ func TestComputeLoadBalancerStatus(t *testing.T) { } } -func TestComputeIngressStatusConditions(t *testing.T) { +func TestComputeDeploymentStatusConditions(t *testing.T) { + type testInputs struct { + desireRepl *int32 + unavailRepl, availRepl int32 + } + type testOutputs struct { + deployReady 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 deployment replicas desired", + inputs: testInputs{}, + outputs: testOutputs{true}, + }, + /*{ + description: "0/2 deployment replicas available", + inputs: testInputs{2, 0}, + outputs: testOutputs{false}, + }, + { + description: "1/2 deployment replicas available", + inputs: testInputs{1, 1}, + outputs: testOutputs{false}, + }, + { + description: "2/2 deployment replicas available", + inputs: testInputs{0, 2}, + outputs: testOutputs{true}, + },*/ } for i, tc := range testCases { - deploy := &appsv1.Deployment{ + var deployStatus operatorv1.ConditionStatus + deployment := &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, }, } - - expected := []operatorv1.OperatorCondition{ - { - Type: tc.condType, - Status: tc.condStatus, - }, + if tc.outputs.deployReady { + deployStatus = operatorv1.ConditionTrue + } else { + deployStatus = operatorv1.ConditionFalse } - actual := computeIngressStatusConditions([]operatorv1.OperatorCondition{}, deploy) + expected := operatorv1.OperatorCondition{Type: "DeploymentReady", Status: deployStatus} + actual := computeDeploymentStatus(deployment) conditionsCmpOpts := []cmp.Option{ cmpopts.IgnoreFields(operatorv1.OperatorCondition{}, "LastTransitionTime", "Reason", "Message"), cmpopts.EquateEmpty(), @@ -231,6 +254,186 @@ func TestComputeIngressStatusConditions(t *testing.T) { } } +func TestComputeIngressStatusConditions(t *testing.T) { + tests := []struct { + description string + domainSet bool + input []operatorv1.OperatorCondition + expect []operatorv1.OperatorCondition + }{ + { + description: "no status domain set", + domainSet: false, + input: []operatorv1.OperatorCondition{ + cond(operatorv1.LoadBalancerManagedIngressConditionType, operatorv1.ConditionTrue, "WantedByEndpointPublishingStrategy"), + cond(operatorv1.LoadBalancerReadyIngressConditionType, operatorv1.ConditionTrue, "LoadBalancerProvisioned"), + }, + expect: []operatorv1.OperatorCondition{ + cond(operatorv1.OperatorStatusTypeAvailable, operatorv1.ConditionFalse, "StatusDomainUnset"), + cond(operatorv1.OperatorStatusTypeDegraded, operatorv1.ConditionTrue, "StatusDomainUnset"), + }, + }, + { + description: "status domain set", + domainSet: true, + input: []operatorv1.OperatorCondition{}, + expect: []operatorv1.OperatorCondition{ + cond(operatorv1.OperatorStatusTypeAvailable, operatorv1.ConditionTrue, "AsExpected"), + cond(operatorv1.OperatorStatusTypeDegraded, operatorv1.ConditionFalse, "AsExpected"), + }, + }, + { + description: "load balancer provisioned", + domainSet: true, + input: []operatorv1.OperatorCondition{ + cond(operatorv1.LoadBalancerManagedIngressConditionType, operatorv1.ConditionTrue, "WantedByEndpointPublishingStrategy"), + cond(operatorv1.LoadBalancerReadyIngressConditionType, operatorv1.ConditionTrue, "LoadBalancerProvisioned"), + }, + expect: []operatorv1.OperatorCondition{ + cond(operatorv1.OperatorStatusTypeAvailable, operatorv1.ConditionTrue, "AsExpected"), + cond(operatorv1.OperatorStatusTypeDegraded, operatorv1.ConditionFalse, "AsExpected"), + }, + }, + { + description: "no load balancer provisioned", + domainSet: true, + input: []operatorv1.OperatorCondition{ + cond(operatorv1.LoadBalancerManagedIngressConditionType, operatorv1.ConditionTrue, "WantedByEndpointPublishingStrategy"), + cond(operatorv1.LoadBalancerReadyIngressConditionType, operatorv1.ConditionFalse, "LoadBalancerProvisioned"), + }, + expect: []operatorv1.OperatorCondition{ + cond(operatorv1.OperatorStatusTypeAvailable, operatorv1.ConditionFalse, "DependantResourceFailure"), + cond(operatorv1.OperatorStatusTypeDegraded, operatorv1.ConditionTrue, "DependantResourceFailure"), + }, + }, + { + description: "dns no failed zones", + domainSet: true, + input: []operatorv1.OperatorCondition{ + cond(operatorv1.DNSManagedIngressConditionType, operatorv1.ConditionTrue, "Normal"), + cond(operatorv1.DNSReadyIngressConditionType, operatorv1.ConditionTrue, "NoFailedZones"), + }, + expect: []operatorv1.OperatorCondition{ + cond(operatorv1.OperatorStatusTypeAvailable, operatorv1.ConditionTrue, "AsExpected"), + cond(operatorv1.OperatorStatusTypeDegraded, operatorv1.ConditionFalse, "AsExpected"), + }, + }, + { + description: "dns record not found", + domainSet: true, + input: []operatorv1.OperatorCondition{ + cond(operatorv1.DNSManagedIngressConditionType, operatorv1.ConditionTrue, "Normal"), + cond(operatorv1.DNSReadyIngressConditionType, operatorv1.ConditionFalse, "RecordNotFound"), + }, + expect: []operatorv1.OperatorCondition{ + cond(operatorv1.OperatorStatusTypeAvailable, operatorv1.ConditionFalse, "DependantResourceFailure"), + cond(operatorv1.OperatorStatusTypeDegraded, operatorv1.ConditionTrue, "DependantResourceFailure"), + }, + }, + { + description: "load balancer provisioned, no failed dns zones", + domainSet: true, + input: []operatorv1.OperatorCondition{ + cond(operatorv1.LoadBalancerManagedIngressConditionType, operatorv1.ConditionTrue, "WantedByEndpointPublishingStrategy"), + cond(operatorv1.LoadBalancerReadyIngressConditionType, operatorv1.ConditionTrue, "LoadBalancerProvisioned"), + cond(operatorv1.DNSManagedIngressConditionType, operatorv1.ConditionTrue, "Normal"), + cond(operatorv1.DNSReadyIngressConditionType, operatorv1.ConditionTrue, "NoFailedZones"), + }, + expect: []operatorv1.OperatorCondition{ + cond(operatorv1.OperatorStatusTypeAvailable, operatorv1.ConditionTrue, "AsExpected"), + cond(operatorv1.OperatorStatusTypeDegraded, operatorv1.ConditionFalse, "AsExpected"), + }, + }, + } + + for i, test := range tests { + t.Logf("evaluating test %s", test.description) + ic := ingressController(fmt.Sprintf("status-test-%d", i+1), operatorv1.LoadBalancerServiceStrategyType) + if test.domainSet { + ic.Status.Domain = "test.local" + } + actual := computeIngressStatusConditions(ic, test.input) + conditionsCmpOpts := []cmp.Option{ + cmpopts.IgnoreFields(operatorv1.OperatorCondition{}, "LastTransitionTime", "Message"), + cmpopts.EquateEmpty(), + cmpopts.SortSlices(func(a, b operatorv1.OperatorCondition) bool { return a.Type < b.Type }), + } + if !cmp.Equal(actual, test.expect, conditionsCmpOpts...) { + t.Fatalf("expected:\n%#v\ngot:\n%#v", test.expect, actual) + } + } +} + +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 diff --git a/pkg/operator/controller/status.go b/pkg/operator/controller/status.go index 7071984461..59403a599b 100644 --- a/pkg/operator/controller/status.go +++ b/pkg/operator/controller/status.go @@ -221,9 +221,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, }