From 63fbab20affc1abb6d4f561b89e4c038b535c9fa Mon Sep 17 00:00:00 2001 From: Sergey Dudoladov Date: Tue, 31 Mar 2020 14:27:58 +0200 Subject: [PATCH] respond to review --- charts/postgres-operator/crds/operatorconfigurations.yaml | 2 +- charts/postgres-operator/values-crd.yaml | 2 +- charts/postgres-operator/values.yaml | 2 +- docs/reference/operator_parameters.md | 2 +- e2e/tests/test_e2e.py | 4 ++-- manifests/configmap.yaml | 2 +- manifests/operatorconfiguration.crd.yaml | 2 +- manifests/postgresql-operator-default-configuration.yaml | 2 +- pkg/cluster/cluster.go | 2 +- pkg/cluster/sync.go | 2 +- pkg/util/config/config.go | 2 +- 11 files changed, 12 insertions(+), 12 deletions(-) diff --git a/charts/postgres-operator/crds/operatorconfigurations.yaml b/charts/postgres-operator/crds/operatorconfigurations.yaml index c6ab0514d..17e1cea01 100644 --- a/charts/postgres-operator/crds/operatorconfigurations.yaml +++ b/charts/postgres-operator/crds/operatorconfigurations.yaml @@ -64,7 +64,7 @@ spec: type: boolean enable_shm_volume: type: boolean - should_delete_unused_pvc: + enable_unused_pvc_deletion: type: boolean etcd_host: type: string diff --git a/charts/postgres-operator/values-crd.yaml b/charts/postgres-operator/values-crd.yaml index 4cf538b2c..45b5c59fa 100644 --- a/charts/postgres-operator/values-crd.yaml +++ b/charts/postgres-operator/values-crd.yaml @@ -22,7 +22,7 @@ configGeneral: # start any new database pod without limitations on shm memory enable_shm_volume: true # delete PVCs of shutdown pods - should_delete_unused_pvc: false + enable_unused_pvc_deletion: false # etcd connection string for Patroni. Empty uses K8s-native DCS. etcd_host: "" # Spilo docker image diff --git a/charts/postgres-operator/values.yaml b/charts/postgres-operator/values.yaml index b3f26d646..000a36b69 100644 --- a/charts/postgres-operator/values.yaml +++ b/charts/postgres-operator/values.yaml @@ -22,7 +22,7 @@ configGeneral: # start any new database pod without limitations on shm memory enable_shm_volume: "true" # delete PVCs of shutdown pods - should_delete_unused_pvc: "false" + enable_unused_pvc_deletion: "false" # etcd connection string for Patroni. Empty uses K8s-native DCS. etcd_host: "" # Spilo docker image diff --git a/docs/reference/operator_parameters.md b/docs/reference/operator_parameters.md index 1a0646af3..645b49aa4 100644 --- a/docs/reference/operator_parameters.md +++ b/docs/reference/operator_parameters.md @@ -131,7 +131,7 @@ Those are top-level keys, containing both leaf keys and groups. container, change the [operator deployment manually](../../manifests/postgres-operator.yaml#L20). The default is `false`. -* **should_delete_unused_pvc** +* **enable_unused_pvc_deletion** Tells the operator to delete persistent volume claims of no longer running pods. That removes respective persistent volumes because operator configures them with the 'Delete' reclaim policy. Note operator deletes unused PVCs for clusters created both before and after this option is turned on. Deletion is not guaranteed: When it fails, operator retries at next Sync() event. The default is `false`. diff --git a/e2e/tests/test_e2e.py b/e2e/tests/test_e2e.py index 34858f2a4..272021c3c 100644 --- a/e2e/tests/test_e2e.py +++ b/e2e/tests/test_e2e.py @@ -504,7 +504,7 @@ class EndToEndTestCase(unittest.TestCase): # enable pvc deletion patch = { "data": { - "should_delete_unused_pvc": "true" + "enable_unused_pvc_deletion": "true" } } k8s.update_config(patch) @@ -542,7 +542,7 @@ class EndToEndTestCase(unittest.TestCase): # clean up patch = { "data": { - "should_delete_unused_pvc": "false" + "enable_unused_pvc_deletion": "false" } } k8s.update_config(patch) diff --git a/manifests/configmap.yaml b/manifests/configmap.yaml index ee6d44dbc..b566a7a5a 100644 --- a/manifests/configmap.yaml +++ b/manifests/configmap.yaml @@ -98,6 +98,6 @@ data: # teams_api_url: http://fake-teams-api.default.svc.cluster.local # toleration: "" # wal_s3_bucket: "" - # should_delete_unused_pvc: "false" + # enable_unused_pvc_deletion: "false" watched_namespace: "*" # listen to all namespaces workers: "4" diff --git a/manifests/operatorconfiguration.crd.yaml b/manifests/operatorconfiguration.crd.yaml index a605b6d04..81a5d63b0 100644 --- a/manifests/operatorconfiguration.crd.yaml +++ b/manifests/operatorconfiguration.crd.yaml @@ -40,7 +40,7 @@ spec: type: boolean enable_shm_volume: type: boolean - should_delete_unused_pvc: + enable_unused_pvc_deletion: type: boolean etcd_host: type: string diff --git a/manifests/postgresql-operator-default-configuration.yaml b/manifests/postgresql-operator-default-configuration.yaml index bba8405db..ce079c832 100644 --- a/manifests/postgresql-operator-default-configuration.yaml +++ b/manifests/postgresql-operator-default-configuration.yaml @@ -7,7 +7,7 @@ configuration: etcd_host: "" docker_image: registry.opensource.zalan.do/acid/spilo-12:1.6-p2 # enable_shm_volume: true - # should_delete_unused_pvc: false + # enable_unused_pvc_deletion: false max_instances: -1 min_instances: -1 resync_period: 30m diff --git a/pkg/cluster/cluster.go b/pkg/cluster/cluster.go index f655735b3..def6ae3ad 100644 --- a/pkg/cluster/cluster.go +++ b/pkg/cluster/cluster.go @@ -679,7 +679,7 @@ func (c *Cluster) Update(oldSpec, newSpec *acidv1.Postgresql) error { } }() - if c.OpConfig.ShouldDeleteUnusedPVC && oldSpec.Spec.NumberOfInstances > newSpec.Spec.NumberOfInstances { + if c.OpConfig.EnableUnusedPVCDeletion && oldSpec.Spec.NumberOfInstances > newSpec.Spec.NumberOfInstances { c.logger.Debug("deleting pvc of shut down pods") for i := oldSpec.Spec.NumberOfInstances - 1; i >= newSpec.Spec.NumberOfInstances; i-- { diff --git a/pkg/cluster/sync.go b/pkg/cluster/sync.go index 08c027f22..5f6a1794a 100644 --- a/pkg/cluster/sync.go +++ b/pkg/cluster/sync.go @@ -113,7 +113,7 @@ func (c *Cluster) Sync(newSpec *acidv1.Postgresql) error { // remove unused PVCs in case deleting them during scale down failed; see Update() // the last pvc stays until the cluster is explicitly deleted as opposed to being scaled down to 0 pods - if c.OpConfig.ShouldDeleteUnusedPVC && c.getNumberOfInstances(&c.Spec) > 0 { + if c.OpConfig.EnableUnusedPVCDeletion && c.getNumberOfInstances(&c.Spec) > 0 { // XXX that also deletes PVC of pods shut down before this change is deployed for i := c.getNumberOfInstances(&c.Spec); ; i++ { diff --git a/pkg/util/config/config.go b/pkg/util/config/config.go index 1c304e0b8..d853b0ab7 100644 --- a/pkg/util/config/config.go +++ b/pkg/util/config/config.go @@ -153,7 +153,7 @@ type Config struct { ProtectedRoles []string `name:"protected_role_names" default:"admin"` PostgresSuperuserTeams []string `name:"postgres_superuser_teams" default:""` SetMemoryRequestToLimit bool `name:"set_memory_request_to_limit" default:"false"` - ShouldDeleteUnusedPVC bool `name:"should_delete_unused_pvc" default:"false"` + EnableUnusedPVCDeletion bool `name:"enable_unused_pvc_deletion" default:"false"` } // MustMarshal marshals the config or panics