From ce6cc2a84cba9bdaf9cadab20dbf9ce2cb3637a3 Mon Sep 17 00:00:00 2001 From: TJ Moore Date: Mon, 16 Dec 2024 11:34:31 -0500 Subject: [PATCH] Update terms for Patroni v4 support This update changes references from "master" to "primary" in support of Patroni v4. This includes updates to tests and various other methods. For the time being, the leader_label_value is manually set to "master" to facilitate the existing label usage. Issue: PGO-1646 --- .../controller/postgrescluster/instance.go | 3 ++- .../postgrescluster/instance_rollout_test.go | 2 +- .../postgrescluster/instance_test.go | 22 +++++++++---------- .../controller/postgrescluster/patroni.go | 2 +- .../postgrescluster/pgbackrest_test.go | 6 ++--- .../postgrescluster/pgmonitor_test.go | 14 ++++++------ .../postgrescluster/postgres_test.go | 4 ++-- .../controller/postgrescluster/watches.go | 2 +- internal/patroni/api.go | 6 ++--- internal/patroni/api_test.go | 2 +- internal/patroni/config.go | 3 +++ internal/patroni/reconcile.go | 6 ++--- internal/patroni/reconcile_test.go | 4 ++-- 13 files changed, 40 insertions(+), 36 deletions(-) diff --git a/internal/controller/postgrescluster/instance.go b/internal/controller/postgrescluster/instance.go index 97cc2cdce..3baaff5dd 100644 --- a/internal/controller/postgrescluster/instance.go +++ b/internal/controller/postgrescluster/instance.go @@ -132,7 +132,8 @@ func (i Instance) IsWritable() (writable, known bool) { // TODO(cbandy): Update this to consider when Patroni is paused. - return strings.HasPrefix(member[role:], `"role":"master"`), true + return strings.HasPrefix(member[role:], `"role":"master"`) || + strings.HasPrefix(member[role:], `"role":"primary"`), true } // PodMatchesPodTemplate returns whether or not the Pod for this instance diff --git a/internal/controller/postgrescluster/instance_rollout_test.go b/internal/controller/postgrescluster/instance_rollout_test.go index bede90861..2f1cda06f 100644 --- a/internal/controller/postgrescluster/instance_rollout_test.go +++ b/internal/controller/postgrescluster/instance_rollout_test.go @@ -132,7 +132,7 @@ func TestReconcilerRolloutInstance(t *testing.T) { // A switchover to any viable candidate. assert.DeepEqual(t, command[:2], []string{"patronictl", "switchover"}) - assert.Assert(t, sets.NewString(command...).Has("--master=the-pod")) + assert.Assert(t, sets.NewString(command...).Has("--primary=the-pod")) assert.Assert(t, sets.NewString(command...).Has("--candidate=")) // Indicate success through stdout. diff --git a/internal/controller/postgrescluster/instance_test.go b/internal/controller/postgrescluster/instance_test.go index 064714872..28502c342 100644 --- a/internal/controller/postgrescluster/instance_test.go +++ b/internal/controller/postgrescluster/instance_test.go @@ -117,7 +117,7 @@ func TestInstanceIsWritable(t *testing.T) { assert.Assert(t, !writable) // Patroni leader - instance.Pods[0].Annotations["status"] = `{"role":"master"}` + instance.Pods[0].Annotations["status"] = `{"role":"primary"}` writable, known = instance.IsWritable() assert.Assert(t, known) assert.Assert(t, writable) @@ -392,7 +392,7 @@ func TestWritablePod(t *testing.T) { Namespace: "namespace", Name: "pod", Annotations: map[string]string{ - "status": `{"role":"master"}`, + "status": `{"role":"primary"}`, }, DeletionTimestamp: &metav1.Time{}, }, @@ -426,7 +426,7 @@ func TestWritablePod(t *testing.T) { Namespace: "namespace", Name: "pod", Annotations: map[string]string{ - "status": `{"role":"master"}`, + "status": `{"role":"primary"}`, }, }, Status: corev1.PodStatus{ @@ -491,7 +491,7 @@ func TestWritablePod(t *testing.T) { Namespace: "namespace", Name: "pod", Annotations: map[string]string{ - "status": `{"role":"master"}`, + "status": `{"role":"primary"}`, }, }, Status: corev1.PodStatus{ @@ -964,7 +964,7 @@ func TestPodsToKeep(t *testing.T) { checks func(*testing.T, []corev1.Pod) }{ { - name: "RemoveSetWithMasterOnly", + name: "RemoveSetWithPrimaryOnly", instances: []corev1.Pod{ { ObjectMeta: metav1.ObjectMeta{ @@ -998,7 +998,7 @@ func TestPodsToKeep(t *testing.T) { assert.Equal(t, len(p), 0) }, }, { - name: "KeepMasterOnly", + name: "KeepPrimaryOnly", instances: []corev1.Pod{ { ObjectMeta: metav1.ObjectMeta{ @@ -1087,7 +1087,7 @@ func TestPodsToKeep(t *testing.T) { assert.Equal(t, len(p), 0) }, }, { - name: "MasterLastInSet", + name: "PrimaryLastInSet", instances: []corev1.Pod{ { ObjectMeta: metav1.ObjectMeta{ @@ -1116,7 +1116,7 @@ func TestPodsToKeep(t *testing.T) { assert.Equal(t, p[0].Labels[naming.LabelRole], "master") }, }, { - name: "ScaleDownSetWithMaster", + name: "ScaleDownSetWithPrimary", instances: []corev1.Pod{ { ObjectMeta: metav1.ObjectMeta{ @@ -1167,7 +1167,7 @@ func TestPodsToKeep(t *testing.T) { assert.Equal(t, p[1].Labels[naming.LabelInstanceSet], "max") }, }, { - name: "ScaleDownSetWithoutMaster", + name: "ScaleDownSetWithoutPrimary", instances: []corev1.Pod{ { ObjectMeta: metav1.ObjectMeta{ @@ -1220,7 +1220,7 @@ func TestPodsToKeep(t *testing.T) { assert.Equal(t, p[2].Labels[naming.LabelRole], "replica") }, }, { - name: "ScaleMasterSetToZero", + name: "ScalePrimarySetToZero", instances: []corev1.Pod{ { ObjectMeta: metav1.ObjectMeta{ @@ -1262,7 +1262,7 @@ func TestPodsToKeep(t *testing.T) { assert.Equal(t, p[1].Labels[naming.LabelInstanceSet], "daisy") }, }, { - name: "RemoveMasterInstanceSet", + name: "RemovePrimaryInstanceSet", instances: []corev1.Pod{ { ObjectMeta: metav1.ObjectMeta{ diff --git a/internal/controller/postgrescluster/patroni.go b/internal/controller/postgrescluster/patroni.go index 1c5ac93ee..b942291de 100644 --- a/internal/controller/postgrescluster/patroni.go +++ b/internal/controller/postgrescluster/patroni.go @@ -94,7 +94,7 @@ func (r *Reconciler) handlePatroniRestarts( return r.PodExec(ctx, pod.Namespace, pod.Name, container, stdin, stdout, stderr, command...) }) - return errors.WithStack(exec.RestartPendingMembers(ctx, "master", naming.PatroniScope(cluster))) + return errors.WithStack(exec.RestartPendingMembers(ctx, "primary", naming.PatroniScope(cluster))) } // When the primary does not need to restart but a replica does, restart all diff --git a/internal/controller/postgrescluster/pgbackrest_test.go b/internal/controller/postgrescluster/pgbackrest_test.go index b7855f173..c63f13cb1 100644 --- a/internal/controller/postgrescluster/pgbackrest_test.go +++ b/internal/controller/postgrescluster/pgbackrest_test.go @@ -751,7 +751,7 @@ func TestReconcileStanzaCreate(t *testing.T) { instances := newObservedInstances(postgresCluster, nil, []corev1.Pod{{ ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"status": `"role":"master"`}, + Annotations: map[string]string{"status": `"role":"primary"`}, Labels: map[string]string{ naming.LabelCluster: postgresCluster.GetName(), naming.LabelInstance: "", @@ -867,7 +867,7 @@ func TestReconcileReplicaCreateBackup(t *testing.T) { } instances := newObservedInstances(postgresCluster, nil, []corev1.Pod{{ ObjectMeta: metav1.ObjectMeta{ - Annotations: map[string]string{"status": `"role":"master"`}, + Annotations: map[string]string{"status": `"role":"primary"`}, Labels: map[string]string{ naming.LabelCluster: postgresCluster.GetName(), naming.LabelInstance: "", @@ -1348,7 +1348,7 @@ func TestReconcileManualBackup(t *testing.T) { instances.forCluster[0].Pods[0].Annotations = map[string]string{} } else { instances.forCluster[0].Pods[0].Annotations = map[string]string{ - "status": `"role":"master"`, + "status": `"role":"primary"`, } } diff --git a/internal/controller/postgrescluster/pgmonitor_test.go b/internal/controller/postgrescluster/pgmonitor_test.go index 8d8c8281d..e7baa689b 100644 --- a/internal/controller/postgrescluster/pgmonitor_test.go +++ b/internal/controller/postgrescluster/pgmonitor_test.go @@ -358,7 +358,7 @@ func TestReconcilePGMonitorExporterSetupErrors(t *testing.T) { Pods: []*corev1.Pod{{ ObjectMeta: metav1.ObjectMeta{ Name: "daisy-pod", - Annotations: map[string]string{"status": `{"role":"master"}`}, + Annotations: map[string]string{"status": `{"role":"primary"}`}, DeletionTimestamp: &metav1.Time{}, }, }}, @@ -388,7 +388,7 @@ func TestReconcilePGMonitorExporterSetupErrors(t *testing.T) { Pods: []*corev1.Pod{{ ObjectMeta: metav1.ObjectMeta{ Name: "daisy-pod", - Annotations: map[string]string{"status": `{"role":"master"}`}, + Annotations: map[string]string{"status": `{"role":"primary"}`}, }, }}, Runner: &appsv1.StatefulSet{}, @@ -410,7 +410,7 @@ func TestReconcilePGMonitorExporterSetupErrors(t *testing.T) { Pods: []*corev1.Pod{{ ObjectMeta: metav1.ObjectMeta{ Name: "daisy-pod", - Annotations: map[string]string{"status": `{"role":"master"}`}, + Annotations: map[string]string{"status": `{"role":"primary"}`}, }, Status: corev1.PodStatus{ ContainerStatuses: []corev1.ContainerStatus{{ @@ -438,7 +438,7 @@ func TestReconcilePGMonitorExporterSetupErrors(t *testing.T) { Pods: []*corev1.Pod{{ ObjectMeta: metav1.ObjectMeta{ Name: "daisy-pod", - Annotations: map[string]string{"status": `{"role":"master"}`}, + Annotations: map[string]string{"status": `{"role":"primary"}`}, }, Status: corev1.PodStatus{ ContainerStatuses: []corev1.ContainerStatus{{ @@ -469,7 +469,7 @@ func TestReconcilePGMonitorExporterSetupErrors(t *testing.T) { Pods: []*corev1.Pod{{ ObjectMeta: metav1.ObjectMeta{ Name: "daisy-pod", - Annotations: map[string]string{"status": `{"role":"master"}`}, + Annotations: map[string]string{"status": `{"role":"primary"}`}, }, Status: corev1.PodStatus{ ContainerStatuses: []corev1.ContainerStatus{{ @@ -536,7 +536,7 @@ func TestReconcilePGMonitorExporter(t *testing.T) { Pods: []*corev1.Pod{{ ObjectMeta: metav1.ObjectMeta{ Name: "one-daisy-pod", - Annotations: map[string]string{"status": `{"role":"master"}`}, + Annotations: map[string]string{"status": `{"role":"primary"}`}, }, Status: corev1.PodStatus{ Phase: corev1.PodRunning, @@ -634,7 +634,7 @@ func TestReconcilePGMonitorExporterStatus(t *testing.T) { Pods: []*corev1.Pod{{ ObjectMeta: metav1.ObjectMeta{ Name: "daisy-pod", - Annotations: map[string]string{"status": `{"role":"master"}`}, + Annotations: map[string]string{"status": `{"role":"primary"}`}, }, Status: corev1.PodStatus{ ContainerStatuses: []corev1.ContainerStatus{{ diff --git a/internal/controller/postgrescluster/postgres_test.go b/internal/controller/postgrescluster/postgres_test.go index 901663b60..23dd424ee 100644 --- a/internal/controller/postgrescluster/postgres_test.go +++ b/internal/controller/postgrescluster/postgres_test.go @@ -951,7 +951,7 @@ func TestReconcileDatabaseInitSQL(t *testing.T) { Namespace: ns.Name, Name: "pod", Annotations: map[string]string{ - "status": `{"role":"master"}`, + "status": `{"role":"primary"}`, }, }, Status: corev1.PodStatus{ @@ -1072,7 +1072,7 @@ func TestReconcileDatabaseInitSQLConfigMap(t *testing.T) { Namespace: ns.Name, Name: "pod", Annotations: map[string]string{ - "status": `{"role":"master"}`, + "status": `{"role":"primary"}`, }, }, Status: corev1.PodStatus{ diff --git a/internal/controller/postgrescluster/watches.go b/internal/controller/postgrescluster/watches.go index 41369254c..50db962c9 100644 --- a/internal/controller/postgrescluster/watches.go +++ b/internal/controller/postgrescluster/watches.go @@ -50,7 +50,7 @@ func (*Reconciler) watchPods() handler.Funcs { } // Queue an event to start applying changes if the PostgreSQL instance - // now has the "master" role. + // now has the "primary" role. if len(cluster) != 0 && !patroni.PodIsPrimary(e.ObjectOld) && patroni.PodIsPrimary(e.ObjectNew) { diff --git a/internal/patroni/api.go b/internal/patroni/api.go index 679da5f4a..8f1212b26 100644 --- a/internal/patroni/api.go +++ b/internal/patroni/api.go @@ -45,7 +45,7 @@ func (exec Executor) ChangePrimaryAndWait( err := exec(ctx, nil, &stdout, &stderr, "patronictl", "switchover", "--scheduled=now", "--force", - "--master="+current, "--candidate="+next) + "--primary="+current, "--candidate="+next) log := logging.FromContext(ctx) log.V(1).Info("changed primary", @@ -65,7 +65,7 @@ func (exec Executor) ChangePrimaryAndWait( // "patronictl". It returns true when an election completes successfully. It // waits up to two "loop_wait" or until an error occurs. When Patroni is paused, // next cannot be blank. Similar to the "POST /switchover" REST endpoint. -// The "patronictl switchover" variant does not require the current master to be passed +// The "patronictl switchover" variant does not require the current primary to be passed // as a flag. func (exec Executor) SwitchoverAndWait( ctx context.Context, target string, @@ -96,7 +96,7 @@ func (exec Executor) SwitchoverAndWait( // "patronictl". It returns true when an election completes successfully. It // waits up to two "loop_wait" or until an error occurs. When Patroni is paused, // next cannot be blank. Similar to the "POST /switchover" REST endpoint. -// The "patronictl failover" variant does not require the current master to be passed +// The "patronictl failover" variant does not require the current primary to be passed // as a flag. func (exec Executor) FailoverAndWait( ctx context.Context, target string, diff --git a/internal/patroni/api_test.go b/internal/patroni/api_test.go index 4eb561ad2..7317cd382 100644 --- a/internal/patroni/api_test.go +++ b/internal/patroni/api_test.go @@ -36,7 +36,7 @@ func TestExecutorChangePrimaryAndWait(t *testing.T) { ) error { called = true assert.DeepEqual(t, command, strings.Fields( - `patronictl switchover --scheduled=now --force --master=old --candidate=new`, + `patronictl switchover --scheduled=now --force --primary=old --candidate=new`, )) assert.Assert(t, stdin == nil, "expected no stdin, got %T", stdin) assert.Assert(t, stderr != nil, "should capture stderr") diff --git a/internal/patroni/config.go b/internal/patroni/config.go index e2bf69443..6381b26f2 100644 --- a/internal/patroni/config.go +++ b/internal/patroni/config.go @@ -60,6 +60,9 @@ func clusterYAML( "role_label": naming.LabelRole, "scope_label": naming.LabelPatroni, "use_endpoints": true, + // To support transitioning to Patroni v4, set the value to 'master'. + // In a future release, this can be removed in favor of the default. + "leader_label_value": naming.RolePatroniLeader, // In addition to "scope_label" above, Patroni will add the following to // every object it creates. It will also use these as filters when doing diff --git a/internal/patroni/reconcile.go b/internal/patroni/reconcile.go index 29f0a0000..77df8d9fd 100644 --- a/internal/patroni/reconcile.go +++ b/internal/patroni/reconcile.go @@ -172,8 +172,7 @@ func instanceProbes(cluster *v1beta1.PostgresCluster, container *corev1.Containe } // PodIsPrimary returns whether or not pod is currently acting as the leader with -// the "master" role. This role will be called "primary" in the future, see: -// - https://github.com/zalando/patroni/blob/master/docs/releases.rst?plain=1#L213 +// the "primary" role. func PodIsPrimary(pod metav1.Object) bool { if pod == nil { return false @@ -186,7 +185,8 @@ func PodIsPrimary(pod metav1.Object) bool { // - https://github.com/zalando/patroni/blob/v3.1.1/patroni/ha.py#L782 // - https://github.com/zalando/patroni/blob/v3.1.1/patroni/ha.py#L1574 status := pod.GetAnnotations()["status"] - return strings.Contains(status, `"role":"master"`) + return strings.Contains(status, `"role":"master"`) || + strings.Contains(status, `"role":"primary"`) } // PodIsStandbyLeader returns whether or not pod is currently acting as a "standby_leader". diff --git a/internal/patroni/reconcile_test.go b/internal/patroni/reconcile_test.go index 5b78acace..a2290232d 100644 --- a/internal/patroni/reconcile_test.go +++ b/internal/patroni/reconcile_test.go @@ -241,7 +241,7 @@ func TestPodIsPrimary(t *testing.T) { assert.Assert(t, !PodIsPrimary(pod)) // Primary - pod.Annotations["status"] = `{"role":"master"}` + pod.Annotations["status"] = `{"role":"primary"}` assert.Assert(t, PodIsPrimary(pod)) } @@ -258,7 +258,7 @@ func TestPodIsStandbyLeader(t *testing.T) { assert.Assert(t, !PodIsStandbyLeader(pod)) // Leader - pod.Annotations["status"] = `{"role":"master"}` + pod.Annotations["status"] = `{"role":"primary"}` assert.Assert(t, !PodIsStandbyLeader(pod)) // Replica