Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Optionally add app.kubernetes.io/managed-by label to all resources #14638

Open
lindhe opened this issue Jul 21, 2023 · 14 comments · May be fixed by #14945 or #17505
Open

Optionally add app.kubernetes.io/managed-by label to all resources #14638

lindhe opened this issue Jul 21, 2023 · 14 comments · May be fixed by #14945 or #17505
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@lindhe
Copy link
Contributor

lindhe commented Jul 21, 2023

Summary

I would like to automatically add the label app.kubernetes.io/managed-by: argocd to all resources deployed by Argo CD.

Motivation

This would make it easier to manually track all instances of resources that are managed by Argo CD and separate them from the resources that are not managed by Argo CD. An example for this would be if I perform some change to Argo and check that all pods are still running (e.g., kubectl get pods -A -l app.kubernetes.io/managed-by=argocd) or for other monitoring systems such as queries in Grafana Loki et al.

The app.kubernetes.io/managed-by label is one of the recommended labels for Kubernetes, so that's also something that strengthens the case for setting this label, in my opinion.

Proposal

I would like this to be an opt-in feature that lets users add this label. Ideally, it would be a general function that can add any label to all resources, or all resources in a certain project even!

This bears some resemblance to the current spec.syncPolicy.managedNamespaceMetadata option in the Application object.

@lindhe lindhe added the enhancement New feature or request label Jul 21, 2023
@percy78
Copy link

percy78 commented Jul 24, 2023

Just add a label to either the statefulset or a deployment to make it persistent.

kubectl label [sts or deployment] --all app.kubernetes.io/managed-by=argocd -n NAMESPACE

For pods already running invoke a label against these by issue this:

kubectl label pods --all app.kubernetes.io/managed-by=argocd -n NAMESPACE

@lindhe
Copy link
Contributor Author

lindhe commented Jul 24, 2023

Thanks. How do I put that into code so Argo does it for me?

@percy78
Copy link

percy78 commented Jul 24, 2023

if it's a helm chart then amend this stanza occuring for every component:

"# Pod Labels"
podLabels: {}

Essentially for your deployment it would like this:

"# Pod Labels"

podLabels:
    app.kubernetes.io/managed-by: argocd

@lindhe
Copy link
Contributor Author

lindhe commented Jul 24, 2023

So then I have to modify the manifest for every object I want to deploy. That's kind of the point of my feature request; it would be much nicer if Argo CD could do that work for me. :)

@mikebryant
Copy link
Contributor

To state the use case another way:
As a cluster-admin, I want all of the resources managed by my ArgoCD installation to have a correct app.kubernetes.io/managed-by label. I can't rely on all of my individual users setting up their helm charts correctly for this

@lindhe
Copy link
Contributor Author

lindhe commented Jul 24, 2023

Yes, exactly what I mean!

@blakepettersson
Copy link
Member

What about argocd.argoproj.io/instance (assuming you're using the default label resource tracking)?

@lindhe
Copy link
Contributor Author

lindhe commented Jul 25, 2023

Good question.

  1. Argo does not set that label for all resources (at least not CRDs). See Managing Argo CD with Argo CD inevitably causes diff from Kustomization template #14665

  2. The label has different values for each app, e.g. the app foo will have argocd.argoproj.io/instance: foo while app bar has argocd.argoproj.io/instance: bar. What I am looking for is to be able to filter on all resources created by Argo CD, no matter the app name.

  3. Kubernetes recommends both those labels to be set on every resource object.

  4. A strength of setting multiple different labels, like the recommended ones, for all objects is that it increases the cardinality of the data. What this means is that it becomes possible to make very precise filters while not restricting the application developers to adhere to convoluted naming schemes.

    For example: say I have one app foo that is deployed in namespace dev by someone running Helm and another app foo that is deployed in namespace prod by Argo CD. In this fairly reasonable scenario, it could very well be that both deployments would have the argocd.argoproj.io/instance label set to foo (because that is, at least from the deployment tool's view, the name of the instance). But complementing this with the app.kubernetes.io/managed-by label, one could very easily make two filters: one for "all objects relating to the foo instance managed by argocd" and other for "all objects relating to the foo instance managed by Helm". In my example, one could of course filter on namespace instead, but that's beside the point.

@blakepettersson
Copy link
Member

Good points,

I realised that argocd.argoproj.io/instance is actually something which is set by argo-helm (I've installed Argo CD using Helm at $dayjob) and what Argo CD actually uses by default is the standard app.kubernetes.io/instance 🤦.

I'd say that this would be a good change in general.

@lindhe
Copy link
Contributor Author

lindhe commented Jul 26, 2023

Important detail! I totally missed the differing labels too, only focusing on the instance part. 😄

@blakepettersson blakepettersson added the good first issue Good for newcomers label Jul 27, 2023
@jmilic1
Copy link

jmilic1 commented Aug 6, 2023

Hi! I would like to take a swing at implementing this. Any pointers on where to start?

jmilic1 added a commit to jmilic1/argo-cd that referenced this issue Aug 6, 2023
@crenshaw-dev
Copy link
Member

@jmilic1 this is where we add resource tracking annotations:

for _, target := range targets {

Might be a reasonable place to add this new metadata.

@jmilic1 jmilic1 linked a pull request Aug 7, 2023 that will close this issue
13 tasks
@jmilic1
Copy link

jmilic1 commented Aug 8, 2023

Hey guys! I prepared a PR at #14945
I'm not sure if I abided by the feature status guidelines correctly.
All criticism is welcome!

@lindhe
Copy link
Contributor Author

lindhe commented Aug 9, 2023

I think it's amazing that you put in the effort to create a PR! Let's make this happen. 🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
6 participants