diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index 926888e9f..42f112e4c 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -185,8 +185,8 @@ class EndToEndTestCase(unittest.TestCase): }) self.eventuallyEqual(lambda: k8s.get_deployment_replica_count(), 2, "Deployment replicas is 2 default") - self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler-name=acid-minimal-cluster-pooler"), 2, "No pooler pods found") - self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler-name=acid-minimal-cluster-pooler-repl"), 2, "No pooler replica pods found") + 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('application=db-connection-pooler,cluster-name=acid-minimal-cluster'), 2, "No pooler service found") #Turn off only master connection pooler @@ -200,9 +200,10 @@ class EndToEndTestCase(unittest.TestCase): } }) + 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-name=acid-minimal-cluster-pooler"), 0, "Master pooler pods not deleted") - self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler-name=acid-minimal-cluster-pooler-repl"), 2, "Pooler replica pods not found") + self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler"), 0, "Master pooler pods not deleted") + self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler-repl"), 2, "Pooler replica pods not found") self.eventuallyEqual(lambda: k8s.count_services_with_label('application=db-connection-pooler,cluster-name=acid-minimal-cluster'), 1, "No pooler service found") #Turn off only replica connection pooler @@ -216,9 +217,10 @@ class EndToEndTestCase(unittest.TestCase): } }) + 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-name=acid-minimal-cluster-pooler"), 2, "Master pooler pods not found") - self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler-name=acid-minimal-cluster-pooler-repl"), 0, "Pooler replica pods not deleted") + self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler"), 2, "Master pooler pods not found") + self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler-repl"), 0, "Pooler replica pods not deleted") self.eventuallyEqual(lambda: k8s.count_services_with_label('application=db-connection-pooler,cluster-name=acid-minimal-cluster'), 1, "No pooler service found") @@ -235,7 +237,7 @@ class EndToEndTestCase(unittest.TestCase): }) self.eventuallyEqual(lambda: k8s.get_deployment_replica_count(), 3, "Deployment replicas is scaled to 3") - self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler-name=acid-minimal-cluster-pooler"), 3, "Scale up of pooler pods does not work") + self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler"), 3, "Scale up of pooler pods does not work") # turn it off, keeping config should be overwritten by false k8s.api.custom_objects_api.patch_namespaced_custom_object( @@ -249,7 +251,7 @@ class EndToEndTestCase(unittest.TestCase): }) - self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler-name=acid-minimal-cluster-pooler"), 0, "Pooler pods not scaled down") + self.eventuallyEqual(lambda: k8s.count_running_pods("connection-pooler=acid-minimal-cluster-pooler"), 0, "Pooler pods not scaled down") self.eventuallyEqual(lambda: k8s.count_services_with_label('application=db-connection-pooler,cluster-name=acid-minimal-cluster'), 0, "Pooler service not removed") # Verify that all the databases have pooler schema installed. diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 6719a3cdf..28552bc6f 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -331,7 +331,7 @@ func (c *Cluster) Create() error { // // Do not consider connection pooler as a strict requirement, and if // something fails, report warning - c.createConnectionPooler() + c.createConnectionPooler(c.installLookupFunction) return nil } @@ -759,8 +759,7 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error { // check which databases we need to process, but even repeating the whole // installation process should be good enough. - if _, err := c.syncConnectionPooler(oldSpec, newSpec, - c.installLookupFunction); err != nil { + if _, err := c.syncConnectionPooler(oldSpec, newSpec, c.installLookupFunction); err != nil { c.logger.Errorf("could not sync connection pooler: %v", err) updateFailed = true } diff --git a/pkg/cluster/connection_pooler.go b/pkg/cluster/connection_pooler.go index bebe00d28..ff1dee6a9 100644 --- a/pkg/cluster/connection_pooler.go +++ b/pkg/cluster/connection_pooler.go @@ -77,15 +77,15 @@ func needReplicaConnectionPoolerWorker(spec *acidv1.PostgresSpec) bool { // have e.g. different `application` label, so that recreatePod operation will // not interfere with it (it lists all the pods via labels, and if there would // be no difference, it will recreate also pooler pods). -func (c *Cluster) connectionPoolerLabelsSelector(name string, role PostgresRole) *metav1.LabelSelector { +func (c *Cluster) connectionPoolerLabelsSelector(role PostgresRole) *metav1.LabelSelector { connectionPoolerLabels := labels.Set(map[string]string{}) extraLabels := labels.Set(map[string]string{ - "connection-pooler-name": name, - "application": "db-connection-pooler", - "role": string(role), - "cluster-name": c.Name, - "Namespace": c.Namespace, + "connection-pooler": c.connectionPoolerName(role), + "application": "db-connection-pooler", + "spilo-role": string(role), + "cluster-name": c.Name, + "Namespace": c.Namespace, }) connectionPoolerLabels = labels.Merge(connectionPoolerLabels, c.labelsSet(false)) @@ -108,12 +108,12 @@ func (c *Cluster) connectionPoolerLabelsSelector(name string, role PostgresRole) // have connectionpooler name in the cp object to have it immutable name // add these cp related functions to a new cp file // opConfig, cluster, and database name -func (c *Cluster) createConnectionPooler() (SyncReason, error) { +func (c *Cluster) createConnectionPooler(LookupFunction InstallFunction) (SyncReason, error) { var reason SyncReason c.setProcessName("creating connection pooler") //this is essentially sync with nil as oldSpec - if reason, err := c.syncConnectionPooler(nil, &c.Postgresql, c.installLookupFunction); err != nil { + if reason, err := c.syncConnectionPooler(nil, &c.Postgresql, LookupFunction); err != nil { return reason, err } return reason, nil @@ -283,7 +283,7 @@ func (c *Cluster) generateConnectionPoolerPodTemplate(role PostgresRole) ( podTemplate := &v1.PodTemplateSpec{ ObjectMeta: metav1.ObjectMeta{ - Labels: c.connectionPoolerLabelsSelector(c.connectionPoolerName(role), role).MatchLabels, + Labels: c.connectionPoolerLabelsSelector(role).MatchLabels, Namespace: c.Namespace, Annotations: c.generatePodAnnotations(spec), }, @@ -337,7 +337,7 @@ func (c *Cluster) generateConnectionPoolerDeployment(connectionPooler *Connectio ObjectMeta: metav1.ObjectMeta{ Name: connectionPooler.Name, Namespace: connectionPooler.Namespace, - Labels: c.connectionPoolerLabelsSelector(connectionPooler.Name, connectionPooler.Role).MatchLabels, + Labels: c.connectionPoolerLabelsSelector(connectionPooler.Role).MatchLabels, Annotations: map[string]string{}, // make StatefulSet object its owner to represent the dependency. // By itself StatefulSet is being deleted with "Orphaned" @@ -349,7 +349,7 @@ func (c *Cluster) generateConnectionPoolerDeployment(connectionPooler *Connectio }, Spec: appsv1.DeploymentSpec{ Replicas: numberOfInstances, - Selector: c.connectionPoolerLabelsSelector(connectionPooler.Name, connectionPooler.Role), + Selector: c.connectionPoolerLabelsSelector(connectionPooler.Role), Template: *podTemplate, }, } @@ -380,7 +380,7 @@ func (c *Cluster) generateConnectionPoolerService(connectionPooler *ConnectionPo }, Type: v1.ServiceTypeClusterIP, Selector: map[string]string{ - "connection-pooler": connectionPooler.Name, + "connection-pooler": c.connectionPoolerName(connectionPooler.Role), }, } @@ -388,7 +388,7 @@ func (c *Cluster) generateConnectionPoolerService(connectionPooler *ConnectionPo ObjectMeta: metav1.ObjectMeta{ Name: connectionPooler.Name, Namespace: connectionPooler.Namespace, - Labels: c.connectionPoolerLabelsSelector(connectionPooler.Name, connectionPooler.Role).MatchLabels, + Labels: c.connectionPoolerLabelsSelector(connectionPooler.Role).MatchLabels, Annotations: map[string]string{}, // make StatefulSet object its owner to represent the dependency. // By itself StatefulSet is being deleted with "Orphaned" @@ -657,7 +657,7 @@ func makeDefaultConnectionPoolerResources(config *config.Config) acidv1.Resource } } -func (c *Cluster) syncConnectionPooler(oldSpec, newSpec *acidv1.Postgresql, installLookupFunction InstallFunction) (SyncReason, error) { +func (c *Cluster) syncConnectionPooler(oldSpec, newSpec *acidv1.Postgresql, LookupFunction InstallFunction) (SyncReason, error) { var reason SyncReason var err error @@ -695,6 +695,7 @@ func (c *Cluster) syncConnectionPooler(oldSpec, newSpec *acidv1.Postgresql, inst Role: role, } } + if newNeedConnectionPooler { // Try to sync in any case. If we didn't needed connection pooler before, // it means we want to create it. If it was already present, still sync @@ -723,7 +724,7 @@ func (c *Cluster) syncConnectionPooler(oldSpec, newSpec *acidv1.Postgresql, inst specUser, c.OpConfig.ConnectionPooler.User) - if err = installLookupFunction(schema, user, role); err != nil { + if err = LookupFunction(schema, user, role); err != nil { return NoSync, err } } diff --git a/pkg/cluster/connection_pooler_test.go b/pkg/cluster/connection_pooler_test.go index b5c6b225b..dfbf43476 100644 --- a/pkg/cluster/connection_pooler_test.go +++ b/pkg/cluster/connection_pooler_test.go @@ -57,8 +57,19 @@ func TestConnectionPoolerCreationAndDeletion(t *testing.T) { ConnectionPooler: &acidv1.ConnectionPooler{}, EnableReplicaConnectionPooler: boolToPointer(true), } - - reason, err := cluster.createConnectionPooler() + cluster.ConnectionPooler = map[PostgresRole]*ConnectionPoolerObjects{ + Master: { + Deployment: nil, + Service: nil, + LookupFunction: true, + }, + Replica: { + Deployment: nil, + Service: nil, + LookupFunction: true, + }, + } + reason, err := cluster.createConnectionPooler(mockInstallLookupFunction) if err != nil { t.Errorf("%s: Cannot create connection pooler, %s, %+v", @@ -347,6 +358,21 @@ func TestConnectionPoolerSynchronization(t *testing.T) { defaultInstances int32 check func(cluster *Cluster, err error, reason SyncReason) error }{ + { + subTest: "create from scratch", + oldSpec: &acidv1.Postgresql{ + Spec: acidv1.PostgresSpec{}, + }, + newSpec: &acidv1.Postgresql{ + Spec: acidv1.PostgresSpec{ + ConnectionPooler: &acidv1.ConnectionPooler{}, + }, + }, + cluster: clusterMissingObjects, + defaultImage: "pooler:1.0", + defaultInstances: 1, + check: MasterobjectsAreSaved, + }, { subTest: "create if doesn't exist", oldSpec: &acidv1.Postgresql{ @@ -379,22 +405,6 @@ func TestConnectionPoolerSynchronization(t *testing.T) { defaultInstances: 1, check: MasterobjectsAreSaved, }, - { - subTest: "create replica if doesn't exist with a flag", - oldSpec: &acidv1.Postgresql{ - Spec: acidv1.PostgresSpec{}, - }, - newSpec: &acidv1.Postgresql{ - Spec: acidv1.PostgresSpec{ - ConnectionPooler: &acidv1.ConnectionPooler{}, - EnableReplicaConnectionPooler: boolToPointer(true), - }, - }, - cluster: clusterDirtyMock, - defaultImage: "pooler:1.0", - defaultInstances: 1, - check: ReplicaobjectsAreSaved, - }, { subTest: "create no replica with flag", oldSpec: &acidv1.Postgresql{ @@ -411,19 +421,20 @@ func TestConnectionPoolerSynchronization(t *testing.T) { check: objectsAreDeleted, }, { - subTest: "create from scratch", + subTest: "create replica if doesn't exist with a flag", oldSpec: &acidv1.Postgresql{ Spec: acidv1.PostgresSpec{}, }, newSpec: &acidv1.Postgresql{ Spec: acidv1.PostgresSpec{ - ConnectionPooler: &acidv1.ConnectionPooler{}, + ConnectionPooler: &acidv1.ConnectionPooler{}, + EnableReplicaConnectionPooler: boolToPointer(true), }, }, - cluster: clusterMissingObjects, + cluster: clusterDirtyMock, defaultImage: "pooler:1.0", defaultInstances: 1, - check: MasterobjectsAreSaved, + check: ReplicaobjectsAreSaved, }, { subTest: "create both master and replica", @@ -443,19 +454,22 @@ func TestConnectionPoolerSynchronization(t *testing.T) { check: objectsAreSaved, }, { - subTest: "delete if not needed", + subTest: "delete only replica if not needed", oldSpec: &acidv1.Postgresql{ + Spec: acidv1.PostgresSpec{ + ConnectionPooler: &acidv1.ConnectionPooler{}, + EnableReplicaConnectionPooler: boolToPointer(true), + }, + }, + newSpec: &acidv1.Postgresql{ Spec: acidv1.PostgresSpec{ ConnectionPooler: &acidv1.ConnectionPooler{}, }, }, - newSpec: &acidv1.Postgresql{ - Spec: acidv1.PostgresSpec{}, - }, - cluster: clusterMock, + cluster: clusterDirtyMock, defaultImage: "pooler:1.0", defaultInstances: 1, - check: objectsAreDeleted, + check: OnlyReplicaDeleted, }, { subTest: "delete only master if not needed", @@ -476,22 +490,19 @@ func TestConnectionPoolerSynchronization(t *testing.T) { check: OnlyMasterDeleted, }, { - subTest: "delete only replica if not needed", + subTest: "delete if not needed", oldSpec: &acidv1.Postgresql{ - Spec: acidv1.PostgresSpec{ - ConnectionPooler: &acidv1.ConnectionPooler{}, - EnableReplicaConnectionPooler: boolToPointer(true), - }, - }, - newSpec: &acidv1.Postgresql{ Spec: acidv1.PostgresSpec{ ConnectionPooler: &acidv1.ConnectionPooler{}, }, }, - cluster: clusterDirtyMock, + newSpec: &acidv1.Postgresql{ + Spec: acidv1.PostgresSpec{}, + }, + cluster: clusterMock, defaultImage: "pooler:1.0", defaultInstances: 1, - check: OnlyReplicaDeleted, + check: objectsAreDeleted, }, { subTest: "cleanup if still there", @@ -742,7 +753,9 @@ func TestConnectionPoolerDeploymentSpec(t *testing.T) { Master: { Deployment: nil, Service: nil, - LookupFunction: false, + LookupFunction: true, + Name: "", + Role: Master, }, } @@ -835,9 +848,9 @@ func testResources(cluster *Cluster, podSpec *v1.PodTemplateSpec, role PostgresR func testLabels(cluster *Cluster, podSpec *v1.PodTemplateSpec, role PostgresRole) error { poolerLabels := podSpec.ObjectMeta.Labels["connection-pooler"] - if poolerLabels != cluster.connectionPoolerLabelsSelector(cluster.connectionPoolerName(role), role).MatchLabels["connection-pooler"] { + if poolerLabels != cluster.connectionPoolerLabelsSelector(role).MatchLabels["connection-pooler"] { return fmt.Errorf("Pod labels do not match, got %+v, expected %+v", - podSpec.ObjectMeta.Labels, cluster.connectionPoolerLabelsSelector(cluster.connectionPoolerName(role), role).MatchLabels) + podSpec.ObjectMeta.Labels, cluster.connectionPoolerLabelsSelector(role).MatchLabels) } return nil @@ -845,7 +858,7 @@ func testLabels(cluster *Cluster, podSpec *v1.PodTemplateSpec, role PostgresRole func testSelector(cluster *Cluster, deployment *appsv1.Deployment) error { labels := deployment.Spec.Selector.MatchLabels - expected := cluster.connectionPoolerLabelsSelector(cluster.connectionPoolerName(Master), Master).MatchLabels + expected := cluster.connectionPoolerLabelsSelector(Master).MatchLabels if labels["connection-pooler"] != expected["connection-pooler"] { return fmt.Errorf("Labels are incorrect, got %+v, expected %+v", @@ -894,18 +907,12 @@ func TestConnectionPoolerServiceSpec(t *testing.T) { Deployment: nil, Service: nil, LookupFunction: false, - Name: cluster.connectionPoolerName(Master), - ClusterName: cluster.ClusterName, - Namespace: cluster.Namespace, Role: Master, }, Replica: { Deployment: nil, Service: nil, LookupFunction: false, - Name: cluster.connectionPoolerName(Replica), - ClusterName: cluster.ClusterName, - Namespace: cluster.Namespace, Role: Replica, }, }