Skip to content

K8SPG-792 Make patroni check check respect default image settings #1167

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 16 additions & 6 deletions internal/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -99,18 +99,28 @@ 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)
// 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)

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.
// Made as a wrapper of PostgresContainerImageString for compat reasons
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 ""
Expand Down
4 changes: 2 additions & 2 deletions percona/controller/pgcluster/controller.go
Original file line number Diff line number Diff line change
Expand Up @@ -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",
},
Expand Down Expand Up @@ -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,
Expand Down
7 changes: 7 additions & 0 deletions pkg/apis/pgv2.percona.com/v2/perconapgcluster_types.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -245,6 +246,12 @@ func (cr *PerconaPGCluster) Default() {
}
}

func (cr *PerconaPGCluster) PostgresImage() string {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can introduce a very basic unit test to verify in a consistent way the functionality in future iterations.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added unit tests for the default image behaviour

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)

Expand Down
61 changes: 61 additions & 0 deletions pkg/apis/pgv2.percona.com/v2/perconapgcluster_types_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ package v2

import (
"testing"
"os"
"fmt"

"gotest.tools/v3/assert"
)
Expand Down Expand Up @@ -41,3 +43,62 @@ 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

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,
},
}

for name, tt := range tests {
t.Run(name, func(t *testing.T) {

cluster.Spec.Image = tt.setImage

if (tt.envImage != "") {
err := os.Setenv(testEnv, tt.envImage)

if (err != nil) {
t.Fatalf("Failed to set %s env variable: %v", testEnv, err)
}

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(), tt.expectedImage)
})
}
}