merge with master and resolve conflicts

pkg/cluster/cluster.go

@@ -29,7 +29,7 @@ import (
 	"github.com/zalando/postgres-operator/pkg/util/patroni"
 	"github.com/zalando/postgres-operator/pkg/util/teams"
 	"github.com/zalando/postgres-operator/pkg/util/users"
-	rbacv1beta1 "k8s.io/api/rbac/v1beta1"
+	rbacv1 "k8s.io/api/rbac/v1"
 )
 
 var (
@@ -45,7 +45,7 @@ type Config struct {
 	RestConfig                   *rest.Config
 	InfrastructureRoles          map[string]spec.PgUser // inherited from the controller
 	PodServiceAccount            *v1.ServiceAccount
-	PodServiceAccountRoleBinding *rbacv1beta1.RoleBinding
+	PodServiceAccountRoleBinding *rbacv1.RoleBinding
 }
 
 type kubeResources struct {
@@ -227,8 +227,8 @@ func (c *Cluster) Create() error {
 
 	c.setStatus(acidv1.ClusterStatusCreating)
 
-	if err = c.validateResources(&c.Spec); err != nil {
-		return fmt.Errorf("insufficient resource limits specified: %v", err)
+	if err = c.enforceMinResourceLimits(&c.Spec); err != nil {
+		return fmt.Errorf("could not enforce minimum resource limits: %v", err)
 	}
 
 	for _, role := range []PostgresRole{Master, Replica} {
@@ -495,38 +495,38 @@ func compareResourcesAssumeFirstNotNil(a *v1.ResourceRequirements, b *v1.Resourc
 
 }
 
-func (c *Cluster) validateResources(spec *acidv1.PostgresSpec) error {
-
-	// setting limits too low can cause unnecessary evictions / OOM kills
-	const (
-		cpuMinLimit    = "256m"
-		memoryMinLimit = "256Mi"
-	)
+func (c *Cluster) enforceMinResourceLimits(spec *acidv1.PostgresSpec) error {
 
 	var (
 		isSmaller bool
 		err       error
 	)
 
+	// setting limits too low can cause unnecessary evictions / OOM kills
+	minCPULimit := c.OpConfig.MinCPULimit
+	minMemoryLimit := c.OpConfig.MinMemoryLimit
+
 	cpuLimit := spec.Resources.ResourceLimits.CPU
 	if cpuLimit != "" {
-		isSmaller, err = util.IsSmallerQuantity(cpuLimit, cpuMinLimit)
+		isSmaller, err = util.IsSmallerQuantity(cpuLimit, minCPULimit)
 		if err != nil {
-			return fmt.Errorf("error validating CPU limit: %v", err)
+			return fmt.Errorf("could not compare defined CPU limit %s with configured minimum value %s: %v", cpuLimit, minCPULimit, err)
 		}
 		if isSmaller {
-			return fmt.Errorf("defined CPU limit %s is below required minimum %s to properly run postgresql resource", cpuLimit, cpuMinLimit)
+			c.logger.Warningf("defined CPU limit %s is below required minimum %s and will be set to it", cpuLimit, minCPULimit)
+			spec.Resources.ResourceLimits.CPU = minCPULimit
 		}
 	}
 
 	memoryLimit := spec.Resources.ResourceLimits.Memory
 	if memoryLimit != "" {
-		isSmaller, err = util.IsSmallerQuantity(memoryLimit, memoryMinLimit)
+		isSmaller, err = util.IsSmallerQuantity(memoryLimit, minMemoryLimit)
 		if err != nil {
-			return fmt.Errorf("error validating memory limit: %v", err)
+			return fmt.Errorf("could not compare defined memory limit %s with configured minimum value %s: %v", memoryLimit, minMemoryLimit, err)
 		}
 		if isSmaller {
-			return fmt.Errorf("defined memory limit %s is below required minimum %s to properly run postgresql resource", memoryLimit, memoryMinLimit)
+			c.logger.Warningf("defined memory limit %s is below required minimum %s and will be set to it", memoryLimit, minMemoryLimit)
+			spec.Resources.ResourceLimits.Memory = minMemoryLimit
 		}
 	}
 
@@ -543,7 +543,6 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	oldStatus := c.Status
 	c.setStatus(acidv1.ClusterStatusUpdating)
 	c.setSpec(newSpec)
 
@@ -555,22 +554,6 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
 		}
 	}()
 
-	if err := c.validateResources(&newSpec.Spec); err != nil {
-		err = fmt.Errorf("insufficient resource limits specified: %v", err)
-
-		// cancel update only when (already too low) pod resources were edited
-		// if cluster was successfully running before the update, continue but log a warning
-		isCPULimitSmaller, err2 := util.IsSmallerQuantity(newSpec.Spec.Resources.ResourceLimits.CPU, oldSpec.Spec.Resources.ResourceLimits.CPU)
-		isMemoryLimitSmaller, err3 := util.IsSmallerQuantity(newSpec.Spec.Resources.ResourceLimits.Memory, oldSpec.Spec.Resources.ResourceLimits.Memory)
-
-		if oldStatus.Running() && !isCPULimitSmaller && !isMemoryLimitSmaller && err2 == nil && err3 == nil {
-			c.logger.Warning(err)
-		} else {
-			updateFailed = true
-			return err
-		}
-	}
-
 	if oldSpec.Spec.PgVersion != newSpec.Spec.PgVersion { // PG versions comparison
 		c.logger.Warningf("postgresql version change(%q -> %q) has no effect", oldSpec.Spec.PgVersion, newSpec.Spec.PgVersion)
 		//we need that hack to generate statefulset with the old version
@@ -616,6 +599,12 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
 
 	// Statefulset
 	func() {
+		if err := c.enforceMinResourceLimits(&c.Spec); err != nil {
+			c.logger.Errorf("could not sync resources: %v", err)
+			updateFailed = true
+			return
+		}
+
 		oldSs, err := c.generateStatefulSet(&oldSpec.Spec)
 		if err != nil {
 			c.logger.Errorf("could not generate old statefulset spec: %v", err)
@@ -623,6 +612,9 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
 			return
 		}
 
+		// update newSpec to for latter comparison with oldSpec
+		c.enforceMinResourceLimits(&newSpec.Spec)
+
 		newSs, err := c.generateStatefulSet(&newSpec.Spec)
 		if err != nil {
 			c.logger.Errorf("could not generate new statefulset spec: %v", err)

pkg/cluster/cluster_test.go

@@ -355,6 +355,12 @@ func TestPodAnnotations(t *testing.T) {
 			database: map[string]string{"foo": "bar"},
 			merged:   map[string]string{"foo": "bar"},
 		},
+		{
+			subTest:  "Both Annotations",
+			operator: map[string]string{"foo": "bar"},
+			database: map[string]string{"post": "gres"},
+			merged:   map[string]string{"foo": "bar", "post": "gres"},
+		},
 		{
 			subTest:  "Database Config overrides Operator Config Annotations",
 			operator: map[string]string{"foo": "bar", "global": "foo"},
@@ -382,3 +388,319 @@ func TestPodAnnotations(t *testing.T) {
 		}
 	}
 }
+
+func TestServiceAnnotations(t *testing.T) {
+	enabled := true
+	disabled := false
+	tests := []struct {
+		about                         string
+		role                          PostgresRole
+		enableMasterLoadBalancerSpec  *bool
+		enableMasterLoadBalancerOC    bool
+		enableReplicaLoadBalancerSpec *bool
+		enableReplicaLoadBalancerOC   bool
+		operatorAnnotations           map[string]string
+		clusterAnnotations            map[string]string
+		expect                        map[string]string
+	}{
+		//MASTER
+		{
+			about:                        "Master with no annotations and EnableMasterLoadBalancer disabled on spec and OperatorConfig",
+			role:                         "master",
+			enableMasterLoadBalancerSpec: &disabled,
+			enableMasterLoadBalancerOC:   false,
+			operatorAnnotations:          make(map[string]string),
+			clusterAnnotations:           make(map[string]string),
+			expect:                       make(map[string]string),
+		},
+		{
+			about:                        "Master with no annotations and EnableMasterLoadBalancer enabled on spec",
+			role:                         "master",
+			enableMasterLoadBalancerSpec: &enabled,
+			enableMasterLoadBalancerOC:   false,
+			operatorAnnotations:          make(map[string]string),
+			clusterAnnotations:           make(map[string]string),
+			expect: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname":                            "test.acid.db.example.com",
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
+			},
+		},
+		{
+			about:                        "Master with no annotations and EnableMasterLoadBalancer enabled only on operator config",
+			role:                         "master",
+			enableMasterLoadBalancerSpec: &disabled,
+			enableMasterLoadBalancerOC:   true,
+			operatorAnnotations:          make(map[string]string),
+			clusterAnnotations:           make(map[string]string),
+			expect:                       make(map[string]string),
+		},
+		{
+			about:                      "Master with no annotations and EnableMasterLoadBalancer defined only on operator config",
+			role:                       "master",
+			enableMasterLoadBalancerOC: true,
+			operatorAnnotations:        make(map[string]string),
+			clusterAnnotations:         make(map[string]string),
+			expect: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname":                            "test.acid.db.example.com",
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
+			},
+		},
+		{
+			about:                      "Master with cluster annotations and load balancer enabled",
+			role:                       "master",
+			enableMasterLoadBalancerOC: true,
+			operatorAnnotations:        make(map[string]string),
+			clusterAnnotations:         map[string]string{"foo": "bar"},
+			expect: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname":                            "test.acid.db.example.com",
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
+				"foo": "bar",
+			},
+		},
+		{
+			about:                        "Master with cluster annotations and load balancer disabled",
+			role:                         "master",
+			enableMasterLoadBalancerSpec: &disabled,
+			enableMasterLoadBalancerOC:   true,
+			operatorAnnotations:          make(map[string]string),
+			clusterAnnotations:           map[string]string{"foo": "bar"},
+			expect:                       map[string]string{"foo": "bar"},
+		},
+		{
+			about:                      "Master with operator annotations and load balancer enabled",
+			role:                       "master",
+			enableMasterLoadBalancerOC: true,
+			operatorAnnotations:        map[string]string{"foo": "bar"},
+			clusterAnnotations:         make(map[string]string),
+			expect: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname":                            "test.acid.db.example.com",
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
+				"foo": "bar",
+			},
+		},
+		{
+			about:                      "Master with operator annotations override default annotations",
+			role:                       "master",
+			enableMasterLoadBalancerOC: true,
+			operatorAnnotations: map[string]string{
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "1800",
+			},
+			clusterAnnotations: make(map[string]string),
+			expect: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname":                            "test.acid.db.example.com",
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "1800",
+			},
+		},
+		{
+			about:                      "Master with cluster annotations override default annotations",
+			role:                       "master",
+			enableMasterLoadBalancerOC: true,
+			operatorAnnotations:        make(map[string]string),
+			clusterAnnotations: map[string]string{
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "1800",
+			},
+			expect: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname":                            "test.acid.db.example.com",
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "1800",
+			},
+		},
+		{
+			about:                      "Master with cluster annotations do not override external-dns annotations",
+			role:                       "master",
+			enableMasterLoadBalancerOC: true,
+			operatorAnnotations:        make(map[string]string),
+			clusterAnnotations: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname": "wrong.external-dns-name.example.com",
+			},
+			expect: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname":                            "test.acid.db.example.com",
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
+			},
+		},
+		{
+			about:                      "Master with operator annotations do not override external-dns annotations",
+			role:                       "master",
+			enableMasterLoadBalancerOC: true,
+			clusterAnnotations:         make(map[string]string),
+			operatorAnnotations: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname": "wrong.external-dns-name.example.com",
+			},
+			expect: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname":                            "test.acid.db.example.com",
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
+			},
+		},
+		// REPLICA
+		{
+			about:                         "Replica with no annotations and EnableReplicaLoadBalancer disabled on spec and OperatorConfig",
+			role:                          "replica",
+			enableReplicaLoadBalancerSpec: &disabled,
+			enableReplicaLoadBalancerOC:   false,
+			operatorAnnotations:           make(map[string]string),
+			clusterAnnotations:            make(map[string]string),
+			expect:                        make(map[string]string),
+		},
+		{
+			about:                         "Replica with no annotations and EnableReplicaLoadBalancer enabled on spec",
+			role:                          "replica",
+			enableReplicaLoadBalancerSpec: &enabled,
+			enableReplicaLoadBalancerOC:   false,
+			operatorAnnotations:           make(map[string]string),
+			clusterAnnotations:            make(map[string]string),
+			expect: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname":                            "test-repl.acid.db.example.com",
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
+			},
+		},
+		{
+			about:                         "Replica with no annotations and EnableReplicaLoadBalancer enabled only on operator config",
+			role:                          "replica",
+			enableReplicaLoadBalancerSpec: &disabled,
+			enableReplicaLoadBalancerOC:   true,
+			operatorAnnotations:           make(map[string]string),
+			clusterAnnotations:            make(map[string]string),
+			expect:                        make(map[string]string),
+		},
+		{
+			about:                       "Replica with no annotations and EnableReplicaLoadBalancer defined only on operator config",
+			role:                        "replica",
+			enableReplicaLoadBalancerOC: true,
+			operatorAnnotations:         make(map[string]string),
+			clusterAnnotations:          make(map[string]string),
+			expect: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname":                            "test-repl.acid.db.example.com",
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
+			},
+		},
+		{
+			about:                       "Replica with cluster annotations and load balancer enabled",
+			role:                        "replica",
+			enableReplicaLoadBalancerOC: true,
+			operatorAnnotations:         make(map[string]string),
+			clusterAnnotations:          map[string]string{"foo": "bar"},
+			expect: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname":                            "test-repl.acid.db.example.com",
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
+				"foo": "bar",
+			},
+		},
+		{
+			about:                         "Replica with cluster annotations and load balancer disabled",
+			role:                          "replica",
+			enableReplicaLoadBalancerSpec: &disabled,
+			enableReplicaLoadBalancerOC:   true,
+			operatorAnnotations:           make(map[string]string),
+			clusterAnnotations:            map[string]string{"foo": "bar"},
+			expect:                        map[string]string{"foo": "bar"},
+		},
+		{
+			about:                       "Replica with operator annotations and load balancer enabled",
+			role:                        "replica",
+			enableReplicaLoadBalancerOC: true,
+			operatorAnnotations:         map[string]string{"foo": "bar"},
+			clusterAnnotations:          make(map[string]string),
+			expect: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname":                            "test-repl.acid.db.example.com",
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
+				"foo": "bar",
+			},
+		},
+		{
+			about:                       "Replica with operator annotations override default annotations",
+			role:                        "replica",
+			enableReplicaLoadBalancerOC: true,
+			operatorAnnotations: map[string]string{
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "1800",
+			},
+			clusterAnnotations: make(map[string]string),
+			expect: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname":                            "test-repl.acid.db.example.com",
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "1800",
+			},
+		},
+		{
+			about:                       "Replica with cluster annotations override default annotations",
+			role:                        "replica",
+			enableReplicaLoadBalancerOC: true,
+			operatorAnnotations:         make(map[string]string),
+			clusterAnnotations: map[string]string{
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "1800",
+			},
+			expect: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname":                            "test-repl.acid.db.example.com",
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "1800",
+			},
+		},
+		{
+			about:                       "Replica with cluster annotations do not override external-dns annotations",
+			role:                        "replica",
+			enableReplicaLoadBalancerOC: true,
+			operatorAnnotations:         make(map[string]string),
+			clusterAnnotations: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname": "wrong.external-dns-name.example.com",
+			},
+			expect: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname":                            "test-repl.acid.db.example.com",
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
+			},
+		},
+		{
+			about:                       "Replica with operator annotations do not override external-dns annotations",
+			role:                        "replica",
+			enableReplicaLoadBalancerOC: true,
+			clusterAnnotations:          make(map[string]string),
+			operatorAnnotations: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname": "wrong.external-dns-name.example.com",
+			},
+			expect: map[string]string{
+				"external-dns.alpha.kubernetes.io/hostname":                            "test-repl.acid.db.example.com",
+				"service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout": "3600",
+			},
+		},
+		// COMMON
+		{
+			about:                       "cluster annotations append to operator annotations",
+			role:                        "replica",
+			enableReplicaLoadBalancerOC: false,
+			operatorAnnotations:         map[string]string{"foo": "bar"},
+			clusterAnnotations:          map[string]string{"post": "gres"},
+			expect:                      map[string]string{"foo": "bar", "post": "gres"},
+		},
+		{
+			about:                       "cluster annotations override operator annotations",
+			role:                        "replica",
+			enableReplicaLoadBalancerOC: false,
+			operatorAnnotations:         map[string]string{"foo": "bar", "post": "gres"},
+			clusterAnnotations:          map[string]string{"post": "greSQL"},
+			expect:                      map[string]string{"foo": "bar", "post": "greSQL"},
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.about, func(t *testing.T) {
+			cl.OpConfig.CustomServiceAnnotations = tt.operatorAnnotations
+			cl.OpConfig.EnableMasterLoadBalancer = tt.enableMasterLoadBalancerOC
+			cl.OpConfig.EnableReplicaLoadBalancer = tt.enableReplicaLoadBalancerOC
+			cl.OpConfig.MasterDNSNameFormat = "{cluster}.{team}.{hostedzone}"
+			cl.OpConfig.ReplicaDNSNameFormat = "{cluster}-repl.{team}.{hostedzone}"
+			cl.OpConfig.DbHostedZone = "db.example.com"
+
+			cl.Postgresql.Spec.ClusterName = "test"
+			cl.Postgresql.Spec.TeamID = "acid"
+			cl.Postgresql.Spec.ServiceAnnotations = tt.clusterAnnotations
+			cl.Postgresql.Spec.EnableMasterLoadBalancer = tt.enableMasterLoadBalancerSpec
+			cl.Postgresql.Spec.EnableReplicaLoadBalancer = tt.enableReplicaLoadBalancerSpec
+
+			got := cl.generateServiceAnnotations(tt.role, &cl.Postgresql.Spec)
+			if len(tt.expect) != len(got) {
+				t.Errorf("expected %d annotation(s), got %d", len(tt.expect), len(got))
+				return
+			}
+			for k, v := range got {
+				if tt.expect[k] != v {
+					t.Errorf("expected annotation '%v' with value '%v', got value '%v'", k, tt.expect[k], v)
+				}
+			}
+		})
+	}
+}

pkg/cluster/k8sres.go

@@ -1051,6 +1051,7 @@ func (c *Cluster) getNumberOfInstances(spec *acidv1.PostgresSpec) int32 {
 	/* Limit the max number of pods to one, if this is standby-cluster */
 	if spec.StandbyCluster != nil {
 		c.logger.Info("Standby cluster can have maximum of 1 pod")
+		min = 1
 		max = 1
 	}
 	if max >= 0 && newcur > max {
@@ -1229,14 +1230,6 @@ func (c *Cluster) shouldCreateLoadBalancerForService(role PostgresRole, spec *ac
 }
 
 func (c *Cluster) generateService(role PostgresRole, spec *acidv1.PostgresSpec) *v1.Service {
-	var dnsName string
-
-	if role == Master {
-		dnsName = c.masterDNSName()
-	} else {
-		dnsName = c.replicaDNSName()
-	}
-
 	serviceSpec := v1.ServiceSpec{
 		Ports: []v1.ServicePort{{Name: "postgresql", Port: 5432, TargetPort: intstr.IntOrString{IntVal: 5432}}},
 		Type:  v1.ServiceTypeClusterIP,
@@ -1246,8 +1239,6 @@ func (c *Cluster) generateService(role PostgresRole, spec *acidv1.PostgresSpec)
 		serviceSpec.Selector = c.roleLabelsSet(false, role)
 	}
 
-	var annotations map[string]string
-
 	if c.shouldCreateLoadBalancerForService(role, spec) {
 
 		// spec.AllowedSourceRanges evaluates to the empty slice of zero length
@@ -1261,18 +1252,6 @@ func (c *Cluster) generateService(role PostgresRole, spec *acidv1.PostgresSpec)
 
 		c.logger.Debugf("final load balancer source ranges as seen in a service spec (not necessarily applied): %q", serviceSpec.LoadBalancerSourceRanges)
 		serviceSpec.Type = v1.ServiceTypeLoadBalancer
-
-		annotations = map[string]string{
-			constants.ZalandoDNSNameAnnotation: dnsName,
-			constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue,
-		}
-
-		if len(c.OpConfig.CustomServiceAnnotations) != 0 {
-			c.logger.Debugf("There are custom annotations defined, creating them.")
-			for customAnnotationKey, customAnnotationValue := range c.OpConfig.CustomServiceAnnotations {
-				annotations[customAnnotationKey] = customAnnotationValue
-			}
-		}
 	} else if role == Replica {
 		// before PR #258, the replica service was only created if allocated a LB
 		// now we always create the service but warn if the LB is absent
@@ -1284,7 +1263,7 @@ func (c *Cluster) generateService(role PostgresRole, spec *acidv1.PostgresSpec)
 			Name:        c.serviceName(role),
 			Namespace:   c.Namespace,
 			Labels:      c.roleLabelsSet(true, role),
-			Annotations: annotations,
+			Annotations: c.generateServiceAnnotations(role, spec),
 		},
 		Spec: serviceSpec,
 	}
@@ -1292,6 +1271,42 @@ func (c *Cluster) generateService(role PostgresRole, spec *acidv1.PostgresSpec)
 	return service
 }
 
+func (c *Cluster) generateServiceAnnotations(role PostgresRole, spec *acidv1.PostgresSpec) map[string]string {
+	annotations := make(map[string]string)
+
+	for k, v := range c.OpConfig.CustomServiceAnnotations {
+		annotations[k] = v
+	}
+	if spec != nil || spec.ServiceAnnotations != nil {
+		for k, v := range spec.ServiceAnnotations {
+			annotations[k] = v
+		}
+	}
+
+	if c.shouldCreateLoadBalancerForService(role, spec) {
+		var dnsName string
+		if role == Master {
+			dnsName = c.masterDNSName()
+		} else {
+			dnsName = c.replicaDNSName()
+		}
+
+		// Just set ELB Timeout annotation with default value, if it does not
+		// have a cutom value
+		if _, ok := annotations[constants.ElbTimeoutAnnotationName]; !ok {
+			annotations[constants.ElbTimeoutAnnotationName] = constants.ElbTimeoutAnnotationValue
+		}
+		// External DNS name annotation is not customizable
+		annotations[constants.ZalandoDNSNameAnnotation] = dnsName
+	}
+
+	if len(annotations) == 0 {
+		return nil
+	}
+
+	return annotations
+}
+
 func (c *Cluster) generateEndpoint(role PostgresRole, subsets []v1.EndpointSubset) *v1.Endpoints {
 	endpoints := &v1.Endpoints{
 		ObjectMeta: metav1.ObjectMeta{
@@ -1588,6 +1603,10 @@ func (c *Cluster) generateLogicalBackupPodEnvVars() []v1.EnvVar {
 			Name:  "LOGICAL_BACKUP_S3_BUCKET",
 			Value: c.OpConfig.LogicalBackup.LogicalBackupS3Bucket,
 		},
+		{
+			Name:  "LOGICAL_BACKUP_S3_REGION",
+			Value: c.OpConfig.LogicalBackup.LogicalBackupS3Region,
+		},
 		{
 			Name:  "LOGICAL_BACKUP_S3_ENDPOINT",
 			Value: c.OpConfig.LogicalBackup.LogicalBackupS3Endpoint,

pkg/cluster/resources.go

@@ -366,6 +366,11 @@ func (c *Cluster) createService(role PostgresRole) (*v1.Service, error) {
 }
 
 func (c *Cluster) updateService(role PostgresRole, newService *v1.Service) error {
+	var (
+		svc *v1.Service
+		err error
+	)
+
 	c.setProcessName("updating %v service", role)
 
 	if c.Services[role] == nil {
@@ -373,70 +378,6 @@ func (c *Cluster) updateService(role PostgresRole, newService *v1.Service) error
 	}
 
 	serviceName := util.NameFromMeta(c.Services[role].ObjectMeta)
-	endpointName := util.NameFromMeta(c.Endpoints[role].ObjectMeta)
-	// TODO: check if it possible to change the service type with a patch in future versions of Kubernetes
-	if newService.Spec.Type != c.Services[role].Spec.Type {
-		// service type has changed, need to replace the service completely.
-		// we cannot use just patch the current service, since it may contain attributes incompatible with the new type.
-		var (
-			currentEndpoint *v1.Endpoints
-			err             error
-		)
-
-		if role == Master {
-			// for the master service we need to re-create the endpoint as well. Get the up-to-date version of
-			// the addresses stored in it before the service is deleted (deletion of the service removes the endpoint)
-			currentEndpoint, err = c.KubeClient.Endpoints(c.Namespace).Get(c.endpointName(role), metav1.GetOptions{})
-			if err != nil {
-				return fmt.Errorf("could not get current cluster %s endpoints: %v", role, err)
-			}
-		}
-		err = c.KubeClient.Services(serviceName.Namespace).Delete(serviceName.Name, c.deleteOptions)
-		if err != nil {
-			return fmt.Errorf("could not delete service %q: %v", serviceName, err)
-		}
-
-		// wait until the service is truly deleted
-		c.logger.Debugf("waiting for service to be deleted")
-
-		err = retryutil.Retry(c.OpConfig.ResourceCheckInterval, c.OpConfig.ResourceCheckTimeout,
-			func() (bool, error) {
-				_, err2 := c.KubeClient.Services(serviceName.Namespace).Get(serviceName.Name, metav1.GetOptions{})
-				if err2 == nil {
-					return false, nil
-				}
-				if k8sutil.ResourceNotFound(err2) {
-					return true, nil
-				}
-				return false, err2
-			})
-		if err != nil {
-			return fmt.Errorf("could not delete service %q: %v", serviceName, err)
-		}
-
-		// make sure we clear the stored service and endpoint status if the subsequent create fails.
-		c.Services[role] = nil
-		c.Endpoints[role] = nil
-		if role == Master {
-			// create the new endpoint using the addresses obtained from the previous one
-			endpointSpec := c.generateEndpoint(role, currentEndpoint.Subsets)
-			ep, err := c.KubeClient.Endpoints(endpointSpec.Namespace).Create(endpointSpec)
-			if err != nil {
-				return fmt.Errorf("could not create endpoint %q: %v", endpointName, err)
-			}
-
-			c.Endpoints[role] = ep
-		}
-
-		svc, err := c.KubeClient.Services(serviceName.Namespace).Create(newService)
-		if err != nil {
-			return fmt.Errorf("could not create service %q: %v", serviceName, err)
-		}
-
-		c.Services[role] = svc
-
-		return nil
-	}
 
 	// update the service annotation in order to propagate ELB notation.
 	if len(newService.ObjectMeta.Annotations) > 0 {
@@ -454,18 +395,30 @@ func (c *Cluster) updateService(role PostgresRole, newService *v1.Service) error
 		}
 	}
 
-	patchData, err := specPatch(newService.Spec)
-	if err != nil {
-		return fmt.Errorf("could not form patch for the service %q: %v", serviceName, 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)
+	oldServiceType := c.Services[role].Spec.Type
+	newServiceType := newService.Spec.Type
+	if newServiceType == "ClusterIP" && newServiceType != oldServiceType {
+		newService.ResourceVersion = c.Services[role].ResourceVersion
+		newService.Spec.ClusterIP = c.Services[role].Spec.ClusterIP
+		svc, err = c.KubeClient.Services(serviceName.Namespace).Update(newService)
+		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)
+		}
 
-	// update the service spec
-	svc, err := c.KubeClient.Services(serviceName.Namespace).Patch(
-		serviceName.Name,
-		types.MergePatchType,
-		patchData, "")
-	if err != nil {
-		return fmt.Errorf("could not patch service %q: %v", serviceName, err)
+		svc, err = c.KubeClient.Services(serviceName.Namespace).Patch(
+			serviceName.Name,
+			types.MergePatchType,
+			patchData, "")
+		if err != nil {
+			return fmt.Errorf("could not patch service %q: %v", serviceName, err)
+		}
 	}
 	c.Services[role] = svc
 

pkg/cluster/sync.go

@@ -23,7 +23,6 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error {
 	c.mu.Lock()
 	defer c.mu.Unlock()
 
-	oldStatus := c.Status
 	c.setSpec(newSpec)
 
 	defer func() {
@@ -35,16 +34,6 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error {
 		}
 	}()
 
-	if err = c.validateResources(&c.Spec); err != nil {
-		err = fmt.Errorf("insufficient resource limits specified: %v", err)
-		if oldStatus.Running() {
-			c.logger.Warning(err)
-			err = nil
-		} else {
-			return err
-		}
-	}
-
 	if err = c.initUsers(); err != nil {
 		err = fmt.Errorf("could not init users: %v", err)
 		return err
@@ -76,6 +65,11 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error {
 		return err
 	}
 
+	if err = c.enforceMinResourceLimits(&c.Spec); err != nil {
+		err = fmt.Errorf("could not enforce minimum resource limits: %v", err)
+		return err
+	}
+
 	c.logger.Debugf("syncing statefulsets")
 	if err = c.syncStatefulSet(); err != nil {
 		if !k8sutil.ResourceAlreadyExists(err) {
@@ -122,7 +116,7 @@ func (c *Cluster) syncServices() error {
 		c.logger.Debugf("syncing %s service", role)
 
 		if err := c.syncEndpoint(role); err != nil {
-			return fmt.Errorf("could not sync %s endpont: %v", role, err)
+			return fmt.Errorf("could not sync %s endpoint: %v", role, err)
 		}
 
 		if err := c.syncService(role); err != nil {