K8SPG-712: Allow overriding default configurations (#1108)

* K8SPG-712: Allow overriding default configurations

* add debug log for replacing patroni config

* enable debug logs with make deploy

* add test

* fix unit test

---------

Co-authored-by: Viacheslav Sarzhan <[email protected]>

Author: egegunes

Makefile

@@ -120,6 +120,7 @@ uninstall: ## Delete the postgrescluster CRD
 deploy: ## Deploy the PostgreSQL Operator (enables the postgrescluster controller)
 	yq eval '(.spec.template.spec.containers[] | select(.name=="operator")).image = "$(IMAGE)"' ./deploy/operator.yaml \
 		| yq eval '(.spec.template.spec.containers[] | select(.name=="operator") | .env[] | select(.name=="DISABLE_TELEMETRY") | .value) = "true"' - \
+		| yq eval '(.spec.template.spec.containers[] | select(.name=="operator") | .env[] | select(.name=="LOG_LEVEL") | .value) = "DEBUG"' - \
 		| kubectl apply -f -
 
 .PHONY: undeploy

internal/controller/postgrescluster/controller.go

@@ -27,13 +27,16 @@ import (
 	"sigs.k8s.io/controller-runtime/pkg/builder"
 	"sigs.k8s.io/controller-runtime/pkg/client"
 	"sigs.k8s.io/controller-runtime/pkg/controller/controllerutil"
+	"sigs.k8s.io/controller-runtime/pkg/event"
 	"sigs.k8s.io/controller-runtime/pkg/manager"
+	"sigs.k8s.io/controller-runtime/pkg/predicate"
 	"sigs.k8s.io/controller-runtime/pkg/reconcile"
 
 	"github.com/percona/percona-postgresql-operator/internal/config"
 	"github.com/percona/percona-postgresql-operator/internal/controller/runtime"
 	"github.com/percona/percona-postgresql-operator/internal/initialize"
 	"github.com/percona/percona-postgresql-operator/internal/logging"
+	"github.com/percona/percona-postgresql-operator/internal/naming"
 	"github.com/percona/percona-postgresql-operator/internal/pgaudit"
 	"github.com/percona/percona-postgresql-operator/internal/pgbackrest"
 	"github.com/percona/percona-postgresql-operator/internal/pgbouncer"
@@ -504,9 +507,22 @@ func (r *Reconciler) SetupWithManager(mgr manager.Manager) error {
 		}
 	}
 
+	// K8SPG-712: Allow overriding default configurations
+	configMapPredicate := builder.WithPredicates(predicate.Funcs{
+		UpdateFunc: func(e event.UpdateEvent) bool {
+			configMap, ok := e.ObjectNew.(*corev1.ConfigMap)
+			if !ok {
+				return true
+			}
+			// Skip reconciliation if the ConfigMap has the specified annotation
+			_, hasAnnotation := configMap.Annotations[naming.OverrideConfigAnnotation]
+			return !hasAnnotation
+		},
+	})
+
 	return builder.ControllerManagedBy(mgr).
 		For(&v1beta1.PostgresCluster{}).
-		Owns(&corev1.ConfigMap{}).
+		Owns(&corev1.ConfigMap{}, configMapPredicate). // K8SPG-712
 		Owns(&corev1.Endpoints{}).
 		Owns(&corev1.PersistentVolumeClaim{}).
 		Owns(&corev1.Secret{}).

internal/controller/postgrescluster/controller_test.go

@@ -512,6 +512,31 @@ spec:
 			Expect(icm.Data["patroni.yaml"]).ToNot(BeZero())
 		})
 
+		It("doesn't reconcile ConfigMap with override annotation", func() {
+			icm := &corev1.ConfigMap{}
+			Expect(suite.Client.Get(context.Background(), client.ObjectKey{
+				Namespace: test.Namespace.Name, Name: instance.Name + "-config",
+			}, icm)).To(Succeed())
+
+			Expect(icm.Labels[naming.LabelCluster]).To(Equal("carlos"))
+			Expect(icm.Labels[naming.LabelInstance]).To(Equal(instance.Name))
+			Expect(icm.Data["patroni.yaml"]).ToNot(BeZero())
+
+			icm.SetAnnotations(map[string]string{
+				naming.OverrideConfigAnnotation: "true",
+			})
+			icm.Data["patroni.yaml"] = ""
+
+			Expect(suite.Client.Update(context.Background(), icm)).To(Succeed())
+
+			Expect(suite.Client.Get(context.Background(), client.ObjectKey{
+				Namespace: test.Namespace.Name, Name: instance.Name + "-config",
+			}, icm)).To(Succeed())
+
+			Expect(icm.Annotations[naming.OverrideConfigAnnotation]).To(Equal("true"))
+			Expect(icm.Data["patroni.yaml"]).To(BeZero())
+		})
+
 		Specify("Instance StatefulSet", func() {
 			Expect(instance.Labels[naming.LabelCluster]).To(Equal("carlos"))
 			Expect(instance.Labels[naming.LabelInstanceSet]).To(Equal("samba"))

internal/controller/postgrescluster/patroni.go

@@ -222,6 +222,8 @@ func (r *Reconciler) reconcilePatroniDynamicConfiguration(
 	}
 	configuration = patroni.DynamicConfiguration(cluster, configuration, pgHBAs, pgParameters)
 
+	logging.FromContext(ctx).V(1).Info("Replacing patroni dynamic configuration")
+
 	return errors.WithStack(
 		patroni.Executor(exec).ReplaceConfiguration(ctx, configuration))
 }

internal/controller/postgrescluster/pgbackrest_test.go

@@ -2060,7 +2060,7 @@ func TestReconcileCloudBasedDataSource(t *testing.T) {
 			result: testResult{
 				configCount: 1, jobCount: 1, pvcCount: 1,
 				expectedClusterCondition: nil,
-				conf:                     "|\n  # Generated by postgres-operator. DO NOT EDIT.\n  # Your changes will not be saved.\n\n  [global]\n  archive-async = y\n  log-path = /pgdata/pgbackrest/log\n  repo1-path = /pgbackrest/repo1\n  spool-path = /pgdata/pgbackrest-spool\n\n  [db]\n  pg1-path = /pgdata/pg13\n  pg1-port = 5432\n  pg1-socket-path = /tmp/postgres\n",
+				conf:                     "|\n  # Generated by postgres-operator. DO NOT EDIT UNLESS YOU KNOW WHAT YOU'RE DOING.\n  # If you want to override the config, annotate this ConfigMap with pgv2.percona.com/override-config=true\n\n  [global]\n  archive-async = y\n  log-path = /pgdata/pgbackrest/log\n  repo1-path = /pgbackrest/repo1\n  spool-path = /pgdata/pgbackrest-spool\n\n  [db]\n  pg1-path = /pgdata/pg13\n  pg1-port = 5432\n  pg1-socket-path = /tmp/postgres\n",
 			},
 		}, {
 			desc: "global/configuration set",
@@ -2077,7 +2077,7 @@ func TestReconcileCloudBasedDataSource(t *testing.T) {
 			result: testResult{
 				configCount: 1, jobCount: 1, pvcCount: 1,
 				expectedClusterCondition: nil,
-				conf:                     "|\n  # Generated by postgres-operator. DO NOT EDIT.\n  # Your changes will not be saved.\n\n  [global]\n  archive-async = y\n  log-path = /pgdata/pgbackrest/log\n  repo1-path = elephant\n  spool-path = /pgdata/pgbackrest-spool\n\n  [db]\n  pg1-path = /pgdata/pg13\n  pg1-port = 5432\n  pg1-socket-path = /tmp/postgres\n",
+				conf:                     "|\n  # Generated by postgres-operator. DO NOT EDIT UNLESS YOU KNOW WHAT YOU'RE DOING.\n  # If you want to override the config, annotate this ConfigMap with pgv2.percona.com/override-config=true\n\n  [global]\n  archive-async = y\n  log-path = /pgdata/pgbackrest/log\n  repo1-path = elephant\n  spool-path = /pgdata/pgbackrest-spool\n\n  [db]\n  pg1-path = /pgdata/pg13\n  pg1-port = 5432\n  pg1-socket-path = /tmp/postgres\n",
 			},
 		}, {
 			desc: "invalid option: stanza",
@@ -2092,7 +2092,7 @@ func TestReconcileCloudBasedDataSource(t *testing.T) {
 			result: testResult{
 				configCount: 1, jobCount: 0, pvcCount: 1,
 				expectedClusterCondition: nil,
-				conf:                     "|\n  # Generated by postgres-operator. DO NOT EDIT.\n  # Your changes will not be saved.\n\n  [global]\n  archive-async = y\n  log-path = /pgdata/pgbackrest/log\n  repo1-path = /pgbackrest/repo1\n  spool-path = /pgdata/pgbackrest-spool\n\n  [db]\n  pg1-path = /pgdata/pg13\n  pg1-port = 5432\n  pg1-socket-path = /tmp/postgres\n",
+				conf:                     "|\n  # Generated by postgres-operator. DO NOT EDIT UNLESS YOU KNOW WHAT YOU'RE DOING.\n  # If you want to override the config, annotate this ConfigMap with pgv2.percona.com/override-config=true\n\n  [global]\n  archive-async = y\n  log-path = /pgdata/pgbackrest/log\n  repo1-path = /pgbackrest/repo1\n  spool-path = /pgdata/pgbackrest-spool\n\n  [db]\n  pg1-path = /pgdata/pg13\n  pg1-port = 5432\n  pg1-socket-path = /tmp/postgres\n",
 			},
 		}, {
 			desc: "cluster bootstrapped init condition missing",
@@ -2111,7 +2111,7 @@ func TestReconcileCloudBasedDataSource(t *testing.T) {
 					Reason:  "ClusterAlreadyBootstrapped",
 					Message: "The cluster is already bootstrapped",
 				},
-				conf: "|\n  # Generated by postgres-operator. DO NOT EDIT.\n  # Your changes will not be saved.\n\n  [global]\n  archive-async = y\n log-path = /pgdata/pgbackrest/log\n  repo1-path = /pgbackrest/repo1\n  spool-path = /pgdata/pgbackrest-spool\n\n  [db]\n  pg1-path = /pgdata/pg13\n  pg1-port = 5432\n  pg1-socket-path = /tmp/postgres\n",
+				conf: "|\n  # Generated by postgres-operator. DO NOT EDIT UNLESS YOU KNOW WHAT YOU'RE DOING.\n  # If you want to override the config, annotate this ConfigMap with pgv2.percona.com/override-config=true\n\n  [global]\n  archive-async = y\n log-path = /pgdata/pgbackrest/log\n  repo1-path = /pgbackrest/repo1\n  spool-path = /pgdata/pgbackrest-spool\n\n  [db]\n  pg1-path = /pgdata/pg13\n  pg1-port = 5432\n  pg1-socket-path = /tmp/postgres\n",
 			},
 		}}
 

internal/naming/annotations.go

@@ -5,7 +5,8 @@
 package naming
 
 const (
-	annotationPrefix = labelPrefix
+	annotationPrefix        = labelPrefix
+	perconaAnnotationPrefix = perconaLabelPrefix
 
 	// Finalizer marks an object to be garbage collected by this module.
 	Finalizer = annotationPrefix + "finalizer"
@@ -68,4 +69,11 @@ const (
 	// to a cluster without backups. As usual with the operator, we do not
 	// touch cloud-based backups.
 	AuthorizeBackupRemovalAnnotation = annotationPrefix + "authorizeBackupRemoval"
+
+	// K8SPG-712
+	// OverrideConfigAnnotation is an annotation used to prevent the controller from reconciling
+	// ConfigMaps when a user wants to manually override their contents. When this annotation
+	// is present, the controller will not update the ConfigMap, allowing users to make custom
+	// modifications that won't be overwritten during reconciliation.
+	OverrideConfigAnnotation = perconaAnnotationPrefix + "override-config"
 )

internal/patroni/config.go

@@ -30,8 +30,8 @@ const (
 
 const (
 	yamlGeneratedWarning = "" +
-		"# Generated by postgres-operator. DO NOT EDIT.\n" +
-		"# Your changes will not be saved.\n"
+		"# Generated by postgres-operator. DO NOT EDIT UNLESS YOU KNOW WHAT YOU'RE DOING.\n" +
+		"# If you want to override the config, annotate this ConfigMap with " + naming.OverrideConfigAnnotation + "=true\n"
 )
 
 // quoteShellWord ensures that s is interpreted by a shell as single word.
@@ -535,7 +535,7 @@ func instanceYAML(
 		// NOTE(cbandy): The "--waldir" option was introduced in PostgreSQL v10.
 		"waldir=" + postgres.WALDirectory(cluster, instance),
 	}
-	methods := []string{"basebackup"}
+	methods := []string{basebackupCreateReplicaMethod}
 
 	// Prefer a pgBackRest method when it is available, and fallback to other
 	// methods when it fails.

internal/patroni/config_test.go

@@ -38,8 +38,8 @@ func TestClusterYAML(t *testing.T) {
 		data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.Parameters{})
 		assert.NilError(t, err)
 		assert.Equal(t, data, strings.TrimSpace(`
-# Generated by postgres-operator. DO NOT EDIT.
-# Your changes will not be saved.
+# Generated by postgres-operator. DO NOT EDIT UNLESS YOU KNOW WHAT YOU'RE DOING.
+# If you want to override the config, annotate this ConfigMap with pgv2.percona.com/override-config=true
 bootstrap:
   dcs:
     loop_wait: 10
@@ -97,8 +97,8 @@ watchdog:
 		data, err := clusterYAML(cluster, postgres.HBAs{}, postgres.Parameters{})
 		assert.NilError(t, err)
 		assert.Equal(t, data, strings.TrimSpace(`
-# Generated by postgres-operator. DO NOT EDIT.
-# Your changes will not be saved.
+# Generated by postgres-operator. DO NOT EDIT UNLESS YOU KNOW WHAT YOU'RE DOING.
+# If you want to override the config, annotate this ConfigMap with pgv2.percona.com/override-config=true
 bootstrap:
   dcs:
     loop_wait: 10
@@ -895,8 +895,8 @@ func TestInstanceYAML(t *testing.T) {
 	data, err := instanceYAML(cluster, instance, nil)
 	assert.NilError(t, err)
 	assert.Equal(t, data, strings.Trim(`
-# Generated by postgres-operator. DO NOT EDIT.
-# Your changes will not be saved.
+# Generated by postgres-operator. DO NOT EDIT UNLESS YOU KNOW WHAT YOU'RE DOING.
+# If you want to override the config, annotate this ConfigMap with pgv2.percona.com/override-config=true
 bootstrap:
   initdb:
   - data-checksums
@@ -918,8 +918,8 @@ tags: {}
 	dataWithReplicaCreate, err := instanceYAML(cluster, instance, []string{"some", "backrest", "cmd"})
 	assert.NilError(t, err)
 	assert.Equal(t, dataWithReplicaCreate, strings.Trim(`
-# Generated by postgres-operator. DO NOT EDIT.
-# Your changes will not be saved.
+# Generated by postgres-operator. DO NOT EDIT UNLESS YOU KNOW WHAT YOU'RE DOING.
+# If you want to override the config, annotate this ConfigMap with pgv2.percona.com/override-config=true
 bootstrap:
   initdb:
   - data-checksums
@@ -958,8 +958,8 @@ tags: {}
 	datawithTDE, err := instanceYAML(cluster, instance, nil)
 	assert.NilError(t, err)
 	assert.Equal(t, datawithTDE, strings.Trim(`
-# Generated by postgres-operator. DO NOT EDIT.
-# Your changes will not be saved.
+# Generated by postgres-operator. DO NOT EDIT UNLESS YOU KNOW WHAT YOU'RE DOING.
+# If you want to override the config, annotate this ConfigMap with pgv2.percona.com/override-config=true
 bootstrap:
   initdb:
   - data-checksums
@@ -983,8 +983,8 @@ tags: {}
 	dataWithCustomMethods, err := instanceYAML(cluster, instance, nil)
 	assert.NilError(t, err)
 	assert.Equal(t, dataWithCustomMethods, strings.Trim(`
-# Generated by postgres-operator. DO NOT EDIT.
-# Your changes will not be saved.
+# Generated by postgres-operator. DO NOT EDIT UNLESS YOU KNOW WHAT YOU'RE DOING.
+# If you want to override the config, annotate this ConfigMap with pgv2.percona.com/override-config=true
 bootstrap:
   initdb:
   - data-checksums

internal/pgbackrest/config.go

@@ -56,8 +56,8 @@ const (
 
 const (
 	iniGeneratedWarning = "" +
-		"# Generated by postgres-operator. DO NOT EDIT.\n" +
-		"# Your changes will not be saved.\n"
+		"# Generated by postgres-operator. DO NOT EDIT UNLESS YOU KNOW WHAT YOU'RE DOING.\n" +
+		"# If you want to override the config, annotate this ConfigMap with " + naming.OverrideConfigAnnotation + "=true\n"
 )
 
 // CreatePGBackRestConfigMapIntent creates a configmap struct with pgBackRest pgbackrest.conf settings in the data field.

internal/pgbackrest/config_test.go

@@ -89,8 +89,8 @@ func TestCreatePGBackRestConfigMapIntent(t *testing.T) {
 
 		assert.Equal(t, configmap.Data["config-hash"], "abcde12345")
 		assert.Equal(t, configmap.Data["pgbackrest_repo.conf"], strings.Trim(`
-# Generated by postgres-operator. DO NOT EDIT.
-# Your changes will not be saved.
+# Generated by postgres-operator. DO NOT EDIT UNLESS YOU KNOW WHAT YOU'RE DOING.
+# If you want to override the config, annotate this ConfigMap with pgv2.percona.com/override-config=true
 
 [global]
 log-path = /pgbackrest/repo1/log
@@ -120,8 +120,8 @@ pg1-socket-path = /tmp/postgres
 		`, "\t\n")+"\n")
 
 		assert.Equal(t, configmap.Data["pgbackrest_instance.conf"], strings.Trim(`
-# Generated by postgres-operator. DO NOT EDIT.
-# Your changes will not be saved.
+# Generated by postgres-operator. DO NOT EDIT UNLESS YOU KNOW WHAT YOU'RE DOING.
+# If you want to override the config, annotate this ConfigMap with pgv2.percona.com/override-config=true
 
 [global]
 archive-async = y