Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patroni v4 upgrade #4049

Merged
merged 2 commits into from
Dec 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
5 changes: 4 additions & 1 deletion 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 Expand Up @@ -588,7 +591,7 @@ func instanceYAML(
postgresql[pgBackRestCreateReplicaMethod] = map[string]any{
"command": strings.Join(quoted, " "),
"keep_data": true,
"no_master": true,
"no_leader": true,
"no_params": true,
}
methods = append([]string{pgBackRestCreateReplicaMethod}, methods...)
Expand Down
2 changes: 1 addition & 1 deletion internal/patroni/config.md
Original file line number Diff line number Diff line change
Expand Up @@ -214,7 +214,7 @@ acquiring the leader lock, the Patroni leader:
| - | postgresql.basebackup | Yes | mutable | either | List of arguments to pass to pg_basebackup when using the `basebackup` replica method.
| - | postgresql.{method}.command | Yes¹ | mutable | either | Command to execute for this replica method.
| - | postgresql.{method}.keep_data | Yes¹ | mutable | either | Whether or not Patroni should empty the data directory before. (default: false)
| - | postgresql.{method}.no_master | Yes¹ | mutable | either | Whether or not Patroni can call this method when no instances are running. (default: false)
| - | postgresql.{method}.no_leader | Yes¹ | mutable | either | Whether or not Patroni can call this method when no instances are running. (default: false)
| - | postgresql.{method}.no_params | Yes¹ | mutable | either | Whether or not Patroni should pass extra arguments to the command. (default: false)
||
|||||| https://github.com/zalando/patroni/blob/v2.0.1/docs/replica_bootstrap.rst#bootstrap
Expand Down
5 changes: 4 additions & 1 deletion internal/patroni/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,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 @@ -113,6 +114,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 @@ -180,6 +182,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 @@ -980,7 +983,7 @@ postgresql:
command: '''bash'' ''-ceu'' ''--'' ''install --directory --mode=0700 "${PGDATA?}"
&& exec "$@"'' ''-'' ''some'' ''backrest'' ''cmd'''
keep_data: true
no_master: true
no_leader: true
no_params: true
pgpass: /tmp/.pgpass
use_unix_socket: true
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