Support metricbeat deployment labels

[v1r7u]

Describe the feature:
Would be great, if labels property would add labels to metricbeat Deployment the same as it does for DaemonSet

Describe a specific use case for the feature:
I use labels to define Kubernetes network-policies. I try to add a custom label to metricbeat pods. It perfectly works for DaemonSet, but not for Deployment

[v1r7u]

I would be happy to implement it, but I need input from maintainers.

If maintainers agree to include the feature, should I use existing labels property and reuse it in both Deployment and DaemonSet or rewrite labeling the same way as annotations are done (separate annotations properties for each). At the moment, I tend to use the same labels property, as it's backward compatible with previous versions and no-one asked opposite :)

Also, I'm not sure to which release version it could be included. I think, 7.x and master both are good candidates

[jmlrt]

Hi @v1r7u,
Thanks for submitting this request.

A PR would be really great for that.

should I use existing labels property and reuse it in both Deployment and DaemonSet or rewrite labeling the same way as annotations are done (separate annotations properties for each).

IMO it would make sense to have different labels values as we have for annotations. In your use case for network policies. I think it could make sense for some people to have different network policies for the DaemonSet (which is usually querying only the host metrics) and Deployment which is used to query external metrics (K8S API for example).

Of course this should be backward compatible with .values.labels used for DaemonSet and Deployment when no .values.daemonset.labels or .values.deployment.labels values exist.

WDYT?

[v1r7u]

@jmlrt I agree that having two labels properties is better.

Maybe then we have to mark general labels property as deprecated? I still going to respect its value in the exact yaml template, but only if a more specific one is absent. For example:

...
daemonset:
  labels:
     {{ DS_LABELS }}
...
deployment:
  labels:
    {{ DEPLOY_LABELS }}
...
labels:
{{ LABELS }}
...

If we agree, that root-level labels property will be marked as deprecated, then Deployment object is not going to use {{ LABELS }} at all, and clause 2 is used only for backward compatibility of existing charts:

1. if {{ DS_LABELS }} is not empty - DaemonSet object uses {{ DS_LABELS }};
2. if {{ DS_LABELS }} is empty, but {{ LABELS }} is not empty - DaemonSet uses {{ LABELS }};
3. if {{ DEPLOY_LABELS }} is not empty - Deployment object uses {{ DEPLOY_LABELS }};
4. if {{ DEPLOY_LABELS }} is empty - nothing happens :)

[jmlrt]

@jmlrt I agree that having two labels properties is better.

Maybe then we have to mark general labels property as deprecated? I still going to respect its value in the exact yaml template, but only if a more specific one is absent. For example:

...
daemonset:
  labels:
     {{ DS_LABELS }}
...
deployment:
  labels:
    {{ DEPLOY_LABELS }}
...
labels:
{{ LABELS }}
...

If we agree, that root-level labels property will be marked as deprecated, then Deployment object is not going to use {{ LABELS }} at all, and clause 2 is used only for backward compatibility of existing charts:

1. if `{{ DS_LABELS }}` is not empty - _DaemonSet_ object uses `{{ DS_LABELS }}`;

2. if `{{ DS_LABELS }}` is empty, but `{{ LABELS }}` is not empty - _DaemonSet_ uses `{{ LABELS }}`;

3. if `{{ DEPLOY_LABELS }}` is not empty - _Deployment_ object uses `{{ DEPLOY_LABELS }}`;

4. if `{{ DEPLOY_LABELS }}` is empty - nothing happens :)

LGTM