OCPBUGS-38217: Clear LB Status Parameters on LB Type Change

When the LB type is changed, the status parameters for the previously selected type should be cleared. This fix ensures that the status remains consistent with the currently selected LB type.
openshift · Aug 16, 2024 · 7fef446 · 7fef446
1 parent 7cf86c7
commit 7fef446
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
-		}
 	}
 }