Merge pull request #1126 from gcs278/clear-lb-param-status

OCPBUGS-38217: Clear LB Status Parameters on LB Type Change
openshift · Aug 16, 2024 · 26f0181 · 26f0181
2 parents 61741b5 + 7fef446
commit 26f0181
Show file tree

Hide file tree

Showing 3 changed files with 18 additions and 12 deletions.
diff --git a/pkg/operator/controller/ingress/controller.go b/pkg/operator/controller/ingress/controller.go
@@ -548,6 +548,7 @@ func setDefaultPublishingStrategy(ic *operatorv1.IngressController, platformStat
 					changed = true
 				}
 				if statusLB.ProviderParameters.AWS.Type == operatorv1.AWSClassicLoadBalancer {
+					statusLB.ProviderParameters.AWS.NetworkLoadBalancerParameters = nil
 					if statusLB.ProviderParameters.AWS.ClassicLoadBalancerParameters == nil {
 						statusLB.ProviderParameters.AWS.ClassicLoadBalancerParameters = &operatorv1.AWSClassicLoadBalancerParameters{}
 					}
@@ -569,6 +570,7 @@ func setDefaultPublishingStrategy(ic *operatorv1.IngressController, platformStat
 					}
 				}
 				if statusLB.ProviderParameters.AWS.Type == operatorv1.AWSNetworkLoadBalancer {
+					statusLB.ProviderParameters.AWS.ClassicLoadBalancerParameters = nil
 					if statusLB.ProviderParameters.AWS.NetworkLoadBalancerParameters == nil {
 						statusLB.ProviderParameters.AWS.NetworkLoadBalancerParameters = &operatorv1.AWSNetworkLoadBalancerParameters{}
 					}

diff --git a/pkg/operator/controller/ingress/controller_test.go b/pkg/operator/controller/ingress/controller_test.go
@@ -624,6 +624,20 @@ func TestSetDefaultPublishingStrategyHandlesUpdates(t *testing.T) {
 			expectedIC:              makeIC(spec(elb()), status(elbWithNullParameters())),
 			domainMatchesBaseDomain: true,
 		},
+		{
+			name:                    "loadbalancer type changed from ELB to NLB, with old ELB parameters removal",
+			ic:                      makeIC(spec(nlb()), status(elbWithNullParameters())),
+			expectedResult:          true,
+			expectedIC:              makeIC(spec(nlb()), status(nlbWithNullParameters())),
+			domainMatchesBaseDomain: true,
+		},
+		{
+			name:                    "loadbalancer type changed from NLB to ELB, with old NLB parameters removal",
+			ic:                      makeIC(spec(elb()), status(nlbWithNullParameters())),
+			expectedResult:          true,
+			expectedIC:              makeIC(spec(elb()), status(elbWithNullParameters())),
+			domainMatchesBaseDomain: true,
+		},
 		{
 			name:                    "loadbalancer type changed from NLB to unset, with Classic LB as default",
 			ic:                      makeIC(spec(eps(lbs(operatorv1.ExternalLoadBalancer, &managedDNS))), status(nlb())),

diff --git a/pkg/operator/controller/ingress/status.go b/pkg/operator/controller/ingress/status.go
@@ -952,29 +952,19 @@ func computeLoadBalancerProgressingStatus(ic *operatorv1.IngressController, serv
 func updateIngressControllerAWSSubnetStatus(ic *operatorv1.IngressController, service *corev1.Service) {
 	// Set the subnets status based on the actual service annotation and the load balancer type,
 	// as NLBs and CLBs have separate subnet configuration fields.
-	clbParams := getAWSClassicLoadBalancerParametersInStatus(ic)
-	nlbParams := getAWSNetworkLoadBalancerParametersInStatus(ic)
 	switch getAWSLoadBalancerTypeInStatus(ic) {
 	case operatorv1.AWSNetworkLoadBalancer:
 		// NetworkLoadBalancerParameters should be initialized by setDefaultPublishingStrategy
 		// when an IngressController is admitted, so we don't need to initialize here.
-		if nlbParams != nil {
+		if nlbParams := getAWSNetworkLoadBalancerParametersInStatus(ic); nlbParams != nil {
 			nlbParams.Subnets = getSubnetsFromServiceAnnotation(service)
 		}
-		// Clear CLB status as the IngressController is now using a NLB.
-		if clbParams != nil {
-			clbParams.Subnets = nil
-		}
 	case operatorv1.AWSClassicLoadBalancer:
 		// ClassicLoadBalancerParameters should be initialized by setDefaultPublishingStrategy
 		// when an IngressController is admitted, so we don't need to initialize here.
-		if clbParams != nil {
+		if clbParams := getAWSClassicLoadBalancerParametersInStatus(ic); clbParams != nil {
 			clbParams.Subnets = getSubnetsFromServiceAnnotation(service)
 		}
-		// Clear NLB status as the IngressController is now using a CLB.
-		if nlbParams != nil {
-			nlbParams.Subnets = nil
-		}
 	}
 }