From 4f3bb6aa8cbecfbc68cfad1fee2a4c5c2b5d6632 Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Mon, 2 Nov 2020 16:49:29 +0100 Subject: [PATCH] Remove operator checks that prevent PG major version upgrade (#1160) * remove checks that prevent major version upgrade Co-authored-by: Sergey Dudoladov --- docs/administrator.md | 12 +++-- docs/user.md | 4 ++ pkg/cluster/cluster.go | 7 ++- pkg/cluster/k8sres.go | 35 --------------- pkg/cluster/k8sres_test.go | 89 -------------------------------------- pkg/cluster/sync.go | 12 ----- 6 files changed, 14 insertions(+), 145 deletions(-) diff --git a/docs/administrator.md b/docs/administrator.md index 5357ddb74..5c1831cfe 100644 --- a/docs/administrator.md +++ b/docs/administrator.md @@ -11,17 +11,15 @@ switchover (planned failover) of the master to the Pod with new minor version. The switch should usually take less than 5 seconds, still clients have to reconnect. -Major version upgrades are supported via [cloning](user.md#how-to-clone-an-existing-postgresql-cluster). -The new cluster manifest must have a higher `version` string than the source +Major version upgrades are supported either via [cloning](user.md#how-to-clone-an-existing-postgresql-cluster)or in-place. + +With cloning, the new cluster manifest must have a higher `version` string than the source cluster and will be created from a basebackup. Depending of the cluster size, downtime in this case can be significant as writes to the database should be stopped and all WAL files should be archived first before cloning is started. -Note, that simply changing the version string in the `postgresql` manifest does -not work at present and leads to errors. Neither Patroni nor Postgres Operator -can do in place `pg_upgrade`. Still, it can be executed manually in the Postgres -container, which is tricky (i.e. systems need to be stopped, replicas have to be -synced) but of course faster than cloning. +Starting with Spilo 13, Postgres Operator can do in-place major version upgrade, which should be faster than cloning. To trigger the upgrade, simply increase the version in the cluster manifest. As the very last step of +processing the manifest update event, the operator will call the `inplace_upgrade.py` script in Spilo. The upgrade is usually fast, well under one minute for most DBs. Note the changes become irrevertible once `pg_upgrade` is called. To understand the upgrade procedure, refer to the [corresponding PR in Spilo](https://github.com/zalando/spilo/pull/488). ## CRD Validation diff --git a/docs/user.md b/docs/user.md index 8cacad0e8..f834c788a 100644 --- a/docs/user.md +++ b/docs/user.md @@ -542,6 +542,10 @@ section in the spec. There are two options here: Note, that cloning can also be used for [major version upgrades](administrator.md#minor-and-major-version-upgrade) of PostgreSQL. +## In-place major version upgrade + +Starting with Spilo 13, operator supports in-place major version upgrade to a higher major version (e.g. from PG 10 to PG 12). To trigger the upgrade, simply increase the version in the manifest. It is your responsibility to test your applications against the new version before the upgrade; downgrading is not supported. The easiest way to do so is to try the upgrade on the cloned cluster first. For details of how Spilo does the upgrade [see here](https://github.com/zalando/spilo/pull/488), operator implementation is described [in the admin docs](administrator.md#minor-and-major-version-upgrade). + ### Clone from S3 Cloning from S3 has the advantage that there is no impact on your production diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index 06388d731..18778fd41 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -626,13 +626,16 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error { } }() - if oldSpec.Spec.PostgresqlParam.PgVersion != newSpec.Spec.PostgresqlParam.PgVersion { // PG versions comparison + if oldSpec.Spec.PostgresqlParam.PgVersion >= newSpec.Spec.PostgresqlParam.PgVersion { c.logger.Warningf("postgresql version change(%q -> %q) has no effect", oldSpec.Spec.PostgresqlParam.PgVersion, newSpec.Spec.PostgresqlParam.PgVersion) c.eventRecorder.Eventf(c.GetReference(), v1.EventTypeWarning, "PostgreSQL", "postgresql version change(%q -> %q) has no effect", oldSpec.Spec.PostgresqlParam.PgVersion, newSpec.Spec.PostgresqlParam.PgVersion) - //we need that hack to generate statefulset with the old version + // we need that hack to generate statefulset with the old version newSpec.Spec.PostgresqlParam.PgVersion = oldSpec.Spec.PostgresqlParam.PgVersion + } else { + c.logger.Infof("postgresql version increased (%q -> %q), major version upgrade can be done manually after StatefulSet Sync", + oldSpec.Spec.PostgresqlParam.PgVersion, newSpec.Spec.PostgresqlParam.PgVersion) } // Service diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 9a328b7df..c07c1d212 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -909,41 +909,6 @@ func extractPgVersionFromBinPath(binPath string, template string) (string, error return fmt.Sprintf("%v", pgVersion), nil } -func (c *Cluster) getNewPgVersion(container v1.Container, newPgVersion string) (string, error) { - var ( - spiloConfiguration spiloConfiguration - runningPgVersion string - err error - ) - - for _, env := range container.Env { - if env.Name != "SPILO_CONFIGURATION" { - continue - } - err = json.Unmarshal([]byte(env.Value), &spiloConfiguration) - if err != nil { - return newPgVersion, err - } - } - - if len(spiloConfiguration.PgLocalConfiguration) > 0 { - currentBinPath := fmt.Sprintf("%v", spiloConfiguration.PgLocalConfiguration[patroniPGBinariesParameterName]) - runningPgVersion, err = extractPgVersionFromBinPath(currentBinPath, pgBinariesLocationTemplate) - if err != nil { - return "", fmt.Errorf("could not extract Postgres version from %v in SPILO_CONFIGURATION", currentBinPath) - } - } else { - return "", fmt.Errorf("could not find %q setting in SPILO_CONFIGURATION", patroniPGBinariesParameterName) - } - - if runningPgVersion != newPgVersion { - c.logger.Warningf("postgresql version change(%q -> %q) has no effect", runningPgVersion, newPgVersion) - newPgVersion = runningPgVersion - } - - return newPgVersion, nil -} - func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.StatefulSet, error) { var ( diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index f44b071bb..f1c0e968b 100644 --- a/pkg/cluster/k8sres_test.go +++ b/pkg/cluster/k8sres_test.go @@ -557,95 +557,6 @@ func TestExtractPgVersionFromBinPath(t *testing.T) { } } -func TestGetPgVersion(t *testing.T) { - testName := "TestGetPgVersion" - tests := []struct { - subTest string - pgContainer v1.Container - currentPgVersion string - newPgVersion string - }{ - { - subTest: "new version with decimal point differs from current SPILO_CONFIGURATION", - pgContainer: v1.Container{ - Name: "postgres", - Env: []v1.EnvVar{ - { - Name: "SPILO_CONFIGURATION", - Value: "{\"postgresql\": {\"bin_dir\": \"/usr/lib/postgresql/9.6/bin\"}}", - }, - }, - }, - currentPgVersion: "9.6", - newPgVersion: "12", - }, - { - subTest: "new version differs from current SPILO_CONFIGURATION", - pgContainer: v1.Container{ - Name: "postgres", - Env: []v1.EnvVar{ - { - Name: "SPILO_CONFIGURATION", - Value: "{\"postgresql\": {\"bin_dir\": \"/usr/lib/postgresql/11/bin\"}}", - }, - }, - }, - currentPgVersion: "11", - newPgVersion: "12", - }, - { - subTest: "new version is lower than the one found in current SPILO_CONFIGURATION", - pgContainer: v1.Container{ - Name: "postgres", - Env: []v1.EnvVar{ - { - Name: "SPILO_CONFIGURATION", - Value: "{\"postgresql\": {\"bin_dir\": \"/usr/lib/postgresql/12/bin\"}}", - }, - }, - }, - currentPgVersion: "12", - newPgVersion: "11", - }, - { - subTest: "new version is the same like in the current SPILO_CONFIGURATION", - pgContainer: v1.Container{ - Name: "postgres", - Env: []v1.EnvVar{ - { - Name: "SPILO_CONFIGURATION", - Value: "{\"postgresql\": {\"bin_dir\": \"/usr/lib/postgresql/12/bin\"}}", - }, - }, - }, - currentPgVersion: "12", - newPgVersion: "12", - }, - } - - var cluster = New( - Config{ - OpConfig: config.Config{ - ProtectedRoles: []string{"admin"}, - Auth: config.Auth{ - SuperUsername: superUserName, - ReplicationUsername: replicationUserName, - }, - }, - }, k8sutil.KubernetesClient{}, acidv1.Postgresql{}, logger, eventRecorder) - - for _, tt := range tests { - pgVersion, err := cluster.getNewPgVersion(tt.pgContainer, tt.newPgVersion) - if err != nil { - t.Errorf("Unexpected error: %v", err) - } - if pgVersion != tt.currentPgVersion { - t.Errorf("%s %s: Expected version %s, have %s instead", - testName, tt.subTest, tt.currentPgVersion, pgVersion) - } - } -} - func TestSecretVolume(t *testing.T) { testName := "TestSecretVolume" tests := []struct { diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index dced69461..ee7672a5b 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -332,18 +332,6 @@ func (c *Cluster) syncStatefulSet() error { // statefulset is already there, make sure we use its definition in order to compare with the spec. c.Statefulset = sset - // check if there is no Postgres version mismatch - for _, container := range c.Statefulset.Spec.Template.Spec.Containers { - if container.Name != "postgres" { - continue - } - pgVersion, err := c.getNewPgVersion(container, c.Spec.PostgresqlParam.PgVersion) - if err != nil { - return fmt.Errorf("could not parse current Postgres version: %v", err) - } - c.Spec.PostgresqlParam.PgVersion = pgVersion - } - desiredSS, err := c.generateStatefulSet(&c.Spec) if err != nil { return fmt.Errorf("could not generate statefulset: %v", err)