From f6f0c148dffbdae1107e1087153f4aeb5938866d Mon Sep 17 00:00:00 2001
From: Felix Kunde <felix.kunde@zalando.de>
Date: Thu, 4 Jun 2020 13:55:50 +0200
Subject: [PATCH] move annotation patch and add hint in docs

---
 docs/administrator.md    |  8 +++++--
 pkg/cluster/resources.go | 48 +++++++++++++++++++++-------------------
 2 files changed, 31 insertions(+), 25 deletions(-)

diff --git a/docs/administrator.md b/docs/administrator.md
index 6664357e2..0e1569aeb 100644
--- a/docs/administrator.md
+++ b/docs/administrator.md
@@ -406,6 +406,10 @@ internal ELB:
 
 - `service.beta.kubernetes.io/aws-load-balancer-internal: "true"`
 
+Note, that changing between public and internal load balancers might require to
+toggle the `enable<Role>LoadBalancer` fields as some cloud infrastructures do
+not support switching the type on-the-fly.
+
 To limit the range of IP addresses that can reach a load balancer, specify the
 desired ranges in the `allowedSourceRanges` field (applies to both master and
 replica load balancers). To prevent exposing load balancers to the entire
@@ -540,9 +544,9 @@ The configuration paramaters that we will be using are:
 
 ### Generate a K8 secret resource
 
-Generate the K8 secret resource that will contain your service account's 
+Generate the K8 secret resource that will contain your service account's
 credentials. It's highly recommended to use a service account and limit its
-scope to just the WAL-E bucket. 
+scope to just the WAL-E bucket.
 
 ```yaml
 apiVersion: v1
diff --git a/pkg/cluster/resources.go b/pkg/cluster/resources.go
index 0d18331a2..db87a49c6 100644
--- a/pkg/cluster/resources.go
+++ b/pkg/cluster/resources.go
@@ -14,6 +14,7 @@ import (
 	"k8s.io/apimachinery/pkg/types"
 
 	"github.com/zalando/postgres-operator/pkg/util"
+	"github.com/zalando/postgres-operator/pkg/util/constants"
 	"github.com/zalando/postgres-operator/pkg/util/k8sutil"
 	"github.com/zalando/postgres-operator/pkg/util/retryutil"
 )
@@ -520,32 +521,14 @@ func (c *Cluster) updateService(role PostgresRole, newService *v1.Service) error
 
 	serviceName := util.NameFromMeta(c.Services[role].ObjectMeta)
 
-	// update the service annotation in order to propagate ELB notation.
-	if len(newService.ObjectMeta.Annotations) > 0 {
-		if annotationsPatchData, err := metaAnnotationsPatch(newService.ObjectMeta.Annotations); err == nil {
-			_, err = c.KubeClient.Services(serviceName.Namespace).Patch(
-				context.TODO(),
-				serviceName.Name,
-				types.MergePatchType,
-				[]byte(annotationsPatchData),
-				metav1.PatchOptions{},
-				"")
-
-			if err != nil {
-				return fmt.Errorf("could not replace annotations for the service %q: %v", serviceName, err)
-			}
-		} else {
-			return fmt.Errorf("could not form patch for the service metadata: %v", err)
-		}
-	}
-
-	// now, patch the service spec, but when disabling LoadBalancers do update instead
-	// patch does not work because of LoadBalancerSourceRanges field (even if set to nil)
+	// update service when disabling LoadBalancers or changing from internal LBs to public LBs
+	// because patch does not remove fields from the service resource
 	oldServiceType := c.Services[role].Spec.Type
 	newServiceType := newService.Spec.Type
+	_, oldInternal := c.Services[role].ObjectMeta.Annotations[constants.ElbInternal]
+	_, newInternal := newService.ObjectMeta.Annotations[constants.ElbInternal]
 	if (newServiceType == "ClusterIP" && newServiceType != oldServiceType) ||
-		newServiceType == "LoadBalancer" && newServiceType == oldServiceType &&
-			len(newService.ObjectMeta.Annotations) != len(c.Services[role].ObjectMeta.Annotations) {
+		(oldInternal && !newInternal) {
 		newService.ResourceVersion = c.Services[role].ResourceVersion
 		newService.Spec.ClusterIP = c.Services[role].Spec.ClusterIP
 		svc, err = c.KubeClient.Services(serviceName.Namespace).Update(context.TODO(), newService, metav1.UpdateOptions{})
@@ -553,6 +536,25 @@ func (c *Cluster) updateService(role PostgresRole, newService *v1.Service) error
 			return fmt.Errorf("could not update service %q: %v", serviceName, err)
 		}
 	} else {
+		// update the service annotation in order to propagate ELB notation.
+		if len(newService.ObjectMeta.Annotations) > 0 {
+			if annotationsPatchData, err := metaAnnotationsPatch(newService.ObjectMeta.Annotations); err == nil {
+				_, err = c.KubeClient.Services(serviceName.Namespace).Patch(
+					context.TODO(),
+					serviceName.Name,
+					types.MergePatchType,
+					[]byte(annotationsPatchData),
+					metav1.PatchOptions{},
+					"")
+
+				if err != nil {
+					return fmt.Errorf("could not replace annotations for the service %q: %v", serviceName, err)
+				}
+			} else {
+				return fmt.Errorf("could not form patch for the service metadata: %v", err)
+			}
+		}
+
 		patchData, err := specPatch(newService.Spec)
 		if err != nil {
 			return fmt.Errorf("could not form patch for the service %q: %v", serviceName, err)