Adjust sync logic

pkg/cluster/cluster.go

@@ -600,7 +600,11 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
 		}
 	}
 
-	if !reflect.DeepEqual(oldSpec.Spec.Users, newSpec.Spec.Users) {
+	// connection pool needs one system user created, which is done in
+	// initUsers. Check if it needs to be called.
+	sameUsers := reflect.DeepEqual(oldSpec.Spec.Users, newSpec.Spec.Users)
+	needConnPool := c.needConnectionPoolWorker(&newSpec.Spec)
+	if !sameUsers || needConnPool {
 		c.logger.Debugf("syncing secrets")
 		if err := c.initUsers(); err != nil {
 			c.logger.Errorf("could not init users: %v", err)
@@ -724,15 +728,9 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error {
 		}
 	}
 
-	// connection pool
-	if !reflect.DeepEqual(oldSpec.Spec.ConnectionPool,
-		newSpec.Spec.ConnectionPool) {
-		c.logger.Debug("syncing connection pool")
-
+	// sync connection pool
 	if err := c.syncConnectionPool(oldSpec, newSpec); err != nil {
-			c.logger.Errorf("could not sync connection pool: %v", err)
-			updateFailed = true
-		}
+		return fmt.Errorf("could not sync connection pool: %v", err)
 	}
 
 	return nil

pkg/cluster/resources.go

@@ -160,7 +160,7 @@ func (c *Cluster) deleteConnectionPool() (err error) {
 
 	// Lack of connection pooler objects is not a fatal error, just log it if
 	// it was present before in the manifest
-	if c.needConnectionPool() && c.ConnectionPool == nil {
+	if c.ConnectionPool == nil {
 		c.logger.Infof("No connection pool to delete")
 		return nil
 	}

pkg/cluster/sync.go

@@ -111,43 +111,8 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error {
 	}
 
 	// sync connection pool
-	if c.needConnectionPool() {
-		oldPool := oldSpec.Spec.ConnectionPool
-		newPool := newSpec.Spec.ConnectionPool
-
-		if newPool == nil {
-			// previously specified connectionPool was removed, so delete
-			// connection pool
-			if err := c.deleteConnectionPool(); err != nil {
-				c.logger.Warningf("could not remove connection pool: %v", err)
-			}
-		}
-
-		// do sync in case if any resources were not remembered (it means they
-		// probably were not created, or if specification differs
-		if c.ConnectionPool == nil ||
-			c.ConnectionPool.Deployment == nil ||
-			c.ConnectionPool.Service == nil ||
-			!reflect.DeepEqual(oldPool, newPool) {
-
-			c.logger.Debug("syncing connection pool")
-
-			if err := c.syncConnectionPool(&oldSpec, newSpec); err != nil {
-				c.logger.Errorf("could not sync connection pool: %v", err)
-				return err
-			}
-		}
-	} else {
-		// check if we need to clean up connection pool resources after it was
-		// disabled
-		if c.ConnectionPool != nil &&
-			(c.ConnectionPool.Deployment != nil ||
-				c.ConnectionPool.Service != nil) {
-
-			if err := c.deleteConnectionPool(); err != nil {
-				c.logger.Warningf("could not remove connection pool: %v", err)
-			}
-		}
+	if err = c.syncConnectionPool(&oldSpec, newSpec); err != nil {
+		return fmt.Errorf("could not sync connection pool: %v", err)
 	}
 
 	return err
@@ -499,10 +464,12 @@ func (c *Cluster) syncRoles() (err error) {
 		userNames = append(userNames, u.Name)
 	}
 
+	if c.needConnectionPool() {
 		// An exception from system users, connection pool user
 		connPoolUser := c.systemUsers[constants.ConnectionPoolUserKeyName]
 		userNames = append(userNames, connPoolUser.Name)
 		c.pgUsers[connPoolUser.Name] = connPoolUser
+	}
 
 	dbUsers, err = c.readPgUsersFromDatabase(userNames)
 	if err != nil {
@@ -637,11 +604,68 @@ func (c *Cluster) syncLogicalBackupJob() error {
 	return nil
 }
 
+func (c *Cluster) syncConnectionPool(oldSpec, newSpec *acidv1.Postgresql) error {
+	newNeedConnPool := c.needConnectionPoolWorker(&newSpec.Spec)
+	oldNeedConnPool := c.needConnectionPoolWorker(&oldSpec.Spec)
+
+	if oldNeedConnPool && newNeedConnPool {
+		// sync in case of differences, or if no resources present
+		oldPool := oldSpec.Spec.ConnectionPool
+		newPool := newSpec.Spec.ConnectionPool
+
+		if c.ConnectionPool == nil ||
+			c.ConnectionPool.Deployment == nil ||
+			c.ConnectionPool.Service == nil ||
+			!reflect.DeepEqual(oldPool, newPool) {
+
+			c.logger.Debug("syncing connection pool")
+
+			if err := c.syncConnectionPoolWorker(oldSpec, newSpec); err != nil {
+				c.logger.Errorf("could not sync connection pool: %v", err)
+				return err
+			}
+		} else {
+			c.logger.Debug("No connection pool sync")
+		}
+	}
+
+	if !oldNeedConnPool && newNeedConnPool {
+		// sync to create everything
+		c.logger.Debug("syncing connection pool")
+
+		if err := c.syncConnectionPoolWorker(oldSpec, newSpec); err != nil {
+			c.logger.Errorf("could not sync connection pool: %v", err)
+			return err
+		}
+	}
+
+	if oldNeedConnPool && !newNeedConnPool {
+		// delete and cleanup resources
+		if err := c.deleteConnectionPool(); err != nil {
+			c.logger.Warningf("could not remove connection pool: %v", err)
+		}
+	}
+
+	if !oldNeedConnPool && !newNeedConnPool {
+		// delete and cleanup resources if not empty
+		if c.ConnectionPool != nil &&
+			(c.ConnectionPool.Deployment != nil ||
+				c.ConnectionPool.Service != nil) {
+
+			if err := c.deleteConnectionPool(); err != nil {
+				c.logger.Warningf("could not remove connection pool: %v", err)
+			}
+		}
+	}
+
+	return nil
+}
+
 // Synchronize connection pool resources. Effectively we're interested only in
 // synchronizing the corresponding deployment, but in case of deployment or
 // service is missing, create it. After checking, also remember an object for
 // the future references.
-func (c *Cluster) syncConnectionPool(oldSpec, newSpec *acidv1.Postgresql) error {
+func (c *Cluster) syncConnectionPoolWorker(oldSpec, newSpec *acidv1.Postgresql) error {
 	if c.ConnectionPool == nil {
 		c.logger.Warning("Connection pool resources are empty")
 		c.ConnectionPool = &ConnectionPoolObjects{}

pkg/cluster/util.go

@@ -496,10 +496,14 @@ func (c *Cluster) patroniUsesKubernetes() bool {
 	return c.OpConfig.EtcdHost == ""
 }
 
-func (c *Cluster) needConnectionPool() bool {
-	if c.Spec.EnableConnectionPool == nil {
-		return c.Spec.ConnectionPool != nil
+func (c *Cluster) needConnectionPoolWorker(spec *acidv1.PostgresSpec) bool {
+	if spec.EnableConnectionPool == nil {
+		return spec.ConnectionPool != nil
 	} else {
-		return *c.Spec.EnableConnectionPool
+		return *spec.EnableConnectionPool
 	}
 }
+
+func (c *Cluster) needConnectionPool() bool {
+	return c.needConnectionPoolWorker(&c.Spec)
+}