From 66129335fd9974248f0f603b5e70c7be2a4ca553 Mon Sep 17 00:00:00 2001 From: Dmitrii Dolgov <9erthalion6@gmail.com> Date: Wed, 19 Feb 2020 14:53:58 +0100 Subject: [PATCH] Adjust sync logic --- pkg/cluster/cluster.go | 18 +++---- pkg/cluster/resources.go | 2 +- pkg/cluster/sync.go | 108 ++++++++++++++++++++++++--------------- pkg/cluster/util.go | 12 +++-- 4 files changed, 83 insertions(+), 57 deletions(-) diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 1328646ac..627c15481 100644 --- a/pkg/cluster/cluster.go +++ b/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") - - if err := c.syncConnectionPool(oldSpec, newSpec); err != nil { - c.logger.Errorf("could not sync connection pool: %v", err) - updateFailed = true - } + // sync connection pool + if err := c.syncConnectionPool(oldSpec, newSpec); err != nil { + return fmt.Errorf("could not sync connection pool: %v", err) } return nil diff --git a/pkg/cluster/resources.go b/pkg/cluster/resources.go index ecda3b44d..5c0a7bbd7 100644 --- a/pkg/cluster/resources.go +++ b/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 } diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index bd4a7fc28..f8d242d99 100644 --- a/pkg/cluster/sync.go +++ b/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) } - // An exception from system users, connection pool user - connPoolUser := c.systemUsers[constants.ConnectionPoolUserKeyName] - userNames = append(userNames, connPoolUser.Name) - c.pgUsers[connPoolUser.Name] = connPoolUser + 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{} diff --git a/pkg/cluster/util.go b/pkg/cluster/util.go index 3e53ffd45..269e94b17 100644 --- a/pkg/cluster/util.go +++ b/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) +}