Skip to content

Commit

Permalink
refactor: use PostgreSQL functions from machinery (cloudnative-pg#5684)
Browse files Browse the repository at this point in the history
Signed-off-by: Jonathan Gonzalez V. <[email protected]>
Signed-off-by: Gabriele Bartolini <[email protected]>
Signed-off-by: Leonardo Cecchi <[email protected]>
Signed-off-by: Francesco Canovai <[email protected]>
Signed-off-by: Marco Nenciarini <[email protected]>
Co-authored-by: Jonathan Gonzalez V. <[email protected]>
Co-authored-by: Gabriele Bartolini <[email protected]>
Co-authored-by: Francesco Canovai <[email protected]>
Co-authored-by: Marco Nenciarini <[email protected]>
  • Loading branch information
5 people authored Oct 1, 2024
1 parent 8298083 commit 0b87a95
Show file tree
Hide file tree
Showing 22 changed files with 142 additions and 445 deletions.
25 changes: 8 additions & 17 deletions api/v1/cluster_funcs.go
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,15 @@ import (
"strings"
"time"

"github.com/cloudnative-pg/machinery/pkg/image/reference"
"github.com/cloudnative-pg/machinery/pkg/log"
"github.com/cloudnative-pg/machinery/pkg/postgres/version"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
"k8s.io/apimachinery/pkg/types"

"github.com/cloudnative-pg/cloudnative-pg/internal/configuration"
"github.com/cloudnative-pg/cloudnative-pg/pkg/postgres"
"github.com/cloudnative-pg/cloudnative-pg/pkg/stringset"
"github.com/cloudnative-pg/cloudnative-pg/pkg/system"
"github.com/cloudnative-pg/cloudnative-pg/pkg/utils"
Expand Down Expand Up @@ -399,26 +400,16 @@ func (cluster *Cluster) GetImageName() string {
// image name or from the ImageCatalogRef.
// Example:
//
// ghcr.io/cloudnative-pg/postgresql:14.0 corresponds to version 140000
// ghcr.io/cloudnative-pg/postgresql:13.2 corresponds to version 130002
// ghcr.io/cloudnative-pg/postgresql:9.6.3 corresponds to version 90603
func (cluster *Cluster) GetPostgresqlVersion() (int, error) {
// ghcr.io/cloudnative-pg/postgresql:14.0 corresponds to version (14,0)
// ghcr.io/cloudnative-pg/postgresql:13.2 corresponds to version (13,2)
func (cluster *Cluster) GetPostgresqlVersion() (version.Data, error) {
if cluster.Spec.ImageCatalogRef != nil {
return postgres.GetPostgresVersionFromTag(strconv.Itoa(cluster.Spec.ImageCatalogRef.Major))
return version.FromTag(strconv.Itoa(cluster.Spec.ImageCatalogRef.Major))
}

image := cluster.GetImageName()
tag := utils.GetImageTag(image)
return postgres.GetPostgresVersionFromTag(tag)
}

// GetPostgresqlMajorVersion gets the PostgreSQL image major version used in the Cluster
func (cluster *Cluster) GetPostgresqlMajorVersion() (int, error) {
version, err := cluster.GetPostgresqlVersion()
if err != nil {
return 0, err
}
return postgres.GetPostgresMajorVersion(version), nil
tag := reference.New(image).Tag
return version.FromTag(tag)
}

// GetImagePullSecret get the name of the pull secret to use
Expand Down
13 changes: 5 additions & 8 deletions api/v1/cluster_funcs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"time"

barmanCatalog "github.com/cloudnative-pg/barman-cloud/pkg/catalog"
"github.com/cloudnative-pg/machinery/pkg/postgres/version"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
Expand Down Expand Up @@ -770,19 +771,15 @@ var _ = Describe("A config map resource version", func() {
var _ = Describe("PostgreSQL version detection", func() {
tests := []struct {
imageName string
postgresVersion int
postgresVersion version.Data
}{
{
"ghcr.io/cloudnative-pg/postgresql:14.0",
140000,
version.New(14, 0),
},
{
"ghcr.io/cloudnative-pg/postgresql:13.2",
130002,
},
{
"ghcr.io/cloudnative-pg/postgresql:9.6.3",
90603,
version.New(13, 2),
},
}

Expand All @@ -802,7 +799,7 @@ var _ = Describe("PostgreSQL version detection", func() {
},
Major: 16,
}
Expect(cluster.GetPostgresqlVersion()).To(Equal(160000))
Expect(cluster.GetPostgresqlVersion()).To(Equal(version.New(16, 0)))
})
})

Expand Down
26 changes: 14 additions & 12 deletions api/v1/cluster_webhook.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ import (
"strings"

barmanWebhooks "github.com/cloudnative-pg/barman-cloud/pkg/api/webhooks"
"github.com/cloudnative-pg/machinery/pkg/image/reference"
"github.com/cloudnative-pg/machinery/pkg/log"
"github.com/cloudnative-pg/machinery/pkg/postgres/version"
"github.com/cloudnative-pg/machinery/pkg/types"
storagesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1"
v1 "k8s.io/api/core/v1"
Expand Down Expand Up @@ -126,7 +128,7 @@ func (r *Cluster) setDefaults(preserveUserSettings bool) {
// validateImageName function
info := postgres.ConfigurationInfo{
Settings: postgres.CnpgConfigurationSettings,
MajorVersion: psqlVersion,
Version: psqlVersion,
UserSettings: r.Spec.PostgresConfiguration.Parameters,
IsReplicaCluster: r.IsReplica(),
PreserveFixedSettingsFromUser: preserveUserSettings,
Expand Down Expand Up @@ -965,7 +967,7 @@ func (r *Cluster) validateImageName() field.ErrorList {
}

// We have to check if the image has a valid tag
tag := utils.GetImageTag(r.Spec.ImageName)
tag := reference.New(r.Spec.ImageName).Tag
switch tag {
case "latest":
result = append(
Expand All @@ -982,7 +984,7 @@ func (r *Cluster) validateImageName() field.ErrorList {
r.Spec.ImageName,
"Can't use just the image sha as we can't detect upgrades"))
default:
_, err := postgres.GetPostgresVersionFromTag(tag)
_, err := version.FromTag(tag)
if err != nil {
result = append(
result,
Expand Down Expand Up @@ -1094,7 +1096,7 @@ func (r *Cluster) validateConfiguration() field.ErrorList {
// validateImageName function
return result
}
if pgVersion < 110000 {
if pgVersion.Major() < 11 {
result = append(result,
field.Invalid(
field.NewPath("spec", "imageName"),
Expand All @@ -1103,7 +1105,7 @@ func (r *Cluster) validateConfiguration() field.ErrorList {
}
info := postgres.ConfigurationInfo{
Settings: postgres.CnpgConfigurationSettings,
MajorVersion: pgVersion,
Version: pgVersion,
UserSettings: r.Spec.PostgresConfiguration.Parameters,
IsReplicaCluster: r.IsReplica(),
IsWalArchivingDisabled: utils.IsWalArchivingDisabled(&r.ObjectMeta),
Expand Down Expand Up @@ -1369,7 +1371,7 @@ func validateSyncReplicaElectionConstraint(constraints SyncReplicaElectionConstr
// to a new one.
func (r *Cluster) validateImageChange(old *Cluster) field.ErrorList {
var result field.ErrorList
var newMajor, oldMajor int
var newVersion, oldVersion version.Data
var err error
var newImagePath *field.Path
if r.Spec.ImageCatalogRef != nil {
Expand All @@ -1379,31 +1381,31 @@ func (r *Cluster) validateImageChange(old *Cluster) field.ErrorList {
}

r.Status.Image = ""
newMajor, err = r.GetPostgresqlVersion()
newVersion, err = r.GetPostgresqlVersion()
if err != nil {
// The validation error will be already raised by the
// validateImageName function
return result
}

old.Status.Image = ""
oldMajor, err = old.GetPostgresqlVersion()
oldVersion, err = old.GetPostgresqlVersion()
if err != nil {
// The validation error will be already raised by the
// validateImageName function
return result
}

status := postgres.IsUpgradePossible(oldMajor, newMajor)
status := version.IsUpgradePossible(oldVersion, newVersion)

if !status {
result = append(
result,
field.Invalid(
newImagePath,
newMajor,
newVersion,
fmt.Sprintf("can't upgrade between majors %v and %v",
oldMajor, newMajor)))
oldVersion, newVersion)))
}

return result
Expand Down Expand Up @@ -2194,7 +2196,7 @@ func (r *Cluster) validateReplicationSlots() field.ErrorList {
return nil
}

if psqlVersion < 110000 {
if psqlVersion.Major() < 11 {
if replicationSlots.HighAvailability.GetEnabled() {
return field.ErrorList{
field.Invalid(
Expand Down
17 changes: 13 additions & 4 deletions api/v1/cluster_webhook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import (
"strings"
"time"

"github.com/cloudnative-pg/machinery/pkg/image/reference"
pgversion "github.com/cloudnative-pg/machinery/pkg/postgres/version"
storagesnapshotv1 "github.com/kubernetes-csi/external-snapshotter/client/v8/apis/volumesnapshot/v1"
corev1 "k8s.io/api/core/v1"
"k8s.io/apimachinery/pkg/api/resource"
Expand Down Expand Up @@ -1440,6 +1442,9 @@ var _ = Describe("validate image name change", func() {
Expect(clusterNew.validateImageChange(&clusterOld)).To(HaveLen(1))
})
It("doesn't complain going from default imageName to same major imageCatalogRef", func() {
defaultImageRef := reference.New(versions.DefaultImageName)
version, err := pgversion.FromTag(defaultImageRef.Tag)
Expect(err).ToNot(HaveOccurred())
clusterOld := Cluster{
Spec: ClusterSpec{},
}
Expand All @@ -1450,7 +1455,7 @@ var _ = Describe("validate image name change", func() {
Name: "test",
Kind: "ImageCatalog",
},
Major: 16,
Major: int(version.Major()),
},
},
}
Expand Down Expand Up @@ -1497,7 +1502,7 @@ var _ = Describe("validate image name change", func() {
}
Expect(clusterNew.validateImageChange(&clusterOld)).To(HaveLen(1))
})
It("complains going from default imageName to different major imageCatalogRef", func() {
It("complains going from imageCatalogRef to different major default imageName", func() {
clusterOld := Cluster{
Spec: ClusterSpec{
ImageCatalogRef: &ImageCatalogRef{
Expand All @@ -1514,15 +1519,19 @@ var _ = Describe("validate image name change", func() {
}
Expect(clusterNew.validateImageChange(&clusterOld)).To(HaveLen(1))
})
It("doesn't complain going from default imageName to same major imageCatalogRef", func() {
It("doesn't complain going from imageCatalogRef to same major default imageName", func() {
imageNameRef := reference.New(versions.DefaultImageName)
version, err := pgversion.FromTag(imageNameRef.Tag)
Expect(err).ToNot(HaveOccurred())

clusterOld := Cluster{
Spec: ClusterSpec{
ImageCatalogRef: &ImageCatalogRef{
TypedLocalObjectReference: corev1.TypedLocalObjectReference{
Name: "test",
Kind: "ImageCatalog",
},
Major: 16,
Major: int(version.Major()),
},
},
}
Expand Down
2 changes: 1 addition & 1 deletion docs/src/supported_releases.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ Git tags for versions are prefixed with `v`.

| Version | Currently supported | Release date | End of life | Supported Kubernetes versions | Tested, but not supported | Supported Postgres versions |
|-----------------|----------------------|-------------------|---------------------|-------------------------------|---------------------------|-----------------------------|
| 1.24.x | Yes | August 22, 2024 | ~ February, 2025 | 1.28, 1.29, 1.30, 1.31 | 1.27 | 12 - 16 |
| 1.24.x | Yes | August 22, 2024 | ~ February, 2025 | 1.28, 1.29, 1.30, 1.31 | 1.27 | 12 - 17 |
| 1.23.x | Yes | April 24, 2024 | ~ November, 2024 | 1.27, 1.28, 1.29 | 1.30, 1.31 | 12 - 16 |
| main | No, development only | | | | | 12 - 16 |

Expand Down
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,13 @@ toolchain go1.23.1

require (
github.com/DATA-DOG/go-sqlmock v1.5.2
github.com/Masterminds/semver/v3 v3.2.1
github.com/Masterminds/semver/v3 v3.3.0
github.com/avast/retry-go/v4 v4.6.0
github.com/blang/semver v3.5.1+incompatible
github.com/cheynewallace/tabby v1.1.1
github.com/cloudnative-pg/barman-cloud v0.0.0-20240924124724-92831d48562a
github.com/cloudnative-pg/cnpg-i v0.0.0-20240820123829-5844b833f4eb
github.com/cloudnative-pg/machinery v0.0.0-20240919131343-9dd62b9257c7
github.com/cloudnative-pg/machinery v0.0.0-20241001075747-34c8797af80f
github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc
github.com/evanphx/json-patch/v5 v5.9.0
github.com/go-logr/logr v1.4.2
Expand Down
8 changes: 4 additions & 4 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@ github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161 h1:L/gRVlceqvL25
github.com/Azure/go-ansiterm v0.0.0-20230124172434-306776ec8161/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E=
github.com/DATA-DOG/go-sqlmock v1.5.2 h1:OcvFkGmslmlZibjAjaHm3L//6LiuBgolP7OputlJIzU=
github.com/DATA-DOG/go-sqlmock v1.5.2/go.mod h1:88MAG/4G7SMwSE3CeA0ZKzrT5CiOU3OJ+JlNzwDqpNU=
github.com/Masterminds/semver/v3 v3.2.1 h1:RN9w6+7QoMeJVGyfmbcgs28Br8cvmnucEXnY0rYXWg0=
github.com/Masterminds/semver/v3 v3.2.1/go.mod h1:qvl/7zhW3nngYb5+80sSMF+FG2BjYrf8m9wsX0PNOMQ=
github.com/Masterminds/semver/v3 v3.3.0 h1:B8LGeaivUe71a5qox1ICM/JLl0NqZSW5CHyL+hmvYS0=
github.com/Masterminds/semver/v3 v3.3.0/go.mod h1:4V+yj/TJE1HU9XfppCwVMZq3I84lprf4nC11bSS5beM=
github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5 h1:0CwZNZbxp69SHPdPJAN/hZIm0C4OItdklCFmMRWYpio=
github.com/armon/go-socks5 v0.0.0-20160902184237-e75332964ef5/go.mod h1:wHh0iHkYZB8zMSxRWpUBQtwG5a7fFgvEO+odwuTv2gs=
github.com/avast/retry-go/v4 v4.6.0 h1:K9xNA+KeB8HHc2aWFuLb25Offp+0iVRXEvFx8IinRJA=
Expand All @@ -22,8 +22,8 @@ github.com/cloudnative-pg/barman-cloud v0.0.0-20240924124724-92831d48562a h1:0v1
github.com/cloudnative-pg/barman-cloud v0.0.0-20240924124724-92831d48562a/go.mod h1:Jm0tOp5oB7utpt8wz6RfSv31h1mThOtffjfyxVupriE=
github.com/cloudnative-pg/cnpg-i v0.0.0-20240820123829-5844b833f4eb h1:kZQk+KUCTHQMEgcH8j2/ypcG2HY58zKocmVUvX6c1IA=
github.com/cloudnative-pg/cnpg-i v0.0.0-20240820123829-5844b833f4eb/go.mod h1:UILpBDaWvXcYC5kY5DMaVEEQY5483CBApMuHIn0GJdg=
github.com/cloudnative-pg/machinery v0.0.0-20240919131343-9dd62b9257c7 h1:glRSFwMeX1tb1wlN6ZxihPH3nMXL9ZlwU1/xvNFB0iE=
github.com/cloudnative-pg/machinery v0.0.0-20240919131343-9dd62b9257c7/go.mod h1:bWp1Es5zlxElg4Z/c5f0RKOkDcyNvDHdYIvNcPQU4WM=
github.com/cloudnative-pg/machinery v0.0.0-20241001075747-34c8797af80f h1:RgPmQJkuSu3eTdfd4T2K95RYQi57LHB2+Jfsu/faKOM=
github.com/cloudnative-pg/machinery v0.0.0-20241001075747-34c8797af80f/go.mod h1:bWp1Es5zlxElg4Z/c5f0RKOkDcyNvDHdYIvNcPQU4WM=
github.com/cpuguy83/go-md2man/v2 v2.0.4/go.mod h1:tgQtvFlXSQOSOSIRvRPT7W67SCa46tRHOmNcaadrF8o=
github.com/creack/pty v1.1.18 h1:n56/Zwd5o6whRC5PMGretI4IdRLlmBXYNjScPaBgsbY=
github.com/creack/pty v1.1.18/go.mod h1:MOBLtS5ELjhRRrroQr9kyvTxUAFNvYEK993ew/Vr4O4=
Expand Down
2 changes: 1 addition & 1 deletion internal/cmd/manager/instance/pgbasebackup/cmd.go
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ func (env *CloneInfo) bootstrapUsingPgbasebackup(ctx context.Context) error {
"Error while parsing PostgreSQL server version to define connection options, defaulting to PostgreSQL 11",
"imageName", cluster.GetImageName(),
"err", err)
} else if pgVersion >= 120000 {
} else if pgVersion.Major() >= 12 {
// We explicitly disable wal_sender_timeout for join-related pg_basebackup executions.
// A short timeout could not be enough in case the instance is slow to send data,
// like when the I/O is overloaded.
Expand Down
6 changes: 3 additions & 3 deletions internal/management/controller/instance_startup.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,7 @@ func (r *InstanceReconciler) verifyPgDataCoherenceForPrimary(ctx context.Context
return err
}

pgMajorVersion, err := cluster.GetPostgresqlMajorVersion()
pgVersion, err := cluster.GetPostgresqlVersion()
if err != nil {
return err
}
Expand All @@ -262,7 +262,7 @@ func (r *InstanceReconciler) verifyPgDataCoherenceForPrimary(ctx context.Context
// The only way to check if we really need to start it up before
// invoking pg_rewind is to try using pg_rewind and, on failures,
// retrying after having started up the instance.
err = r.instance.Rewind(ctx, pgMajorVersion)
err = r.instance.Rewind(ctx, pgVersion)
if err != nil {
contextLogger.Info(
"pg_rewind failed, starting the server to complete the crash recovery",
Expand All @@ -277,7 +277,7 @@ func (r *InstanceReconciler) verifyPgDataCoherenceForPrimary(ctx context.Context
}

// Then let's go back to the point of the new primary
err = r.instance.Rewind(ctx, pgMajorVersion)
err = r.instance.Rewind(ctx, pgVersion)
if err != nil {
return err
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/management/postgres/configuration.go
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ func (instance *Instance) GeneratePostgresqlHBA(cluster *apiv1.Cluster, ldapBind
// See:
// https://www.postgresql.org/docs/14/release-14.html
defaultAuthenticationMethod := "scram-sha-256"
if version < 140000 {
if version.Major() < 14 {
defaultAuthenticationMethod = "md5"
}

Expand Down Expand Up @@ -429,7 +429,7 @@ func createPostgresqlConfiguration(cluster *apiv1.Cluster, preserveUserSettings

info := postgres.ConfigurationInfo{
Settings: postgres.CnpgConfigurationSettings,
MajorVersion: fromVersion,
Version: fromVersion,
UserSettings: cluster.Spec.PostgresConfiguration.Parameters,
IncludingSharedPreloadLibraries: true,
AdditionalSharedPreloadLibraries: cluster.Spec.PostgresConfiguration.AdditionalLibraries,
Expand Down
5 changes: 3 additions & 2 deletions pkg/management/postgres/instance.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,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"
"go.uber.org/atomic"
"k8s.io/apimachinery/pkg/util/wait"
"k8s.io/client-go/util/retry"
Expand Down Expand Up @@ -982,7 +983,7 @@ func (instance *Instance) removePgControlFileBackup() error {

// Rewind uses pg_rewind to align this data directory with the contents of the primary node.
// If postgres major version is >= 13, add "--restore-target-wal" option
func (instance *Instance) Rewind(ctx context.Context, postgresMajorVersion int) error {
func (instance *Instance) Rewind(ctx context.Context, postgresVersion version.Data) error {
contextLogger := log.FromContext(ctx)

// Signal the liveness probe that we are running pg_rewind before starting postgres
Expand All @@ -1002,7 +1003,7 @@ func (instance *Instance) Rewind(ctx context.Context, postgresMajorVersion int)

// As PostgreSQL 13 introduces support of restore from the WAL archive in pg_rewind,
// let’s automatically use it, if possible
if postgresMajorVersion >= 130000 {
if postgresVersion.Major() >= 13 {
options = append(options, "--restore-target-wal")
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/management/postgres/join.go
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ func (info InitInfo) Join(cluster *apiv1.Cluster) error {
"Error while parsing PostgreSQL server version to define connection options, defaulting to PostgreSQL 11",
"imageName", cluster.GetImageName(),
"err", err)
} else if pgVersion >= 120000 {
} else if pgVersion.Major() >= 12 {
// We explicitly disable wal_sender_timeout for join-related pg_basebackup executions.
// A short timeout could not be enough in case the instance is slow to send data,
// like when the I/O is overloaded.
Expand Down
Loading

0 comments on commit 0b87a95

Please sign in to comment.