[bitnami/etcd] keep snapshot history

[dacodas]

Description of the change

Keep a timestamped etcd snapshot history.

Benefits

For organizations that would like some history of changes, this might be helpful. One could argue for rotating these via another Kubernetes cronjob acting on the same persistent volume. Having this as part of the same chart with just a few lines is more helpful for the end user.

Possible drawbacks

Depending on your setting for history length, more disk usage. With the default history length and with the use of hardlinks, there is teensy impact by default.

Tried to limit any blast from this find | rm -f by minimizing the depth of find. Perhaps another filter for file names would be helpful---this carries the assumption that the PV is meant primarily for backups.

I believe hard links depend on the filesystem. FAT or ReFS users, for example, will not be able to use this according to Wikipedia. I assert the vast majority of users would not experience this issue.

The use of the $(( 1 + $VARIABLE )) may not work with certain shs. I see that these scripts have bash in the she-bang, so I don't foresee this to be an issue.

Applicable issues

None

Additional information

Checklist

• Chart version bumped in Chart.yaml according to semver.
• Variables are documented in the README.md
• Title of the PR starts with chart name (e.g. [bitnami/chart])
• If the chart contains a values-production.yaml apart from values.yaml, ensure that you implement the changes in both files

[rafariossaa]

Hi,
When testing it, I got an error.
Could you please take a look at it ?

$ helm install --name etcd -f values.yaml  .
Error: render error in "etcd/templates/scripts-configmap.yaml": template: etcd/templates/scripts-configmap.yaml:13:36: executing "etcd/templates/scripts-configmap.yaml" at <.Values.cronjob.snap...>: can't evaluate field snapshotHistoryLimit in type interface {}

[nvtkaszpir]

Variable in template is incorrect, because it is nested under disasterRecovery, below is code change suggestion (not tested, though):

Suggested change

	{{- $snapshotHistoryLimit := .Values.cronjob.snapshotHistoryLimit }}
	{{- $snapshotHistoryLimit := .Values.disasterRecovery.cronjob.snapshotHistoryLimit }}

[dacodas]

My apologies for this; I was a bit cheeky and made this pull request without testing. I have now tested, and will be submitting a different pull requests that both implements this features and addresses #1921

[dacodas]

This PR includes the commits from #1958

[dacodas]

Alright, the related PR #1958 was merged this morning, so the details of this PR here are a bit different. I am requesting review from @miguelaeh, and re-requesting from @rafariossaa if you would be so kind.

[rafariossaa]

LGTM

[miguelaeh]

Thank you very much for the clarifications!!
LGTM!