From 7fef44643ba04946a23b591a312cb46dbc25aac5 Mon Sep 17 00:00:00 2001
From: Grant Spence <gspence@redhat.com>
Date: Tue, 13 Aug 2024 14:30:48 -0400
Subject: [PATCH] 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.
---
 pkg/operator/controller/ingress/controller.go      |  2 ++
 pkg/operator/controller/ingress/controller_test.go | 14 ++++++++++++++
 pkg/operator/controller/ingress/status.go          | 14 ++------------
 3 files changed, 18 insertions(+), 12 deletions(-)

diff --git a/pkg/operator/controller/ingress/controller.go b/pkg/operator/controller/ingress/controller.go
index f35f4446c..43984ec1a 100644
--- 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
index ed5426c60..530de3071 100644
--- 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
index 99d41d3ee..3baea68b8 100644
--- 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
-		}
 	}
 }