From 08089ed4b4783f3fa2203708c7479efd59f3c528 Mon Sep 17 00:00:00 2001 From: Felix Kunde Date: Thu, 14 Mar 2024 17:01:26 +0100 Subject: [PATCH] add option to prevent PVC removal on cluster deletion (#2579) * add option to prevent PVC removal on cluster deletion * Update docs/reference/operator_parameters.md Co-authored-by: Motte <37443982+dmotte@users.noreply.github.com> --- .../crds/operatorconfigurations.yaml | 3 +++ charts/postgres-operator/values.yaml | 2 ++ docs/reference/operator_parameters.md | 20 +++++++++++++++++++ e2e/tests/test_e2e.py | 4 +++- manifests/configmap.yaml | 1 + manifests/operatorconfiguration.crd.yaml | 3 +++ ...gresql-operator-default-configuration.yaml | 1 + pkg/apis/acid.zalan.do/v1/crds.go | 3 +++ .../v1/operator_configuration_type.go | 1 + .../acid.zalan.do/v1/zz_generated.deepcopy.go | 5 +++++ pkg/cluster/resources.go | 8 ++++++-- pkg/controller/operator_config.go | 1 + pkg/util/config/config.go | 1 + 13 files changed, 50 insertions(+), 3 deletions(-) diff --git a/charts/postgres-operator/crds/operatorconfigurations.yaml b/charts/postgres-operator/crds/operatorconfigurations.yaml index bfe17d07b..c75eef541 100644 --- a/charts/postgres-operator/crds/operatorconfigurations.yaml +++ b/charts/postgres-operator/crds/operatorconfigurations.yaml @@ -211,6 +211,9 @@ spec: enable_init_containers: type: boolean default: true + enable_persistent_volume_claim_deletion: + type: boolean + default: true enable_pod_antiaffinity: type: boolean default: false diff --git a/charts/postgres-operator/values.yaml b/charts/postgres-operator/values.yaml index a7bc7310c..477142b40 100644 --- a/charts/postgres-operator/values.yaml +++ b/charts/postgres-operator/values.yaml @@ -129,6 +129,8 @@ configKubernetes: enable_finalizers: false # enables initContainers to run actions before Spilo is started enable_init_containers: true + # toggles if operator should delete PVCs on cluster deletion + enable_persistent_volume_claim_deletion: true # toggles pod anti affinity on the Postgres pods enable_pod_antiaffinity: false # toggles PDB to set to MinAvailabe 0 or 1 diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index 341570a55..41214e730 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -346,6 +346,26 @@ configuration they are grouped under the `kubernetes` key. gone at this point. The default is `false`. +* **persistent_volume_claim_retention_policy** + The operator tries to protect volumes as much as possible. If somebody + accidentally deletes the statefulset or scales in the `numberOfInstances` the + Persistent Volume Claims and thus Persistent Volumes will be retained. + However, this can have some consequences when you scale out again at a much + later point, for example after the cluster's Postgres major version has been + upgraded, because the old volume runs the old Postgres version with stale data. + Even if the version has not changed the replication lag could be massive. In + this case a reinitialization of the re-added member would make sense. You can + also modify the [retention policy of PVCs](https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#persistentvolumeclaim-retention) in the operator configuration. + The behavior can be changed for two scenarios: `when_deleted` - default is + `"retain"` - or `when_scaled` - default is also `"retain"`. The other possible + option is `delete`. + +* **enable_persistent_volume_claim_deletion** + By default, the operator deletes PersistentVolumeClaims when removing the + Postgres cluster manifest, no matter if `persistent_volume_claim_retention_policy` + on the statefulset is set to `retain`. To keep PVCs set this option to `false`. + The default is `true`. + * **enable_pod_disruption_budget** PDB is enabled by default to protect the cluster from voluntarily disruptions and hence unwanted DB downtime. However, on some cloud providers it could be diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index e0854f1f7..7ffaba126 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -2048,7 +2048,8 @@ class EndToEndTestCase(unittest.TestCase): patch_delete_annotations = { "data": { "delete_annotation_date_key": "delete-date", - "delete_annotation_name_key": "delete-clustername" + "delete_annotation_name_key": "delete-clustername", + "enable_persistent_volume_claim_deletion": "false" } } k8s.update_config(patch_delete_annotations) @@ -2109,6 +2110,7 @@ class EndToEndTestCase(unittest.TestCase): self.eventuallyEqual(lambda: k8s.count_deployments_with_label(cluster_label), 0, "Deployments not deleted") self.eventuallyEqual(lambda: k8s.count_pdbs_with_label(cluster_label), 0, "Pod disruption budget not deleted") self.eventuallyEqual(lambda: k8s.count_secrets_with_label(cluster_label), 0, "Secrets not deleted") + self.eventuallyEqual(lambda: k8s.count_pvcs_with_label(cluster_label), 3, "PVCs were deleted although disabled in config") except timeout_decorator.TimeoutError: print('Operator log: {}'.format(k8s.get_operator_log())) diff --git a/manifests/configmap.yaml b/manifests/configmap.yaml index a4d91e63d..5d36a267e 100644 --- a/manifests/configmap.yaml +++ b/manifests/configmap.yaml @@ -49,6 +49,7 @@ data: enable_master_pooler_load_balancer: "false" enable_password_rotation: "false" enable_patroni_failsafe_mode: "false" + enable_persistent_volume_claim_deletion: "true" 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 51ffb24a4..7ef285e92 100644 --- a/manifests/operatorconfiguration.crd.yaml +++ b/manifests/operatorconfiguration.crd.yaml @@ -209,6 +209,9 @@ spec: enable_init_containers: type: boolean default: true + enable_persistent_volume_claim_deletion: + type: boolean + default: true enable_pod_antiaffinity: type: boolean default: false diff --git a/manifests/postgresql-operator-default-configuration.yaml b/manifests/postgresql-operator-default-configuration.yaml index aff79ef71..85a7dd23c 100644 --- a/manifests/postgresql-operator-default-configuration.yaml +++ b/manifests/postgresql-operator-default-configuration.yaml @@ -59,6 +59,7 @@ configuration: # enable_cross_namespace_secret: "false" enable_finalizers: false enable_init_containers: true + enable_persistent_volume_claim_deletion: true enable_pod_antiaffinity: false enable_pod_disruption_budget: true enable_readiness_probe: false diff --git a/pkg/apis/acid.zalan.do/v1/crds.go b/pkg/apis/acid.zalan.do/v1/crds.go index ae0a180ac..852993ea0 100644 --- a/pkg/apis/acid.zalan.do/v1/crds.go +++ b/pkg/apis/acid.zalan.do/v1/crds.go @@ -1320,6 +1320,9 @@ var OperatorConfigCRDResourceValidation = apiextv1.CustomResourceValidation{ "enable_init_containers": { Type: "boolean", }, + "enable_persistent_volume_claim_deletion": { + Type: "boolean", + }, "enable_pod_antiaffinity": { Type: "boolean", }, 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 fb099c746..42debc7f6 100644 --- a/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go +++ b/pkg/apis/acid.zalan.do/v1/operator_configuration_type.go @@ -102,6 +102,7 @@ type KubernetesMetaConfiguration struct { PodAntiAffinityTopologyKey string `json:"pod_antiaffinity_topology_key,omitempty"` PodManagementPolicy string `json:"pod_management_policy,omitempty"` PersistentVolumeClaimRetentionPolicy map[string]string `json:"persistent_volume_claim_retention_policy,omitempty"` + EnablePersistentVolumeClaimDeletion *bool `json:"enable_persistent_volume_claim_deletion,omitempty"` EnableReadinessProbe bool `json:"enable_readiness_probe,omitempty"` EnableCrossNamespaceSecret bool `json:"enable_cross_namespace_secret,omitempty"` EnableFinalizers *bool `json:"enable_finalizers,omitempty"` 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 6a1698d54..3a12387a1 100644 --- a/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go +++ b/pkg/apis/acid.zalan.do/v1/zz_generated.deepcopy.go @@ -272,6 +272,11 @@ func (in *KubernetesMetaConfiguration) DeepCopyInto(out *KubernetesMetaConfigura (*out)[key] = val } } + if in.EnablePersistentVolumeClaimDeletion != nil { + in, out := &in.EnablePersistentVolumeClaimDeletion, &out.EnablePersistentVolumeClaimDeletion + *out = new(bool) + **out = **in + } if in.EnableFinalizers != nil { in, out := &in.EnableFinalizers, &out.EnableFinalizers *out = new(bool) diff --git a/pkg/cluster/resources.go b/pkg/cluster/resources.go index 297559b53..1d4758c02 100644 --- a/pkg/cluster/resources.go +++ b/pkg/cluster/resources.go @@ -261,8 +261,12 @@ func (c *Cluster) deleteStatefulSet() error { return fmt.Errorf("could not delete pods: %v", err) } - if err := c.deletePersistentVolumeClaims(); err != nil { - return fmt.Errorf("could not delete PersistentVolumeClaims: %v", err) + if c.OpConfig.EnablePersistentVolumeClaimDeletion != nil && *c.OpConfig.EnablePersistentVolumeClaimDeletion { + if err := c.deletePersistentVolumeClaims(); err != nil { + return fmt.Errorf("could not delete PersistentVolumeClaims: %v", err) + } + } else { + c.logger.Info("not deleting PersistentVolumeClaims because disabled in configuration") } return nil diff --git a/pkg/controller/operator_config.go b/pkg/controller/operator_config.go index eaad11dff..4c0f0dfbe 100644 --- a/pkg/controller/operator_config.go +++ b/pkg/controller/operator_config.go @@ -122,6 +122,7 @@ func (c *Controller) importConfigurationFromCRD(fromCRD *acidv1.OperatorConfigur result.PodPriorityClassName = fromCRD.Kubernetes.PodPriorityClassName result.PodManagementPolicy = util.Coalesce(fromCRD.Kubernetes.PodManagementPolicy, "ordered_ready") result.PersistentVolumeClaimRetentionPolicy = fromCRD.Kubernetes.PersistentVolumeClaimRetentionPolicy + result.EnablePersistentVolumeClaimDeletion = util.CoalesceBool(fromCRD.Kubernetes.EnablePersistentVolumeClaimDeletion, util.True()) result.EnableReadinessProbe = fromCRD.Kubernetes.EnableReadinessProbe result.MasterPodMoveTimeout = util.CoalesceDuration(time.Duration(fromCRD.Kubernetes.MasterPodMoveTimeout), "10m") result.EnablePodAntiAffinity = fromCRD.Kubernetes.EnablePodAntiAffinity diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 4c6575218..5834cc92d 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -249,6 +249,7 @@ type Config struct { 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"` + EnablePersistentVolumeClaimDeletion *bool `name:"enable_persistent_volume_claim_deletion" default:"true"` PersistentVolumeClaimRetentionPolicy map[string]string `name:"persistent_volume_claim_retention_policy" default:"when_deleted:retain,when_scaled:retain"` }