Helm Chart: Support StatefulSet PVC retention

[alemuro]

Add new variable on the Helm chart to define the PVC retention policy for the StatefulSet, as requested in #3101

Changes proposed in this PR:

• Add new persistentVolumeClaimRetentionPolicy to control the PVC retention policy when the statefulset is deleted or the number of replicas is decreased.
• Only supported since kubernetes 1.23

How I've tested this PR:

$ bats test/unit/server-statefulset.bats
server-statefulset.bats
 ✓ server/StatefulSet: enabled by default
 ✓ server/StatefulSet: enable with global.enabled false
 ✓ server/StatefulSet: disable with server.enabled
 ✓ server/StatefulSet: disable with global.enabled
 ✓ server/StatefulSet: errors if bootstrapExpect < replicas
 ✓ server/StatefulSet: federation and admin partitions cannot be enabled together
 ✓ server/StatefulSet: image defaults to global.image
 ✓ server/StatefulSet: image can be overridden with server.image
 ✓ server/StatefulSet: resources defined by default
 ✓ server/StatefulSet: resources can be overridden
 ✓ server/StatefulSet: resources can be overridden with string
 ✓ server/StatefulSet: no updateStrategy when not updating
 ✓ server/StatefulSet: updateStrategy during update
 ✓ server/StatefulSet: no truncation for namespace <= 58 chars
 ✓ server/StatefulSet: truncation for namespace > 58 chars
 ✓ server/StatefulSet: no storageClass on claim by default
 ✓ server/StatefulSet: can set storageClass
 ✓ server/StatefulSet: persistentVolumeClaimRetentionPolicy not set by default when kubernetes < 1.23
 ✓ server/StatefulSet: unset persistentVolumeClaimRetentionPolicy.whenDeleted when kubernetes < 1.23
 ✓ server/StatefulSet: unset persistentVolumeClaimRetentionPolicy.whenScaled when kubernetes < 1.23
 ✓ server/StatefulSet: persistentVolumeClaimRetentionPolicy not set by default when kubernetes >= 1.23
 ✓ server/StatefulSet: can set persistentVolumeClaimRetentionPolicy.whenDeleted when kubernetes >= 1.23
 ✓ server/StatefulSet: can set persistentVolumeClaimRetentionPolicy.whenScaled when kubernetes >= 1.23
 ✓ server/StatefulSet: consul-server-cert uses default cert when serverCert.secretName not set
 ✓ server/StatefulSet: consul-server-cert uses serverCert.secretName when serverCert (and caCert) are set
 ✓ server/StatefulSet: when server.serverCert.secretName!=null and global.tls.caCert.secretName=null, fail
 ✓ server/StatefulSet: server gossip and RPC ports are not exposed by default
 ✓ server/StatefulSet: server gossip and RPC ports can be exposed
 ✓ server/StatefulSet: server.ports.serflan.port is set to 8301 by default
 ✓ server/StatefulSet: server.ports.serflan.port can be customized
 ✓ server/StatefulSet: has extra-config volume
 ✓ server/StatefulSet: adds extra volume
 ✓ server/StatefulSet: adds extra secret volume
 ✓ server/StatefulSet: adds loadable volume
 ✓ server/StatefulSet: adds extra secret volume with items
 ✓ server/StatefulSet: affinity not set with server.affinity=null
 ✓ server/StatefulSet: affinity set by default
 ✓ server/StatefulSet: nodeSelector is not set by default
 ✓ server/StatefulSet: specified nodeSelector
 ✓ server/StatefulSet: priorityClassName is not set by default
 ✓ server/StatefulSet: specified priorityClassName
 ✓ server/StatefulSet: no extra labels defined by default
 ✓ server/StatefulSet: extra labels can be set
 ✓ server/StatefulSet: multiple extra labels can be set
 ✓ server/StatefulSet: extra global labels can be set
 ✓ server/StatefulSet: multiple extra global labels can be set
 ✓ server/StatefulSet: no annotations defined by default
 ✓ server/StatefulSet: annotations can be set
 ✓ server/StatefulSet: when global.metrics.enableAgentMetrics=true, adds prometheus scrape=true annotations
 ✓ server/StatefulSet: when global.metrics.enableAgentMetrics=true, adds prometheus port=8500 annotation
 ✓ server/StatefulSet: when global.metrics.enableAgentMetrics=true, adds prometheus path=/v1/agent/metrics annotation
 ✓ server/StatefulSet: when global.metrics.enableAgentMetrics=true, adds prometheus scheme=http annotation
 ✓ server/StatefulSet: when global.metrics.enableAgentMetrics=true and global.tls.enabled=true, adds prometheus port=8501 annotation
 ✓ server/StatefulSet: when global.metrics.enableAgentMetrics=true and global.tls.enabled=true, adds prometheus scheme=https annotation
 ✓ server/StatefulSet: adds config-checksum annotation when extraConfig is blank
 ✓ server/StatefulSet: adds config-checksum annotation when extraConfig is provided
 ✓ server/StatefulSet: adds config-checksum annotation when config is updated
 ✓ server/StatefulSet: tolerations not set by default
 ✓ server/StatefulSet: tolerations can be set
 ✓ server/StatefulSet: topologySpreadConstraints not set by default
 ✓ server/StatefulSet: topologySpreadConstraints can be set
 ✓ server/StatefulSet: setting server.disableFsGroupSecurityContext fails
 ✓ server/StatefulSet: securityContext is not set when global.openshift.enabled=true
 ✓ server/StatefulSet: sets default security context settings
 ✓ server/StatefulSet: can overwrite security context settings
 ✓ server/StatefulSet: Can set container level securityContexts
 ✓ server/StatefulSet: Can set container level securityContexts when global.openshift.enabled=true
 ✓ server/StatefulSet: restricted container securityContexts are set when global.openshift.enabled=true
 ✓ server/StatefulSet: restricted container securityContexts are set by default
 ✓ server/StatefulSet: gossip encryption disabled in server StatefulSet by default
 ✓ server/StatefulSet: gossip encryption autogeneration properly sets secretName and secretKey
 ✓ server/StatefulSet: gossip encryption key is passed via the -encrypt flag
 ✓ server/StatefulSet: fail if global.gossipEncyption.gossipEncryption.secretName is set but not global.gossipEncyption.secretKey
 ✓ server/StatefulSet: fail if global.gossipEncyption.gossipEncryption.secretKey is set but not global.gossipEncyption.secretName
 ✓ server/StatefulSet: gossip environment variable present in server StatefulSet when all config is provided
 ✓ server/StatefulSet: encrypt CLI option not present in server StatefulSet when encryption disabled
 ✓ server/StatefulSet: encrypt CLI option present in server StatefulSet when all config is provided
 ✓ server/StatefulSet: custom environment variables
 ✓ server/StatefulSet: CA volume present when TLS is enabled
 ✓ server/StatefulSet: server cert volume present when TLS is enabled
 ✓ server/StatefulSet: CA volume mounted when TLS is enabled
 ✓ server/StatefulSet: server certificate volume mounted when TLS is enabled
 ✓ server/StatefulSet: port 8501 is not exposed when TLS is disabled
 ✓ server/StatefulSet: port 8501 is exposed when TLS is enabled
 ✓ server/StatefulSet: port 8500 is still exposed when httpsOnly is not enabled
 ✓ server/StatefulSet: port 8500 is not exposed when httpsOnly is enabled
 ✓ server/StatefulSet: readiness checks are over HTTP when TLS is disabled
 ✓ server/StatefulSet: readiness checks are over HTTPS when TLS is enabled
 ✓ server/StatefulSet: sets Consul environment variables when global.tls.enabled
 ✓ server/StatefulSet: sets Consul environment variables when global.tls.enabled and global.secretsBackend.vault.enabled
 ✓ server/StatefulSet: can overwrite CA secret with the provided one
 ✓ server/StatefulSet: fails when federation.enabled=true and tls.enabled=false
 ✓ server/StatefulSet: when global.acls.bootstrapToken.secretKey!=null and global.acls.bootstrapToken.secretName=null, fail
 ✓ server/StatefulSet: when global.acls.bootstrapToken.secretName!=null and global.acls.bootstrapToken.secretKey=null, fail
 ✓ server/StatefulSet: acl bootstrap token config is not set by default
 ✓ server/StatefulSet: acl bootstrap token config is set when acls.bootstrapToken.secretKey and secretName are set
 ✓ server/StatefulSet: acl replication token config is not set by default
 ✓ server/StatefulSet: acl replication token config is not set when acls.replicationToken.secretName is set but secretKey is not
 ✓ server/StatefulSet: acl replication token config is not set when acls.replicationToken.secretKey is set but secretName is not
 ✓ server/StatefulSet: acl replication token config is set when acls.replicationToken.secretKey and secretName are set
 ✓ server/StatefulSet: adds volume for license secret when enterprise license secret name and key are provided
 ✓ server/StatefulSet: adds volume mount for license secret when enterprise license secret name and key are provided
 ✓ server/StatefulSet: adds env var for license path when enterprise license secret name and key are provided
 ✓ server/StatefulSet: when global.enterpriseLicense.secretKey!=null and global.enterpriseLicense.secretName=null, fail
 ✓ server/StatefulSet: when global.enterpriseLicense.secretName!=null and global.enterpriseLicense.secretKey=null, fail
 ✓ server/StatefulSet: adds extra container
 ✓ server/StatefulSet: adds two extra containers
 ✓ server/StatefulSet: no extra containers added by default
 ✓ server/StatefulSet: fail when vault is enabled but the consulServerRole is not provided
 ✓ server/StatefulSet: fail when vault, tls are enabled but no caCert provided
 ✓ server/StatefulSet: vault annotations not set by default
 ✓ server/StatefulSet: vault annotations added when vault is enabled
 ✓ server/StatefulSet: vault gossip annotations are correct when enabled
 ✓ server/StatefulSet: vault no GOSSIP_KEY env variable and command defines GOSSIP_KEY
 ✓ server/StatefulSet: vault CA is not configured by default
 ✓ server/StatefulSet: vault namespace annotations is set when global.secretsBackend.vault.vaultNamespace is set
 ✓ server/StatefulSet: correct vault namespace annotations is set when global.secretsBackend.vault.vaultNamespace is set and agentAnnotations are also set without vaultNamespace annotation
 ✓ server/StatefulSet: correct vault namespace annotations is set when global.secretsBackend.vault.vaultNamespace is set and agentAnnotations are also set with vaultNamespace annotation
 ✓ server/StatefulSet: vault CA is not configured when secretName is set but secretKey is not
 ✓ server/StatefulSet: vault CA is not configured when secretKey is set but secretName is not
 ✓ server/StatefulSet: vault CA is configured when both secretName and secretKey are set
 ✓ server/StatefulSet: vault tls annotations are set when tls is enabled and command modified correctly
 ✓ server/StatefulSet: tls related volumes not attached when tls is enabled on vault
 ✓ server/StatefulSet: vault - can set additional alt_names on server cert when tls is enabled
 ✓ server/StatefulSet: vault - can set additional ip_sans on server cert when tls is enabled
 ✓ server/StatefulSet: vault enterprise license annotations are correct when enabled
 ✓ server/StatefulSet: vault CONSUL_LICENSE_PATH is set to /vault/secrets/enterpriselicense.txt
 ✓ server/StatefulSet: vault does not add volume for license secret
 ✓ server/StatefulSet: vault does not add volume mount for license secret
 ✓ server/StatefulSet: no vault agent annotations defined by default
 ✓ server/StatefulSet: vault agent annotations can be set
 ✓ server/StatefulSet: vault bootstrap token is configured when secret provided
 ✓ server/StatefulSet: vault replication token is configured when secret provided and createReplicationToken is false
 ✓ server/StatefulSet: cloud config is not set in command when global.cloud.enabled is not set
 ✓ server/StatefulSet: does not create HCP_RESOURCE_ID, HCP_CLIENT_ID, HCP_CLIENT_SECRET, HCP_AUTH_URL, HCP_SCADA_ADDRESS, and HCP_API_HOSTNAME envvars in consul container when global.cloud.enabled is not set
 ✓ server/StatefulSet: cloud config is set in command when global.cloud.enabled and global.cloud.resourceId are set
 ✓ server/StatefulSet: creates HCP_RESOURCE_ID, HCP_CLIENT_ID, HCP_CLIENT_SECRET envvars in consul container when global.cloud.enabled is true
 ✓ server/StatefulSet: creates HCP_AUTH_URL, HCP_SCADA_ADDRESS, and HCP_API_HOSTNAME envvars in consul container when global.cloud.enabled is true and those cloud values are specified
 ✓ server/StatefulSet: cloud config is set in command global.cloud.enabled is not set
 ✓ server/StatefulSet: fails when global.cloud.enabled is true and global.cloud.clientId.secretName is not set but global.cloud.clientSecret.secretName and global.cloud.resourceId.secretName is set
 ✓ server/StatefulSet: fails when global.cloud.enabled is true and global.cloud.clientSecret.secretName is not set but global.cloud.clientId.secretName and global.cloud.resourceId.secretName is set
 ✓ server/StatefulSet: fails when global.cloud.enabled is true and global.cloud.resourceId.secretName is not set but global.cloud.clientId.secretName and global.cloud.clientSecret.secretName is set
 ✓ server/StatefulSet: fails when global.cloud.resourceId.secretName is set but global.cloud.resourceId.secretKey is not set.
 ✓ server/StatefulSet: fails when global.cloud.authURL.secretName is set but global.cloud.authURL.secretKey is not set.
 ✓ server/StatefulSet: fails when global.cloud.authURL.secretKey is set but global.cloud.authURL.secretName is not set.
 ✓ server/StatefulSet: fails when global.cloud.apiHost.secretName is set but global.cloud.apiHost.secretKey is not set.
 ✓ server/StatefulSet: fails when global.cloud.apiHost.secretKey is set but global.cloud.apiHost.secretName is not set.
 ✓ server/StatefulSet: fails when global.cloud.scadaAddress.secretName is set but global.cloud.scadaAddress.secretKey is not set.
 ✓ server/StatefulSet: fails when global.cloud.scadaAddress.secretKey is set but global.cloud.scadaAddress.secretName is not set.
 ✓ server/StatefulSet: snapshot-agent: snapshot agent container not added by default
 ✓ server/StatefulSet: snapshot-agent: snapshot agent container added with server.snapshotAGent.enabled=true
 ✓ server/StatefulSet: snapshot-agent: when server.snapshotAgent.configSecret.secretKey!=null and server.snapshotAgent.configSecret.secretName=null, fail
 ✓ server/StatefulSet: snapshot-agent: when server.snapshotAgent.configSecret.secretName!=null and server.snapshotAgent.configSecret.secretKey=null, fail
 ✓ server/StatefulSet: snapshot-agent: adds volume for snapshot agent config secret when secret is configured
 ✓ server/StatefulSet: snapshot-agent: adds volume mount to snapshot container for snapshot agent config secret when secret is configured
 ✓ server/StatefulSet: snapshot-agent: set config-dir argument on snapshot agent command to volume mount
 ✓ server/StatefulSet: snapshot-agent: does not configure snapshot agent login config secret when acls are disabled
 ✓ server/StatefulSet: snapshot-agent: adds volume for snapshot agent login config secret when acls are enabled
 ✓ server/StatefulSet: snapshot-agent: adds volume mount to snapshot container for snapshot agent login config secret when acls are enabled
 ✓ server/StatefulSet: snapshot-agent: set config-file argument on snapshot agent command to login config when acls are enabled
 ✓ server/StatefulSet: snapshot-agent: uses default consul addr when TLS is disabled
 ✓ server/StatefulSet: snapshot-agent: sets TLS env vars when global.tls.enabled
 ✓ server/StatefulSet: snapshot-agent: populates container volumeMounts when global.tls.enabled is true
 ✓ server/StatefulSet: snapshot-agent: default resources
 ✓ server/StatefulSet: snapshot-agent: can set resources
 ✓ server/StatefulSet: snapshot-agent: if caCert is set command is modified correctly
 ✓ server/StatefulSet: snapshot-agent: if caCert is set extra-ssl-certs volumeMount is added
 ✓ server/StatefulSet: snapshot-agent: if caCert is set SSL_CERT_DIR env var is set
 ✓ server/StatefulSet: trustedCAs: if trustedCAs is set command is modified correctly
 ✓ server/StatefulSet: trustedCAs: if tustedCAs multiple are set
 ✓ server/StatefulSet: trustedCAs: if trustedCAs is set /trusted-cas volumeMount is added
 ✓ server/StatefulSet: trustedCAs: if trustedCAs is set SSL_CERT_DIR env var is set
 ✓ server/StatefulSet: snapshot-agent: adds volume mount for license secret when enterprise license secret name and key are provided
 ✓ server/StatefulSet: snapshot-agent: adds env var for license path when enterprise license secret name and key are provided
 ✓ server/StatefulSet: snapshot-agent: does not add license secret volume mount if manageSystemACLs are enabled
 ✓ server/StatefulSet: snapshot-agent: does not add license env if manageSystemACLs are enabled
 ✓ server/StatefulSet: snapshot-agent: vault CONSUL_LICENSE_PATH is set to /vault/secrets/enterpriselicense.txt
 ✓ server/StatefulSet: snapshot-agent: vault does not add volume mount for license secret
 ✓ server/StatefulSet: snapshot-agent: vault snapshot agent config annotations are correct when enabled
 ✓ server/StatefulSet: snapshot-agent: vault does not add volume for snapshot agent config secret
 ✓ server/StatefulSet: snapshot-agent: vault does not add volume mount for snapshot agent config secret
 ✓ server/StatefulSet: snapshot-agent: vault sets config-file argument on snapshot agent command to config downloaded by vault agent injector
 ✓ server/StatefulSet: snapshot-agent: interval defaults to 1h
 ✓ server/StatefulSet: snapshot-agent: interval can be set
 ✓ server/StatefulSet: experiments=["resource-apis"] is not set in command when global.experiments is empty
 ✓ server/StatefulSet: experiments=["resource-apis"] is set in command when global.experiments contains "resource-apis"

186 tests, 0 failures

How I expect reviewers to test this PR:

Checklist:

• Tests added
• CHANGELOG entry added

closes #3101

[david-yu]

@alemuro Thanks for the contribution! What would be the default behavior when a persistentVolumeClaimRetentionPolicy is not specified? Would it not be deleted as is today? We can also try to take a look. Also what version of Consul K8s are you using?

[alemuro]

Hello @david-yu !

What would be the default behavior when a persistentVolumeClaimRetentionPolicy is not specified?

On this implementation, the persistentVolumeClaimRetentionPolicy setting has a null value by default, and there is a condition that set this setting on the StatefulSet if some value is provided. If the user does not set this value, the functionality works as it is today.

Currently, Kubernetes uses Remain by default for both parameters (whenDeleted and whenScaled).

https://kubernetes.io/docs/concepts/workloads/controllers/statefulset/#persistentvolumeclaim-retention

There is more information about this change on KEP-1847.

Would it not be deleted as is today?

No, because the PVC is managed by the StatefulSet, and StatefulSets do not remove the PVC/PV they create (there is more information on the ADR above).

There are some cases (for example for test environments) where it is not required to maintain those PVCs. We are currently removing these PVCs manually, but we would like to take benefit from this new feature (was released on 1.23) to avoid executing manual cleanup actions on the cluster.

Also what version of Consul K8s are you using?

We are currently using the latest stable version of the chart (v1.2.2) and default Consul version provided by that chart (v1.16.2).

Thanks!

[zalimeni]

Thanks for the contribution, @alemuro ! I've opened #3180 with your change (just updated changelog filename) to run the full CI suite and hopefully get this merged soon.

I also went ahead and hand-tested the change against Helm 3.6 (our min. supported version) just to verify semverCompare behaves as expected; looks good:

[nix-shell:~/workspace/consul-k8s/charts/consul]$ helm version
version.BuildInfo{Version:"v3.6.1", GitCommit:"", GitTreeState:"", GoVersion:"go1.16.5"}

[nix-shell:~/workspace/consul-k8s/charts/consul]$ helm template -s templates/server-statefulset.yaml --kube-version "1.23" --set 'server.persistentVolumeClaimRetentionPolicy.whenDeleted=Delete' . | yq -r '.spec.persistentVolumeClaimRetentionPolicy.whenDeleted'
Delete

[nix-shell:~/workspace/consul-k8s/charts/consul]$ helm template -s templates/server-statefulset.yaml --kube-version "1.23-beta" --set 'server.persistentVolumeClaimRetentionPolicy.whenDeleted=Delete' . | yq -r '.spec.persistentVolumeClaimRetentionPolicy.whenDeleted'
Delete

[nix-shell:~/workspace/consul-k8s/charts/consul]$ helm template -s templates/server-statefulset.yaml --kube-version "1.22" --set 'server.persistentVolumeClaimRetentionPolicy.whenDeleted=Delete' . | yq -r '.spec.persistentVolumeClaimRetentionPolicy.whenDeleted'
null

@david-yu given this could be useful to earlier consul-k8s versions including Aleix's, I'll plan to backport this through 1.1.x (which targets k8s 1.23+) unless you have other thoughts.

[david-yu]

Up to 1.1.x makes sense to me. Thanks @zalimeni

[alemuro]

Thanks guys! Then I think it's worth to close this PR and follow this conversation on #3180

[david-yu]

Thanks @alemuro you should be mentioned as a contributor on the PR we put up. I'll go ahead and close, we still need to figure out how to get acceptance tests to run on PRs from contributors but wanted to thank you for your contribution!