Sync due to defaults

Since we can miss it while checking only spec

pkg/cluster/cluster.go

@@ -1206,7 +1206,7 @@ func (c *Cluster) deletePatroniClusterConfigMaps() error {
 // Test if two connection pool configuration needs to be synced. For simplicity
 // compare not the actual K8S objects, but the configuration itself and request
 // sync if there is any difference.
-func (c *Cluster) needSyncConnPoolDeployments(oldSpec, newSpec *acidv1.ConnectionPool) (sync bool, reasons []string) {
+func (c *Cluster) needSyncConnPoolSpecs(oldSpec, newSpec *acidv1.ConnectionPool) (sync bool, reasons []string) {
 	reasons = []string{}
 	sync = false
 
@@ -1228,3 +1228,69 @@ func (c *Cluster) needSyncConnPoolDeployments(oldSpec, newSpec *acidv1.Connectio
 
 	return sync, reasons
 }
+
+func syncResources(a, b *v1.ResourceRequirements) bool {
+	for _, res := range []v1.ResourceName{
+		v1.ResourceCPU,
+		v1.ResourceMemory,
+	} {
+		if !a.Limits[res].Equal(b.Limits[res]) ||
+			!a.Requests[res].Equal(b.Requests[res]) {
+			return true
+		}
+	}
+
+	return false
+}
+
+// Check if we need to synchronize connection pool deploymend due to new
+// defaults, that are different from what we see in the DeploymentSpec
+func (c *Cluster) needSyncConnPoolDefaults(
+	spec *acidv1.ConnectionPool,
+	deployment *appsv1.Deployment) (sync bool, reasons []string) {
+
+	reasons = []string{}
+	sync = false
+
+	config := c.OpConfig.ConnectionPool
+	podTemplate := deployment.Spec.Template
+	poolContainer := podTemplate.Spec.Containers[constants.ConnPoolContainer]
+
+	if spec.NumberOfInstances == nil &&
+		deployment.Spec.Replicas != config.NumberOfInstances {
+
+		sync = true
+		msg := fmt.Sprintf("NumberOfInstances is different (%d vs %d)",
+			deployment.Spec.Replicas, config.NumberOfInstances)
+		reasons = append(reasons, msg)
+	}
+
+	if spec.DockerImage == "" &&
+		poolContainer.Image != config.Image {
+
+		sync = true
+		msg := fmt.Sprintf("DockerImage is different (%s vs %s)",
+			poolContainer.Image, config.Image)
+		reasons = append(reasons, msg)
+	}
+
+	expectedResources, err := generateResourceRequirements(spec.Resources,
+		c.makeDefaultConnPoolResources())
+
+	// An error to generate expected resources means something is not quite
+	// right, but for the purpose of robustness do not panic here, just report
+	// and ignore resources comparison (in the worst case there will be no
+	// updates for new resource values).
+	if err == nil && syncResources(&poolContainer.Resources, expectedResources) {
+		sync = true
+		msg := fmt.Sprintf("Resources are different (%+v vs %+v)",
+			poolContainer.Resources, expectedResources)
+		reasons = append(reasons, msg)
+	}
+
+	if err != nil {
+		c.logger.Warningf("Cannot generate expected resources, %v", err)
+	}
+
+	return sync, reasons
+}

pkg/cluster/k8sres.go

@@ -1754,7 +1754,7 @@ func (c *Cluster) generateConnPoolPodTemplate(spec *acidv1.PostgresSpec) (
 	secretSelector := func(key string) *v1.SecretKeySelector {
 		return &v1.SecretKeySelector{
 			LocalObjectReference: v1.LocalObjectReference{
-				Name: c.credentialSecretName(c.OpConfig.SuperUsername),
+				Name: c.credentialSecretName(c.OpConfig.ConnectionPool.User),
 			},
 			Key: key,
 		}

pkg/cluster/sync.go

@@ -702,8 +702,10 @@ func (c *Cluster) syncConnectionPoolWorker(oldSpec, newSpec *acidv1.Postgresql)
 		// actual synchronization
 		oldConnPool := oldSpec.Spec.ConnectionPool
 		newConnPool := newSpec.Spec.ConnectionPool
-		sync, reason := c.needSyncConnPoolDeployments(oldConnPool, newConnPool)
+		specSync, specReason := c.needSyncConnPoolSpecs(oldConnPool, newConnPool)
-		if sync {
+		defaultsSync, defaultsReason := c.needSyncConnPoolDefaults(newConnPool, deployment)
+		reason := append(specReason, defaultsReason...)
+		if specSync || defaultsSync {
 			c.logger.Infof("Update connection pool deployment %s, reason: %s",
 				c.connPoolName(), reason)
 

pkg/cluster/sync_test.go

@@ -88,6 +88,11 @@ func TestConnPoolSynchronization(t *testing.T) {
 		Service:    &v1.Service{},
 	}
 
+	clusterNewDefaultsMock := *cluster
+	clusterNewDefaultsMock.KubeClient = k8sutil.NewMockKubernetesClient()
+	cluster.OpConfig.ConnectionPool.Image = "pooler:2.0"
+	cluster.OpConfig.ConnectionPool.NumberOfInstances = int32ToPointer(2)
+
 	tests := []struct {
 		subTest string
 		oldSpec *acidv1.Postgresql
@@ -166,6 +171,21 @@ func TestConnPoolSynchronization(t *testing.T) {
 			cluster: &clusterMock,
 			check:   deploymentUpdated,
 		},
+		{
+			subTest: "update image from changed defaults",
+			oldSpec: &acidv1.Postgresql{
+				Spec: acidv1.PostgresSpec{
+					ConnectionPool: &acidv1.ConnectionPool{},
+				},
+			},
+			newSpec: &acidv1.Postgresql{
+				Spec: acidv1.PostgresSpec{
+					ConnectionPool: &acidv1.ConnectionPool{},
+				},
+			},
+			cluster: &clusterNewDefaultsMock,
+			check:   deploymentUpdated,
+		},
 	}
 	for _, tt := range tests {
 		err := tt.cluster.syncConnectionPool(tt.oldSpec, tt.newSpec)

pkg/util/constants/pooler.go

@@ -10,4 +10,6 @@ const (
 	ConnectionPoolDefaultCpuLimit      = "100m"
 	ConnectionPoolDefaultMemoryRequest = "100Mi"
 	ConnectionPoolDefaultMemoryLimit   = "100Mi"
+
+	ConnPoolContainer = 0
 )

pkg/util/k8sutil/k8sutil.go

@@ -300,6 +300,18 @@ func (mock *mockDeployment) Get(name string, opts metav1.GetOptions) (*apiappsv1
 		ObjectMeta: metav1.ObjectMeta{
 			Name: "test-deployment",
 		},
+		Spec: apiappsv1.DeploymentSpec{
+			Replicas: int32ToPointer(1),
+			Template: v1.PodTemplateSpec{
+				Spec: v1.PodSpec{
+					Containers: []v1.Container{
+						v1.Container{
+							Image: "pooler:1.0",
+						},
+					},
+				},
+			},
+		},
 	}, nil
 }