From 0fd2834c5af646049f8fd612721dc7e019193350 Mon Sep 17 00:00:00 2001 From: Stefan Majer Date: Fri, 7 Nov 2025 15:34:03 +0100 Subject: [PATCH 1/8] Show how it breaks --- integration/postgres_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/integration/postgres_test.go b/integration/postgres_test.go index 5a55112..afdd62e 100644 --- a/integration/postgres_test.go +++ b/integration/postgres_test.go @@ -56,6 +56,7 @@ func Test_Postgres_Upgrade(t *testing.T) { "postgres:14.18-alpine", "postgres:15-alpine", "postgres:17-alpine", + "postgres:18-alpine", }, }) } From e874024116b1943310edde50e6a5939437092cf9 Mon Sep 17 00:00:00 2001 From: Stefan Majer Date: Fri, 7 Nov 2025 16:07:33 +0100 Subject: [PATCH 2/8] Simple Fix --- cmd/internal/database/postgres/upgrade.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cmd/internal/database/postgres/upgrade.go b/cmd/internal/database/postgres/upgrade.go index edce54e..7615ede 100644 --- a/cmd/internal/database/postgres/upgrade.go +++ b/cmd/internal/database/postgres/upgrade.go @@ -133,7 +133,8 @@ func (db *Postgres) Upgrade(ctx context.Context) error { } // initdb -D /data/postgres-new - cmd := exec.Command(postgresInitDBCmd, "-D", newDataDirTemp) + // TODO enable checksums for newer postgres versions after the upgrade + cmd := exec.Command(postgresInitDBCmd, "-D", newDataDirTemp, "--no-data-checksums") cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr cmd.Env = os.Environ() From 042ed6769e1680d087235e02330914f6b18a5990 Mon Sep 17 00:00:00 2001 From: Stefan Majer Date: Sat, 8 Nov 2025 12:30:48 +0100 Subject: [PATCH 3/8] Simple Fix --- cmd/internal/database/postgres/upgrade.go | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/cmd/internal/database/postgres/upgrade.go b/cmd/internal/database/postgres/upgrade.go index 7615ede..8d7ff3c 100644 --- a/cmd/internal/database/postgres/upgrade.go +++ b/cmd/internal/database/postgres/upgrade.go @@ -134,7 +134,12 @@ func (db *Postgres) Upgrade(ctx context.Context) error { // initdb -D /data/postgres-new // TODO enable checksums for newer postgres versions after the upgrade - cmd := exec.Command(postgresInitDBCmd, "-D", newDataDirTemp, "--no-data-checksums") + // This is enabled by default since v18 + initdbCommandArgs := []string{"-D", newDataDirTemp} + if oldBinaryVersionMajor > 17 { + initdbCommandArgs = append(initdbCommandArgs, "--no-data-checksums") + } + cmd := exec.Command(postgresInitDBCmd, initdbCommandArgs...) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr cmd.Env = os.Environ() @@ -235,6 +240,9 @@ func (db *Postgres) Upgrade(ctx context.Context) error { return fmt.Errorf("unable to rename upgraded datadir to destination, a full restore is required: %w", err) } + // TODO run: + // pg_checksums --enable --pgdata /var/lib/postgres/data + db.log.Info("pg_upgrade done and new data in place", "took", time.Since(start).String()) return nil From e261d276f8adc129d49af95219e2154471ed6c08 Mon Sep 17 00:00:00 2001 From: Stefan Majer Date: Sat, 8 Nov 2025 18:47:42 +0100 Subject: [PATCH 4/8] Upgrade works --- Makefile | 5 +++++ api/v1/backup.pb.go | 2 +- api/v1/database.pb.go | 2 +- api/v1/initializer.pb.go | 2 +- cmd/internal/database/postgres/upgrade.go | 3 ++- integration/postgres_test.go | 8 ++++---- 6 files changed, 14 insertions(+), 8 deletions(-) diff --git a/Makefile b/Makefile index 86f4f24..b6757e8 100644 --- a/Makefile +++ b/Makefile @@ -40,6 +40,11 @@ test-integration: kind-cluster-create kind --name backup-restore-sidecar load docker-image ghcr.io/metal-stack/backup-restore-sidecar:latest KUBECONFIG=$(KUBECONFIG) go test $(GO_RUN_ARG) -tags=integration -count 1 -v -p 1 -timeout 30m ./... +.PHONY: test-integration-postgres +test-integration-postgres: kind-cluster-create + kind --name backup-restore-sidecar load docker-image ghcr.io/metal-stack/backup-restore-sidecar:latest + KUBECONFIG=$(KUBECONFIG) go test $(GO_RUN_ARG) -tags=integration -count 1 -v -p 1 -timeout 10m -run "Test_Postgres" ./... + .PHONY: test-integration-valkey test-integration-valkey: kind-cluster-create kind --name backup-restore-sidecar load docker-image ghcr.io/metal-stack/backup-restore-sidecar:latest diff --git a/api/v1/backup.pb.go b/api/v1/backup.pb.go index 01ccf4e..96670c7 100644 --- a/api/v1/backup.pb.go +++ b/api/v1/backup.pb.go @@ -1,6 +1,6 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.36.6 +// protoc-gen-go v1.36.10 // protoc (unknown) // source: v1/backup.proto diff --git a/api/v1/database.pb.go b/api/v1/database.pb.go index cd47017..1b1f31b 100644 --- a/api/v1/database.pb.go +++ b/api/v1/database.pb.go @@ -1,6 +1,6 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.36.6 +// protoc-gen-go v1.36.10 // protoc (unknown) // source: v1/database.proto diff --git a/api/v1/initializer.pb.go b/api/v1/initializer.pb.go index acb0d95..caa06eb 100644 --- a/api/v1/initializer.pb.go +++ b/api/v1/initializer.pb.go @@ -1,6 +1,6 @@ // Code generated by protoc-gen-go. DO NOT EDIT. // versions: -// protoc-gen-go v1.36.6 +// protoc-gen-go v1.36.10 // protoc (unknown) // source: v1/initializer.proto diff --git a/cmd/internal/database/postgres/upgrade.go b/cmd/internal/database/postgres/upgrade.go index 8d7ff3c..a28b198 100644 --- a/cmd/internal/database/postgres/upgrade.go +++ b/cmd/internal/database/postgres/upgrade.go @@ -136,9 +136,10 @@ func (db *Postgres) Upgrade(ctx context.Context) error { // TODO enable checksums for newer postgres versions after the upgrade // This is enabled by default since v18 initdbCommandArgs := []string{"-D", newDataDirTemp} - if oldBinaryVersionMajor > 17 { + if oldBinaryVersionMajor == 17 { initdbCommandArgs = append(initdbCommandArgs, "--no-data-checksums") } + db.log.Info("running", "initdb with args", initdbCommandArgs) cmd := exec.Command(postgresInitDBCmd, initdbCommandArgs...) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr diff --git a/integration/postgres_test.go b/integration/postgres_test.go index afdd62e..51f09d2 100644 --- a/integration/postgres_test.go +++ b/integration/postgres_test.go @@ -47,14 +47,14 @@ func Test_Postgres_Upgrade(t *testing.T) { verifyTestData: verifyPostgresTestData, }, databaseImages: []string{ - "postgres:12-alpine", + // "postgres:12-alpine", // Upgrade from 12-alpine to 13-alpine is not possible because of library differences in icu-lib. // The solution is to upgrade to a older 14.10-alpine which has the same icu-lib version as 12-alpine // and then update to 14.18-alpine or newer which does not require to run pg_upgrade. // "postgres:13-alpine", - "postgres:14.10-alpine", - "postgres:14.18-alpine", - "postgres:15-alpine", + // "postgres:14.10-alpine", + // "postgres:14.18-alpine", + // "postgres:15-alpine", "postgres:17-alpine", "postgres:18-alpine", }, From 891464c30d01564575d61baeee7dc3ff275905c0 Mon Sep 17 00:00:00 2001 From: Stefan Majer Date: Sat, 8 Nov 2025 18:55:30 +0100 Subject: [PATCH 5/8] Enable checksums for new postgres versions --- Makefile | 2 +- cmd/internal/database/postgres/upgrade.go | 18 ++++++++++++++++-- integration/postgres_test.go | 8 ++++---- 3 files changed, 21 insertions(+), 7 deletions(-) diff --git a/Makefile b/Makefile index b6757e8..1423a9d 100644 --- a/Makefile +++ b/Makefile @@ -43,7 +43,7 @@ test-integration: kind-cluster-create .PHONY: test-integration-postgres test-integration-postgres: kind-cluster-create kind --name backup-restore-sidecar load docker-image ghcr.io/metal-stack/backup-restore-sidecar:latest - KUBECONFIG=$(KUBECONFIG) go test $(GO_RUN_ARG) -tags=integration -count 1 -v -p 1 -timeout 10m -run "Test_Postgres" ./... + KUBECONFIG=$(KUBECONFIG) go test $(GO_RUN_ARG) -tags=integration -count 1 -v -p 1 -timeout 10m -run "Test_Postgres_Upgrade" ./... .PHONY: test-integration-valkey test-integration-valkey: kind-cluster-create diff --git a/cmd/internal/database/postgres/upgrade.go b/cmd/internal/database/postgres/upgrade.go index a28b198..5aafca1 100644 --- a/cmd/internal/database/postgres/upgrade.go +++ b/cmd/internal/database/postgres/upgrade.go @@ -25,6 +25,7 @@ const ( postgresConfigCmd = "pg_config" postgresUpgradeCmd = "pg_upgrade" postgresInitDBCmd = "initdb" + postgresChecksumsCmd = "pg_checksums" postgresVersionFile = "PG_VERSION" postgresBinBackupPrefix = "pg-bin-v" ) @@ -241,8 +242,21 @@ func (db *Postgres) Upgrade(ctx context.Context) error { return fmt.Errorf("unable to rename upgraded datadir to destination, a full restore is required: %w", err) } - // TODO run: - // pg_checksums --enable --pgdata /var/lib/postgres/data + if oldBinaryVersionMajor == 17 { + checksumsCommandArgs := []string{"--enable", "--pgdata", db.datadir} + db.log.Info("running", "command", postgresChecksumsCmd, "args", checksumsCommandArgs) + cmd := exec.Command(postgresChecksumsCmd, checksumsCommandArgs...) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + cmd.Env = os.Environ() + cmd.SysProcAttr = &syscall.SysProcAttr{ + Credential: &syscall.Credential{Uid: uint32(uid)}, // nolint:gosec + } + err = cmd.Run() + if err != nil { + db.log.Warn("unable to run checksums on new new datadir, ignoring", "error", err) + } + } db.log.Info("pg_upgrade done and new data in place", "took", time.Since(start).String()) diff --git a/integration/postgres_test.go b/integration/postgres_test.go index 51f09d2..afdd62e 100644 --- a/integration/postgres_test.go +++ b/integration/postgres_test.go @@ -47,14 +47,14 @@ func Test_Postgres_Upgrade(t *testing.T) { verifyTestData: verifyPostgresTestData, }, databaseImages: []string{ - // "postgres:12-alpine", + "postgres:12-alpine", // Upgrade from 12-alpine to 13-alpine is not possible because of library differences in icu-lib. // The solution is to upgrade to a older 14.10-alpine which has the same icu-lib version as 12-alpine // and then update to 14.18-alpine or newer which does not require to run pg_upgrade. // "postgres:13-alpine", - // "postgres:14.10-alpine", - // "postgres:14.18-alpine", - // "postgres:15-alpine", + "postgres:14.10-alpine", + "postgres:14.18-alpine", + "postgres:15-alpine", "postgres:17-alpine", "postgres:18-alpine", }, From 5821a5749d8d9cedfa75ef491155f76349446c5d Mon Sep 17 00:00:00 2001 From: Stefan Majer Date: Sat, 8 Nov 2025 18:55:52 +0100 Subject: [PATCH 6/8] Enable checksums for new postgres versions --- cmd/internal/database/postgres/upgrade.go | 1 + 1 file changed, 1 insertion(+) diff --git a/cmd/internal/database/postgres/upgrade.go b/cmd/internal/database/postgres/upgrade.go index 5aafca1..de4b622 100644 --- a/cmd/internal/database/postgres/upgrade.go +++ b/cmd/internal/database/postgres/upgrade.go @@ -242,6 +242,7 @@ func (db *Postgres) Upgrade(ctx context.Context) error { return fmt.Errorf("unable to rename upgraded datadir to destination, a full restore is required: %w", err) } + // FIXME check if this is possible earlier if oldBinaryVersionMajor == 17 { checksumsCommandArgs := []string{"--enable", "--pgdata", db.datadir} db.log.Info("running", "command", postgresChecksumsCmd, "args", checksumsCommandArgs) From 1f19f456def1220aeb7b83dbb125c426c116e799 Mon Sep 17 00:00:00 2001 From: Stefan Majer Date: Sun, 9 Nov 2025 10:25:39 +0100 Subject: [PATCH 7/8] Remove comments --- Makefile | 2 +- cmd/internal/database/postgres/upgrade.go | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/Makefile b/Makefile index 1423a9d..b6757e8 100644 --- a/Makefile +++ b/Makefile @@ -43,7 +43,7 @@ test-integration: kind-cluster-create .PHONY: test-integration-postgres test-integration-postgres: kind-cluster-create kind --name backup-restore-sidecar load docker-image ghcr.io/metal-stack/backup-restore-sidecar:latest - KUBECONFIG=$(KUBECONFIG) go test $(GO_RUN_ARG) -tags=integration -count 1 -v -p 1 -timeout 10m -run "Test_Postgres_Upgrade" ./... + KUBECONFIG=$(KUBECONFIG) go test $(GO_RUN_ARG) -tags=integration -count 1 -v -p 1 -timeout 10m -run "Test_Postgres" ./... .PHONY: test-integration-valkey test-integration-valkey: kind-cluster-create diff --git a/cmd/internal/database/postgres/upgrade.go b/cmd/internal/database/postgres/upgrade.go index de4b622..fab44da 100644 --- a/cmd/internal/database/postgres/upgrade.go +++ b/cmd/internal/database/postgres/upgrade.go @@ -134,7 +134,6 @@ func (db *Postgres) Upgrade(ctx context.Context) error { } // initdb -D /data/postgres-new - // TODO enable checksums for newer postgres versions after the upgrade // This is enabled by default since v18 initdbCommandArgs := []string{"-D", newDataDirTemp} if oldBinaryVersionMajor == 17 { @@ -242,7 +241,7 @@ func (db *Postgres) Upgrade(ctx context.Context) error { return fmt.Errorf("unable to rename upgraded datadir to destination, a full restore is required: %w", err) } - // FIXME check if this is possible earlier + // TODO check if we should enable checksum with earlier versions if oldBinaryVersionMajor == 17 { checksumsCommandArgs := []string{"--enable", "--pgdata", db.datadir} db.log.Info("running", "command", postgresChecksumsCmd, "args", checksumsCommandArgs) From 462fab842bb6d2ef3f2857a54eca20fe06cadbda Mon Sep 17 00:00:00 2001 From: Stefan Majer Date: Tue, 18 Nov 2025 16:14:54 +0100 Subject: [PATCH 8/8] Checksum on Check --- cmd/internal/database/postgres/postgres.go | 28 ++++++++++++++++++++++ cmd/internal/database/postgres/upgrade.go | 4 ++++ 2 files changed, 32 insertions(+) diff --git a/cmd/internal/database/postgres/postgres.go b/cmd/internal/database/postgres/postgres.go index ddc5294..365d614 100644 --- a/cmd/internal/database/postgres/postgres.go +++ b/cmd/internal/database/postgres/postgres.go @@ -6,8 +6,11 @@ import ( "fmt" "log/slog" "os" + "os/exec" + "os/user" "path" "strconv" + "syscall" "github.com/metal-stack/backup-restore-sidecar/cmd/internal/utils" "github.com/metal-stack/backup-restore-sidecar/pkg/constants" @@ -56,6 +59,31 @@ func (db *Postgres) Check(_ context.Context) (bool, error) { return true, err } + pgUser, err := user.Lookup("postgres") + if err != nil { + return false, err + } + uid, err := strconv.Atoi(pgUser.Uid) + if err != nil { + return false, err + } + + // calling pg_checksums --pgdaata db.datadir --check + checksumsCommandArgs := []string{"--check", "--pgdata", db.datadir} + db.log.Info("running", "command", postgresChecksumsCmd, "args", checksumsCommandArgs) + cmd := exec.Command(postgresChecksumsCmd, checksumsCommandArgs...) + cmd.Stdout = os.Stdout + cmd.Stderr = os.Stderr + cmd.Env = os.Environ() + cmd.SysProcAttr = &syscall.SysProcAttr{ + Credential: &syscall.Credential{Uid: uint32(uid)}, // nolint:gosec + } + err = cmd.Run() + if err != nil { + db.log.Error("pg_checksums reported errors", "args", checksumsCommandArgs, "error", err) + return true, err + } + return false, nil } diff --git a/cmd/internal/database/postgres/upgrade.go b/cmd/internal/database/postgres/upgrade.go index fab44da..ddca96b 100644 --- a/cmd/internal/database/postgres/upgrade.go +++ b/cmd/internal/database/postgres/upgrade.go @@ -81,6 +81,10 @@ func (db *Postgres) Upgrade(ctx context.Context) error { db.log.Error("database is newer than postgres binary, aborting", "database-version", pgVersion, "binary-version", binaryVersionMajor) return fmt.Errorf("database is newer than postgres binary") } + if pgVersion < 17 && binaryVersionMajor == 18 { + db.log.Error("you must not skip a major version before upgrading to v18, skipping upgrade", "old database", pgVersion, "binary-version", binaryVersionMajor) + return nil + } oldPostgresBinDir := path.Join(db.datadir, fmt.Sprintf("%s%d", postgresBinBackupPrefix, pgVersion))