chore(helm): Refactor loki health probes

[lindhe]

What this PR does / why we need it:

Some components used .Values.loki.livenessProbe and .Values.loki.startupProbe despite them being unset in values.yaml. This change adds those parameters.

Introducing these parameters with {} as the default value breaks the existing logic, since they previously had the value null (because they were unset).
But we want to have {} as the default value, not null, to indicate that the parameter is an object.
So therefore, this change also refactors the existing logic so it handles {} properly.

Which issue(s) this PR fixes:

Partially addresses #13678

Special notes for your reviewer:

Partially replaces #15359

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
• 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

[jkroepke]

Some components used .Values.loki.livenessProbe and .Values.loki.startupProbe despite them being unset in values.yaml. This change adds those parameters.

My local search for toYaml .Values.loki.readinessProbe returns 14 hits. Why it is only necessary to change 3 of them?

[lindhe]

I only did the ones that clearly caused errors with the values files from ci. But you are probably right – all of them should be updated.

EDIT: Or should they? 🤔 Clearly, all the "bad" uses should be improved, so a {} does not break things. But I'm not so sure that this PR should introduce new probes in all locations where a readinessProbe is used today.

I'll try and split it up into different commits so you can review them individually and choose which ones belong here.

[lindhe]

I've reworked the PR now. In my opinion, only 68b815e and maybe 91d8d1c belongs here, but I pushed 32e22d6 just in case you think that belongs here. If you agree with me, I'll remove that commit and just keep the ones you want.

[jkroepke]

Great PR! I personally like this pattern and should be used everywhere.

I just found one odd thing, it's optional here and maybe a separate PR worth.

Just comment what did you think.

[jkroepke]

Missing a Changelog entry and this PR feel more "feat(helm)" than "chore(helm)"

[lindhe]

Missing a Changelog entry

Thanks, I'll add that!

this PR feel more "feat(helm)" than "chore(helm)"

Indeed, that's why I think 32e22d6 does not belong here – because that is a new feature:

The more I think about it, the more strongly I feel that I should revert that commit. Otherwise, this PR is no longer "Refactor loki health probes" but has grown from it's original scope. Do you agree?

[jkroepke]

From my point of view, both is fine.

• revert the commit and remain the original scope
• leave the PR as is and change the scope.

We should not over engineer this. In best case, all your PRs are included in the same Loki Release and there is zero difference for the end-user.

[lindhe]

Thanks! Then I'll revert the commit! Just a minute!

[jkroepke]

LGTM