fix condition to decide on syncing pooler

pkg/cluster/cluster.go

@@ -1006,9 +1006,9 @@ func (c *Cluster) initSystemUsers() {
 	// Connection pooler user is an exception, if requested it's going to be
 	// created by operator as a normal pgUser
 	if needConnectionPooler(&c.Spec) {
-		// initialize empty connection pooler if not done yet
+		connectionPoolerSpec := c.Spec.ConnectionPooler
-		if c.Spec.ConnectionPooler == nil {
+		if connectionPoolerSpec == nil {
-			c.Spec.ConnectionPooler = &acidv1.ConnectionPooler{}
+			connectionPoolerSpec = &acidv1.ConnectionPooler{}
 		}
 
 		// Using superuser as pooler user is not a good idea. First of all it's
@@ -1016,13 +1016,13 @@ func (c *Cluster) initSystemUsers() {
 		// and second it's a bad practice.
 		username := c.OpConfig.ConnectionPooler.User
 
-		isSuperUser := c.Spec.ConnectionPooler.User == c.OpConfig.SuperUsername
+		isSuperUser := connectionPoolerSpec.User == c.OpConfig.SuperUsername
 		isProtectedUser := c.shouldAvoidProtectedOrSystemRole(
-			c.Spec.ConnectionPooler.User, "connection pool role")
+			connectionPoolerSpec.User, "connection pool role")
 
 		if !isSuperUser && !isProtectedUser {
 			username = util.Coalesce(
-				c.Spec.ConnectionPooler.User,
+				connectionPoolerSpec.User,
 				c.OpConfig.ConnectionPooler.User)
 		}
 

pkg/cluster/connection_pooler.go

@@ -3,6 +3,7 @@ package cluster
 import (
 	"context"
 	"fmt"
+	"reflect"
 	"strings"
 
 	"github.com/r3labs/diff"
@@ -114,7 +115,7 @@ func (c *Cluster) createConnectionPooler(LookupFunction InstallFunction) (SyncRe
 	c.setProcessName("creating connection pooler")
 
 	//this is essentially sync with nil as oldSpec
-	if reason, err := c.syncConnectionPooler(nil, &c.Postgresql, LookupFunction); err != nil {
+	if reason, err := c.syncConnectionPooler(&acidv1.Postgresql{}, &c.Postgresql, LookupFunction); err != nil {
 		return reason, err
 	}
 	return reason, nil
@@ -321,12 +322,13 @@ func (c *Cluster) generateConnectionPoolerDeployment(connectionPooler *Connectio
 	// default values, initialize it to an empty structure. It could be done
 	// anywhere, but here is the earliest common entry point between sync and
 	// create code, so init here.
-	if spec.ConnectionPooler == nil {
+	connectionPoolerSpec := spec.ConnectionPooler
-		spec.ConnectionPooler = &acidv1.ConnectionPooler{}
+	if connectionPoolerSpec == nil {
+		connectionPoolerSpec = &acidv1.ConnectionPooler{}
 	}
 	podTemplate, err := c.generateConnectionPoolerPodTemplate(connectionPooler.Role)
 
-	numberOfInstances := spec.ConnectionPooler.NumberOfInstances
+	numberOfInstances := connectionPoolerSpec.NumberOfInstances
 	if numberOfInstances == nil {
 		numberOfInstances = util.CoalesceInt32(
 			c.OpConfig.ConnectionPooler.NumberOfInstances,
@@ -371,16 +373,6 @@ func (c *Cluster) generateConnectionPoolerDeployment(connectionPooler *Connectio
 func (c *Cluster) generateConnectionPoolerService(connectionPooler *ConnectionPoolerObjects) *v1.Service {
 
 	spec := &c.Spec
-	// there are two ways to enable connection pooler, either to specify a
-	// connectionPooler section or enableConnectionPooler. In the second case
-	// spec.connectionPooler will be nil, so to make it easier to calculate
-	// default values, initialize it to an empty structure. It could be done
-	// anywhere, but here is the earliest common entry point between sync and
-	// create code, so init here.
-	if spec.ConnectionPooler == nil {
-		spec.ConnectionPooler = &acidv1.ConnectionPooler{}
-	}
-
 	serviceSpec := v1.ServiceSpec{
 		Ports: []v1.ServicePort{
 			{
@@ -693,15 +685,7 @@ func (c *Cluster) syncConnectionPooler(oldSpec, newSpec *acidv1.Postgresql, Look
 	var err error
 	var connectionPoolerNeeded bool
 
-	if oldSpec == nil {
+	needSync := !reflect.DeepEqual(oldSpec.Spec.ConnectionPooler, newSpec.Spec.ConnectionPooler)
-		oldSpec = &acidv1.Postgresql{
-			Spec: acidv1.PostgresSpec{
-				ConnectionPooler: &acidv1.ConnectionPooler{},
-			},
-		}
-	}
-
-	needSync, _ := needSyncConnectionPoolerSpecs(oldSpec.Spec.ConnectionPooler, newSpec.Spec.ConnectionPooler, c.logger)
 	masterChanges, err := diff.Diff(oldSpec.Spec.EnableConnectionPooler, newSpec.Spec.EnableConnectionPooler)
 	if err != nil {
 		c.logger.Error("Error in getting diff of master connection pooler changes")
@@ -711,15 +695,14 @@ func (c *Cluster) syncConnectionPooler(oldSpec, newSpec *acidv1.Postgresql, Look
 		c.logger.Error("Error in getting diff of replica connection pooler changes")
 	}
 
-	// skip pooler sync only
+	// skip pooler sync when theres no diff or it's deactivated
-	// 1. if there is no diff in spec, AND
+	// but, handling the case when connectionPooler is not there but it is required
-	// 2. if connection pooler is already there and is also required as per newSpec
-	//
-	// Handling the case when connectionPooler is not there but it is required
 	// as per spec, hence do not skip syncing in that case, even though there
 	// is no diff in specs
 	if (!needSync && len(masterChanges) <= 0 && len(replicaChanges) <= 0) &&
-		(c.ConnectionPooler != nil && (needConnectionPooler(&newSpec.Spec))) {
+		((c.ConnectionPooler == nil && !needConnectionPooler(&newSpec.Spec)) ||
+			(c.ConnectionPooler != nil && needConnectionPooler(&newSpec.Spec) &&
+				c.ConnectionPooler[Master].LookupFunction && c.ConnectionPooler[Replica].LookupFunction)) {
 		c.logger.Debugln("syncing pooler is not required")
 		return nil, nil
 	}
@@ -761,7 +744,24 @@ func (c *Cluster) syncConnectionPooler(oldSpec, newSpec *acidv1.Postgresql, Look
 
 			// in this case also do not forget to install lookup function
 			if !c.ConnectionPooler[role].LookupFunction {
-				if err = c.syncConnectionPoolerSchema(LookupFunction); err != nil {
+				connectionPooler := c.Spec.ConnectionPooler
+				specSchema := ""
+				specUser := ""
+
+				if connectionPooler != nil {
+					specSchema = connectionPooler.Schema
+					specUser = connectionPooler.User
+				}
+
+				schema := util.Coalesce(
+					specSchema,
+					c.OpConfig.ConnectionPooler.Schema)
+
+				user := util.Coalesce(
+					specUser,
+					c.OpConfig.ConnectionPooler.User)
+
+				if err = LookupFunction(schema, user); err != nil {
 					return NoSync, err
 				}
 				c.ConnectionPooler[role].LookupFunction = true
@@ -849,8 +849,6 @@ func (c *Cluster) syncConnectionPoolerWorker(oldSpec, newSpec *acidv1.Postgresql
 			newConnectionPooler = &acidv1.ConnectionPooler{}
 		}
 
-		c.logger.Infof("old: %+v, new %+v", oldConnectionPooler, newConnectionPooler)
-
 		var specSync bool
 		var specReason []string
 
@@ -917,29 +915,3 @@ func (c *Cluster) syncConnectionPoolerWorker(oldSpec, newSpec *acidv1.Postgresql
 
 	return NoSync, nil
 }
-
-func (c *Cluster) syncConnectionPoolerSchema(LookupFunction InstallFunction) error {
-
-	connectionPooler := c.Spec.ConnectionPooler
-	specSchema := ""
-	specUser := ""
-
-	if connectionPooler != nil {
-		specSchema = connectionPooler.Schema
-		specUser = connectionPooler.User
-	}
-
-	schema := util.Coalesce(
-		specSchema,
-		c.OpConfig.ConnectionPooler.Schema)
-
-	user := util.Coalesce(
-		specUser,
-		c.OpConfig.ConnectionPooler.User)
-
-	if err := LookupFunction(schema, user); err != nil {
-		return err
-	}
-
-	return nil
-}

pkg/cluster/sync.go

@@ -759,9 +759,9 @@ func (c *Cluster) syncDatabases() error {
 	}
 
 	if len(createDatabases) > 0 {
-		// create the pooler objects in new database if needed
+		// trigger creation of pooler objects in new database in syncConnectionPooler
-		if c.ConnectionPooler != nil && needConnectionPooler(&c.Spec) {
+		for _, role := range [2]PostgresRole{Master, Replica} {
-			c.syncConnectionPoolerSchema(c.installLookupFunction)
+			c.ConnectionPooler[role].LookupFunction = false
 		}
 	}