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

fix: enforce postgres.auto.conf to rw before run rewind #3728

Merged
merged 3 commits into from
Jan 29, 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.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions internal/management/controller/instance_controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -133,9 +133,6 @@ func (r *InstanceReconciler) Reconcile(
}
reloadNeeded = reloadNeeded || reloadConfigNeeded

// Reconcile postgresql.auto.conf file
r.reconcileAutoConf(ctx, cluster)

// here we execute initialization tasks that need to be executed only on the first reconciliation loop
if !r.firstReconcileDone.Load() {
if err = r.initialize(ctx, cluster); err != nil {
Expand All @@ -144,6 +141,10 @@ func (r *InstanceReconciler) Reconcile(
r.firstReconcileDone.Store(true)
}

// Reconcile postgresql.auto.conf file. This operation must be after the initialize() call
// because it could interfere with pg_rewind execution
r.reconcileAutoConf(ctx, cluster)

// Reconcile cluster role without DB
reloadClusterRoleConfig, err := r.reconcileClusterRoleWithoutDB(ctx, cluster)
if err != nil {
Expand Down
9 changes: 9 additions & 0 deletions internal/management/controller/instance_startup.go
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,15 @@ func (r *InstanceReconciler) verifyPgDataCoherenceForPrimary(ctx context.Context
return err
}

// Set permission of postgres.auto.conf to 0600 to allow pg_rewind to write to it
// the mode will be later reset by the reconciliation again, skip the error as
// rewind may be not needed
err = r.instance.SetPostgreSQLAutoConfWritable(true)
if err != nil {
contextLogger.Error(
err, "Error while changing mode of the postgresql.auto.conf file before pg_rewind, skipped")
}

// pg_rewind could require a clean shutdown of the old primary to
// work. Unfortunately, if the old primary is already clean starting
// it up may make it advance in respect to the new one.
Expand Down
13 changes: 9 additions & 4 deletions pkg/management/postgres/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,14 +197,19 @@ type Instance struct {
tablespaceSynchronizerChan chan map[string]apiv1.TablespaceConfiguration
}

// SetAlterSystemEnabled allows or deny writes to the
// `postgresql.auto.conf` file in PGDATA, allowing and denying the
// usage of the ALTER SYSTEM SQL command
// SetAlterSystemEnabled allows or deny the usage of the
// ALTER SYSTEM SQL command
func (instance *Instance) SetAlterSystemEnabled(enabled bool) error {
return instance.SetPostgreSQLAutoConfWritable(enabled)
}

// SetPostgreSQLAutoConfWritable allows or deny writes to the
// `postgresql.auto.conf` file in PGDATA
func (instance *Instance) SetPostgreSQLAutoConfWritable(writeable bool) error {
autoConfFileName := path.Join(instance.PgData, "postgresql.auto.conf")

mode := fs.FileMode(0o600)
if !enabled {
if !writeable {
mode = 0o400
}
return os.Chmod(autoConfFileName, mode)
Expand Down