Skip to content

Commit

Permalink
Update terms for Patroni v4 support
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tjmoore4 committed Dec 20, 2024
1 parent a01d26a commit 744f708
Show file tree
Hide file tree
Showing 14 changed files with 43 additions and 36 deletions.
3 changes: 2 additions & 1 deletion internal/controller/postgrescluster/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
22 changes: 11 additions & 11 deletions internal/controller/postgrescluster/instance_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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{},
},
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/postgrescluster/patroni.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions internal/controller/postgrescluster/pgbackrest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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: "",
Expand Down Expand Up @@ -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: "",
Expand Down Expand Up @@ -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"`,
}
}

Expand Down
14 changes: 7 additions & 7 deletions internal/controller/postgrescluster/pgmonitor_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{},
},
}},
Expand Down Expand Up @@ -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{},
Expand All @@ -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{{
Expand Down Expand Up @@ -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{{
Expand Down Expand Up @@ -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{{
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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{{
Expand Down
4 changes: 2 additions & 2 deletions internal/controller/postgrescluster/postgres_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down
2 changes: 1 addition & 1 deletion internal/controller/postgrescluster/watches.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
6 changes: 3 additions & 3 deletions internal/patroni/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion internal/patroni/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down
3 changes: 3 additions & 0 deletions internal/patroni/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions internal/patroni/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ ctl:
kubernetes:
labels:
postgres-operator.crunchydata.com/cluster: cluster-name
leader_label_value: master
namespace: some-namespace
role_label: postgres-operator.crunchydata.com/role
scope_label: postgres-operator.crunchydata.com/patroni
Expand Down Expand Up @@ -112,6 +113,7 @@ ctl:
kubernetes:
labels:
postgres-operator.crunchydata.com/cluster: cluster-name
leader_label_value: master
namespace: some-namespace
role_label: postgres-operator.crunchydata.com/role
scope_label: postgres-operator.crunchydata.com/patroni
Expand Down Expand Up @@ -179,6 +181,7 @@ ctl:
kubernetes:
labels:
postgres-operator.crunchydata.com/cluster: cluster-name
leader_label_value: master
namespace: some-namespace
role_label: postgres-operator.crunchydata.com/role
scope_label: postgres-operator.crunchydata.com/patroni
Expand Down
6 changes: 3 additions & 3 deletions internal/patroni/reconcile.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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".
Expand Down
4 changes: 2 additions & 2 deletions internal/patroni/reconcile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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))
}

Expand All @@ -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
Expand Down

0 comments on commit 744f708

Please sign in to comment.