diff --git a/charts/postgres-operator/crds/postgresqls.yaml b/charts/postgres-operator/crds/postgresqls.yaml index 198afe119..b4b676236 100644 --- a/charts/postgres-operator/crds/postgresqls.yaml +++ b/charts/postgres-operator/crds/postgresqls.yaml @@ -266,6 +266,10 @@ spec: pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' # Note: the value specified here must not be zero or be higher # than the corresponding limit. + serviceAnnotations: + type: object + additionalProperties: + type: string sidecars: type: array nullable: true diff --git a/docs/administrator.md b/docs/administrator.md index 5b8769edb..2e86193c0 100644 --- a/docs/administrator.md +++ b/docs/administrator.md @@ -376,6 +376,17 @@ cluster manifest. In the case any of these variables are omitted from the manifest, the operator configuration settings `enable_master_load_balancer` and `enable_replica_load_balancer` apply. Note that the operator settings affect all Postgresql services running in all namespaces watched by the operator. +If load balancing is enabled two default annotations will be applied to its +services: + +- `external-dns.alpha.kubernetes.io/hostname` with the value defined by the + operator configs `master_dns_name_format` and `replica_dns_name_format`. + This value can't be overwritten. If any changing in its value is needed, it + MUST be done changing the DNS format operator config parameters; and +- `service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout` with + a default value of "3600". This value can be overwritten with the operator + config parameter `custom_service_annotations` or the cluster parameter + `serviceAnnotations`. 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 diff --git a/docs/reference/cluster_manifest.md b/docs/reference/cluster_manifest.md index bf6df681b..7b049b6fa 100644 --- a/docs/reference/cluster_manifest.md +++ b/docs/reference/cluster_manifest.md @@ -122,6 +122,11 @@ These parameters are grouped directly under the `spec` key in the manifest. A map of key value pairs that gets attached as [annotations](https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/) to each pod created for the database. +* **serviceAnnotations** + A map of key value pairs that gets attached as [annotations](https://kubernetes.io/docs/concepts/overview/working-with-objects/annotations/) + to the services created for the database cluster. Check the + [administrator docs](https://github.com/zalando/postgres-operator/blob/master/docs/administrator.md#load-balancers-and-allowed-ip-ranges) + for more information regarding default values and overwrite rules. * **enableShmVolume** Start a database pod without limitations on shm memory. By default Docker diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index 1055d89b6..b0e148289 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -380,8 +380,9 @@ In the CRD-based configuration they are grouped under the `load_balancer` key. `false`. * **custom_service_annotations** - when load balancing is enabled, LoadBalancer service is created and - this parameter takes service annotations that are applied to service. + This key/value map provides a list of annotations that get attached to each + service of a cluster created by the operator. If the annotation key is also + provided by the cluster definition, the manifest value is used. Optional. * **master_dns_name_format** defines the DNS name string template for the diff --git a/e2e/README.md b/e2e/README.md index 1d611bcd0..f1bc5f9ed 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -44,3 +44,4 @@ The current tests are all bundled in [`test_e2e.py`](tests/test_e2e.py): * taint-based eviction of Postgres pods * invoking logical backup cron job * uniqueness of master pod +* custom service annotations diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index 88a7f1f34..3ffdb746a 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -161,8 +161,8 @@ def test_logical_backup_cron_job(self): schedule = "7 7 7 7 *" pg_patch_enable_backup = { "spec": { - "enableLogicalBackup": True, - "logicalBackupSchedule": schedule + "enableLogicalBackup": True, + "logicalBackupSchedule": schedule } } k8s.api.custom_objects_api.patch_namespaced_custom_object( @@ -184,7 +184,7 @@ def test_logical_backup_cron_job(self): image = "test-image-name" patch_logical_backup_image = { "data": { - "logical_backup_docker_image": image, + "logical_backup_docker_image": image, } } k8s.update_config(patch_logical_backup_image) @@ -197,7 +197,7 @@ def test_logical_backup_cron_job(self): # delete the logical backup cron job pg_patch_disable_backup = { "spec": { - "enableLogicalBackup": False, + "enableLogicalBackup": False, } } k8s.api.custom_objects_api.patch_namespaced_custom_object( @@ -207,6 +207,37 @@ def test_logical_backup_cron_job(self): self.assertEqual(0, len(jobs), "Expected 0 logical backup jobs, found {}".format(len(jobs))) + @timeout_decorator.timeout(TEST_TIMEOUT_SEC) + def test_service_annotations(self): + ''' + Create a Postgres cluster with service annotations and check them. + ''' + k8s = self.k8s + patch_custom_service_annotations = { + "data": { + "custom_service_annotations": "foo:bar", + } + } + k8s.update_config(patch_custom_service_annotations) + + k8s.create_with_kubectl("manifests/postgres-manifest-with-service-annotations.yaml") + annotations = { + "annotation.key": "value", + "foo": "bar", + } + self.assertTrue(k8s.check_service_annotations( + "version=acid-service-annotations,spilo-role=master", annotations)) + self.assertTrue(k8s.check_service_annotations( + "version=acid-service-annotations,spilo-role=replica", annotations)) + + # clean up + unpatch_custom_service_annotations = { + "data": { + "custom_service_annotations": "", + } + } + k8s.update_config(unpatch_custom_service_annotations) + def assert_master_is_unique(self, namespace='default', version="acid-minimal-cluster"): """ Check that there is a single pod in the k8s cluster with the label "spilo-role=master" @@ -272,6 +303,16 @@ def wait_for_pod_start(self, pod_labels, namespace='default'): pod_phase = pods[0].status.phase time.sleep(self.RETRY_TIMEOUT_SEC) + def check_service_annotations(self, svc_labels, annotations, namespace='default'): + svcs = self.api.core_v1.list_namespaced_service(namespace, label_selector=svc_labels, limit=1).items + for svc in svcs: + if len(svc.metadata.annotations) != len(annotations): + return False + for key in svc.metadata.annotations: + if svc.metadata.annotations[key] != annotations[key]: + return False + return True + def wait_for_pg_to_scale(self, number_of_instances, namespace='default'): body = { @@ -280,7 +321,7 @@ def wait_for_pg_to_scale(self, number_of_instances, namespace='default'): } } _ = self.api.custom_objects_api.patch_namespaced_custom_object( - "acid.zalan.do", "v1", namespace, "postgresqls", "acid-minimal-cluster", body) + "acid.zalan.do", "v1", namespace, "postgresqls", "acid-minimal-cluster", body) labels = 'version=acid-minimal-cluster' while self.count_pods_with_label(labels) != number_of_instances: diff --git a/manifests/complete-postgres-manifest.yaml b/manifests/complete-postgres-manifest.yaml index cf450ef94..bfd4b7da7 100644 --- a/manifests/complete-postgres-manifest.yaml +++ b/manifests/complete-postgres-manifest.yaml @@ -32,6 +32,8 @@ spec: # spiloFSGroup: 103 # podAnnotations: # annotation.key: value +# serviceAnnotations: +# annotation.key: value # podPriorityClassName: "spilo-pod-priority" # tolerations: # - key: postgres diff --git a/manifests/postgres-manifest-with-service-annotations.yaml b/manifests/postgres-manifest-with-service-annotations.yaml new file mode 100644 index 000000000..ab3096740 --- /dev/null +++ b/manifests/postgres-manifest-with-service-annotations.yaml @@ -0,0 +1,20 @@ +apiVersion: "acid.zalan.do/v1" +kind: postgresql +metadata: + name: acid-service-annotations +spec: + teamId: "acid" + volume: + size: 1Gi + numberOfInstances: 2 + users: + zalando: # database owner + - superuser + - createdb + foo_user: [] # role for application foo + databases: + foo: zalando # dbname: owner + postgresql: + version: "11" + serviceAnnotations: + annotation.key: value diff --git a/manifests/postgresql.crd.yaml b/manifests/postgresql.crd.yaml index 3b0f652ea..276bc94b8 100644 --- a/manifests/postgresql.crd.yaml +++ b/manifests/postgresql.crd.yaml @@ -230,6 +230,10 @@ spec: pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' # Note: the value specified here must not be zero or be higher # than the corresponding limit. + serviceAnnotations: + type: object + additionalProperties: + type: string sidecars: type: array nullable: true diff --git a/pkg/apis/acid.zalan.do/v1/crds.go b/pkg/apis/acid.zalan.do/v1/crds.go index 20fa37138..df378515f 100644 --- a/pkg/apis/acid.zalan.do/v1/crds.go +++ b/pkg/apis/acid.zalan.do/v1/crds.go @@ -383,6 +383,14 @@ var PostgresCRDResourceValidation = apiextv1beta1.CustomResourceValidation{ }, }, }, + "serviceAnnotations": { + Type: "object", + AdditionalProperties: &apiextv1beta1.JSONSchemaPropsOrBool{ + Schema: &apiextv1beta1.JSONSchemaProps{ + Type: "string", + }, + }, + }, "sidecars": { Type: "array", Items: &apiextv1beta1.JSONSchemaPropsOrArray{ diff --git a/pkg/apis/acid.zalan.do/v1/postgresql_type.go b/pkg/apis/acid.zalan.do/v1/postgresql_type.go index 515a73ff0..07b42d4d4 100644 --- a/pkg/apis/acid.zalan.do/v1/postgresql_type.go +++ b/pkg/apis/acid.zalan.do/v1/postgresql_type.go @@ -60,6 +60,7 @@ type PostgresSpec struct { LogicalBackupSchedule string `json:"logicalBackupSchedule,omitempty"` StandbyCluster *StandbyDescription `json:"standby"` PodAnnotations map[string]string `json:"podAnnotations"` + ServiceAnnotations map[string]string `json:"serviceAnnotations"` // deprecated json tags InitContainersOld []v1.Container `json:"init_containers,omitempty"` diff --git a/pkg/apis/acid.zalan.do/v1/util_test.go b/pkg/apis/acid.zalan.do/v1/util_test.go index a1e01825f..28e9e8ca4 100644 --- a/pkg/apis/acid.zalan.do/v1/util_test.go +++ b/pkg/apis/acid.zalan.do/v1/util_test.go @@ -456,18 +456,84 @@ var postgresqlList = []struct { PostgresqlList{}, errors.New("unexpected end of JSON input")}} -var annotations = []struct { +var podAnnotations = []struct { about string in []byte annotations map[string]string err error }{{ - about: "common annotations", - in: []byte(`{"kind": "Postgresql","apiVersion": "acid.zalan.do/v1","metadata": {"name": "acid-testcluster1"}, "spec": {"podAnnotations": {"foo": "bar"},"teamId": "acid", "clone": {"cluster": "team-batman"}}}`), + about: "common annotations", + in: []byte(`{ + "kind": "Postgresql", + "apiVersion": "acid.zalan.do/v1", + "metadata": { + "name": "acid-testcluster1" + }, + "spec": { + "podAnnotations": { + "foo": "bar" + }, + "teamId": "acid", + "clone": { + "cluster": "team-batman" + } + } + }`), annotations: map[string]string{"foo": "bar"}, err: nil}, } +var serviceAnnotations = []struct { + about string + in []byte + annotations map[string]string + err error +}{ + { + about: "common single annotation", + in: []byte(`{ + "kind": "Postgresql", + "apiVersion": "acid.zalan.do/v1", + "metadata": { + "name": "acid-testcluster1" + }, + "spec": { + "serviceAnnotations": { + "foo": "bar" + }, + "teamId": "acid", + "clone": { + "cluster": "team-batman" + } + } + }`), + annotations: map[string]string{"foo": "bar"}, + err: nil, + }, + { + about: "common two annotations", + in: []byte(`{ + "kind": "Postgresql", + "apiVersion": "acid.zalan.do/v1", + "metadata": { + "name": "acid-testcluster1" + }, + "spec": { + "serviceAnnotations": { + "foo": "bar", + "post": "gres" + }, + "teamId": "acid", + "clone": { + "cluster": "team-batman" + } + } + }`), + annotations: map[string]string{"foo": "bar", "post": "gres"}, + err: nil, + }, +} + func mustParseTime(s string) metav1.Time { v, err := time.Parse("15:04", s) if err != nil { @@ -517,21 +583,42 @@ func TestWeekdayTime(t *testing.T) { } } -func TestClusterAnnotations(t *testing.T) { - for _, tt := range annotations { +func TestPodAnnotations(t *testing.T) { + for _, tt := range podAnnotations { t.Run(tt.about, func(t *testing.T) { var cluster Postgresql err := cluster.UnmarshalJSON(tt.in) if err != nil { if tt.err == nil || err.Error() != tt.err.Error() { - t.Errorf("Unable to marshal cluster with annotations: expected %v got %v", tt.err, err) + t.Errorf("Unable to marshal cluster with podAnnotations: expected %v got %v", tt.err, err) } return } for k, v := range cluster.Spec.PodAnnotations { found, expected := v, tt.annotations[k] if found != expected { - t.Errorf("Didn't find correct value for key %v in for podAnnotations: Expected %v found %v", k, expected, found) + t.Errorf("Didn't find correct value for key %v in for podAnnotations: Expected %v found %v", k, expected, found) + } + } + }) + } +} + +func TestServiceAnnotations(t *testing.T) { + for _, tt := range serviceAnnotations { + t.Run(tt.about, func(t *testing.T) { + var cluster Postgresql + err := cluster.UnmarshalJSON(tt.in) + if err != nil { + if tt.err == nil || err.Error() != tt.err.Error() { + t.Errorf("Unable to marshal cluster with serviceAnnotations: expected %v got %v", tt.err, err) + } + return + } + for k, v := range cluster.Spec.ServiceAnnotations { + found, expected := v, tt.annotations[k] + if found != expected { + t.Errorf("Didn't find correct value for key %v in for serviceAnnotations: Expected %v found %v", k, expected, found) } } }) diff --git a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go index dc07aa2cf..aaae1f04b 100644 --- a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go +++ b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go @@ -514,6 +514,13 @@ func (in *PostgresSpec) DeepCopyInto(out *PostgresSpec) { (*out)[key] = val } } + if in.ServiceAnnotations != nil { + in, out := &in.ServiceAnnotations, &out.ServiceAnnotations + *out = make(map[string]string, len(*in)) + for key, val := range *in { + (*out)[key] = val + } + } if in.InitContainersOld != nil { in, out := &in.InitContainersOld, &out.InitContainersOld *out = make([]corev1.Container, len(*in)) diff --git a/pkg/cluster/cluster_test.go b/pkg/cluster/cluster_test.go index 85d014d8a..9efbc51c6 100644 --- a/pkg/cluster/cluster_test.go +++ b/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) + } + } + }) + } +} diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index aed0c6e83..c9c0362be 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -1230,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, @@ -1247,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 @@ -1262,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 @@ -1285,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, } @@ -1293,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{