Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add persistentVolumeClaimRetentionPolicy variable to values.yaml #965

Merged
merged 3 commits into from
Nov 13, 2023
Merged

Add persistentVolumeClaimRetentionPolicy variable to values.yaml #965

merged 3 commits into from
Nov 13, 2023

Conversation

alemuro
Copy link
Contributor

@alemuro alemuro commented Oct 19, 2023 •
edited
Loading

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: disabled server.enabled
 ✓ server/StatefulSet: disabled server.enabled random string
 ✓ server/StatefulSet: enabled server.enabled explicit true
 ✓ server/standalone-StatefulSet: default server.standalone.enabled
 ✓ server/standalone-StatefulSet: enable with server.standalone.enabled true
 ✓ server/standalone-StatefulSet: disable with global.enabled
 ✓ server/standalone-StatefulSet: disable with injector.externalVaultAddr
 ✓ server/standalone-StatefulSet: namespace
 ✓ server/standalone-StatefulSet: image defaults to server.image.repository:tag
 ✓ server/standalone-StatefulSet: image tag defaults to latest
 ✓ server/standalone-StatefulSet: default imagePullPolicy
 ✓ server/standalone-StatefulSet: Custom imagePullPolicy
 ✓ server/standalone-StatefulSet: Custom imagePullSecrets
 ✓ server/standalone-StatefulSet: Custom imagePullSecrets - string array
 ✓ server/standalone-StatefulSet: default imagePullSecrets
 ✓ server/standalone-StatefulSet: OnDelete updateStrategy
 ✓ server/standalone-StatefulSet: persistentVolumeClaimRetentionPolicy not set by default when kubernetes < 1.23
 ✓ server/standalone-StatefulSet: unset persistentVolumeClaimRetentionPolicy.whenDeleted when kubernetes < 1.23
 ✓ server/standalone-StatefulSet: unset persistentVolumeClaimRetentionPolicy.whenScaled when kubernetes < 1.23
 ✓ server/standalone-StatefulSet: persistentVolumeClaimRetentionPolicy not set by default when kubernetes >= 1.23
 ✓ server/standalone-StatefulSet: can set persistentVolumeClaimRetentionPolicy.whenDeleted when kubernetes >= 1.23
 ✓ server/standalone-StatefulSet: can set persistentVolumeClaimRetentionPolicy.whenScaled when kubernetes >= 1.23
 ✓ server/standalone-StatefulSet: default replicas
 ✓ server/standalone-StatefulSet: custom replicas
 ✓ server/standalone-StatefulSet: default resources
 ✓ server/standalone-StatefulSet: custom resources
 ✓ server/standalone-StatefulSet: server.extraVolumes adds extra volume
 ✓ server/standalone-StatefulSet: server.extraVolumes adds extra secret volume
 ✓ server/standalone-StatefulSet: can mount audit
 ✓ server/standalone-StatefulSet: server.volumes adds volume
 ✓ server/standalone-StatefulSet: server.volumeMounts adds volumeMount
 ✓ server/standalone-StatefulSet: default log level to empty
 ✓ server/standalone-StatefulSet: log level can be changed
 ✓ server/standalone-StatefulSet: default log format to empty
 ✓ server/standalone-StatefulSet: can set log format
 ✓ server/standalone-StatefulSet: set extraEnvironmentVars
 ✓ server/standalone-StatefulSet: storageClass on claim by default
 ✓ server/standalone-StatefulSet: can set storageClass
 ✓ server/standalone-StatefulSet: can disable storage
 ✓ server/standalone-StatefulSet: default audit storage mount path
 ✓ server/standalone-StatefulSet: can configure audit storage mount path
 ✓ server/standalone-StatefulSet: default data storage mount path
 ✓ server/standalone-StatefulSet: can configure data storage mount path
 ✓ server/standalone-StatefulSet: affinity is set by default
 ✓ server/standalone-StatefulSet: affinity can be set as string
 ✓ server/standalone-StatefulSet: affinity can be set as YAML
 ✓ server/standalone-StatefulSet: topologySpreadConstraints is null by default
 ✓ server/standalone-StatefulSet: topologySpreadConstraints can be set as YAML
 ✓ server/standalone-StatefulSet: tolerations not set by default
 ✓ server/standalone-StatefulSet: tolerations can be set as string
 ✓ server/standalone-StatefulSet: tolerations can be set as YAML
 ✓ server/standalone-StatefulSet: nodeSelector is not set by default
 ✓ server/standalone-StatefulSet: specified nodeSelector as string
 ✓ server/standalone-StatefulSet: nodeSelector can be set as YAML
 ✓ server/standalone-StatefulSet: adds extra init containers
 ✓ server/standalone-StatefulSet: add two extra init containers
 ✓ server/standalone-StatefulSet: adds extra containers
 ✓ server/standalone-StatefulSet: add two extra containers
 ✓ server/standalone-StatefulSet: no extra containers added
 ✓ server/standalone-StatefulSet: shareProcessNamespace disabled by default
 ✓ server/standalone-StatefulSet: shareProcessNamespace enabled
 ✓ server/standalone-StatefulSet: specify extraLabels
 ✓ server/standalone-StatefulSet: default statefulSet.annotations
 ✓ server/standalone-StatefulSet: specify statefulSet.annotations yaml
 ✓ server/standalone-StatefulSet: specify statefulSet.annotations yaml string
 ✓ server/standalone-StatefulSet: uid default
 ✓ server/standalone-StatefulSet: uid configurable
 ✓ server/standalone-StatefulSet: gid default
 ✓ server/standalone-StatefulSet: gid configurable
 ✓ server/standalone-StatefulSet: fsgroup default
 ✓ server/standalone-StatefulSet: fsgroup configurable
 ✓ server/standalone-StatefulSet: readinessProbe default
 ✓ server/standalone-StatefulSet: readinessProbe configurable
 ✓ server/standalone-StatefulSet: readiness failureThreshold default
 ✓ server/standalone-StatefulSet: readiness failureThreshold configurable
 ✓ server/standalone-StatefulSet: readiness initialDelaySeconds default
 ✓ server/standalone-StatefulSet: readiness initialDelaySeconds configurable
 ✓ server/standalone-StatefulSet: readiness periodSeconds default
 ✓ server/standalone-StatefulSet: readiness periodSeconds configurable
 ✓ server/standalone-StatefulSet: readiness successThreshold default
 ✓ server/standalone-StatefulSet: readiness successThreshold configurable
 ✓ server/standalone-StatefulSet: readiness timeoutSeconds default
 ✓ server/standalone-StatefulSet: readiness timeoutSeconds configurable
 ✓ server/standalone-StatefulSet: livenessProbe default
 ✓ server/standalone-StatefulSet: livenessProbe configurable
 ✓ server/standalone-StatefulSet: liveness failureThreshold default
 ✓ server/standalone-StatefulSet: liveness failureThreshold configurable
 ✓ server/standalone-StatefulSet: liveness initialDelaySeconds default
 ✓ server/standalone-StatefulSet: liveness initialDelaySeconds configurable
 ✓ server/standalone-StatefulSet: liveness periodSeconds default
 ✓ server/standalone-StatefulSet: liveness periodSeconds configurable
 ✓ server/standalone-StatefulSet: liveness successThreshold default
 ✓ server/standalone-StatefulSet: liveness successThreshold configurable
 ✓ server/standalone-StatefulSet: liveness timeoutSeconds default
 ✓ server/standalone-StatefulSet: liveness timeoutSeconds configurable
 ✓ server/standalone-StatefulSet: add extraArgs
 ✓ server/standalone-StatefulSet: terminationGracePeriodSeconds default
 ✓ server/standalone-StatefulSet: terminationGracePeriodSeconds 30
 ✓ server/standalone-StatefulSet: preStop sleep duration default
 ✓ server/standalone-StatefulSet: preStop sleep duration 10
 ✓ server/standalone-StatefulSet: vault port name is http, when tlsDisable is true
 ✓ server/standalone-StatefulSet: vault replication port name is http-rep, when tlsDisable is true
 ✓ server/standalone-StatefulSet: vault port name is https, when tlsDisable is false
 ✓ server/standalone-StatefulSet: vault replication port name is https-rep, when tlsDisable is false
 ✓ server/standalone-StatefulSet: generic annotations string
 ✓ server/standalone-StatefulSet: auditStorage volumeClaim annotations string
 ✓ server/standalone-StatefulSet: dataStorage volumeClaim annotations string
 ✓ server/standalone-StatefulSet: auditStorage volumeClaim annotations yaml
 ✓ server/standalone-StatefulSet: dataStorage volumeClaim annotations yaml
 ✓ server/ha-standby-Service: generic annotations yaml
 ✓ server/standalone-StatefulSet: priorityClassName not set by default
 ✓ server/standalone-StatefulSet: priorityClassName can be set
 ✓ server/standalone-StatefulSet: postStart disabled by default
 ✓ server/standalone-StatefulSet: postStart can be set
 ✓ server/standalone-StatefulSet: OpenShift - runAsUser disabled
 ✓ server/standalone-StatefulSet: OpenShift - runAsGroup disabled
 ✓ server/standalone-StatefulSet: serviceAccount.name is set
 ✓ server/standalone-StatefulSet: serviceAccount.name is not 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: blank secretName does not set env var
 ✓ server/standalone-StatefulSet: default statefulSet.securityContext.pod
 ✓ server/standalone-StatefulSet: default statefulSet.securityContext.container
 ✓ server/standalone-StatefulSet: specify statefulSet.securityContext.pod yaml
 ✓ server/standalone-StatefulSet: specify statefulSet.securityContext.container yaml
 ✓ server/standalone-StatefulSet: specify statefulSet.securityContext.pod yaml string
 ✓ server/standalone-StatefulSet: specify statefulSet.securityContext.container yaml string
 ✓ server/StatefulSet: server.hostNetwork not set
 ✓ server/StatefulSet: server.hostNetwork is set
 ✓ server/StatefulSet: server.hostAliases not set
 ✓ server/StatefulSet: server.hostAliases is set
 ✓ server/standalone-StatefulSet: adds extra ports
 ✓ server/StatefulSet: server.readinessProbe.port is set
 ✓ server/StatefulSet: server.livenessProbe.port is set

135 tests, 0 failures

closes #964

@alemuro alemuro requested a review from a team as a code owner October 19, 2023 16:36
tvoran
tvoran reviewed Nov 8, 2023
View reviewed changes
Copy link
Member

@tvoran tvoran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for being so thorough with the tests here! Could you also add an entry in values.schema.json for the new parameter?

values.yaml Outdated
# persistentVolumeClaimRetentionPolicy:
# whenDeleted: Retain
# whenScaled: Retain
persistentVolumeClaimRetentionPolicy: null
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks like this setting affects all PVCs on the statefulset, which would affect audit PVCs as well. I wonder if we should we move it up one level, i.e. server.persistentVolumeClaimRetentionPolicy?

I also think we could just default this to {}:

Suggested change
persistentVolumeClaimRetentionPolicy: null
persistentVolumeClaimRetentionPolicy: {}

Copy link
Contributor Author

@alemuro alemuro Nov 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello @tvoran ! Thanks for your feedback.

I've addressed all the changes requested 😄:

  • Change default from null to empty object {}.
  • Move key from server.dataStorage.persistentVolumeClaimRetentionPolicy to server.persistentVolumeClaimRetentionPolicy
  • Updated tests to match changes.
  • Add new variable into the values.schema.json (sorry for that, I missed it when I raised this PR!)

Thanks!

@alemuro alemuro force-pushed the 964-add-statefulset-pvc-retention branch from 98987a4 to c782cbb Compare November 8, 2023 08:33
@alemuro
Add persistentVolumeClaimRetentionPolicy variable to values.yaml
9fe21c6 
This variable is used to set the persistentVolumeClaimRetentionPolicy
value in the server-statefulset.yaml template, which is used to
configure the retention policy for the PVCs used by the server
statefulset.
@alemuro alemuro force-pushed the 964-add-statefulset-pvc-retention branch from c782cbb to 9fe21c6 Compare November 8, 2023 08:36
tvoran
tvoran approved these changes Nov 13, 2023
View reviewed changes
Copy link
Member

@tvoran tvoran left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this looks great!

@tvoran tvoran merged commit ad64f92 into hashicorp:main Nov 13, 2023
1 check passed
@tvoran tvoran added this to the v0.27.0 milestone Nov 14, 2023
@tvoran tvoran mentioned this pull request Nov 16, 2023
Merged
@alemuro alemuro deleted the 964-add-statefulset-pvc-retention branch December 21, 2023 07:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@tvoran tvoran tvoran approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v0.27.0
Development

Successfully merging this pull request may close these issues.

Support StatefulSet PVC retention feature
2 participants
@alemuro @tvoran