Make Patroni Status Non-Pointer

Initially the patroni status only had one purpose meaning it could be
overwritten when its one field (SystemIdentifier) was invalid. Now that
another Patroni status is being added we need to be more careful when
making changes to it. By making this change we can update the status in
both places (i.e. the SystemIdentifier field and the new Switchover
field) without having to worry about instantiating the pointer value. We
can do this because we create the Patroni status on every cluster that
gets created by the operator. There are also some checks throughout the
code that get updated to check for a value in
`Status.Patroni.SystemIdentifier` instead of the existence of
`Status.Patroni`.

Author: jmckulk

internal/controller/postgrescluster/patroni.go

@@ -312,9 +312,7 @@ func (r *Reconciler) reconcilePatroniStatus(
 	if err == nil {
 		if dcs.Annotations["initialize"] != "" {
 			// After bootstrap, Patroni writes the cluster system identifier to DCS.
-			cluster.Status.Patroni = &v1beta1.PatroniStatus{
-				SystemIdentifier: dcs.Annotations["initialize"],
-			}
+			cluster.Status.Patroni.SystemIdentifier = dcs.Annotations["initialize"]
 		} else if readyInstance {
 			// While we typically expect a value for the initialize key to be present in the
 			// Endpoints above by the time the StatefulSet for any instance indicates "ready"

internal/controller/postgrescluster/pgbackrest.go

@@ -954,7 +954,7 @@ func (r *Reconciler) prepareForRestore(ctx context.Context,
 			Message:            "Restoring cluster in-place",
 		})
 		// the cluster is no longer bootstrapped
-		cluster.Status.Patroni = nil
+		cluster.Status.Patroni.SystemIdentifier = ""
 		// the restore will change the contents of the database, so the pgbouncer and exporter hashes
 		// are no longer valid
 		cluster.Status.Proxy.PGBouncer.PostgreSQLRevision = ""

internal/controller/postgrescluster/pgbackrest_test.go

@@ -223,7 +223,7 @@ func TestReconcilePGBackRest(t *testing.T) {
 
 	// set status
 	postgresCluster.Status = v1beta1.PostgresClusterStatus{
-		Patroni: &v1beta1.PatroniStatus{SystemIdentifier: "12345abcde"},
+		Patroni: v1beta1.PatroniStatus{SystemIdentifier: "12345abcde"},
 		PGBackRest: &v1beta1.PGBackRestStatus{
 			RepoHost: &v1beta1.RepoHostStatus{Ready: true},
 			Repos:    []v1beta1.RepoStatus{{Name: "repo1", StanzaCreated: true}}},
@@ -539,7 +539,7 @@ func TestReconcilePGBackRest(t *testing.T) {
 
 		// set status
 		postgresCluster.Status = v1beta1.PostgresClusterStatus{
-			Patroni: &v1beta1.PatroniStatus{SystemIdentifier: "12345abcde"},
+			Patroni: v1beta1.PatroniStatus{SystemIdentifier: "12345abcde"},
 			PGBackRest: &v1beta1.PGBackRestStatus{
 				Repos: []v1beta1.RepoStatus{{Name: "repo1", StanzaCreated: true}}},
 		}
@@ -828,7 +828,7 @@ func TestReconcileStanzaCreate(t *testing.T) {
 		Reason:             "RepoHostReady",
 		Message:            "pgBackRest dedicated repository host is ready",
 	})
-	postgresCluster.Status.Patroni = &v1beta1.PatroniStatus{
+	postgresCluster.Status.Patroni = v1beta1.PatroniStatus{
 		SystemIdentifier: "6952526174828511264",
 	}
 
@@ -977,7 +977,7 @@ func TestReconcileReplicaCreateBackup(t *testing.T) {
 		Reason:             "StanzaCreated",
 		Message:            "pgBackRest replica create repo is ready for backups",
 	})
-	postgresCluster.Status.Patroni = &v1beta1.PatroniStatus{
+	postgresCluster.Status.Patroni = v1beta1.PatroniStatus{
 		SystemIdentifier: "6952526174828511264",
 	}
 
@@ -2162,7 +2162,7 @@ func TestReconcilePostgresClusterDataSource(t *testing.T) {
 				cluster.Spec.DataSource = tc.dataSource
 				assert.NilError(t, tClient.Create(ctx, cluster))
 				if tc.clusterBootstrapped {
-					cluster.Status.Patroni = &v1beta1.PatroniStatus{
+					cluster.Status.Patroni = v1beta1.PatroniStatus{
 						SystemIdentifier: "123456789",
 					}
 				}
@@ -2952,7 +2952,7 @@ func TestPrepareForRestore(t *testing.T) {
 				}
 				clusterUID := clusterName
 				cluster := fakePostgresCluster(clusterName, namespace, clusterUID, dedicated)
-				cluster.Status.Patroni = &v1beta1.PatroniStatus{SystemIdentifier: "abcde12345"}
+				cluster.Status.Patroni = v1beta1.PatroniStatus{SystemIdentifier: "abcde12345"}
 				cluster.Status.Proxy.PGBouncer.PostgreSQLRevision = "abcde12345"
 				cluster.Status.Monitoring.ExporterConfiguration = "abcde12345"
 				meta.SetStatusCondition(&cluster.Status.Conditions, metav1.Condition{
@@ -3026,7 +3026,7 @@ func TestPrepareForRestore(t *testing.T) {
 						assert.Equal(t, tc.result.expectedClusterCondition.Message, condition.Message)
 					}
 					if tc.result.expectedClusterCondition.Reason == ReasonReadyForRestore {
-						assert.Assert(t, cluster.Status.Patroni == nil)
+						assert.Assert(t, cluster.Status.Patroni.SystemIdentifier == "")
 						assert.Assert(t, cluster.Status.Proxy.PGBouncer.PostgreSQLRevision == "")
 						assert.Assert(t, cluster.Status.Monitoring.ExporterConfiguration == "")
 						assert.Assert(t, meta.FindStatusCondition(cluster.Status.Conditions,
@@ -3089,7 +3089,7 @@ func TestReconcileScheduledBackups(t *testing.T) {
 				ConditionReplicaCreate: metav1.ConditionTrue,
 			},
 			status: &v1beta1.PostgresClusterStatus{
-				Patroni: &v1beta1.PatroniStatus{SystemIdentifier: "12345abcde"},
+				Patroni: v1beta1.PatroniStatus{SystemIdentifier: "12345abcde"},
 				PGBackRest: &v1beta1.PGBackRestStatus{
 					Repos: []v1beta1.RepoStatus{{Name: "repo1", StanzaCreated: true}}},
 			},
@@ -3107,7 +3107,7 @@ func TestReconcileScheduledBackups(t *testing.T) {
 			testDesc:      "no repo host ready condition, should not reconcile",
 			dedicatedOnly: true,
 			status: &v1beta1.PostgresClusterStatus{
-				Patroni: &v1beta1.PatroniStatus{SystemIdentifier: "12345abcde"},
+				Patroni: v1beta1.PatroniStatus{SystemIdentifier: "12345abcde"},
 				PGBackRest: &v1beta1.PGBackRestStatus{
 					Repos: []v1beta1.RepoStatus{{Name: "repo1", StanzaCreated: true}}},
 			},
@@ -3116,7 +3116,7 @@ func TestReconcileScheduledBackups(t *testing.T) {
 		}, {
 			testDesc: "no replica create condition, should not reconcile",
 			status: &v1beta1.PostgresClusterStatus{
-				Patroni: &v1beta1.PatroniStatus{SystemIdentifier: "12345abcde"},
+				Patroni: v1beta1.PatroniStatus{SystemIdentifier: "12345abcde"},
 				PGBackRest: &v1beta1.PGBackRestStatus{
 					Repos: []v1beta1.RepoStatus{{Name: "repo1", StanzaCreated: true}}},
 			},
@@ -3126,7 +3126,7 @@ func TestReconcileScheduledBackups(t *testing.T) {
 			testDesc:      "false repo host ready condition, should not reconcile",
 			dedicatedOnly: true,
 			status: &v1beta1.PostgresClusterStatus{
-				Patroni: &v1beta1.PatroniStatus{SystemIdentifier: "12345abcde"},
+				Patroni: v1beta1.PatroniStatus{SystemIdentifier: "12345abcde"},
 				PGBackRest: &v1beta1.PGBackRestStatus{
 					Repos: []v1beta1.RepoStatus{{Name: "repo1", StanzaCreated: true}}},
 			},
@@ -3135,7 +3135,7 @@ func TestReconcileScheduledBackups(t *testing.T) {
 		}, {
 			testDesc: "false replica create condition, should not reconcile",
 			status: &v1beta1.PostgresClusterStatus{
-				Patroni: &v1beta1.PatroniStatus{SystemIdentifier: "12345abcde"},
+				Patroni: v1beta1.PatroniStatus{SystemIdentifier: "12345abcde"},
 				PGBackRest: &v1beta1.PGBackRestStatus{
 					Repos: []v1beta1.RepoStatus{{Name: "repo1", StanzaCreated: true}}},
 			},
@@ -3148,7 +3148,7 @@ func TestReconcileScheduledBackups(t *testing.T) {
 				ConditionReplicaCreate: metav1.ConditionTrue,
 			},
 			status: &v1beta1.PostgresClusterStatus{
-				Patroni: &v1beta1.PatroniStatus{SystemIdentifier: "12345abcde"},
+				Patroni: v1beta1.PatroniStatus{SystemIdentifier: "12345abcde"},
 				PGBackRest: &v1beta1.PGBackRestStatus{
 					Repos: []v1beta1.RepoStatus{}},
 			},

internal/patroni/reconcile.go

@@ -34,8 +34,7 @@ import (
 // ClusterBootstrapped returns a bool indicating whether or not Patroni has successfully
 // bootstrapped the PostgresCluster
 func ClusterBootstrapped(postgresCluster *v1beta1.PostgresCluster) bool {
-	return (postgresCluster.Status.Patroni != nil &&
-		postgresCluster.Status.Patroni.SystemIdentifier != "")
+	return postgresCluster.Status.Patroni.SystemIdentifier != ""
 }
 
 // ClusterConfigMap populates the shared ConfigMap with fields needed to run Patroni.

pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_test.go

@@ -63,6 +63,7 @@ spec:
   postgresVersion: 0
 status:
   monitoring: {}
+  patroni: {}
   proxy:
     pgBouncer: {}
 		`)+"\n")
@@ -99,6 +100,7 @@ spec:
   postgresVersion: 0
 status:
   monitoring: {}
+  patroni: {}
   proxy:
     pgBouncer: {}
 		`)+"\n")

pkg/apis/postgres-operator.crunchydata.com/v1beta1/postgrescluster_types.go

@@ -315,7 +315,7 @@ type PostgresClusterStatus struct {
 	InstanceSets []PostgresInstanceSetStatus `json:"instances,omitempty"`
 
 	// +optional
-	Patroni *PatroniStatus `json:"patroni,omitempty"`
+	Patroni PatroniStatus `json:"patroni,omitempty"`
 
 	// Status information for pgBackRest
 	// +optional