From 6aa16e9858909cce9e6c693cc4835d5af7ef3104 Mon Sep 17 00:00:00 2001 From: Jakub Jaruszewski Date: Tue, 10 Jun 2025 13:42:00 +0200 Subject: [PATCH 1/5] K8SPG-792 Make patroni check check respect default image settings Signed-off-by: Jakub Jaruszewski --- internal/config/config.go | 20 +++++++++++++------ percona/controller/pgcluster/controller.go | 4 ++-- .../v2/perconapgcluster_types.go | 7 +++++++ 3 files changed, 23 insertions(+), 8 deletions(-) diff --git a/internal/config/config.go b/internal/config/config.go index 783f56c3b..469ea0ae1 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -99,18 +99,26 @@ func PGExporterContainerImage(cluster *v1beta1.PostgresCluster) string { return defaultFromEnv(image, "RELATED_IMAGE_PGEXPORTER") } -// PostgresContainerImage returns the container image to use for PostgreSQL. -func PostgresContainerImage(cluster *v1beta1.PostgresCluster) string { - image := cluster.Spec.Image - key := "RELATED_IMAGE_POSTGRES_" + fmt.Sprint(cluster.Spec.PostgresVersion) +// PostgresContainerImage returns the container image to use for PostgreSQL (from string params). +func PostgresContainerImageString(image string, postgresVersion int, postGISVersion string) string { + key := "RELATED_IMAGE_POSTGRES_" + fmt.Sprint(postgresVersion) - if version := cluster.Spec.PostGISVersion; version != "" { - key += "_GIS_" + version + if postGISVersion != "" { + key += "_GIS_" + postGISVersion } return defaultFromEnv(image, key) } +// PostgresContainerImage returns the container image to use for PostgreSQL. +func PostgresContainerImage(cluster *v1beta1.PostgresCluster) string { + image := cluster.Spec.Image + postgresVersion := cluster.Spec.PostgresVersion + postGISVersion := cluster.Spec.PostGISVersion + + return PostgresContainerImageString(image, postgresVersion, postGISVersion) +} + // PGONamespace returns the namespace where the PGO is running, // based on the env var from the DownwardAPI // If no env var is found, returns "" diff --git a/percona/controller/pgcluster/controller.go b/percona/controller/pgcluster/controller.go index b13360340..28cb9b0cb 100644 --- a/percona/controller/pgcluster/controller.go +++ b/percona/controller/pgcluster/controller.go @@ -399,7 +399,7 @@ func (r *PGClusterReconciler) reconcilePatroniVersionCheck(ctx context.Context, Containers: []corev1.Container{ { Name: pNaming.ContainerPatroniVersionCheck, - Image: cr.Spec.Image, + Image: cr.PostgresImage(), Command: []string{ "bash", }, @@ -781,7 +781,7 @@ func (r *PGClusterReconciler) reconcileCustomExtensions(ctx context.Context, cr for i := 0; i < len(cr.Spec.InstanceSets); i++ { set := &cr.Spec.InstanceSets[i] set.InitContainers = append(set.InitContainers, extensions.ExtensionRelocatorContainer( - cr, cr.Spec.Image, cr.Spec.ImagePullPolicy, cr.Spec.PostgresVersion, + cr, cr.PostgresImage(), cr.Spec.ImagePullPolicy, cr.Spec.PostgresVersion, )) set.InitContainers = append(set.InitContainers, extensions.ExtensionInstallerContainer( cr, diff --git a/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types.go b/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types.go index bcb1d2e16..a63503c4f 100644 --- a/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types.go +++ b/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types.go @@ -10,6 +10,7 @@ import ( "k8s.io/apimachinery/pkg/util/intstr" "sigs.k8s.io/controller-runtime/pkg/controller/controllerutil" + "github.com/percona/percona-postgresql-operator/internal/config" "github.com/percona/percona-postgresql-operator/internal/logging" "github.com/percona/percona-postgresql-operator/internal/naming" pNaming "github.com/percona/percona-postgresql-operator/percona/naming" @@ -245,6 +246,12 @@ func (cr *PerconaPGCluster) Default() { } } +func (cr *PerconaPGCluster) PostgresImage() string { + image := cr.Spec.Image + postgresVersion := cr.Spec.PostgresVersion + return config.PostgresContainerImageString(image, postgresVersion, "") +} + func (cr *PerconaPGCluster) ToCrunchy(ctx context.Context, postgresCluster *crunchyv1beta1.PostgresCluster, scheme *runtime.Scheme) (*crunchyv1beta1.PostgresCluster, error) { log := logging.FromContext(ctx) From 28663f9208522233298c7861a4f1ae04c976dd69 Mon Sep 17 00:00:00 2001 From: Jakub Jaruszewski Date: Fri, 13 Jun 2025 15:02:27 +0200 Subject: [PATCH 2/5] K8SPG-792 Add unit tests for default image behaviour Signed-off-by: Jakub Jaruszewski --- .../v2/perconapgcluster_types_test.go | 35 +++++++++++++++++++ 1 file changed, 35 insertions(+) diff --git a/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types_test.go b/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types_test.go index d5ef12489..e5a2d5cfd 100644 --- a/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types_test.go +++ b/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types_test.go @@ -2,6 +2,8 @@ package v2 import ( "testing" + "os" + "fmt" "gotest.tools/v3/assert" ) @@ -41,3 +43,36 @@ func TestPerconaPGCluster_BackupsEnabled(t *testing.T) { }) } } + + +func TestPerconaPGCluster_PostgresImage(t *testing.T) { + cluster := new(PerconaPGCluster) + cluster.Default() + + postgresVersion := 16 + testDefaultImage := fmt.Sprintf("test_default_image:%d", postgresVersion) + testSpecificImage := fmt.Sprintf("test_defined_image:%d", postgresVersion) + testEnv := fmt.Sprintf("RELATED_IMAGE_POSTGRES_%d", postgresVersion) + + cluster.Spec.PostgresVersion = postgresVersion + + t.Run("Spec.Image should be empty by default", func(t *testing.T) { + assert.Equal(t, cluster.PostgresImage(), "") + }) + + t.Run("Spec.Image should use env variables if present", func(t *testing.T) { + os.Setenv(testEnv, testDefaultImage) + defer os.Unsetenv(testEnv) + + assert.Equal(t, cluster.PostgresImage(), testDefaultImage) + }) + + t.Run("Spec.Image should use defined variable", func (t *testing.T) { + os.Setenv(testEnv, testDefaultImage) + defer os.Unsetenv(testEnv) + + cluster.Spec.Image = testSpecificImage + + assert.Equal(t, cluster.PostgresImage(), testSpecificImage) + }) +} From a2de96f6cba42004106d5d9308cb8fe73c04cfec Mon Sep 17 00:00:00 2001 From: Jakub Jaruszewski Date: Fri, 13 Jun 2025 15:04:48 +0200 Subject: [PATCH 3/5] K8SPG-792 Add comments regarding changed upstream func Signed-off-by: Jakub Jaruszewski --- internal/config/config.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/internal/config/config.go b/internal/config/config.go index 469ea0ae1..032663fea 100644 --- a/internal/config/config.go +++ b/internal/config/config.go @@ -99,7 +99,8 @@ func PGExporterContainerImage(cluster *v1beta1.PostgresCluster) string { return defaultFromEnv(image, "RELATED_IMAGE_PGEXPORTER") } -// PostgresContainerImage returns the container image to use for PostgreSQL (from string params). +// PostgresContainerImageString returns the container image to use for PostgreSQL (from string params). +// This func copies logic from original PostgresContainerImage as is, leaving PostgresContainerImage as a wrapper for upstream compatibility func PostgresContainerImageString(image string, postgresVersion int, postGISVersion string) string { key := "RELATED_IMAGE_POSTGRES_" + fmt.Sprint(postgresVersion) @@ -111,6 +112,7 @@ func PostgresContainerImageString(image string, postgresVersion int, postGISVers } // PostgresContainerImage returns the container image to use for PostgreSQL. +// Made as a wrapper of PostgresContainerImageString for compat reasons func PostgresContainerImage(cluster *v1beta1.PostgresCluster) string { image := cluster.Spec.Image postgresVersion := cluster.Spec.PostgresVersion From 0e45517fd5be7c0e655b34aa25d57f2cb4ff391c Mon Sep 17 00:00:00 2001 From: Jakub Jaruszewski Date: Tue, 17 Jun 2025 11:36:30 +0200 Subject: [PATCH 4/5] K8SPG-792 Use table for tests Signed-off-by: Jakub Jaruszewski --- .../v2/perconapgcluster_types_test.go | 54 ++++++++++++++----- 1 file changed, 40 insertions(+), 14 deletions(-) diff --git a/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types_test.go b/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types_test.go index e5a2d5cfd..4637bdb3c 100644 --- a/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types_test.go +++ b/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types_test.go @@ -56,23 +56,49 @@ func TestPerconaPGCluster_PostgresImage(t *testing.T) { cluster.Spec.PostgresVersion = postgresVersion - t.Run("Spec.Image should be empty by default", func(t *testing.T) { - assert.Equal(t, cluster.PostgresImage(), "") - }) + tests := map[string]struct { + expectedImage string + setImage string + envImage string + }{ + "Spec.Image should be empty by default": { + expectedImage: "", + setImage: "", + envImage: "", + }, + "Spec.Image should use env variables if present": { + expectedImage: testDefaultImage, + setImage: "", + envImage: testDefaultImage, + }, + "Spec.Image should use defined variable": { + expectedImage: testSpecificImage, + setImage: testSpecificImage, + envImage: testDefaultImage, + }, + } - t.Run("Spec.Image should use env variables if present", func(t *testing.T) { - os.Setenv(testEnv, testDefaultImage) - defer os.Unsetenv(testEnv) + for name, tt := range tests { + t.Run(name, func(t *testing.T) { + + cluster.Spec.Image = tt.setImage - assert.Equal(t, cluster.PostgresImage(), testDefaultImage) - }) + if (tt.envImage != "") { + err := os.Setenv(testEnv, tt.envImage) - t.Run("Spec.Image should use defined variable", func (t *testing.T) { - os.Setenv(testEnv, testDefaultImage) - defer os.Unsetenv(testEnv) + if (err != nil) { + t.Fatalf("Failed to set %s env variable: %v", testEnv, err) + } - cluster.Spec.Image = testSpecificImage + defer func() { + err := os.Unsetenv(testEnv) + if (err != nil) { + t.Errorf("Failed to unset %s env variable: %v", testEnv, err) + } + }() + } - assert.Equal(t, cluster.PostgresImage(), testSpecificImage) - }) + assert.Equal(t, cluster.PostgresImage(), tt.expectedImage) + }) + } } From 688c3d3555758474122070ef9637e628c8654180 Mon Sep 17 00:00:00 2001 From: Jakub Jaruszewski Date: Wed, 25 Jun 2025 20:58:53 +0200 Subject: [PATCH 5/5] K8SPG-792 Fix gofmt warning Signed-off-by: Jakub Jaruszewski --- .../v2/perconapgcluster_types_test.go | 27 +++++++++---------- 1 file changed, 13 insertions(+), 14 deletions(-) diff --git a/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types_test.go b/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types_test.go index 4637bdb3c..228056957 100644 --- a/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types_test.go +++ b/pkg/apis/pgv2.percona.com/v2/perconapgcluster_types_test.go @@ -1,9 +1,9 @@ package v2 import ( - "testing" - "os" "fmt" + "os" + "testing" "gotest.tools/v3/assert" ) @@ -44,7 +44,6 @@ func TestPerconaPGCluster_BackupsEnabled(t *testing.T) { } } - func TestPerconaPGCluster_PostgresImage(t *testing.T) { cluster := new(PerconaPGCluster) cluster.Default() @@ -58,23 +57,23 @@ func TestPerconaPGCluster_PostgresImage(t *testing.T) { tests := map[string]struct { expectedImage string - setImage string - envImage string + setImage string + envImage string }{ "Spec.Image should be empty by default": { expectedImage: "", - setImage: "", - envImage: "", + setImage: "", + envImage: "", }, "Spec.Image should use env variables if present": { expectedImage: testDefaultImage, - setImage: "", - envImage: testDefaultImage, + setImage: "", + envImage: testDefaultImage, }, "Spec.Image should use defined variable": { expectedImage: testSpecificImage, - setImage: testSpecificImage, - envImage: testDefaultImage, + setImage: testSpecificImage, + envImage: testDefaultImage, }, } @@ -83,16 +82,16 @@ func TestPerconaPGCluster_PostgresImage(t *testing.T) { cluster.Spec.Image = tt.setImage - if (tt.envImage != "") { + if tt.envImage != "" { err := os.Setenv(testEnv, tt.envImage) - if (err != nil) { + if err != nil { t.Fatalf("Failed to set %s env variable: %v", testEnv, err) } defer func() { err := os.Unsetenv(testEnv) - if (err != nil) { + if err != nil { t.Errorf("Failed to unset %s env variable: %v", testEnv, err) } }()