move annotation patch and add hint in docs

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

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,6 +521,21 @@ func (c *Cluster) updateService(role PostgresRole, newService *v1.Service) error
 
 	serviceName := util.NameFromMeta(c.Services[role].ObjectMeta)
 
+	// 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) ||
+		(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{})
+		if err != nil {
+			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 {
@@ -539,20 +555,6 @@ func (c *Cluster) updateService(role PostgresRole, newService *v1.Service) error
 			}
 		}
 
-	// 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)
-	oldServiceType := c.Services[role].Spec.Type
-	newServiceType := newService.Spec.Type
-	if (newServiceType == "ClusterIP" && newServiceType != oldServiceType) ||
-		newServiceType == "LoadBalancer" && newServiceType == oldServiceType &&
-			len(newService.ObjectMeta.Annotations) != len(c.Services[role].ObjectMeta.Annotations) {
-		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{})
-		if err != nil {
-			return fmt.Errorf("could not update service %q: %v", serviceName, err)
-		}
-	} else {
 		patchData, err := specPatch(newService.Spec)
 		if err != nil {
 			return fmt.Errorf("could not form patch for the service %q: %v", serviceName, err)