feat: Add persistence option to memcached on helm chart

[AndreZiviani]

What this PR does / why we need it:
Add an option to configure persistence on memcached, very useful with extstore option.
More info on Grafana Blog post, based mostly on loki-distributed helm chart

Checklist

• Reviewed the CONTRIBUTING.md guide (required)
• Documentation added
• Tests updated
• Title matches the required conventional commits format, see here
  • Note that Promtail is considered to be feature complete, and future development for logs collection will be in Grafana Alloy. As such, feat PRs are unlikely to be accepted unless a case can be made for the feature actually being a bug fix to existing behavior.
• Changes that require user attention or interaction to upgrade are documented in docs/sources/setup/upgrade/_index.md
• For Helm chart changes bump the Helm chart version in production/helm/loki/Chart.yaml and update production/helm/loki/CHANGELOG.md and production/helm/loki/README.md. Example PR
• If the change is deprecating or removing a configuration option, update the deprecated-config.yaml and deleted-config.yaml files respectively in the tools/deprecated-config-checker directory. Example PR

[AndreZiviani]

not sure why helm-ci / Test Helm Chart (pull_request) failed, seems unrelated to these changes

[AndreZiviani]

@JStickler sorry to tag you directly but can I get a review here please?

[JStickler]

@AndreZiviani I'm the technical writer on the Loki team, not a developer. So I do not review code, just docs PRs.

[AndreZiviani]

my bad, can you tag the correct person to review this please?

[AndreZiviani]

@vlad-diachenko @DylanGuedes sorry to tag you directly but could you take a look here please? thanks

[chaudum]

How does this behave when there are neither .extraVolumeMounts and .persistence.enabled is false? I would assume it renders the key volumeMounts: but without list items. I assume this would cause invalid YAML.
In that case, it should either not render volumeMounts at all, or volumeMounts: [].

[AndreZiviani]

I would assume it renders the key volumeMounts: but without list items

that is correct

I assume this would cause invalid YAML.

that is not an issue, for exemple env is empty by default but I've added another check just in case

[chaudum]

Hm, interesting, I would expect that to be invalid. Nevertheless, thanks for the additional check.

[chaudum]

One other question I still have is:

Do we want to set the --extended CLI arguments on the memcached container that are required for using the extstore feature, if persistence is enabled?

I see that they could be set using the extraExtendedOptions, but maybe it would be nice if you don't have to remember the the options.

Wdyt?

[AndreZiviani]

Do we want to set the --extended CLI arguments on the memcached container that are required for using the extstore feature, if persistence is enabled?

I see that they could be set using the extraExtendedOptions, but maybe it would be nice if you don't have to remember the the options.

yeah... I thought about that, loki-distributed does something like that but that was an issue for me because that paremeter makes memcached use the disk for persistency which means that my performance was bound to disk speed and it is not possible to remove/disable it so I had to declare the volume outside of the helm chart. I used the persistency as a second tier (like swap) which makes the hot data fast but could store less frequently accessed data on disk, more details on the blog post.
I guess I could that and the other -o ext_path=/data/file parameter, which actually enables the storage backend, as values in order to allow people to override them, let me see what I can do

[AndreZiviani]

I tried doing in a more flexible way but it became too complex, I believe how I did now is enough

I also rebased the branch to fix the conflicts

[chaudum]

lgtm