fix(upgrades): consider previous controldata when creating the new datadir (cloudnative-pg#7274)

Closes cloudnative-pg#7273 cloudnative-pg#7276

---------

Signed-off-by: Marco Nenciarini <marco.nenciarini@enterprisedb.com>
Signed-off-by: Armando Ruocco <armando.ruocco@enterprisedb.com>
Co-authored-by: Armando Ruocco <armando.ruocco@enterprisedb.com>

Author: mnencia

internal/cmd/manager/instance/upgrade/execute/cmd.go

@@ -27,6 +27,7 @@ import (
 	"os/exec"
 	"path"
 	"path/filepath"
+	"strconv"
 	"strings"
 	"time"
 
@@ -36,6 +37,7 @@ import (
 	"github.com/cloudnative-pg/machinery/pkg/fileutils"
 	"github.com/cloudnative-pg/machinery/pkg/fileutils/compatibility"
 	"github.com/cloudnative-pg/machinery/pkg/log"
+	"github.com/cloudnative-pg/machinery/pkg/postgres/version"
 	"github.com/spf13/cobra"
 	"k8s.io/utils/ptr"
 	ctrl "sigs.k8s.io/controller-runtime/pkg/client"
@@ -48,9 +50,10 @@ import (
 	"github.com/cloudnative-pg/cloudnative-pg/pkg/management"
 	"github.com/cloudnative-pg/cloudnative-pg/pkg/management/postgres"
 	"github.com/cloudnative-pg/cloudnative-pg/pkg/management/postgres/constants"
-	"github.com/cloudnative-pg/cloudnative-pg/pkg/management/postgres/utils"
+	postgresutils "github.com/cloudnative-pg/cloudnative-pg/pkg/management/postgres/utils"
 	"github.com/cloudnative-pg/cloudnative-pg/pkg/management/postgres/webserver/metricserver"
 	"github.com/cloudnative-pg/cloudnative-pg/pkg/specs"
+	"github.com/cloudnative-pg/cloudnative-pg/pkg/utils"
 )
 
 // NewCmd creates the cobra command
@@ -187,8 +190,19 @@ func upgradeSubCommand(
 		}
 	}
 
+	// Extract controldata information from the old data directory
+	controlData, err := getControlData(oldBinDir, pgData)
+	if err != nil {
+		return fmt.Errorf("error while getting old data directory control data: %w", err)
+	}
+
+	targetVersion, err := cluster.GetPostgresqlVersion()
+	if err != nil {
+		return fmt.Errorf("error while getting the target version from the cluster object: %w", err)
+	}
+
 	contextLogger.Info("Creating data directory", "directory", newDataDir)
-	if err := runInitDB(newDataDir, newWalDir); err != nil {
+	if err := runInitDB(newDataDir, newWalDir, controlData, targetVersion); err != nil {
 		return fmt.Errorf("error while creating the data directory: %w", err)
 	}
 
@@ -199,12 +213,12 @@ func upgradeSubCommand(
 
 	contextLogger.Info("Checking if we have anything to update")
 	// Read pg_version from both the old and new data directories
-	oldVersion, err := utils.GetPgdataVersion(pgData)
+	oldVersion, err := postgresutils.GetPgdataVersion(pgData)
 	if err != nil {
 		return fmt.Errorf("error while reading the old version: %w", err)
 	}
 
-	newVersion, err := utils.GetPgdataVersion(newDataDir)
+	newVersion, err := postgresutils.GetPgdataVersion(newDataDir)
 	if err != nil {
 		return fmt.Errorf("error while reading the new version: %w", err)
 	}
@@ -262,7 +276,19 @@ func upgradeSubCommand(
 	return nil
 }
 
-func runInitDB(destDir string, walDir *string) error {
+func getControlData(binDir, pgData string) (map[string]string, error) {
+	pgControlDataCmd := exec.Command(path.Join(binDir, "pg_controldata")) // #nosec
+	pgControlDataCmd.Env = append(os.Environ(), "PGDATA="+pgData)
+
+	out, err := pgControlDataCmd.CombinedOutput()
+	if err != nil {
+		return nil, fmt.Errorf("while executing pg_controldata: %w", err)
+	}
+
+	return utils.ParsePgControldataOutput(string(out)), nil
+}
+
+func runInitDB(destDir string, walDir *string, pgControlData map[string]string, targetVersion version.Data) error {
 	// Invoke initdb to generate a data directory
 	options := []string{
 		"--username",
@@ -275,6 +301,17 @@ func runInitDB(destDir string, walDir *string) error {
 		options = append(options, "--waldir", *walDir)
 	}
 
+	// Extract the WAL segment size from the pg_controldata output
+	options, err := tryAddWalSegmentSize(pgControlData, options)
+	if err != nil {
+		return err
+	}
+
+	options, err = tryAddDataChecksums(pgControlData, targetVersion, options)
+	if err != nil {
+		return err
+	}
+
 	// Certain CSI drivers may add setgid permissions on newly created folders.
 	// A default umask is set to attempt to avoid this, by revoking group/other
 	// permission bits on the PGDATA
@@ -288,6 +325,46 @@ func runInitDB(destDir string, walDir *string) error {
 	return nil
 }
 
+// TODO: refactor it should be a method of pgControlData
+func tryAddDataChecksums(
+	pgControlData map[string]string,
+	targetVersion version.Data,
+	options []string,
+) ([]string, error) {
+	dataPageChecksumVersion, ok := pgControlData[utils.PgControlDataDataPageChecksumVersion]
+	if !ok {
+		return nil, fmt.Errorf("no '%s' section into pg_controldata output", utils.PgControlDataDataPageChecksumVersion)
+	}
+
+	if dataPageChecksumVersion != "1" {
+		// In postgres 18 we will have to set "--no-data-checksums" if checksums are disabled (they are enabled by default)
+		if targetVersion.Major() >= 18 {
+			return append(options, "--no-data-checksums"), nil
+		}
+		return options, nil
+	}
+
+	return append(options, "--data-checksums"), nil
+}
+
+// TODO: refactor it should be a method of pgControlData
+func tryAddWalSegmentSize(pgControlData map[string]string, options []string) ([]string, error) {
+	walSegmentSizeString, ok := pgControlData[utils.PgControlDataBytesPerWALSegment]
+	if !ok {
+		return nil, fmt.Errorf("no '%s' section into pg_controldata output", utils.PgControlDataBytesPerWALSegment)
+	}
+
+	walSegmentSize, err := strconv.Atoi(walSegmentSizeString)
+	if err != nil {
+		return nil, fmt.Errorf(
+			"wrong '%s' pg_controldata value (not an integer): '%s' %w",
+			utils.PgControlDataBytesPerWALSegment, walSegmentSizeString, err)
+	}
+
+	param := "--wal-segsize=" + strconv.Itoa(walSegmentSize/(1024*1024))
+	return append(options, param), nil
+}
+
 func prepareConfigurationFiles(ctx context.Context, cluster apiv1.Cluster, destDir string) error {
 	// Always read the custom and override configuration files created by the operator
 	_, err := configfile.EnsureIncludes(path.Join(destDir, "postgresql.conf"),
@@ -298,8 +375,20 @@ func prepareConfigurationFiles(ctx context.Context, cluster apiv1.Cluster, destD
 		return fmt.Errorf("appending inclusion directives to postgresql.conf file resulted in an error: %w", err)
 	}
 
+	// Set `max_slot_wal_keep_size` to the default value because any other value it is not supported in pg_upgrade
+	tmpCluster := cluster.DeepCopy()
+	tmpCluster.Spec.PostgresConfiguration.Parameters["max_slot_wal_keep_size"] = "-1"
+
+	pgVersion, err := postgresutils.GetPgdataVersion(destDir)
+	if err != nil {
+		return fmt.Errorf("error while reading the new data directory version: %w", err)
+	}
+	if pgVersion.Major >= 18 {
+		tmpCluster.Spec.PostgresConfiguration.Parameters["idle_replication_slot_timeout"] = "0"
+	}
+
 	newInstance := postgres.Instance{PgData: destDir}
-	if _, err := newInstance.RefreshConfigurationFilesFromCluster(ctx, &cluster, false); err != nil {
+	if _, err := newInstance.RefreshConfigurationFilesFromCluster(ctx, tmpCluster, false); err != nil {
 		return fmt.Errorf("error while creating the configuration files for new datadir %q: %w", destDir, err)
 	}
 

pkg/management/postgres/instance.go

@@ -271,7 +271,7 @@ func (instance *Instance) CheckHasDiskSpaceForWAL(ctx context.Context) (bool, er
 	}
 
 	pgControlData := utils.ParsePgControldataOutput(pgControlDataString)
-	walSegmentSizeString, ok := pgControlData["Bytes per WAL segment"]
+	walSegmentSizeString, ok := pgControlData[utils.PgControlDataBytesPerWALSegment]
 	if !ok {
 		return false, fmt.Errorf("no 'Bytes per WAL segment' section into pg_controldata output")
 	}

pkg/utils/parser.go

@@ -57,6 +57,12 @@ const (
 	// PgControlDataDatabaseClusterStateKey is the status
 	// of the latest primary that run on this data directory.
 	PgControlDataDatabaseClusterStateKey pgControlDataKey = "Database cluster state"
+
+	// PgControlDataDataPageChecksumVersion reports whether the checksums are enabled in the cluster
+	PgControlDataDataPageChecksumVersion pgControlDataKey = "Data page checksum version"
+
+	// PgControlDataBytesPerWALSegment reports the size of the WAL segments
+	PgControlDataBytesPerWALSegment pgControlDataKey = "Bytes per WAL segment"
 )
 
 // PgDataState represents the "Database cluster state" field of pg_controldata
@@ -82,7 +88,7 @@ func (state PgDataState) IsShutdown(ctx context.Context) bool {
 }
 
 // ParsePgControldataOutput parses a pg_controldata output into a map of key-value pairs
-func ParsePgControldataOutput(data string) map[string]string {
+func ParsePgControldataOutput(data string) map[pgControlDataKey]string {
 	pairs := make(map[string]string)
 	lines := strings.Split(data, "\n")
 	for _, line := range lines {

tests/e2e/cluster_major_upgrade_test.go

@@ -32,6 +32,7 @@ import (
 	metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
 	"k8s.io/apimachinery/pkg/types"
 	"k8s.io/apimachinery/pkg/util/rand"
+	"k8s.io/utils/ptr"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 
 	v1 "github.com/cloudnative-pg/cloudnative-pg/api/v1"
@@ -78,6 +79,12 @@ var _ = Describe("Postgres Major Upgrade", Label(tests.LabelPostgresMajorUpgrade
 			},
 			Spec: v1.ClusterSpec{
 				Instances: 3,
+				Bootstrap: &v1.BootstrapConfiguration{
+					InitDB: &v1.BootstrapInitDB{
+						DataChecksums:  ptr.To(true),
+						WalSegmentSize: 32,
+					},
+				},
 				StorageConfiguration: v1.StorageConfiguration{
 					StorageClass: &storageClass,
 					Size:         "1Gi",
@@ -95,6 +102,7 @@ var _ = Describe("Postgres Major Upgrade", Label(tests.LabelPostgresMajorUpgrade
 						"log_temp_files":              "1024",
 						"log_autovacuum_min_duration": "1000",
 						"log_replication_commands":    "on",
+						"max_slot_wal_keep_size":      "1GB",
 					},
 				},
 			},