From 31d04cbdfcd82670eddd43c290b14b1c15116472 Mon Sep 17 00:00:00 2001 From: Sergey Shatunov Date: Tue, 28 Apr 2020 14:45:25 +0700 Subject: [PATCH 1/7] Add support for pooler load balancer Signed-off-by: Sergey Shatunov --- charts/postgres-operator/values-crd.yaml | 2 ++ charts/postgres-operator/values.yaml | 2 ++ manifests/operatorconfiguration.crd.yaml | 2 ++ manifests/postgresql.crd.yaml | 2 ++ .../v1/operator_configuration_type.go | 1 + pkg/apis/acid.zalan.do/v1/postgresql_type.go | 1 + .../acid.zalan.do/v1/zz_generated.deepcopy.go | 5 +++++ pkg/cluster/k8sres.go | 15 ++++++++++++++- pkg/controller/operator_config.go | 1 + pkg/util/config/config.go | 1 + 10 files changed, 31 insertions(+), 1 deletion(-) diff --git a/charts/postgres-operator/values-crd.yaml b/charts/postgres-operator/values-crd.yaml index caa4dda4d..1f9473835 100644 --- a/charts/postgres-operator/values-crd.yaml +++ b/charts/postgres-operator/values-crd.yaml @@ -163,6 +163,8 @@ configLoadBalancer: enable_master_load_balancer: false # toggles service type load balancer pointing to the replica pod of the cluster enable_replica_load_balancer: false + # toggles service type load balancer pointing to the pooler pod of the cluster + enable_pooler_load_balancer: false # defines the DNS name string template for the master load balancer cluster master_dns_name_format: "{cluster}.{team}.{hostedzone}" # defines the DNS name string template for the replica load balancer cluster diff --git a/charts/postgres-operator/values.yaml b/charts/postgres-operator/values.yaml index e7db249f0..0c3394f77 100644 --- a/charts/postgres-operator/values.yaml +++ b/charts/postgres-operator/values.yaml @@ -154,6 +154,8 @@ configLoadBalancer: enable_master_load_balancer: "false" # toggles service type load balancer pointing to the replica pod of the cluster enable_replica_load_balancer: "false" + # toggles service type load balancer pointing to the pooler pod of the cluster + enable_pooler_load_balancer: "false" # defines the DNS name string template for the master load balancer cluster master_dns_name_format: '{cluster}.{team}.{hostedzone}' # defines the DNS name string template for the replica load balancer cluster diff --git a/manifests/operatorconfiguration.crd.yaml b/manifests/operatorconfiguration.crd.yaml index b2496c9c9..2b3e007ee 100644 --- a/manifests/operatorconfiguration.crd.yaml +++ b/manifests/operatorconfiguration.crd.yaml @@ -197,6 +197,8 @@ spec: type: boolean enable_replica_load_balancer: type: boolean + enable_pooler_load_balancer: + type: boolean master_dns_name_format: type: string replica_dns_name_format: diff --git a/manifests/postgresql.crd.yaml b/manifests/postgresql.crd.yaml index 1ee6a1ae5..468917eb4 100644 --- a/manifests/postgresql.crd.yaml +++ b/manifests/postgresql.crd.yaml @@ -156,6 +156,8 @@ spec: type: boolean enableReplicaLoadBalancer: type: boolean + enablePoolerLoadBalancer: + type: boolean enableShmVolume: type: boolean init_containers: # deprecated diff --git a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go index c377a294b..f1dea3ff9 100644 --- a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go +++ b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go @@ -100,6 +100,7 @@ type LoadBalancerConfiguration struct { DbHostedZone string `json:"db_hosted_zone,omitempty"` EnableMasterLoadBalancer bool `json:"enable_master_load_balancer,omitempty"` EnableReplicaLoadBalancer bool `json:"enable_replica_load_balancer,omitempty"` + EnablePoolerLoadBalancer bool `json:"enable_pooler_load_balancer,omitempty"` CustomServiceAnnotations map[string]string `json:"custom_service_annotations,omitempty"` MasterDNSNameFormat config.StringTemplate `json:"master_dns_name_format,omitempty"` ReplicaDNSNameFormat config.StringTemplate `json:"replica_dns_name_format,omitempty"` diff --git a/pkg/apis/acid.zalan.do/v1/postgresql_type.go b/pkg/apis/acid.zalan.do/v1/postgresql_type.go index e36009208..32815907c 100644 --- a/pkg/apis/acid.zalan.do/v1/postgresql_type.go +++ b/pkg/apis/acid.zalan.do/v1/postgresql_type.go @@ -41,6 +41,7 @@ type PostgresSpec struct { // in that case the var evaluates to nil and the value is taken from the operator config EnableMasterLoadBalancer *bool `json:"enableMasterLoadBalancer,omitempty"` EnableReplicaLoadBalancer *bool `json:"enableReplicaLoadBalancer,omitempty"` + EnablePoolerLoadBalancer *bool `json:"enablePoolerLoadBalancer,omitempty"` // deprecated load balancer settings maintained for backward compatibility // see "Load balancers" operator docs 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 e2e1d5bd1..11de49b3f 100644 --- a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go +++ b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go @@ -525,6 +525,11 @@ func (in *PostgresSpec) DeepCopyInto(out *PostgresSpec) { *out = new(bool) **out = **in } + if in.EnablePoolerLoadBalancer != nil { + in, out := &in.EnablePoolerLoadBalancer, &out.EnablePoolerLoadBalancer + *out = new(bool) + **out = **in + } if in.UseLoadBalancer != nil { in, out := &in.UseLoadBalancer, &out.UseLoadBalancer *out = new(bool) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 43190491b..e1a77dbcd 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -2210,6 +2210,14 @@ func (c *Cluster) generateConnectionPoolerDeployment(spec *acidv1.PostgresSpec) return deployment, nil } +func (c *Cluster) shouldCreateLoadBalancerForPoolerService(spec *acidv1.PostgresSpec) bool { + if spec.EnablePoolerLoadBalancer != nil { + return *spec.EnablePoolerLoadBalancer + } + + return c.OpConfig.EnablePoolerLoadBalancer +} + func (c *Cluster) generateConnectionPoolerService(spec *acidv1.PostgresSpec) *v1.Service { // there are two ways to enable connection pooler, either to specify a @@ -2230,12 +2238,17 @@ func (c *Cluster) generateConnectionPoolerService(spec *acidv1.PostgresSpec) *v1 TargetPort: intstr.IntOrString{StrVal: c.servicePort(Master)}, }, }, - Type: v1.ServiceTypeClusterIP, Selector: map[string]string{ "connection-pooler": c.connectionPoolerName(), }, } + if c.shouldCreateLoadBalancerForPoolerService(spec) { + serviceSpec.Type = v1.ServiceTypeLoadBalancer + } else { + serviceSpec.Type = v1.ServiceTypeClusterIP + } + service := &v1.Service{ ObjectMeta: metav1.ObjectMeta{ Name: c.connectionPoolerName(), diff --git a/pkg/controller/operator_config.go b/pkg/controller/operator_config.go index c1756604b..73ac1d58a 100644 --- a/pkg/controller/operator_config.go +++ b/pkg/controller/operator_config.go @@ -100,6 +100,7 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur result.DbHostedZone = fromCRD.LoadBalancer.DbHostedZone result.EnableMasterLoadBalancer = fromCRD.LoadBalancer.EnableMasterLoadBalancer result.EnableReplicaLoadBalancer = fromCRD.LoadBalancer.EnableReplicaLoadBalancer + result.EnablePoolerLoadBalancer = fromCRD.LoadBalancer.EnablePoolerLoadBalancer result.CustomServiceAnnotations = fromCRD.LoadBalancer.CustomServiceAnnotations result.MasterDNSNameFormat = fromCRD.LoadBalancer.MasterDNSNameFormat result.ReplicaDNSNameFormat = fromCRD.LoadBalancer.ReplicaDNSNameFormat diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 9c2257e78..c146ba088 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -135,6 +135,7 @@ type Config struct { EnableAdminRoleForUsers bool `name:"enable_admin_role_for_users" default:"true"` EnableMasterLoadBalancer bool `name:"enable_master_load_balancer" default:"true"` EnableReplicaLoadBalancer bool `name:"enable_replica_load_balancer" default:"false"` + EnablePoolerLoadBalancer bool `name:"enable_pooler_load_balancer" default:"false"` CustomServiceAnnotations map[string]string `name:"custom_service_annotations"` CustomPodAnnotations map[string]string `name:"custom_pod_annotations"` EnablePodAntiAffinity bool `name:"enable_pod_antiaffinity" default:"false"` From f45fc5cffea97b0ae3fa8f9425a9260fac34323e Mon Sep 17 00:00:00 2001 From: Sergey Shatunov Date: Wed, 29 Apr 2020 07:30:19 +0700 Subject: [PATCH 2/7] Rename to enable_master_pooler_load_balancer Signed-off-by: Sergey Shatunov --- charts/postgres-operator/values-crd.yaml | 4 ++-- charts/postgres-operator/values.yaml | 4 ++-- manifests/operatorconfiguration.crd.yaml | 2 +- manifests/postgresql.crd.yaml | 2 +- .../v1/operator_configuration_type.go | 14 +++++++------- pkg/apis/acid.zalan.do/v1/postgresql_type.go | 6 +++--- pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go | 4 ++-- pkg/cluster/k8sres.go | 6 +++--- pkg/controller/operator_config.go | 2 +- pkg/util/config/config.go | 2 +- 10 files changed, 23 insertions(+), 23 deletions(-) diff --git a/charts/postgres-operator/values-crd.yaml b/charts/postgres-operator/values-crd.yaml index 1f9473835..bd125d783 100644 --- a/charts/postgres-operator/values-crd.yaml +++ b/charts/postgres-operator/values-crd.yaml @@ -163,8 +163,8 @@ configLoadBalancer: enable_master_load_balancer: false # toggles service type load balancer pointing to the replica pod of the cluster enable_replica_load_balancer: false - # toggles service type load balancer pointing to the pooler pod of the cluster - enable_pooler_load_balancer: false + # toggles service type load balancer pointing to the master pooler pod of the cluster + enable_master_pooler_load_balancer: false # defines the DNS name string template for the master load balancer cluster master_dns_name_format: "{cluster}.{team}.{hostedzone}" # defines the DNS name string template for the replica load balancer cluster diff --git a/charts/postgres-operator/values.yaml b/charts/postgres-operator/values.yaml index 0c3394f77..b22a4f673 100644 --- a/charts/postgres-operator/values.yaml +++ b/charts/postgres-operator/values.yaml @@ -154,8 +154,8 @@ configLoadBalancer: enable_master_load_balancer: "false" # toggles service type load balancer pointing to the replica pod of the cluster enable_replica_load_balancer: "false" - # toggles service type load balancer pointing to the pooler pod of the cluster - enable_pooler_load_balancer: "false" + # toggles service type load balancer pointing to the master pooler pod of the cluster + enable_master_pooler_load_balancer: "false" # defines the DNS name string template for the master load balancer cluster master_dns_name_format: '{cluster}.{team}.{hostedzone}' # defines the DNS name string template for the replica load balancer cluster diff --git a/manifests/operatorconfiguration.crd.yaml b/manifests/operatorconfiguration.crd.yaml index 2b3e007ee..667556f0a 100644 --- a/manifests/operatorconfiguration.crd.yaml +++ b/manifests/operatorconfiguration.crd.yaml @@ -197,7 +197,7 @@ spec: type: boolean enable_replica_load_balancer: type: boolean - enable_pooler_load_balancer: + enable_master_pooler_load_balancer: type: boolean master_dns_name_format: type: string diff --git a/manifests/postgresql.crd.yaml b/manifests/postgresql.crd.yaml index 468917eb4..f493b9401 100644 --- a/manifests/postgresql.crd.yaml +++ b/manifests/postgresql.crd.yaml @@ -156,7 +156,7 @@ spec: type: boolean enableReplicaLoadBalancer: type: boolean - enablePoolerLoadBalancer: + enableMasterPoolerLoadBalancer: type: boolean enableShmVolume: type: boolean diff --git a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go index f1dea3ff9..c02f14634 100644 --- a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go +++ b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go @@ -97,13 +97,13 @@ type OperatorTimeouts struct { // LoadBalancerConfiguration defines the LB configuration type LoadBalancerConfiguration struct { - DbHostedZone string `json:"db_hosted_zone,omitempty"` - EnableMasterLoadBalancer bool `json:"enable_master_load_balancer,omitempty"` - EnableReplicaLoadBalancer bool `json:"enable_replica_load_balancer,omitempty"` - EnablePoolerLoadBalancer bool `json:"enable_pooler_load_balancer,omitempty"` - CustomServiceAnnotations map[string]string `json:"custom_service_annotations,omitempty"` - MasterDNSNameFormat config.StringTemplate `json:"master_dns_name_format,omitempty"` - ReplicaDNSNameFormat config.StringTemplate `json:"replica_dns_name_format,omitempty"` + DbHostedZone string `json:"db_hosted_zone,omitempty"` + EnableMasterLoadBalancer bool `json:"enable_master_load_balancer,omitempty"` + EnableReplicaLoadBalancer bool `json:"enable_replica_load_balancer,omitempty"` + EnableMasterPoolerLoadBalancer bool `json:"enable_master_pooler_load_balancer,omitempty"` + CustomServiceAnnotations map[string]string `json:"custom_service_annotations,omitempty"` + MasterDNSNameFormat config.StringTemplate `json:"master_dns_name_format,omitempty"` + ReplicaDNSNameFormat config.StringTemplate `json:"replica_dns_name_format,omitempty"` } // AWSGCPConfiguration defines the configuration for AWS diff --git a/pkg/apis/acid.zalan.do/v1/postgresql_type.go b/pkg/apis/acid.zalan.do/v1/postgresql_type.go index 32815907c..21f5aca3c 100644 --- a/pkg/apis/acid.zalan.do/v1/postgresql_type.go +++ b/pkg/apis/acid.zalan.do/v1/postgresql_type.go @@ -39,9 +39,9 @@ type PostgresSpec struct { // vars that enable load balancers are pointers because it is important to know if any of them is omitted from the Postgres manifest // in that case the var evaluates to nil and the value is taken from the operator config - EnableMasterLoadBalancer *bool `json:"enableMasterLoadBalancer,omitempty"` - EnableReplicaLoadBalancer *bool `json:"enableReplicaLoadBalancer,omitempty"` - EnablePoolerLoadBalancer *bool `json:"enablePoolerLoadBalancer,omitempty"` + EnableMasterLoadBalancer *bool `json:"enableMasterLoadBalancer,omitempty"` + EnableReplicaLoadBalancer *bool `json:"enableReplicaLoadBalancer,omitempty"` + EnableMasterPoolerLoadBalancer *bool `json:"enableMasterPoolerLoadBalancer,omitempty"` // deprecated load balancer settings maintained for backward compatibility // see "Load balancers" operator docs 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 11de49b3f..eac352b1d 100644 --- a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go +++ b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go @@ -525,8 +525,8 @@ func (in *PostgresSpec) DeepCopyInto(out *PostgresSpec) { *out = new(bool) **out = **in } - if in.EnablePoolerLoadBalancer != nil { - in, out := &in.EnablePoolerLoadBalancer, &out.EnablePoolerLoadBalancer + if in.EnableMasterPoolerLoadBalancer != nil { + in, out := &in.EnableMasterPoolerLoadBalancer, &out.EnableMasterPoolerLoadBalancer *out = new(bool) **out = **in } diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index e1a77dbcd..2ae17efef 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -2211,11 +2211,11 @@ func (c *Cluster) generateConnectionPoolerDeployment(spec *acidv1.PostgresSpec) } func (c *Cluster) shouldCreateLoadBalancerForPoolerService(spec *acidv1.PostgresSpec) bool { - if spec.EnablePoolerLoadBalancer != nil { - return *spec.EnablePoolerLoadBalancer + if spec.EnableMasterPoolerLoadBalancer != nil { + return *spec.EnableMasterPoolerLoadBalancer } - return c.OpConfig.EnablePoolerLoadBalancer + return c.OpConfig.EnableMasterPoolerLoadBalancer } func (c *Cluster) generateConnectionPoolerService(spec *acidv1.PostgresSpec) *v1.Service { diff --git a/pkg/controller/operator_config.go b/pkg/controller/operator_config.go index 73ac1d58a..9d42c75d0 100644 --- a/pkg/controller/operator_config.go +++ b/pkg/controller/operator_config.go @@ -100,7 +100,7 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur result.DbHostedZone = fromCRD.LoadBalancer.DbHostedZone result.EnableMasterLoadBalancer = fromCRD.LoadBalancer.EnableMasterLoadBalancer result.EnableReplicaLoadBalancer = fromCRD.LoadBalancer.EnableReplicaLoadBalancer - result.EnablePoolerLoadBalancer = fromCRD.LoadBalancer.EnablePoolerLoadBalancer + result.EnableMasterPoolerLoadBalancer = fromCRD.LoadBalancer.EnableMasterPoolerLoadBalancer result.CustomServiceAnnotations = fromCRD.LoadBalancer.CustomServiceAnnotations result.MasterDNSNameFormat = fromCRD.LoadBalancer.MasterDNSNameFormat result.ReplicaDNSNameFormat = fromCRD.LoadBalancer.ReplicaDNSNameFormat diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index c146ba088..d8a88d2d1 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -135,7 +135,7 @@ type Config struct { EnableAdminRoleForUsers bool `name:"enable_admin_role_for_users" default:"true"` EnableMasterLoadBalancer bool `name:"enable_master_load_balancer" default:"true"` EnableReplicaLoadBalancer bool `name:"enable_replica_load_balancer" default:"false"` - EnablePoolerLoadBalancer bool `name:"enable_pooler_load_balancer" default:"false"` + EnableMasterPoolerLoadBalancer bool `name:"enable_master_pooler_load_balancer" default:"false"` CustomServiceAnnotations map[string]string `name:"custom_service_annotations"` CustomPodAnnotations map[string]string `name:"custom_pod_annotations"` EnablePodAntiAffinity bool `name:"enable_pod_antiaffinity" default:"false"` From 43f275c89ce9856605a7165340add15eca72212e Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Tue, 1 Mar 2022 09:46:14 +0100 Subject: [PATCH 3/7] target port should be intval --- pkg/cluster/connection_pooler.go | 4 ++-- pkg/cluster/k8sres.go | 8 ++++---- pkg/cluster/k8sres_test.go | 4 ++-- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/pkg/cluster/connection_pooler.go b/pkg/cluster/connection_pooler.go index c7dee5443..12c0be6ae 100644 --- a/pkg/cluster/connection_pooler.go +++ b/pkg/cluster/connection_pooler.go @@ -247,7 +247,7 @@ func (c *Cluster) generateConnectionPoolerPodTemplate(role PostgresRole) ( }, { Name: "PGPORT", - Value: c.servicePort(role), + Value: fmt.Sprint(c.servicePort(role)), }, { Name: "PGUSER", @@ -386,7 +386,7 @@ func (c *Cluster) generateConnectionPoolerService(connectionPooler *ConnectionPo { Name: connectionPooler.Name, Port: pgPort, - TargetPort: intstr.IntOrString{Type: 1, StrVal: c.servicePort(connectionPooler.Role)}, + TargetPort: intstr.IntOrString{IntVal: c.servicePort(connectionPooler.Role)}, }, }, Type: v1.ServiceTypeClusterIP, diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 21b4589c4..22ee4d33c 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -99,15 +99,15 @@ func (c *Cluster) serviceAddress(role PostgresRole) string { return "" } -func (c *Cluster) servicePort(role PostgresRole) string { +func (c *Cluster) servicePort(role PostgresRole) int32 { service, exist := c.Services[role] if exist { - return fmt.Sprint(service.Spec.Ports[0].Port) + return service.Spec.Ports[0].Port } - c.logger.Warningf("No service for role %s", role) - return "" + c.logger.Warningf("No service for role %s - defaulting to port 5432", role) + return 5432 } func (c *Cluster) podDisruptionBudgetName() string { diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index 42bec224d..0a56e2d4b 100644 --- a/pkg/cluster/k8sres_test.go +++ b/pkg/cluster/k8sres_test.go @@ -1479,7 +1479,7 @@ func getServices(serviceType v1.ServiceType, sourceRanges []string, extTrafficPo v1.ServiceSpec{ ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyType(extTrafficPolicy), LoadBalancerSourceRanges: sourceRanges, - Ports: []v1.ServicePort{{Name: clusterName + "-pooler", Port: 5432, TargetPort: intstr.IntOrString{Type: 1, StrVal: "5432"}}}, + Ports: []v1.ServicePort{{Name: clusterName + "-pooler", Port: 5432, TargetPort: intstr.IntOrString{IntVal: 5432}}}, Selector: map[string]string{"connection-pooler": clusterName + "-pooler"}, Type: serviceType, }, @@ -1493,7 +1493,7 @@ func getServices(serviceType v1.ServiceType, sourceRanges []string, extTrafficPo v1.ServiceSpec{ ExternalTrafficPolicy: v1.ServiceExternalTrafficPolicyType(extTrafficPolicy), LoadBalancerSourceRanges: sourceRanges, - Ports: []v1.ServicePort{{Name: clusterName + "-pooler-repl", Port: 5432, TargetPort: intstr.IntOrString{Type: 1, StrVal: "5432"}}}, + Ports: []v1.ServicePort{{Name: clusterName + "-pooler-repl", Port: 5432, TargetPort: intstr.IntOrString{IntVal: 5432}}}, Selector: map[string]string{"connection-pooler": clusterName + "-pooler-repl"}, Type: serviceType, }, From e805f60b778d8c0c32bde711295277bb7adf8dd3 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Tue, 1 Mar 2022 12:30:05 +0100 Subject: [PATCH 4/7] enhance pooler e2e test --- e2e/tests/test_e2e.py | 65 +++++++++++++++++++++++-------------------- 1 file changed, 35 insertions(+), 30 deletions(-) diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index 81977f508..db16fecc4 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -468,6 +468,8 @@ def test_enable_disable_connection_pooler(self): ''' k8s = self.k8s pooler_label = 'application=db-connection-pooler,cluster-name=acid-minimal-cluster' + master_pooler_label = 'connection-pooler=acid-minimal-cluster-pooler' + replica_pooler_label = master_pooler_label + '-repl' self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, "Operator does not get in sync") k8s.api.custom_objects_api.patch_namespaced_custom_object( @@ -477,27 +479,31 @@ def test_enable_disable_connection_pooler(self): 'spec': { 'enableConnectionPooler': True, 'enableReplicaConnectionPooler': True, + } + }) + self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, "Operator does not get in sync") + + self.eventuallyEqual(lambda: k8s.get_deployment_replica_count(), 2, "Deployment replicas is 2 default") + self.eventuallyEqual(lambda: k8s.count_running_pods(master_pooler_label), 2, "No pooler pods found") + self.eventuallyEqual(lambda: k8s.count_running_pods(replica_pooler_label), 2, "No pooler replica pods found") + self.eventuallyEqual(lambda: k8s.count_services_with_label(pooler_label), 2, "No pooler service found") + self.eventuallyEqual(lambda: k8s.count_secrets_with_label(pooler_label), 1, "Pooler secret not created") + + k8s.api.custom_objects_api.patch_namespaced_custom_object( + 'acid.zalan.do', 'v1', 'default', + 'postgresqls', 'acid-minimal-cluster', + { + 'spec': { 'enableMasterPoolerLoadBalancer': True, 'enableReplicaPoolerLoadBalancer': True, } }) + self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, "Operator does not get in sync") - self.eventuallyEqual(lambda: k8s.get_deployment_replica_count(), 2, - "Deployment replicas is 2 default") - self.eventuallyEqual(lambda: k8s.count_running_pods( - "connection-pooler=acid-minimal-cluster-pooler"), - 2, "No pooler pods found") - self.eventuallyEqual(lambda: k8s.count_running_pods( - "connection-pooler=acid-minimal-cluster-pooler-repl"), - 2, "No pooler replica pods found") - self.eventuallyEqual(lambda: k8s.count_services_with_label(pooler_label), - 2, "No pooler service found") - self.eventuallyEqual(lambda: k8s.count_secrets_with_label(pooler_label), - 1, "Pooler secret not created") - self.eventuallyEqual(lambda: k8s.get_service_type("spilo-role=master,"+pooler_label), + self.eventuallyEqual(lambda: k8s.get_service_type(master_pooler_label+","+pooler_label), 'LoadBalancer', "Expected LoadBalancer service type for master pooler pod, found {}") - self.eventuallyEqual(lambda: k8s.get_service_type("spilo-role=replica,"+pooler_label), + self.eventuallyEqual(lambda: k8s.get_service_type(replica_pooler_label+","+pooler_label), 'LoadBalancer', "Expected LoadBalancer service type for replica pooler pod, found {}") @@ -512,15 +518,13 @@ def test_enable_disable_connection_pooler(self): } }) - self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, - "Operator does not get in sync") + self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, "Operator does not get in sync") + self.eventuallyEqual(lambda: k8s.get_deployment_replica_count(name="acid-minimal-cluster-pooler-repl"), 2, "Deployment replicas is 2 default") - self.eventuallyEqual(lambda: k8s.count_running_pods( - "connection-pooler=acid-minimal-cluster-pooler"), + self.eventuallyEqual(lambda: k8s.count_running_pods(master_pooler_label), 0, "Master pooler pods not deleted") - self.eventuallyEqual(lambda: k8s.count_running_pods( - "connection-pooler=acid-minimal-cluster-pooler-repl"), + self.eventuallyEqual(lambda: k8s.count_running_pods(replica_pooler_label), 2, "Pooler replica pods not found") self.eventuallyEqual(lambda: k8s.count_services_with_label(pooler_label), 1, "No pooler service found") @@ -539,17 +543,17 @@ def test_enable_disable_connection_pooler(self): } }) - self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, - "Operator does not get in sync") + self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, "Operator does not get in sync") + self.eventuallyEqual(lambda: k8s.get_deployment_replica_count(), 2, "Deployment replicas is 2 default") - self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler"), + self.eventuallyEqual(lambda: k8s.count_running_pods(master_pooler_label), 2, "Master pooler pods not found") - self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler-repl"), + self.eventuallyEqual(lambda: k8s.count_running_pods(replica_pooler_label), 0, "Pooler replica pods not deleted") self.eventuallyEqual(lambda: k8s.count_services_with_label(pooler_label), 1, "No pooler service found") - self.eventuallyEqual(lambda: k8s.get_service_type("spilo-role=master,"+pooler_label), + self.eventuallyEqual(lambda: k8s.get_service_type(master_pooler_label+","+pooler_label), 'ClusterIP', "Expected LoadBalancer service type for master, found {}") self.eventuallyEqual(lambda: k8s.count_secrets_with_label(pooler_label), @@ -569,7 +573,7 @@ def test_enable_disable_connection_pooler(self): self.eventuallyEqual(lambda: k8s.get_deployment_replica_count(), 3, "Deployment replicas is scaled to 3") - self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler"), + self.eventuallyEqual(lambda: k8s.count_running_pods(master_pooler_label), 3, "Scale up of pooler pods does not work") # turn it off, keeping config should be overwritten by false @@ -583,7 +587,7 @@ def test_enable_disable_connection_pooler(self): } }) - self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler"), + self.eventuallyEqual(lambda: k8s.count_running_pods(master_pooler_label), 0, "Pooler pods not scaled down") self.eventuallyEqual(lambda: k8s.count_services_with_label(pooler_label), 0, "Pooler service not removed") @@ -1188,10 +1192,11 @@ def test_node_readiness_label(self): @timeout_decorator.timeout(TEST_TIMEOUT_SEC) def test_overwrite_pooler_deployment(self): + pooler_name = 'acid-minimal-cluster-pooler' k8s = self.k8s k8s.create_with_kubectl("manifests/minimal-fake-pooler-deployment.yaml") self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, "Operator does not get in sync") - self.eventuallyEqual(lambda: k8s.get_deployment_replica_count(name="acid-minimal-cluster-pooler"), 1, + self.eventuallyEqual(lambda: k8s.get_deployment_replica_count(name=pooler_name), 1, "Initial broken deployment not rolled out") k8s.api.custom_objects_api.patch_namespaced_custom_object( @@ -1204,7 +1209,7 @@ def test_overwrite_pooler_deployment(self): }) self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, "Operator does not get in sync") - self.eventuallyEqual(lambda: k8s.get_deployment_replica_count(name="acid-minimal-cluster-pooler"), 2, + self.eventuallyEqual(lambda: k8s.get_deployment_replica_count(name=pooler_name), 2, "Operator did not succeed in overwriting labels") k8s.api.custom_objects_api.patch_namespaced_custom_object( @@ -1217,7 +1222,7 @@ def test_overwrite_pooler_deployment(self): }) self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, "Operator does not get in sync") - self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler"), + self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler="+pooler_name), 0, "Pooler pods not scaled down") @timeout_decorator.timeout(TEST_TIMEOUT_SEC) From e61add5f6f2638de03888fe5fdd4d9c1dc2c2af7 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Tue, 1 Mar 2022 14:39:19 +0100 Subject: [PATCH 5/7] add new options to crds.go --- e2e/tests/test_e2e.py | 15 +++------------ pkg/apis/acid.zalan.do/v1/crds.go | 6 ++++++ pkg/cluster/k8sres.go | 4 ---- 3 files changed, 9 insertions(+), 16 deletions(-) diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index db16fecc4..c75ef108c 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -479,6 +479,8 @@ def test_enable_disable_connection_pooler(self): 'spec': { 'enableConnectionPooler': True, 'enableReplicaConnectionPooler': True, + 'enableMasterPoolerLoadBalancer': True, + 'enableReplicaPoolerLoadBalancer': True, } }) self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, "Operator does not get in sync") @@ -488,18 +490,6 @@ def test_enable_disable_connection_pooler(self): self.eventuallyEqual(lambda: k8s.count_running_pods(replica_pooler_label), 2, "No pooler replica pods found") self.eventuallyEqual(lambda: k8s.count_services_with_label(pooler_label), 2, "No pooler service found") self.eventuallyEqual(lambda: k8s.count_secrets_with_label(pooler_label), 1, "Pooler secret not created") - - k8s.api.custom_objects_api.patch_namespaced_custom_object( - 'acid.zalan.do', 'v1', 'default', - 'postgresqls', 'acid-minimal-cluster', - { - 'spec': { - 'enableMasterPoolerLoadBalancer': True, - 'enableReplicaPoolerLoadBalancer': True, - } - }) - self.eventuallyEqual(lambda: k8s.get_operator_state(), {"0": "idle"}, "Operator does not get in sync") - self.eventuallyEqual(lambda: k8s.get_service_type(master_pooler_label+","+pooler_label), 'LoadBalancer', "Expected LoadBalancer service type for master pooler pod, found {}") @@ -584,6 +574,7 @@ def test_enable_disable_connection_pooler(self): 'spec': { 'enableConnectionPooler': False, 'enableReplicaConnectionPooler': False, + 'enableReplicaPoolerLoadBalancer': False, } }) diff --git a/pkg/apis/acid.zalan.do/v1/crds.go b/pkg/apis/acid.zalan.do/v1/crds.go index b74727057..bf6da4a58 100644 --- a/pkg/apis/acid.zalan.do/v1/crds.go +++ b/pkg/apis/acid.zalan.do/v1/crds.go @@ -302,9 +302,15 @@ var PostgresCRDResourceValidation = apiextv1.CustomResourceValidation{ "enableMasterLoadBalancer": { Type: "boolean", }, + "enableMasterPoolerLoadBalancer": { + Type: "boolean", + }, "enableReplicaLoadBalancer": { Type: "boolean", }, + "enableReplicaPoolerLoadBalancer": { + Type: "boolean", + }, "enableShmVolume": { Type: "boolean", }, diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 22ee4d33c..6717238ee 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -1684,10 +1684,6 @@ func (c *Cluster) generateService(role PostgresRole, spec *acidv1.PostgresSpec) if c.shouldCreateLoadBalancerForService(role, spec) { c.configureLoadBalanceService(&serviceSpec, spec.AllowedSourceRanges) - } 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 - c.logger.Debugf("No load balancer created for the replica service") } service := &v1.Service{ From 63368d63af9c29861fb7e06f508ad9910d068f25 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Tue, 1 Mar 2022 14:55:33 +0100 Subject: [PATCH 6/7] final fix for pooler e2e test --- e2e/tests/test_e2e.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index c75ef108c..09b2c9c60 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -582,7 +582,7 @@ def test_enable_disable_connection_pooler(self): 0, "Pooler pods not scaled down") self.eventuallyEqual(lambda: k8s.count_services_with_label(pooler_label), 0, "Pooler service not removed") - self.eventuallyEqual(lambda: k8s.count_secrets_with_label(pooler_label), + self.eventuallyEqual(lambda: k8s.count_secrets_with_label('application=spilo,cluster-name=acid-minimal-cluster'), 4, "Secrets not deleted") # Verify that all the databases have pooler schema installed. From ee55b2db6b9a8c41a536e40bb82d0b96a0ff0bed Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Thu, 3 Mar 2022 10:47:10 +0100 Subject: [PATCH 7/7] return constant for standard pg port --- pkg/cluster/k8sres.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 6717238ee..9d4c614a5 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -107,7 +107,7 @@ func (c *Cluster) servicePort(role PostgresRole) int32 { } c.logger.Warningf("No service for role %s - defaulting to port 5432", role) - return 5432 + return pgPort } func (c *Cluster) podDisruptionBudgetName() string {