Add _tplvalues.tpl and define commonLabels, podLabels, commonAnnotations and podAnnotations attributes

[ecthelion77]

• Add _tplvalues.tpl
• Update values file with default commonLabels, commonAnnotations, podLabels and podAnnotations
• Update all template to render properly labels and annotations

This change allows to use this helm chart with argocd for example and use multistring labels and annotations with the standardized way from bitnami

[floreks]

Add dependency to bitnami common helm chart

Could you provide a little more background behind this?
Why do we need this and why should we rely on this particular external dependency?

[ecthelion77]

Today, it's not possible to provide common information to all resources deployed by the chart. It's really usefull to be able to do that for monitoring for example.

While I integrate those new information (commonLabels, podLabels, commonAnnotations and podAnnotations) seems to be really interesting to use the bitnami common chart as subchart to avoid adding complexity on the concerned chart.

If we integrate basically those new information on templates with :

 {{- toYaml .Values.commonLabels | nindent 4 }}

it's always impossible to use a multistring to set information :

commonLabels:
  label1: value1
  label2: value2

is working, but

commonLabels: |
  "
  label1: value1
  label2: value2
  "

not working

with the templating from bitnami

{{- include "common.tplvalues.render" ( dict "value" .Values.commonAnnotations "context" $ ) | nindent 4 }}

both possibilites are working.

And the second solution is really usefull to simplify deployment with argocd

[maciaszczykm]

I don't think that this is a good place for external dependencies too.

[floreks]

We are usually quite hesitant to add dependencies of third-party software that is not part of the k8s core (especially when it comes to deployment). We are only providing a helm chart because it is basically the main alternative to deploying apps in K8S. That's why we are not providing i.e. any example reverse-proxy configurations. There are just too many options and we'd rather force users to configure everything on their own without influencing their selection of third-party software.

[desaintmartin]

I was hesitating before answering. I personally use common but if I'm not wrong, you only use https://github.com/bitnami/charts/blob/master/bitnami/common/templates/_tplvalues.tpl which could be included into this chart instead of adding the whole dependency. What do you think?

(Thanks a lot for the files reformatting)

[ecthelion77]

Yes currently I simply use _tplvalues and we can imagine simply add this template to the chart. In my sense, I consider it's interesting to add the dependency to the common to avoid a rewrite of existing template and to let the possibility for future to use others powerfull objects of the dependency.
But if you consider that it's too much and you validate solution based on the integration of the template, I can modify my PR.

[desaintmartin]

It would be easy to add this dependency if there are other significant needs later, from now on I think the template integration directly in the chart would be simpler to manage.

[linux-foundation-easycla]

• ✅ Olivier Gintrand (923945c)

• ❌ The commit (402ed0e) is missing the User's ID, preventing the EasyCLA check. Consult GitHub Help to resolve.For further assistance with EasyCLA, please submit a support request ticket.

[k8s-ci-robot]

Thanks for your pull request. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please follow instructions at https://git.k8s.io/community/CLA.md#the-contributor-license-agreement to sign the CLA.

It may take a couple minutes for the CLA signature to be fully registered; after that, please reply here with a new comment and we'll verify. Thanks.

---

• If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
• If you signed the CLA as a corporation, please sign in with your organization's credentials at https://identity.linuxfoundation.org/projects/cncf to be authorized.
• If you have done the above and are still having issues with the CLA being reported as unsigned, please log a ticket with the Linux Foundation Helpdesk: https://support.linuxfoundation.org/
• Should you encounter any issues with the Linux Foundation Helpdesk, send a message to the backup e-mail support address at: login-issues@jira.linuxfoundation.org

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

[linux-foundation-easycla]

• ✅ Olivier Gintrand (923945c, 3454bd1)

• ❌ The commit (402ed0e) is missing the User's ID, preventing the EasyCLA check. Consult GitHub Help to resolve.For further assistance with EasyCLA, please submit a support request ticket.

[ecthelion77]

@desaintmartin change are done, can you review

[desaintmartin]

hehe this could have been indented as well, but honestly thanks for the work so far in streamlining good usage of newlines.

[ecthelion77]

with pleasure :)

[desaintmartin]

Could a member trigger the CI, please?

[desaintmartin]

Oh, by the way, @ecthelion77, could you update PR name and description to refer to actual content?

[ecthelion77]

Oh, by the way, @ecthelion77, could you update PR name and description to refer to actual content?

done :)

[ecthelion77]

up

[desaintmartin]

@floreks @maciaszczykm Could one of you trigger CI?

[codecov]

Codecov Report

Merging #6638 (27cb4b0) into master (870b069) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #6638   +/-   ##
=======================================
  Coverage   42.08%   42.08%           
=======================================
  Files          44       44           
  Lines        1226     1226           
  Branches      161      161           
=======================================
  Hits          516      516           
  Misses        710      710           

[ecthelion77]

@desaintmartin can you review again please, the chart version was bumped during waiting for worflow approval so i bumped again to version 5.0.5

[ecthelion77]

up

[desaintmartin]

/lgtm

[k8s-ci-robot]

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: desaintmartin, ecthelion77

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

• aio/deploy/helm-chart/kubernetes-dashboard/OWNERS [desaintmartin]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment