From 4d585250dbaa1003c4b8f61068e4bb97765dcf20 Mon Sep 17 00:00:00 2001 From: Polina Bungina Date: Fri, 2 Dec 2022 13:33:02 +0100 Subject: [PATCH] Add Patroni failsafe_mode parameter (#2076) This commit adds support of a not-yet-released Patroni feature that allows postgres to run as primary in case of a failed leader lock update. * Add Patroni 'failsafe_mode' local parameter (enable for a single PG cluster) * Allow configuring Patroni 'failsafe_mode' parameter globally --- .../crds/operatorconfigurations.yaml | 6 + .../postgres-operator/crds/postgresqls.yaml | 2 + charts/postgres-operator/values.yaml | 4 + docs/reference/cluster_manifest.md | 3 + e2e/tests/test_e2e.py | 5 +- manifests/complete-postgres-manifest.yaml | 1 + manifests/configmap.yaml | 1 + manifests/operatorconfiguration.crd.yaml | 6 + ...gresql-operator-default-configuration.yaml | 2 + manifests/postgresql.crd.yaml | 2 + pkg/apis/acid.zalan.do/v1/crds.go | 11 ++ .../v1/operator_configuration_type.go | 8 +- pkg/apis/acid.zalan.do/v1/postgresql_type.go | 1 + .../acid.zalan.do/v1/zz_generated.deepcopy.go | 27 ++++ pkg/cluster/k8sres.go | 14 +- pkg/cluster/k8sres_test.go | 74 ++++++++-- pkg/cluster/sync.go | 15 ++ pkg/cluster/sync_test.go | 133 +++++++++++++++--- pkg/controller/operator_config.go | 3 + .../clientset/versioned/fake/register.go | 14 +- .../clientset/versioned/scheme/register.go | 14 +- pkg/util/config/config.go | 1 + 22 files changed, 297 insertions(+), 50 deletions(-) diff --git a/charts/postgres-operator/crds/operatorconfigurations.yaml b/charts/postgres-operator/crds/operatorconfigurations.yaml index 02fab645f..e6305fada 100644 --- a/charts/postgres-operator/crds/operatorconfigurations.yaml +++ b/charts/postgres-operator/crds/operatorconfigurations.yaml @@ -633,6 +633,12 @@ spec: type: string pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' default: "100Mi" + patroni: + type: object + properties: + failsafe_mode: + type: boolean + default: false status: type: object additionalProperties: diff --git a/charts/postgres-operator/crds/postgresqls.yaml b/charts/postgres-operator/crds/postgresqls.yaml index d2ad89da6..d78bdb2fb 100644 --- a/charts/postgres-operator/crds/postgresqls.yaml +++ b/charts/postgres-operator/crds/postgresqls.yaml @@ -320,6 +320,8 @@ spec: patroni: type: object properties: + failsafe_mode: + type: boolean initdb: type: object additionalProperties: diff --git a/charts/postgres-operator/values.yaml b/charts/postgres-operator/values.yaml index 99e1092be..1ca429a99 100644 --- a/charts/postgres-operator/values.yaml +++ b/charts/postgres-operator/values.yaml @@ -409,6 +409,10 @@ configConnectionPooler: connection_pooler_default_cpu_limit: "1" connection_pooler_default_memory_limit: 100Mi +configPatroni: + # enable Patroni DCS failsafe_mode feature + failsafe_mode: false + # Zalando's internal CDC stream feature enableStreams: false diff --git a/docs/reference/cluster_manifest.md b/docs/reference/cluster_manifest.md index ba4006f64..718479d50 100644 --- a/docs/reference/cluster_manifest.md +++ b/docs/reference/cluster_manifest.md @@ -316,6 +316,9 @@ explanation of `ttl` and `loop_wait` parameters. * **synchronous_node_count** Patroni `synchronous_node_count` parameter value. Note, this option is only available for Spilo images with Patroni 2.0+. The default is set to `1`. Optional. + +* **failsafe_mode** + Patroni `failsafe_mode` parameter value. If enabled, allows Patroni to cope with DCS outages and avoid leader demotion. Note, this option is currently not included in any Patroni release. The default is set to `false`. Optional. ## Postgres container resources diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index d13a51c19..638cd05b2 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -407,7 +407,8 @@ class EndToEndTestCase(unittest.TestCase): "ttl": 29, "loop_wait": 9, "retry_timeout": 9, - "synchronous_mode": True + "synchronous_mode": True, + "failsafe_mode": True, } } } @@ -434,6 +435,8 @@ class EndToEndTestCase(unittest.TestCase): "retry_timeout not updated") self.assertEqual(desired_config["synchronous_mode"], effective_config["synchronous_mode"], "synchronous_mode not updated") + self.assertEqual(desired_config["failsafe_mode"], effective_config["failsafe_mode"], + "failsafe_mode not updated") return True # check if Patroni config has been updated diff --git a/manifests/complete-postgres-manifest.yaml b/manifests/complete-postgres-manifest.yaml index cb86dc272..bc8309684 100644 --- a/manifests/complete-postgres-manifest.yaml +++ b/manifests/complete-postgres-manifest.yaml @@ -109,6 +109,7 @@ spec: cpu: 500m memory: 500Mi patroni: + failsafe_mode: false initdb: encoding: "UTF8" locale: "en_US.UTF-8" diff --git a/manifests/configmap.yaml b/manifests/configmap.yaml index 94b7c5f1c..6eb391ba4 100644 --- a/manifests/configmap.yaml +++ b/manifests/configmap.yaml @@ -47,6 +47,7 @@ data: enable_master_load_balancer: "false" enable_master_pooler_load_balancer: "false" enable_password_rotation: "false" + # enable_patroni_failsafe_mode: "false" enable_pgversion_env_var: "true" # enable_pod_antiaffinity: "false" # enable_pod_disruption_budget: "true" diff --git a/manifests/operatorconfiguration.crd.yaml b/manifests/operatorconfiguration.crd.yaml index 5a4383e82..2be301c84 100644 --- a/manifests/operatorconfiguration.crd.yaml +++ b/manifests/operatorconfiguration.crd.yaml @@ -631,6 +631,12 @@ spec: type: string pattern: '^(\d+(e\d+)?|\d+(\.\d+)?(e\d+)?[EPTGMK]i?)$' default: "100Mi" + patroni: + type: object + properties: + failsafe_mode: + type: boolean + default: false status: type: object additionalProperties: diff --git a/manifests/postgresql-operator-default-configuration.yaml b/manifests/postgresql-operator-default-configuration.yaml index 80643b767..e02ef8bae 100644 --- a/manifests/postgresql-operator-default-configuration.yaml +++ b/manifests/postgresql-operator-default-configuration.yaml @@ -198,3 +198,5 @@ configuration: connection_pooler_number_of_instances: 2 # connection_pooler_schema: "pooler" # connection_pooler_user: "pooler" + patroni: + # failsafe_mode: "false" diff --git a/manifests/postgresql.crd.yaml b/manifests/postgresql.crd.yaml index b113c849f..ae92a756a 100644 --- a/manifests/postgresql.crd.yaml +++ b/manifests/postgresql.crd.yaml @@ -318,6 +318,8 @@ spec: patroni: type: object properties: + failsafe_mode: + type: boolean initdb: type: object additionalProperties: diff --git a/pkg/apis/acid.zalan.do/v1/crds.go b/pkg/apis/acid.zalan.do/v1/crds.go index 58186a5e5..cbff1e3ad 100644 --- a/pkg/apis/acid.zalan.do/v1/crds.go +++ b/pkg/apis/acid.zalan.do/v1/crds.go @@ -503,6 +503,9 @@ var PostgresCRDResourceValidation = apiextv1.CustomResourceValidation{ "patroni": { Type: "object", Properties: map[string]apiextv1.JSONSchemaProps{ + "failsafe_mode": { + Type: "boolean", + }, "initdb": { Type: "object", AdditionalProperties: &apiextv1.JSONSchemaPropsOrBool{ @@ -1458,6 +1461,14 @@ var OperatorConfigCRDResourceValidation = apiextv1.CustomResourceValidation{ }, }, }, + "patroni": { + Type: "object", + Properties: map[string]apiextv1.JSONSchemaProps{ + "failsafe_mode": { + Type: "boolean", + }, + }, + }, "postgres_pod_resources": { Type: "object", Properties: map[string]apiextv1.JSONSchemaProps{ diff --git a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go index 85ba25e34..fc5746413 100644 --- a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go +++ b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go @@ -227,6 +227,11 @@ type OperatorLogicalBackupConfiguration struct { JobPrefix string `json:"logical_backup_job_prefix,omitempty"` } +// PatroniConfiguration defines configuration for Patroni +type PatroniConfiguration struct { + FailsafeMode *bool `json:"failsafe_mode,omitempty"` +} + // OperatorConfigurationData defines the operation config type OperatorConfigurationData struct { EnableCRDRegistration *bool `json:"enable_crd_registration,omitempty"` @@ -259,11 +264,12 @@ type OperatorConfigurationData struct { Scalyr ScalyrConfiguration `json:"scalyr"` LogicalBackup OperatorLogicalBackupConfiguration `json:"logical_backup"` ConnectionPooler ConnectionPoolerConfiguration `json:"connection_pooler"` + Patroni PatroniConfiguration `json:"patroni"` MinInstances int32 `json:"min_instances,omitempty"` MaxInstances int32 `json:"max_instances,omitempty"` IgnoreInstanceLimitsAnnotationKey string `json:"ignore_instance_limits_annotation_key,omitempty"` } -//Duration shortens this frequently used name +// Duration shortens this frequently used name type Duration time.Duration diff --git a/pkg/apis/acid.zalan.do/v1/postgresql_type.go b/pkg/apis/acid.zalan.do/v1/postgresql_type.go index e46a43636..dde47622c 100644 --- a/pkg/apis/acid.zalan.do/v1/postgresql_type.go +++ b/pkg/apis/acid.zalan.do/v1/postgresql_type.go @@ -171,6 +171,7 @@ type Patroni struct { SynchronousMode bool `json:"synchronous_mode,omitempty"` SynchronousModeStrict bool `json:"synchronous_mode_strict,omitempty"` SynchronousNodeCount uint32 `json:"synchronous_node_count,omitempty" defaults:"1"` + FailsafeMode *bool `json:"failsafe_mode,omitempty"` } // StandbyDescription contains remote primary config or s3/gs wal path diff --git a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go index b338421a2..2cae5743e 100644 --- a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go +++ b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go @@ -423,6 +423,7 @@ func (in *OperatorConfigurationData) DeepCopyInto(out *OperatorConfigurationData out.Scalyr = in.Scalyr out.LogicalBackup = in.LogicalBackup in.ConnectionPooler.DeepCopyInto(&out.ConnectionPooler) + in.Patroni.DeepCopyInto(&out.Patroni) return } @@ -549,6 +550,11 @@ func (in *Patroni) DeepCopyInto(out *Patroni) { (*out)[key] = outVal } } + if in.FailsafeMode != nil { + in, out := &in.FailsafeMode, &out.FailsafeMode + *out = new(bool) + **out = **in + } return } @@ -562,6 +568,27 @@ func (in *Patroni) DeepCopy() *Patroni { return out } +// DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. +func (in *PatroniConfiguration) DeepCopyInto(out *PatroniConfiguration) { + *out = *in + if in.FailsafeMode != nil { + in, out := &in.FailsafeMode, &out.FailsafeMode + *out = new(bool) + **out = **in + } + return +} + +// DeepCopy is an autogenerated deepcopy function, copying the receiver, creating a new PatroniConfiguration. +func (in *PatroniConfiguration) DeepCopy() *PatroniConfiguration { + if in == nil { + return nil + } + out := new(PatroniConfiguration) + in.DeepCopyInto(out) + return out +} + // DeepCopyInto is an autogenerated deepcopy function, copying the receiver, writing into out. in must be non-nil. func (in *PostgresPodResourcesDefaults) DeepCopyInto(out *PostgresPodResourcesDefaults) { *out = *in diff --git a/pkg/cluster/k8sres.go b/pkg/cluster/k8sres.go index aa3229848..73f4090d8 100644 --- a/pkg/cluster/k8sres.go +++ b/pkg/cluster/k8sres.go @@ -59,6 +59,7 @@ type patroniDCS struct { SynchronousNodeCount uint32 `json:"synchronous_node_count,omitempty"` PGBootstrapConfiguration map[string]interface{} `json:"postgresql,omitempty"` Slots map[string]map[string]string `json:"slots,omitempty"` + FailsafeMode *bool `json:"failsafe_mode,omitempty"` } type pgBootstrap struct { @@ -296,7 +297,7 @@ func (c *Cluster) generateResourceRequirements( return &result, nil } -func generateSpiloJSONConfiguration(pg *acidv1.PostgresqlParam, patroni *acidv1.Patroni, pamRoleName string, EnablePgVersionEnvVar bool, logger *logrus.Entry) (string, error) { +func generateSpiloJSONConfiguration(pg *acidv1.PostgresqlParam, patroni *acidv1.Patroni, opConfig *config.Config, logger *logrus.Entry) (string, error) { config := spiloConfiguration{} config.Bootstrap = pgBootstrap{} @@ -378,6 +379,11 @@ PatroniInitDBParams: if patroni.SynchronousNodeCount >= 1 { config.Bootstrap.DCS.SynchronousNodeCount = patroni.SynchronousNodeCount } + if patroni.FailsafeMode != nil { + config.Bootstrap.DCS.FailsafeMode = patroni.FailsafeMode + } else if opConfig.EnablePatroniFailsafeMode != nil { + config.Bootstrap.DCS.FailsafeMode = opConfig.EnablePatroniFailsafeMode + } config.PgLocalConfiguration = make(map[string]interface{}) @@ -385,7 +391,7 @@ PatroniInitDBParams: // setting postgresq.bin_dir in the SPILO_CONFIGURATION still works and takes precedence over PGVERSION // so we add postgresq.bin_dir only if PGVERSION is unused // see PR 222 in Spilo - if !EnablePgVersionEnvVar { + if !opConfig.EnablePgVersionEnvVar { config.PgLocalConfiguration[patroniPGBinariesParameterName] = fmt.Sprintf(pgBinariesLocationTemplate, pg.PgVersion) } if len(pg.Parameters) > 0 { @@ -407,7 +413,7 @@ PatroniInitDBParams: } config.Bootstrap.Users = map[string]pgUser{ - pamRoleName: { + opConfig.PamRoleName: { Password: "", Options: []string{constants.RoleFlagCreateDB, constants.RoleFlagNoLogin}, }, @@ -1179,7 +1185,7 @@ func (c *Cluster) generateStatefulSet(spec *acidv1.PostgresSpec) (*appsv1.Statef } } - spiloConfiguration, err := generateSpiloJSONConfiguration(&spec.PostgresqlParam, &spec.Patroni, c.OpConfig.PamRoleName, c.OpConfig.EnablePgVersionEnvVar, c.logger) + spiloConfiguration, err := generateSpiloJSONConfiguration(&spec.PostgresqlParam, &spec.Patroni, &c.OpConfig, c.logger) if err != nil { return nil, fmt.Errorf("could not generate Spilo JSON configuration: %v", err) } diff --git a/pkg/cluster/k8sres_test.go b/pkg/cluster/k8sres_test.go index 797c1426c..4781bfabc 100644 --- a/pkg/cluster/k8sres_test.go +++ b/pkg/cluster/k8sres_test.go @@ -69,17 +69,19 @@ func TestGenerateSpiloJSONConfiguration(t *testing.T) { subtest string pgParam *acidv1.PostgresqlParam patroni *acidv1.Patroni - role string - opConfig config.Config + opConfig *config.Config result string }{ { - subtest: "Patroni default configuration", - pgParam: &acidv1.PostgresqlParam{PgVersion: "9.6"}, - patroni: &acidv1.Patroni{}, - role: "zalandos", - opConfig: config.Config{}, - result: `{"postgresql":{"bin_dir":"/usr/lib/postgresql/9.6/bin"},"bootstrap":{"initdb":[{"auth-host":"md5"},{"auth-local":"trust"}],"users":{"zalandos":{"password":"","options":["CREATEDB","NOLOGIN"]}},"dcs":{}}}`, + subtest: "Patroni default configuration", + pgParam: &acidv1.PostgresqlParam{PgVersion: "9.6"}, + patroni: &acidv1.Patroni{}, + opConfig: &config.Config{ + Auth: config.Auth{ + PamRoleName: "zalandos", + }, + }, + result: `{"postgresql":{"bin_dir":"/usr/lib/postgresql/9.6/bin"},"bootstrap":{"initdb":[{"auth-host":"md5"},{"auth-local":"trust"}],"users":{"zalandos":{"password":"","options":["CREATEDB","NOLOGIN"]}},"dcs":{}}}`, }, { subtest: "Patroni configured", @@ -99,21 +101,65 @@ func TestGenerateSpiloJSONConfiguration(t *testing.T) { SynchronousModeStrict: true, SynchronousNodeCount: 1, Slots: map[string]map[string]string{"permanent_logical_1": {"type": "logical", "database": "foo", "plugin": "pgoutput"}}, + FailsafeMode: util.True(), }, - role: "zalandos", - opConfig: config.Config{}, - result: `{"postgresql":{"bin_dir":"/usr/lib/postgresql/11/bin","pg_hba":["hostssl all all 0.0.0.0/0 md5","host all all 0.0.0.0/0 md5"]},"bootstrap":{"initdb":[{"auth-host":"md5"},{"auth-local":"trust"},"data-checksums",{"encoding":"UTF8"},{"locale":"en_US.UTF-8"}],"users":{"zalandos":{"password":"","options":["CREATEDB","NOLOGIN"]}},"dcs":{"ttl":30,"loop_wait":10,"retry_timeout":10,"maximum_lag_on_failover":33554432,"synchronous_mode":true,"synchronous_mode_strict":true,"synchronous_node_count":1,"slots":{"permanent_logical_1":{"database":"foo","plugin":"pgoutput","type":"logical"}}}}}`, + opConfig: &config.Config{ + Auth: config.Auth{ + PamRoleName: "zalandos", + }, + }, + result: `{"postgresql":{"bin_dir":"/usr/lib/postgresql/11/bin","pg_hba":["hostssl all all 0.0.0.0/0 md5","host all all 0.0.0.0/0 md5"]},"bootstrap":{"initdb":[{"auth-host":"md5"},{"auth-local":"trust"},"data-checksums",{"encoding":"UTF8"},{"locale":"en_US.UTF-8"}],"users":{"zalandos":{"password":"","options":["CREATEDB","NOLOGIN"]}},"dcs":{"ttl":30,"loop_wait":10,"retry_timeout":10,"maximum_lag_on_failover":33554432,"synchronous_mode":true,"synchronous_mode_strict":true,"synchronous_node_count":1,"slots":{"permanent_logical_1":{"database":"foo","plugin":"pgoutput","type":"logical"}},"failsafe_mode":true}}}`, + }, + { + subtest: "Patroni failsafe_mode configured globally", + pgParam: &acidv1.PostgresqlParam{PgVersion: "14"}, + patroni: &acidv1.Patroni{}, + opConfig: &config.Config{ + Auth: config.Auth{ + PamRoleName: "zalandos", + }, + EnablePatroniFailsafeMode: util.True(), + }, + result: `{"postgresql":{"bin_dir":"/usr/lib/postgresql/14/bin"},"bootstrap":{"initdb":[{"auth-host":"md5"},{"auth-local":"trust"}],"users":{"zalandos":{"password":"","options":["CREATEDB","NOLOGIN"]}},"dcs":{"failsafe_mode":true}}}`, + }, + { + subtest: "Patroni failsafe_mode configured globally, disabled for cluster", + pgParam: &acidv1.PostgresqlParam{PgVersion: "14"}, + patroni: &acidv1.Patroni{ + FailsafeMode: util.False(), + }, + opConfig: &config.Config{ + Auth: config.Auth{ + PamRoleName: "zalandos", + }, + EnablePatroniFailsafeMode: util.True(), + }, + result: `{"postgresql":{"bin_dir":"/usr/lib/postgresql/14/bin"},"bootstrap":{"initdb":[{"auth-host":"md5"},{"auth-local":"trust"}],"users":{"zalandos":{"password":"","options":["CREATEDB","NOLOGIN"]}},"dcs":{"failsafe_mode":false}}}`, + }, + { + subtest: "Patroni failsafe_mode disabled globally, configured for cluster", + pgParam: &acidv1.PostgresqlParam{PgVersion: "14"}, + patroni: &acidv1.Patroni{ + FailsafeMode: util.True(), + }, + opConfig: &config.Config{ + Auth: config.Auth{ + PamRoleName: "zalandos", + }, + EnablePatroniFailsafeMode: util.False(), + }, + result: `{"postgresql":{"bin_dir":"/usr/lib/postgresql/14/bin"},"bootstrap":{"initdb":[{"auth-host":"md5"},{"auth-local":"trust"}],"users":{"zalandos":{"password":"","options":["CREATEDB","NOLOGIN"]}},"dcs":{"failsafe_mode":true}}}`, }, } for _, tt := range tests { - cluster.OpConfig = tt.opConfig - result, err := generateSpiloJSONConfiguration(tt.pgParam, tt.patroni, tt.role, false, logger) + cluster.OpConfig = *tt.opConfig + result, err := generateSpiloJSONConfiguration(tt.pgParam, tt.patroni, tt.opConfig, logger) if err != nil { t.Errorf("Unexpected error: %v", err) } if tt.result != result { t.Errorf("%s %s: Spilo Config is %v, expected %v for role %#v and param %#v", - testName, tt.subtest, result, tt.result, tt.role, tt.pgParam) + testName, tt.subtest, result, tt.result, tt.opConfig.Auth.PamRoleName, tt.pgParam) } } } diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index ed1511311..76c9fd12a 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -564,6 +564,21 @@ func (c *Cluster) checkAndSetGlobalPostgreSQLConfiguration(pod *v1.Pod, effectiv configToSet["ttl"] = desiredPatroniConfig.TTL } + var desiredFailsafe *bool + if desiredPatroniConfig.FailsafeMode != nil { + desiredFailsafe = desiredPatroniConfig.FailsafeMode + } else if c.OpConfig.EnablePatroniFailsafeMode != nil { + desiredFailsafe = c.OpConfig.EnablePatroniFailsafeMode + } + + effectiveFailsafe := effectivePatroniConfig.FailsafeMode + + if desiredFailsafe != nil { + if effectiveFailsafe == nil || *desiredFailsafe != *effectiveFailsafe { + configToSet["failsafe_mode"] = *desiredFailsafe + } + } + // check if specified slots exist in config and if they differ slotsToSet := make(map[string]map[string]string) for slotName, desiredSlot := range desiredPatroniConfig.Slots { diff --git a/pkg/cluster/sync_test.go b/pkg/cluster/sync_test.go index 785a7cca2..4d50b791f 100644 --- a/pkg/cluster/sync_test.go +++ b/pkg/cluster/sync_test.go @@ -20,6 +20,7 @@ import ( acidv1 "github.com/zalando/postgres-operator/pkg/apis/acid.zalan.do/v1" fakeacidv1 "github.com/zalando/postgres-operator/pkg/generated/clientset/versioned/fake" "github.com/zalando/postgres-operator/pkg/spec" + "github.com/zalando/postgres-operator/pkg/util" "github.com/zalando/postgres-operator/pkg/util/config" "github.com/zalando/postgres-operator/pkg/util/k8sutil" "github.com/zalando/postgres-operator/pkg/util/patroni" @@ -147,20 +148,23 @@ func TestCheckAndSetGlobalPostgreSQLConfiguration(t *testing.T) { ctrl := gomock.NewController(t) defer ctrl.Finish() + defaultPgParameters := map[string]string{ + "log_min_duration_statement": "200", + "max_connections": "50", + } + defaultPatroniParameters := acidv1.Patroni{ + TTL: 20, + } + pg := acidv1.Postgresql{ ObjectMeta: metav1.ObjectMeta{ Name: clusterName, Namespace: namespace, }, Spec: acidv1.PostgresSpec{ - Patroni: acidv1.Patroni{ - TTL: 20, - }, + Patroni: defaultPatroniParameters, PostgresqlParam: acidv1.PostgresqlParam{ - Parameters: map[string]string{ - "log_min_duration_statement": "200", - "max_connections": "50", - }, + Parameters: defaultPgParameters, }, Volume: acidv1.Volume{ Size: "1Gi", @@ -222,9 +226,7 @@ func TestCheckAndSetGlobalPostgreSQLConfiguration(t *testing.T) { }, { subtest: "multiple Postgresql.Parameters differ - restart replica first", - patroni: acidv1.Patroni{ - TTL: 20, - }, + patroni: defaultPatroniParameters, pgParams: map[string]string{ "log_min_duration_statement": "500", // desired 200 "max_connections": "100", // desired 50 @@ -233,9 +235,7 @@ func TestCheckAndSetGlobalPostgreSQLConfiguration(t *testing.T) { }, { subtest: "desired max_connections bigger - restart replica first", - patroni: acidv1.Patroni{ - TTL: 20, - }, + patroni: defaultPatroniParameters, pgParams: map[string]string{ "log_min_duration_statement": "200", "max_connections": "30", // desired 50 @@ -244,9 +244,7 @@ func TestCheckAndSetGlobalPostgreSQLConfiguration(t *testing.T) { }, { subtest: "desired max_connections smaller - restart master first", - patroni: acidv1.Patroni{ - TTL: 20, - }, + patroni: defaultPatroniParameters, pgParams: map[string]string{ "log_min_duration_statement": "200", "max_connections": "100", // desired 50 @@ -265,6 +263,109 @@ func TestCheckAndSetGlobalPostgreSQLConfiguration(t *testing.T) { t.Errorf("%s - %s: wrong master restart strategy, got restart %v, expected restart %v", testName, tt.subtest, requirePrimaryRestart, tt.restartPrimary) } } + + testsFailsafe := []struct { + subtest string + operatorVal *bool + effectiveVal *bool + desiredVal bool + shouldBePatched bool + restartPrimary bool + }{ + { + subtest: "Not set in operator config, not set for pg cluster. Set to true in the pg config.", + operatorVal: nil, + effectiveVal: nil, + desiredVal: true, + shouldBePatched: true, + restartPrimary: false, + }, + { + subtest: "Not set in operator config, disabled for pg cluster. Set to true in the pg config.", + operatorVal: nil, + effectiveVal: util.False(), + desiredVal: true, + shouldBePatched: true, + restartPrimary: false, + }, + { + subtest: "Not set in operator config, not set for pg cluster. Set to false in the pg config.", + operatorVal: nil, + effectiveVal: nil, + desiredVal: false, + shouldBePatched: true, + restartPrimary: false, + }, + { + subtest: "Not set in operator config, enabled for pg cluster. Set to false in the pg config.", + operatorVal: nil, + effectiveVal: util.True(), + desiredVal: false, + shouldBePatched: true, + restartPrimary: false, + }, + { + subtest: "Enabled in operator config, not set for pg cluster. Set to false in the pg config.", + operatorVal: util.True(), + effectiveVal: nil, + desiredVal: false, + shouldBePatched: true, + restartPrimary: false, + }, + { + subtest: "Enabled in operator config, disabled for pg cluster. Set to true in the pg config.", + operatorVal: util.True(), + effectiveVal: util.False(), + desiredVal: true, + shouldBePatched: true, + restartPrimary: false, + }, + { + subtest: "Disabled in operator config, not set for pg cluster. Set to true in the pg config.", + operatorVal: util.False(), + effectiveVal: nil, + desiredVal: true, + shouldBePatched: true, + restartPrimary: false, + }, + { + subtest: "Disabled in operator config, enabled for pg cluster. Set to false in the pg config.", + operatorVal: util.False(), + effectiveVal: util.True(), + desiredVal: false, + shouldBePatched: true, + restartPrimary: false, + }, + { + subtest: "Disabled in operator config, enabled for pg cluster. Set to true in the pg config.", + operatorVal: util.False(), + effectiveVal: util.True(), + desiredVal: true, + shouldBePatched: false, // should not require patching + restartPrimary: true, + }, + } + + for _, tt := range testsFailsafe { + patroniConf := defaultPatroniParameters + + if tt.operatorVal != nil { + cluster.OpConfig.EnablePatroniFailsafeMode = tt.operatorVal + } + if tt.effectiveVal != nil { + patroniConf.FailsafeMode = tt.effectiveVal + } + cluster.Spec.Patroni.FailsafeMode = &tt.desiredVal + + configPatched, requirePrimaryRestart, err := cluster.checkAndSetGlobalPostgreSQLConfiguration(mockPod, patroniConf, cluster.Spec.Patroni, defaultPgParameters, cluster.Spec.Parameters) + assert.NoError(t, err) + if configPatched != tt.shouldBePatched { + t.Errorf("%s - %s: expected update went wrong", testName, tt.subtest) + } + if requirePrimaryRestart != tt.restartPrimary { + t.Errorf("%s - %s: wrong master restart strategy, got restart %v, expected restart %v", testName, tt.subtest, requirePrimaryRestart, tt.restartPrimary) + } + } } func TestUpdateSecret(t *testing.T) { diff --git a/pkg/controller/operator_config.go b/pkg/controller/operator_config.go index 57c3c7110..03afee358 100644 --- a/pkg/controller/operator_config.go +++ b/pkg/controller/operator_config.go @@ -216,6 +216,9 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur result.ScalyrCPULimit = fromCRD.Scalyr.ScalyrCPULimit result.ScalyrMemoryLimit = fromCRD.Scalyr.ScalyrMemoryLimit + // Patroni config + result.EnablePatroniFailsafeMode = util.CoalesceBool(fromCRD.Patroni.FailsafeMode, util.False()) + // Connection pooler. Looks like we can't use defaulting in CRD before 1.17, // so ensure default values here. result.ConnectionPooler.NumberOfInstances = util.CoalesceInt32( diff --git a/pkg/generated/clientset/versioned/fake/register.go b/pkg/generated/clientset/versioned/fake/register.go index 313eeacc2..a5d94de6b 100644 --- a/pkg/generated/clientset/versioned/fake/register.go +++ b/pkg/generated/clientset/versioned/fake/register.go @@ -45,14 +45,14 @@ var localSchemeBuilder = runtime.SchemeBuilder{ // AddToScheme adds all types of this clientset into the given scheme. This allows composition // of clientsets, like in: // -// import ( -// "k8s.io/client-go/kubernetes" -// clientsetscheme "k8s.io/client-go/kubernetes/scheme" -// aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme" -// ) +// import ( +// "k8s.io/client-go/kubernetes" +// clientsetscheme "k8s.io/client-go/kubernetes/scheme" +// aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme" +// ) // -// kclientset, _ := kubernetes.NewForConfig(c) -// _ = aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme) +// kclientset, _ := kubernetes.NewForConfig(c) +// _ = aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme) // // After this, RawExtensions in Kubernetes types will serialize kube-aggregator types // correctly. diff --git a/pkg/generated/clientset/versioned/scheme/register.go b/pkg/generated/clientset/versioned/scheme/register.go index 823909bcb..96ab81670 100644 --- a/pkg/generated/clientset/versioned/scheme/register.go +++ b/pkg/generated/clientset/versioned/scheme/register.go @@ -45,14 +45,14 @@ var localSchemeBuilder = runtime.SchemeBuilder{ // AddToScheme adds all types of this clientset into the given scheme. This allows composition // of clientsets, like in: // -// import ( -// "k8s.io/client-go/kubernetes" -// clientsetscheme "k8s.io/client-go/kubernetes/scheme" -// aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme" -// ) +// import ( +// "k8s.io/client-go/kubernetes" +// clientsetscheme "k8s.io/client-go/kubernetes/scheme" +// aggregatorclientsetscheme "k8s.io/kube-aggregator/pkg/client/clientset_generated/clientset/scheme" +// ) // -// kclientset, _ := kubernetes.NewForConfig(c) -// _ = aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme) +// kclientset, _ := kubernetes.NewForConfig(c) +// _ = aggregatorclientsetscheme.AddToScheme(clientsetscheme.Scheme) // // After this, RawExtensions in Kubernetes types will serialize kube-aggregator types // correctly. diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 8e51dd020..234486c24 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -234,6 +234,7 @@ type Config struct { TargetMajorVersion string `name:"target_major_version" default:"14"` PatroniAPICheckInterval time.Duration `name:"patroni_api_check_interval" default:"1s"` PatroniAPICheckTimeout time.Duration `name:"patroni_api_check_timeout" default:"5s"` + EnablePatroniFailsafeMode *bool `name:"enable_patroni_failsafe_mode" default:"false"` } // MustMarshal marshals the config or panics