diff --git a/charts/postgres-operator/crds/operatorconfigurations.yaml b/charts/postgres-operator/crds/operatorconfigurations.yaml index 4c5ebdf66..c7325907a 100644 --- a/charts/postgres-operator/crds/operatorconfigurations.yaml +++ b/charts/postgres-operator/crds/operatorconfigurations.yaml @@ -327,7 +327,7 @@ spec: connection_pool_user: type: string #default: "pooler" - connection_pool_instances_number: + connection_pool_replicas: type: integer #default: 1 connection_pool_image: @@ -354,7 +354,7 @@ spec: connection_pool_default_memory_request: type: string pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' - #default: "100m" + #default: "100Mi" status: type: object additionalProperties: diff --git a/charts/postgres-operator/crds/postgresqls.yaml b/charts/postgres-operator/crds/postgresqls.yaml index aa4d40b1d..1efdbc7b7 100644 --- a/charts/postgres-operator/crds/postgresqls.yaml +++ b/charts/postgres-operator/crds/postgresqls.yaml @@ -157,6 +157,8 @@ spec: # Note: usernames specified here as database owners must be declared in the users key of the spec key. dockerImage: type: string + enableConnectionPool: + type: boolean enableLogicalBackup: type: boolean enableMasterLoadBalancer: diff --git a/docs/user.md b/docs/user.md index 47be76773..7758d1c34 100644 --- a/docs/user.md +++ b/docs/user.md @@ -494,10 +494,10 @@ spec: resources: requests: cpu: "100m" - memory: "100M" + memory: "100Mi" limits: cpu: "100m" - memory: "100M" + memory: "100Mi" ``` By default `pgbouncer` is used to create a connection pool. To find out about diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index e3f00da9c..e92aba11f 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -270,7 +270,16 @@ class EndToEndTestCase(unittest.TestCase): } k8s.update_config(patch_custom_service_annotations) - k8s.create_with_kubectl("manifests/postgres-manifest-with-service-annotations.yaml") + pg_patch_custom_annotations = { + "spec": { + "serviceAnnotations": { + "annotation.key": "value" + } + } + } + k8s.api.custom_objects_api.patch_namespaced_custom_object( + "acid.zalan.do", "v1", "default", "postgresqls", "acid-minimal-cluster", pg_patch_custom_annotations) + annotations = { "annotation.key": "value", "foo": "bar", diff --git a/manifests/configmap.yaml b/manifests/configmap.yaml index 05f22a388..6d02f0f58 100644 --- a/manifests/configmap.yaml +++ b/manifests/configmap.yaml @@ -16,7 +16,7 @@ data: # connection_pool_default_memory_limit: 100m # connection_pool_default_memory_request: "100Mi" # connection_pool_image: "" - # connection_pool_instances_number: 1 + # connection_pool_replicas: 1 # connection_pool_mode: "transaction" # connection_pool_schema: "pooler" # connection_pool_user: "pooler" diff --git a/manifests/minimal-postgres-manifest.yaml b/manifests/minimal-postgres-manifest.yaml index ff7785cec..61b250c35 100644 --- a/manifests/minimal-postgres-manifest.yaml +++ b/manifests/minimal-postgres-manifest.yaml @@ -17,5 +17,4 @@ spec: foo: zalando # dbname: owner postgresql: version: "11" - connectionPool: - type: "pgbouncer" + enableConnectionPool: true diff --git a/manifests/operatorconfiguration.crd.yaml b/manifests/operatorconfiguration.crd.yaml index aa64c6f6d..1c4443ba3 100644 --- a/manifests/operatorconfiguration.crd.yaml +++ b/manifests/operatorconfiguration.crd.yaml @@ -303,7 +303,7 @@ spec: connection_pool_user: type: string #default: "pooler" - connection_pool_instances_number: + connection_pool_replicas: type: integer #default: 1 connection_pool_image: diff --git a/manifests/postgres-manifest-with-service-annotations.yaml b/manifests/postgres-manifest-with-service-annotations.yaml deleted file mode 100644 index ab3096740..000000000 --- a/manifests/postgres-manifest-with-service-annotations.yaml +++ /dev/null @@ -1,20 +0,0 @@ -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-operator-default-configuration.yaml b/manifests/postgresql-operator-default-configuration.yaml index 037ae5e35..e9ce07fda 100644 --- a/manifests/postgresql-operator-default-configuration.yaml +++ b/manifests/postgresql-operator-default-configuration.yaml @@ -127,7 +127,7 @@ configuration: connection_pool_default_memory_limit: 100m connection_pool_default_memory_request: "100Mi" # connection_pool_image: "" - connection_pool_instances_number: 1 + connection_pool_replicas: 1 connection_pool_mode: "transaction" # connection_pool_schema: "pooler" # connection_pool_user: "pooler" diff --git a/manifests/postgresql.crd.yaml b/manifests/postgresql.crd.yaml index ff9366421..a85c81043 100644 --- a/manifests/postgresql.crd.yaml +++ b/manifests/postgresql.crd.yaml @@ -121,6 +121,8 @@ spec: # Note: usernames specified here as database owners must be declared in the users key of the spec key. dockerImage: type: string + enableConnectionPool: + type: boolean enableLogicalBackup: type: boolean enableMasterLoadBalancer: diff --git a/pkg/apis/acid.zalan.do/v1/crds.go b/pkg/apis/acid.zalan.do/v1/crds.go index f760d63e5..7cc7385f7 100644 --- a/pkg/apis/acid.zalan.do/v1/crds.go +++ b/pkg/apis/acid.zalan.do/v1/crds.go @@ -247,6 +247,9 @@ var PostgresCRDResourceValidation = apiextv1beta1.CustomResourceValidation{ "dockerImage": { Type: "string", }, + "enableConnectionPool": { + Type: "boolean", + }, "enableLogicalBackup": { Type: "boolean", }, @@ -1118,7 +1121,7 @@ var OperatorConfigCRDResourceValidation = apiextv1beta1.CustomResourceValidation "connection_pool_image": { Type: "string", }, - "connection_pool_instances_number": { + "connection_pool_replicas": { Type: "integer", Minimum: &min1, }, diff --git a/pkg/util/k8sutil/k8sutil.go b/pkg/util/k8sutil/k8sutil.go index 44a293025..a58261167 100644 --- a/pkg/util/k8sutil/k8sutil.go +++ b/pkg/util/k8sutil/k8sutil.go @@ -9,7 +9,6 @@ import ( batchv1beta1 "k8s.io/api/batch/v1beta1" clientbatchv1beta1 "k8s.io/client-go/kubernetes/typed/batch/v1beta1" - "github.com/zalando/postgres-operator/pkg/util/constants" apiappsv1 "k8s.io/api/apps/v1" v1 "k8s.io/api/core/v1" policybeta1 "k8s.io/api/policy/v1beta1" @@ -172,21 +171,37 @@ func SameService(cur, new *v1.Service) (match bool, reason string) { } } - oldDNSAnnotation := cur.Annotations[constants.ZalandoDNSNameAnnotation] - newDNSAnnotation := new.Annotations[constants.ZalandoDNSNameAnnotation] - oldELBAnnotation := cur.Annotations[constants.ElbTimeoutAnnotationName] - newELBAnnotation := new.Annotations[constants.ElbTimeoutAnnotationName] + match = true - if oldDNSAnnotation != newDNSAnnotation { - return false, fmt.Sprintf("new service's %q annotation value %q doesn't match the current one %q", - constants.ZalandoDNSNameAnnotation, newDNSAnnotation, oldDNSAnnotation) - } - if oldELBAnnotation != newELBAnnotation { - return false, fmt.Sprintf("new service's %q annotation value %q doesn't match the current one %q", - constants.ElbTimeoutAnnotationName, oldELBAnnotation, newELBAnnotation) + reasonPrefix := "new service's annotations doesn't match the current one:" + for ann := range cur.Annotations { + if _, ok := new.Annotations[ann]; !ok { + match = false + if len(reason) == 0 { + reason = reasonPrefix + } + reason += fmt.Sprintf(" Removed '%s'.", ann) + } } - return true, "" + for ann := range new.Annotations { + v, ok := cur.Annotations[ann] + if !ok { + if len(reason) == 0 { + reason = reasonPrefix + } + reason += fmt.Sprintf(" Added '%s' with value '%s'.", ann, new.Annotations[ann]) + match = false + } else if v != new.Annotations[ann] { + if len(reason) == 0 { + reason = reasonPrefix + } + reason += fmt.Sprintf(" '%s' changed from '%s' to '%s'.", ann, v, new.Annotations[ann]) + match = false + } + } + + return match, reason } // SamePDB compares the PodDisruptionBudgets diff --git a/pkg/util/k8sutil/k8sutil_test.go b/pkg/util/k8sutil/k8sutil_test.go new file mode 100644 index 000000000..12288243e --- /dev/null +++ b/pkg/util/k8sutil/k8sutil_test.go @@ -0,0 +1,310 @@ +package k8sutil + +import ( + "strings" + "testing" + + "github.com/zalando/postgres-operator/pkg/util/constants" + + v1 "k8s.io/api/core/v1" +) + +func newsService(ann map[string]string, svcT v1.ServiceType, lbSr []string) *v1.Service { + svc := &v1.Service{ + Spec: v1.ServiceSpec{ + Type: svcT, + LoadBalancerSourceRanges: lbSr, + }, + } + svc.Annotations = ann + return svc +} + +func TestServiceAnnotations(t *testing.T) { + tests := []struct { + about string + current *v1.Service + new *v1.Service + reason string + match bool + }{ + { + about: "two equal services", + current: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + }, + v1.ServiceTypeClusterIP, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + new: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + }, + v1.ServiceTypeClusterIP, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + match: true, + }, + { + about: "services differ on service type", + current: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + }, + v1.ServiceTypeClusterIP, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + new: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + match: false, + reason: `new service's type "LoadBalancer" doesn't match the current one "ClusterIP"`, + }, + { + about: "services differ on lb source ranges", + current: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + new: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + }, + v1.ServiceTypeLoadBalancer, + []string{"185.249.56.0/22"}), + match: false, + reason: `new service's LoadBalancerSourceRange doesn't match the current one`, + }, + { + about: "new service doesn't have lb source ranges", + current: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + new: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + }, + v1.ServiceTypeLoadBalancer, + []string{}), + match: false, + reason: `new service's LoadBalancerSourceRange doesn't match the current one`, + }, + { + about: "services differ on DNS annotation", + current: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + new: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "new_clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + match: false, + reason: `new service's annotations doesn't match the current one: 'external-dns.alpha.kubernetes.io/hostname' changed from 'clstr.acid.zalan.do' to 'new_clstr.acid.zalan.do'.`, + }, + { + about: "services differ on AWS ELB annotation", + current: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + new: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: "1800", + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + match: false, + reason: `new service's annotations doesn't match the current one: 'service.beta.kubernetes.io/aws-load-balancer-connection-idle-timeout' changed from '3600' to '1800'.`, + }, + { + about: "service changes existing annotation", + current: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + "foo": "bar", + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + new: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + "foo": "baz", + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + match: false, + reason: `new service's annotations doesn't match the current one: 'foo' changed from 'bar' to 'baz'.`, + }, + { + about: "service changes multiple existing annotations", + current: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + "foo": "bar", + "bar": "foo", + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + new: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + "foo": "baz", + "bar": "fooz", + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + match: false, + // Test just the prefix to avoid flakiness and map sorting + reason: `new service's annotations doesn't match the current one:`, + }, + { + about: "service adds a new custom annotation", + current: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + new: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + "foo": "bar", + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + match: false, + reason: `new service's annotations doesn't match the current one: Added 'foo' with value 'bar'.`, + }, + { + about: "service removes a custom annotation", + current: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + "foo": "bar", + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + new: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + match: false, + reason: `new service's annotations doesn't match the current one: Removed 'foo'.`, + }, + { + about: "service removes a custom annotation and adds a new one", + current: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + "foo": "bar", + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + new: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + "bar": "foo", + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + match: false, + reason: `new service's annotations doesn't match the current one: Removed 'foo'. Added 'bar' with value 'foo'.`, + }, + { + about: "service removes a custom annotation, adds a new one and change another", + current: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + "foo": "bar", + "zalan": "do", + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + new: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + "bar": "foo", + "zalan": "do.com", + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + match: false, + reason: `new service's annotations doesn't match the current one: Removed 'foo'. Added 'bar' with value 'foo'. 'zalan' changed from 'do' to 'do.com'`, + }, + { + about: "service add annotations", + current: newsService( + map[string]string{}, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + new: newsService( + map[string]string{ + constants.ZalandoDNSNameAnnotation: "clstr.acid.zalan.do", + constants.ElbTimeoutAnnotationName: constants.ElbTimeoutAnnotationValue, + }, + v1.ServiceTypeLoadBalancer, + []string{"128.141.0.0/16", "137.138.0.0/16"}), + match: false, + // Test just the prefix to avoid flakiness and map sorting + reason: `new service's annotations doesn't match the current one: Added `, + }, + } + for _, tt := range tests { + t.Run(tt.about, func(t *testing.T) { + match, reason := SameService(tt.current, tt.new) + if match && !tt.match { + t.Errorf("expected services to do not match: '%q' and '%q'", tt.current, tt.new) + return + } + if !match && tt.match { + t.Errorf("expected services to be the same: '%q' and '%q'", tt.current, tt.new) + return + } + if !match && !tt.match { + if !strings.HasPrefix(reason, tt.reason) { + t.Errorf("expected reason '%s', found '%s'", tt.reason, reason) + return + } + } + }) + } +}