diff --git a/internal/controller/postgrescluster/pgbouncer.go b/internal/controller/postgrescluster/pgbouncer.go index e3fdcbc9b..9bcc77f0b 100644 --- a/internal/controller/postgrescluster/pgbouncer.go +++ b/internal/controller/postgrescluster/pgbouncer.go @@ -69,7 +69,7 @@ func (r *Reconciler) reconcilePGBouncerConfigMap( configmap := &corev1.ConfigMap{ObjectMeta: naming.ClusterPGBouncer(cluster)} configmap.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("ConfigMap")) - if cluster.Spec.Proxy == nil || cluster.Spec.Proxy.PGBouncer == nil { + if !cluster.Spec.Proxy.PGBouncerEnabled() { // PgBouncer is disabled; delete the ConfigMap if it exists. Check the // client cache first using Get. key := client.ObjectKeyFromObject(configmap) @@ -134,7 +134,7 @@ func (r *Reconciler) reconcilePGBouncerInPostgreSQL( // K8SPG-345 var exposeSuperusers bool - if cluster.Spec.Proxy != nil && cluster.Spec.Proxy.PGBouncer != nil { + if cluster.Spec.Proxy.PGBouncerEnabled() { exposeSuperusers = cluster.Spec.Proxy.PGBouncer.ExposeSuperusers if exposeSuperusers { log.Info("Superusers are exposed through PGBouncer") @@ -144,7 +144,7 @@ func (r *Reconciler) reconcilePGBouncerInPostgreSQL( action := func(ctx context.Context, exec postgres.Executor) error { return errors.WithStack(pgbouncer.EnableInPostgreSQL(ctx, exec, clusterSecret, exposeSuperusers)) } - if cluster.Spec.Proxy == nil || cluster.Spec.Proxy.PGBouncer == nil { + if !cluster.Spec.Proxy.PGBouncerEnabled() { // PgBouncer is disabled. action = func(ctx context.Context, exec postgres.Executor) error { return errors.WithStack(pgbouncer.DisableInPostgreSQL(ctx, exec)) @@ -254,7 +254,7 @@ func (r *Reconciler) reconcilePGBouncerSecret( return nil, err } - if cluster.Spec.Proxy == nil || cluster.Spec.Proxy.PGBouncer == nil { + if !cluster.Spec.Proxy.PGBouncerEnabled() { // PgBouncer is disabled; delete the Secret if it exists. if err == nil { err = errors.WithStack(r.deleteControlled(ctx, cluster, existing)) @@ -397,7 +397,7 @@ func (r *Reconciler) generatePGBouncerService( service := &corev1.Service{ObjectMeta: naming.ClusterPGBouncer(cluster)} service.SetGroupVersionKind(corev1.SchemeGroupVersion.WithKind("Service")) - if cluster.Spec.Proxy == nil || cluster.Spec.Proxy.PGBouncer == nil { + if !cluster.Spec.Proxy.PGBouncerEnabled() { return service, false, nil } @@ -504,7 +504,7 @@ func (r *Reconciler) generatePGBouncerDeployment( deploy := &appsv1.Deployment{ObjectMeta: naming.ClusterPGBouncer(cluster)} deploy.SetGroupVersionKind(appsv1.SchemeGroupVersion.WithKind("Deployment")) - if cluster.Spec.Proxy == nil || cluster.Spec.Proxy.PGBouncer == nil { + if !cluster.Spec.Proxy.PGBouncerEnabled() { return deploy, false, nil } @@ -693,7 +693,7 @@ func (r *Reconciler) reconcilePGBouncerPodDisruptionBudget( return client.IgnoreNotFound(err) } - if cluster.Spec.Proxy == nil || cluster.Spec.Proxy.PGBouncer == nil { + if !cluster.Spec.Proxy.PGBouncerEnabled() { return deleteExistingPDB(cluster) } diff --git a/internal/controller/postgrescluster/pgbouncer_test.go b/internal/controller/postgrescluster/pgbouncer_test.go index fc1932d7c..0c36c30b2 100644 --- a/internal/controller/postgrescluster/pgbouncer_test.go +++ b/internal/controller/postgrescluster/pgbouncer_test.go @@ -171,6 +171,24 @@ namespace: ns5 } }) + t.Run("ZeroReplicas", func(t *testing.T) { + cluster := cluster.DeepCopy() + cluster.Spec.Proxy = &v1beta1.PostgresProxySpec{ + PGBouncer: &v1beta1.PGBouncerPodSpec{ + Replicas: new(int32), + }, + } + + service, specified, err := reconciler.generatePGBouncerService(cluster) + assert.NilError(t, err) + assert.Assert(t, !specified) + + assert.Assert(t, cmp.MarshalMatches(service.ObjectMeta, ` +name: pg7-pgbouncer +namespace: ns5 + `)) + }) + cluster.Spec.Proxy = &v1beta1.PostgresProxySpec{ PGBouncer: &v1beta1.PGBouncerPodSpec{ Port: new(int32(9651)), diff --git a/internal/controller/postgrescluster/postgres.go b/internal/controller/postgrescluster/postgres.go index 8811c2f25..8c8a85621 100644 --- a/internal/controller/postgrescluster/postgres.go +++ b/internal/controller/postgrescluster/postgres.go @@ -139,7 +139,7 @@ func (r *Reconciler) generatePostgresUserSecret( } // When PgBouncer is enabled, include values for connecting through it. - if cluster.Spec.Proxy != nil && cluster.Spec.Proxy.PGBouncer != nil { + if cluster.Spec.Proxy.PGBouncerEnabled() { pgBouncer := naming.ClusterPGBouncer(cluster) hostname := pgBouncer.Name + "." + pgBouncer.Namespace + ".svc" port := fmt.Sprint(*cluster.Spec.Proxy.PGBouncer.Port) diff --git a/internal/pgbouncer/reconcile.go b/internal/pgbouncer/reconcile.go index 83f5c01ff..632b4e5c9 100644 --- a/internal/pgbouncer/reconcile.go +++ b/internal/pgbouncer/reconcile.go @@ -25,7 +25,7 @@ func ConfigMap( inCluster *v1beta1.PostgresCluster, outConfigMap *corev1.ConfigMap, ) { - if inCluster.Spec.Proxy == nil || inCluster.Spec.Proxy.PGBouncer == nil { + if !inCluster.Spec.Proxy.PGBouncerEnabled() { // PgBouncer is disabled; there is nothing to do. return } @@ -56,7 +56,7 @@ func Secret(ctx context.Context, frontendCertManagerSecret *corev1.Secret, additionalCAs [][]byte, ) error { - if inCluster.Spec.Proxy == nil || inCluster.Spec.Proxy.PGBouncer == nil { + if !inCluster.Spec.Proxy.PGBouncerEnabled() { // PgBouncer is disabled; there is nothing to do. return nil } @@ -171,7 +171,7 @@ func Pod( inSecret *corev1.Secret, outPod *corev1.PodSpec, ) { - if inCluster.Spec.Proxy == nil || inCluster.Spec.Proxy.PGBouncer == nil { + if !inCluster.Spec.Proxy.PGBouncerEnabled() { // PgBouncer is disabled; there is nothing to do. return } @@ -302,7 +302,7 @@ func PostgreSQL( inCluster *v1beta1.PostgresCluster, outHBAs *postgres.HBAs, ) { - if inCluster.Spec.Proxy == nil || inCluster.Spec.Proxy.PGBouncer == nil { + if !inCluster.Spec.Proxy.PGBouncerEnabled() { // PgBouncer is disabled; there is nothing to do. return } diff --git a/percona/controller/pgcluster/gethost_test.go b/percona/controller/pgcluster/gethost_test.go index 27a0e52a0..63910566f 100644 --- a/percona/controller/pgcluster/gethost_test.go +++ b/percona/controller/pgcluster/gethost_test.go @@ -41,6 +41,13 @@ func TestGetHost(t *testing.T) { proxy: &v2.PGProxySpec{PGBouncer: nil}, expectedHost: clusterName + "-primary." + ns + ".svc", }, + { + name: "PGBouncer configured with zero replicas", + proxy: &v2.PGProxySpec{PGBouncer: &v2.PGBouncerSpec{ + Replicas: new(int32), + }}, + expectedHost: clusterName + "-primary." + ns + ".svc", + }, { name: "PGBouncer configured, no ServiceExpose", proxy: &v2.PGProxySpec{PGBouncer: &v2.PGBouncerSpec{}}, diff --git a/percona/controller/pgcluster/status.go b/percona/controller/pgcluster/status.go index fa8dbdf6b..fa61ed10b 100644 --- a/percona/controller/pgcluster/status.go +++ b/percona/controller/pgcluster/status.go @@ -32,7 +32,7 @@ func (r *PGClusterReconciler) getHost(ctx context.Context, cr *v2.PerconaPGClust } // If proxy is not configured, use the primary service as host. - if cr.Spec.Proxy == nil || cr.Spec.Proxy.PGBouncer == nil { + if !cr.Spec.Proxy.PGBouncerEnabled() { return svcFQDN(naming.ClusterPrimaryService(postgresCluster).Name, postgresCluster.Namespace), nil } @@ -190,7 +190,6 @@ func updateConditions(cr *v2.PerconaPGCluster, status *v1beta1.PostgresClusterSt } setClusterNotReadyCondition(metav1.ConditionTrue, "AllConditionsAreTrue") - } func syncConditionsFromPostgresToPercona(cr *v2.PerconaPGCluster, postgresStatus *v1beta1.PostgresClusterStatus) { @@ -207,7 +206,6 @@ func syncConditionsFromPostgresToPercona(cr *v2.PerconaPGCluster, postgresStatus } func syncPatroniFromPostgresToPercona(cr *v2.PerconaPGCluster, postgresStatus *v1beta1.PostgresClusterStatus) { - if cr.Status.Patroni.Status == nil { cr.Status.Patroni.Status = &v1beta1.PatroniStatus{} } @@ -227,5 +225,4 @@ func syncPgbackrestFromPostgresToPercona(cr *v2.PerconaPGCluster, postgresStatus if postgresStatus.PGBackRest != nil { cr.Status.PGBackRest = postgresStatus.PGBackRest.DeepCopy() } - } diff --git a/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types.go b/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types.go index 41f32bb54..7a107d66a 100644 --- a/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types.go +++ b/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types.go @@ -1182,6 +1182,10 @@ func (p *PGProxySpec) IsSet() bool { return p != nil && p.PGBouncer != nil } +func (p *PGProxySpec) PGBouncerEnabled() bool { + return p.IsSet() && (p.PGBouncer.Replicas == nil || *p.PGBouncer.Replicas != 0) +} + func (p *PGProxySpec) ToCrunchy(version string) *crunchyv1beta1.PostgresProxySpec { if p == nil { return nil diff --git a/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types_test.go b/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types_test.go index 70178a8d4..efa685054 100644 --- a/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types_test.go +++ b/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types_test.go @@ -145,6 +145,46 @@ func TestPerconaPGCluster_Proxy(t *testing.T) { }) } +func TestPGProxySpec_PGBouncerEnabled(t *testing.T) { + tests := map[string]struct { + spec *PGProxySpec + expected bool + }{ + "nil proxy": { + spec: nil, + expected: false, + }, + "nil PgBouncer": { + spec: &PGProxySpec{}, + expected: false, + }, + "replicas unspecified": { + spec: &PGProxySpec{ + PGBouncer: &PGBouncerSpec{}, + }, + expected: true, + }, + "zero replicas": { + spec: &PGProxySpec{ + PGBouncer: &PGBouncerSpec{Replicas: new(int32(0))}, + }, + expected: false, + }, + "non-zero replicas": { + spec: &PGProxySpec{ + PGBouncer: &PGBouncerSpec{Replicas: new(int32(1))}, + }, + expected: true, + }, + } + + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + assert.Equal(t, tt.expected, tt.spec.PGBouncerEnabled()) + }) + } +} + func TestPerconaPGCluster_PostgresImage(t *testing.T) { cluster := new(PerconaPGCluster) cluster.Default() diff --git a/pkg/apis/upstream.pgv2.percona.com/v1beta1/postgrescluster_types.go b/pkg/apis/upstream.pgv2.percona.com/v1beta1/postgrescluster_types.go index 37fe1e620..6e684db83 100644 --- a/pkg/apis/upstream.pgv2.percona.com/v1beta1/postgrescluster_types.go +++ b/pkg/apis/upstream.pgv2.percona.com/v1beta1/postgrescluster_types.go @@ -706,6 +706,11 @@ func (s *PostgresProxySpec) Default() { } } +// K8SPG-1062 +func (s *PostgresProxySpec) PGBouncerEnabled() bool { + return s != nil && s.PGBouncer != nil && (s.PGBouncer.Replicas == nil || *s.PGBouncer.Replicas != 0) +} + type RegistrationRequirementStatus struct { PGOVersion string `json:"pgoVersion,omitempty"` }