allow awsLBAdditionalResourceTags to detect changes

Signed-off-by: chiragkyal <[email protected]>

pkg/operator/controller/ingress/load_balancer_service.go

@@ -254,7 +254,7 @@ var (
 			//
 			// https://cloud.ibm.com/docs/containers?topic=containers-vpc-lbaas
 			iksLBEnableFeaturesAnnotation,
-			//TODO
+			// TODO: Any changes to managedLoadBalancerServiceAnnotations will cause cluster upgrade issue
 			// awsLBAdditionalResourceTags,
 		)
 
@@ -289,17 +289,21 @@ func (r *reconciler) ensureLoadBalancerService(ci *operatorv1.IngressController,
 	}
 
 	haveLBS, currentLBService, err := r.currentLoadBalancerService(ci)
+	log.Info("@chirag: currentLoadBalancerService %v", "annotations", currentLBService.Annotations)
 	if err != nil {
 		return false, nil, err
 	}
 
 	// BZ2054200: Don't modify/delete services that are not directly owned by this controller.
 	ownLBS := isServiceOwnedByIngressController(currentLBService, ci)
+	log.Info("@chirag: switch case ", "wantLBS", wantLBS, "haveLBS", haveLBS)
 
 	switch {
 	case !wantLBS && !haveLBS:
+		log.Info("@chirag: switch case !wantLBS && !haveLBS", "wantLBS", wantLBS, "haveLBS", haveLBS)
 		return false, nil, nil
 	case !wantLBS && haveLBS:
+		log.Info("@chirag: switch case !wantLBS && haveLBS", "wantLBS", wantLBS, "haveLBS", haveLBS)
 		if !ownLBS {
 			return false, nil, fmt.Errorf("a conflicting load balancer service exists that is not owned by the ingress controller: %s", controller.LoadBalancerServiceName(ci))
 		}
@@ -308,11 +312,13 @@ func (r *reconciler) ensureLoadBalancerService(ci *operatorv1.IngressController,
 		}
 		return false, nil, nil
 	case wantLBS && !haveLBS:
+		log.Info("@chirag: switch case wantLBS && !haveLBS", "wantLBS", wantLBS, "haveLBS", haveLBS)
 		if err := r.createLoadBalancerService(desiredLBService); err != nil {
 			return false, nil, err
 		}
 		return r.currentLoadBalancerService(ci)
 	case wantLBS && haveLBS:
+		log.Info("@chirag: switch case wantLBS && haveLBS", "wantLBS", wantLBS, "haveLBS", haveLBS)
 		if !ownLBS {
 			return false, nil, fmt.Errorf("a conflicting load balancer service exists that is not owned by the ingress controller: %s", controller.LoadBalancerServiceName(ci))
 		}
@@ -604,6 +610,8 @@ func (r *reconciler) deleteLoadBalancerService(service *corev1.Service, options
 // updateLoadBalancerService updates a load balancer service.  Returns a Boolean
 // indicating whether the service was updated, and an error value.
 func (r *reconciler) updateLoadBalancerService(current, desired *corev1.Service, platform *configv1.PlatformStatus, autoDeleteLB bool) (bool, error) {
+	log.Info("@chirag: inside updateLoadBalancerService()")
+
 	if shouldRecreateLB, reason := shouldRecreateLoadBalancer(current, desired, platform); shouldRecreateLB && autoDeleteLB {
 		log.Info("deleting and recreating the load balancer because "+reason, "namespace", desired.Namespace, "name", desired.Name)
 		foreground := metav1.DeletePropagationForeground
@@ -681,7 +689,10 @@ func loadBalancerServiceChanged(current, expected *corev1.Service) (bool, *corev
 	// avoid problems, make sure the previous release blocks upgrades when
 	// the user has modified an annotation or spec field that the new
 	// release manages.
-	changed, updated := loadBalancerServiceAnnotationsChanged(current, expected, managedLoadBalancerServiceAnnotations)
+	log.Info("@chirag: Inside loadBalancerServiceChanged()", "current", current.Annotations, "expected", expected.Annotations)
+	// check whether managedAnnotations and awsLBAdditionalResourceTags annotation are changed
+	annotations := managedLoadBalancerServiceAnnotations.Union(sets.NewString(awsLBAdditionalResourceTags))
+	changed, updated := loadBalancerServiceAnnotationsChanged(current, expected, annotations)
 
 	// If spec.loadBalancerSourceRanges is nonempty on the service, that
 	// means that allowedSourceRanges is nonempty on the ingresscontroller,
@@ -703,6 +714,7 @@ func loadBalancerServiceChanged(current, expected *corev1.Service) (bool, *corev
 			updated.Spec.LoadBalancerSourceRanges = expected.Spec.LoadBalancerSourceRanges
 		}
 	}
+	log.Info("@chirag: Inside loadBalancerServiceChanged()", "changed", changed, "updated", updated)
 
 	return changed, updated
 }

pkg/operator/controller/ingress/load_balancer_service_test.go

@@ -1141,7 +1141,7 @@ func Test_loadBalancerServiceChanged(t *testing.T) {
 			mutate: func(svc *corev1.Service) {
 				svc.Annotations["service.beta.kubernetes.io/aws-load-balancer-additional-resource-tags"] = "Key3=Value3,Key4=Value4"
 			},
-			expect: false,
+			expect: true,
 		},
 		{
 			description: "if the service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout annotation changes",

pkg/operator/controller/ingress/status_test.go

@@ -3026,7 +3026,7 @@ func Test_computeIngressUpgradeableCondition(t *testing.T) {
 			mutate: func(svc *corev1.Service) {
 				svc.Annotations[awsLBAdditionalResourceTags] = "Key2=Value2"
 			},
-			expect: false,
+			expect: true, // allow to update aws tags
 		},
 		{
 			description: "if the service.beta.kubernetes.io/load-balancer-source-ranges is set",