[bitnami/redis] Enables Redis to utilize external-dns

[jtslear]

Description of the change

• By way of using external-dns we can assign a DNS entry for each Pod
• Doing so allows anyone who's got the ability to talk to the IP's for which Redis utilize, external to the cluster, to become members of the Redis cluster
• This is helpful during Redis migrations where backing up and restoring are destructive, but instead, we can enable both Redis in Kubernetes and Redis on VM's to talk to each other, perform application configuration changes, and turn down the VM's.
  • Theoretically vice versa as well
• Note that this REQUIRES the person leveraging this option to have external-dns in a working fashion prior to this being usable.

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/<name_of_the_chart>])

[github-actions]

Great PR! Please pay attention to the following items before merging:

Files matching bitnami/*/values.yaml:

• Is the PR adding a new container? Please reviewer, add it to the models (internal process)
• Is the PR adding a new parameter? Please, ensure it’s documented in the README.md

This is an automatically generated QA checklist based on modified files

[alemorcuq]

Thanks for the PR, @jtslear. I've left some comments, but since I'm not very familiar with the technology, I'd like another colleague to review this as well. I'll ping him on Monday.

[alemorcuq]

{{ printf "%s.%s" (include "common.names.fullname" .) .Values.externalDNS.suffix }} is used quite a bit on this PR, I think it's worth to add a helper to include it.

[jtslear]

Done!

[alemorcuq]

I'm not sure this is a good way to name the parameters, as users could assume that this also deploys a installation of the bitnami/external-dns chart.

[jtslear]

I would agree. I've renamed it to useExternalDNS. Though I'm not thrilled about that name either. Open to further suggestions.

[jtslear]

@alemorcuq I could use some guidance on the test, generate-chart-readme. I've added the values in place, but the job claims I did not and also I added one that does not exist. I'm not sure what I need to do to fix this. I created the values in the README manually, alongside an example configuration to assist persons whom may want to leverage this configuration option.

[WarheadsSE]

Suggested change

	{{- include "common.tplvalues.render" ( dict "value" .Values.commonAnnotations "context" $ ) | nindent 4 }}
	{{- end }}
	{{- if .Values.useExternalDNS.enabled }}
	{{ .Values.useExternalDNS.annotationKey }}hostname: {{ include "redis.externalDNS.suffix" . }}
	{{- range $key, $val := .Values.useExternalDNS.additionalAnnotations }}
	{{ $.Values.useExternalDNS.annotationKey }}{{ $key }}: {{ $val | quote }}
	{{- end }}
	{{- include "common.tplvalues.render" ( dict "value" .Values.commonAnnotations "context" $ ) | nindent 4 }}
	{{- end }}
	{{- if .Values.useExternalDNS.enabled }}
	{{ .Values.useExternalDNS.annotationKey }}hostname: {{ include "redis.externalDNS.suffix" . }}
	{{- range $key, $val := .Values.useExternalDNS.additionalAnnotations }}
	{{ $.Values.useExternalDNS.annotationKey }}{{ $key }}: {{ $val | quote }}
	{{- end }}

We can notch the indentation of the commonAnnoations render. Ideally, we could do the same for the additionalAnnotations , however, the spaces currently matter within the range loop because of how this is composed within.

What do you think of making this into a single template for all of these annotations, say redis.externalDNS.annotations, and instead simply gating this here?

[jtslear]

@WarheadsSE thanks for the suggestion, I proceeded to do exactly this in ba304f9

[alemorcuq]

@alemorcuq I could use some guidance on the test, generate-chart-readme. I've added the values in place, but the job claims I did not and also I added one that does not exist. I'm not sure what I need to do to fix this. I created the values in the README manually, alongside an example configuration to assist persons whom may want to leverage this configuration option.

It is missing the @param metadata in the values.yaml. The README table can be generated from that metadata. More on this tool here: https://github.com/bitnami/charts/blob/master/CONTRIBUTING.md#documentation-requirements

@juan131, could you take a look at this PR?

[jtslear]

@alemorcuq Readme is fixed, thank you much! This is now ready for review.

[juan131]

Thanks for your PR @jtslear ! Please check my comments when you have a chance.

[alemorcuq]

Sorry for the late reply. Let's try to get this merged, I'm rerunning the jobs.

[jtslear]

@alemorcuq I suspect that #8662 is preventing the tests from being successful. The behavior is slightly different, so I'm not 100% confident: Reference log output: https://github.com/bitnami/charts/runs/4842365585?check_suite_focus=true#step:6:1081

[alemorcuq]

A PR has been merged as a result of the task you mentioned, so let's check again.