From 2b1d74cb4a1aa48e810716696ec67e0d0fe19f25 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Fri, 13 Mar 2020 11:02:34 +0100 Subject: [PATCH] update getNewPgVersion and added tests --- pkg/cluster/k8sres.go | 17 ++--- pkg/cluster/k8sres_test.go | 129 +++++++++++++++++++++++++++++++++++++ pkg/cluster/sync.go | 13 ++-- 3 files changed, 144 insertions(+), 15 deletions(-) diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index 4f1fea1d1..fb53b0c66 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -725,25 +725,20 @@ func extractPgVersionFromBinPath(binPath string, template string) (string, error return fmt.Sprintf("%v", pgVersion), nil } -func (c *Cluster) getNewPgVersion(containers []v1.Container, newPgVersion string) (string, error) { +func (c *Cluster) getNewPgVersion(container v1.Container, newPgVersion string) (string, error) { var ( spiloConfiguration spiloConfiguration runningPgVersion string err error ) - for _, container := range containers { - if container.Name != "postgres" { + for _, env := range container.Env { + if env.Name != "SPILO_CONFIGURATION" { continue } - for _, env := range container.Env { - if env.Name != "SPILO_CONFIGURATION" { - continue - } - err = json.Unmarshal([]byte(env.Value), &spiloConfiguration) - if err != nil { - return newPgVersion, err - } + err = json.Unmarshal([]byte(env.Value), &spiloConfiguration) + if err != nil { + return newPgVersion, err } } diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index e8fe05456..5c63c09c8 100644 --- a/pkg/cluster/k8sres_test.go +++ b/pkg/cluster/k8sres_test.go @@ -381,6 +381,135 @@ func TestCloneEnv(t *testing.T) { } } +func TestExtractPgVersionFromBinPath(t *testing.T) { + testName := "TestExtractPgVersionFromBinPath" + tests := []struct { + subTest string + binPath string + template string + expected string + }{ + { + subTest: "test current bin path with decimal against hard coded template", + binPath: "/usr/lib/postgresql/9.6/bin", + template: pgBinariesLocationTemplate, + expected: "9.6", + }, + { + subTest: "test current bin path against hard coded template", + binPath: "/usr/lib/postgresql/12/bin", + template: pgBinariesLocationTemplate, + expected: "12", + }, + { + subTest: "test alternative bin path against a matching template", + binPath: "/usr/pgsql-12/bin", + template: "/usr/pgsql-%v/bin", + expected: "12", + }, + } + + for _, tt := range tests { + pgVersion, err := extractPgVersionFromBinPath(tt.binPath, tt.template) + if err != nil { + t.Errorf("Unexpected error: %v", err) + } + if pgVersion != tt.expected { + t.Errorf("%s %s: Expected version %s, have %s instead", + testName, tt.subTest, tt.expected, pgVersion) + } + } +} + +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) + + 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 63b842806..b04ff863b 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -286,11 +286,16 @@ func (c *Cluster) syncStatefulSet() error { c.Statefulset = sset // check if there is no Postgres version mismatch - pgVersion, err := c.getNewPgVersion(c.Statefulset.Spec.Template.Spec.Containers, c.Spec.PostgresqlParam.PgVersion) - if err != nil { - return fmt.Errorf("could not parse current Postgres version: %v", err) + 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 } - c.Spec.PostgresqlParam.PgVersion = pgVersion desiredSS, err := c.generateStatefulSet(&c.Spec) if err != nil {