From 5e61b28a5c5bc6555b809897b54080e45dfbe056 Mon Sep 17 00:00:00 2001 From: Miciah Masters Date: Fri, 4 Dec 2020 14:13:14 -0500 Subject: [PATCH] Assume ingresscontroller is external absent status MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If an ingresscontroller's status.endpointPublishingStrategy.type field is "LoadBalancerService" but status.endpointPublishingStrategy.loadBalancer is nil, assume the ingresscontroller should have an external load balancer. Commit 8cd622ff010b84b761cd8557e5e42a0d17600891 added support for internal scope for load balancers. In doing so, it changed the operator logic to initialize status.endpointPublishingStrategy.loadBalancer to indicate the scope when an ingresscontroller was created, using the corresponding fields in the ingresscontroller's spec to determine the scope. The same commit also changed operator logic to assume that the ingresscontroller has internal scope if status.endpointPublishingStrategy.loadBalancer is nil. However, if an ingresscontroller already exists, the operator does not update status.endpointPublishingStrategy.loadBalancer. This commit was made in OpenShift 4.2, which means that if a cluster was upgraded from 4.1, any ingresscontrollers that were created prior to the upgrade would not have status.endpointPublishingStrategy.loadBalancer set. Subsequently, commit 982682f10d57041e81fe0983b00610e036286ee1 made the ingresscontroller's scope mutable, meaning if an ingresscontroller has an external load balancer and status.endpointPublishingStrategy.loadBalancer indicates that the ingresscontroller should have an internal load balancer, then the operator changes the load balancer from external to internal. In combination, those two commits cause the operator to change an ingresscontroller's load balancer from external to internal if the cluster has been upgraded from 4.1 → 4.2 → ... → 4.6 and the ingresscontroller was originally created on OpenShift 4.1. This commit rectifies the situation by amending the logic that was added in commit 8cd622ff010b84b761cd8557e5e42a0d17600891 to assume that a nil value for status.endpointPublishingStrategy.loadBalancer means that the load balancer should be external. This assumption is valid because status.endpointPublishingStrategy.loadBalancer is nil exactly when the ingresscontroller was created on an OpenShift 4.1 cluster, and external scope was the only supported option on OpenShift 4.1. * pkg/operator/controller/ingress/load_balancer_service.go (desiredLoadBalancerService): Assume nil status.endpointPublishingStrategy.loadBalancer means external scope. --- pkg/operator/controller/ingress/load_balancer_service.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/operator/controller/ingress/load_balancer_service.go b/pkg/operator/controller/ingress/load_balancer_service.go index 55cf95265c..9aeeeee783 100644 --- a/pkg/operator/controller/ingress/load_balancer_service.go +++ b/pkg/operator/controller/ingress/load_balancer_service.go @@ -222,7 +222,7 @@ func desiredLoadBalancerService(ci *operatorv1.IngressController, deploymentRef service.Spec.Selector = controller.IngressControllerDeploymentPodSelector(ci).MatchLabels - isInternal := ci.Status.EndpointPublishingStrategy.LoadBalancer == nil || ci.Status.EndpointPublishingStrategy.LoadBalancer.Scope == operatorv1.InternalLoadBalancer + isInternal := ci.Status.EndpointPublishingStrategy.LoadBalancer != nil && ci.Status.EndpointPublishingStrategy.LoadBalancer.Scope == operatorv1.InternalLoadBalancer if service.Annotations == nil { service.Annotations = map[string]string{}