[fluent-bit] optional podVersionLabel feature flag to include app.kubernetes.io/version applied to all pods

[jhulick]

What this PR does / why we need it:

• It adds common labels to metadata labels in the pod spec. The main reasoning is to persist the app.kubernetes.io/version and app.kubernetes.io/name labels to the pod for standardization. Per docs, (https://kubernetes.io/docs/concepts/overview/working-with-objects/common-labels/#labels) "In order to take full advantage of using these labels, they should be applied on every resource object.".

• Improves traceability throughout the application. Ex. via Istio/Kiali dashboard.

[stevehipwell]

Thanks for the PR @jhulick but as was the case for Metrics Server this isn't a desirable change for the chart. If you make this change optional and off by default I think it would be OK; my preference would be to use a value like verbosePodLabels: false.

[jhulick]

@stevehipwell, I was hesitant to create this PR for that reason, but since fluent-bit is data plane rather than control plane I thought it would be considered acceptable in this case, unless there are other reasons? Is it primarily due to telemetry latency? But I'll update per your proposed suggested.

[stevehipwell]

@jhulick I don't think the labels are a good idea, but even if they were we'd still be looking to follow the "principal of least astonishment"; so making them opt in is the only real way of introducing this change.

[stevehipwell]

@jhulick looking at this again with fresh eyes am I right in summarising your requirement as only needing the app.kubernetes.io/version label adding to pods so they can be part of an Istio mesh?

If we ignore the question of "why"; would the best implementation for this not be to have a podVersionLabel: false value which if set to true just adds the singular label? The concept of verbose pod labels is fuzzy and could be co-opted for other purposes, with unintended consequences.

[jhulick]

@stevehipwell, yes, only the app.kubernetes.io/version label is needed in this case since the app label is already supplied via selectorLabels. Istio recommends adding app and version labels to pods to attach this information to telemetry. Kiali relies on correctness of these labels for several features...

"...would the best implementation for this not be to have a podVersionLabel: false value which if set to true just adds the singular label?" Yes, I think this is a better approach. I'll update the PR.

[stevehipwell]

@jhulick I've added some comments and it looks like you need to rebase this PR locally.

[stevehipwell]

Can this be removed?

[stevehipwell]

This would make more sense being below the standard labels and implemented with the following pattern.

      {{- if and .Values.podVersionLabel .Chart.AppVersion }}
        app.kubernetes.io/version: {{ .Chart.AppVersion | quote }}
      {{- end }}

[stevehipwell]

Same as above.