Ignore Owner References

[spjmurray]

Summary

Provide a way to ignore owner references, and force ArgoCD to delete things.

Motivation

Related to kubernetes-sigs/cluster-api#7913. What we are seeing here is cluster-api adding its own owner references to resources managed by Argo. When it comes to deletion, it's not generated by the Helm template any more, but Argo thinks it's an implicitly created by cluster-api because of the owner references CAPI has added. Basically I cannot delete things unless I do it manually.

Proposal

One idea to handle naughty apps is to have Argo spot the tracking ID, and the fact there is no requested resource any more, and just ignore owners, it created it and has all the power here.

[spjmurray]

Another proposal now I consider it, is an explicit annotation that we can attach to a resource that says "you own this, delete it if not backed by helm template". Probably a better solution than just considering the tracking ID implicitly as I did see something historically about why tracking IDs came about in the first place due to inheritance.

[spjmurray]

Added a potential--and remarkably small--PoC fix for consideration.

[jannfis]

Thanks for bringing this up.

Probably we need to discuss general pruning behavior and how Argo CD handles owner references. I think the initial assumption was, that a resource would never have an owner outside of what the Argo CD application manages (think Deployment -> ReplicaSet, etc).

While it most likely contradicts with how Argo CD views things right now (e.g. that it "owns" a resource it has reconciled from Git to a cluster), I think it's perfectly fine for some other controller to claim ownership over a resource - I guess that's why there can be more than one owner references for any resource in the first place. I'd assume once CAPI puts an owner reference on the resource, Argo CD would also ignore the resource for changes originating on the cluster, and not enter an OutOfSync state when the resource change on the cluster?

Maybe a sync option on the resource, as implemented in your PoC, is a proper way to tell Argo CD to still let it handle a resource despite it having one or more owner references.

However, it sounds like it would need a little more discussion amongst contributors and maintainers on how we can support this use case. I'd like to bring this to the next contributor's meeting, happening weekly each Thursday at 4:15pm UTC, so that we'll find a good solution. You are of course invited to join, see https://github.com/argoproj/argo-cd#contribution-discussion-and-support for more details.

[spjmurray]

Yeah, my train doesn't leave London until 7, so I should be able to hop on. Thanks for having a look

[spjmurray]

Note this looks like a dupe of #4764 so linking

Did you come to any conclusions @jannfis I missed the important bit of that meeting due to Zoom (and broken library deps in dpkg!)

[jaideepr97]

There's another potential approach suggested to this problem in #12210

[spjmurray]

Ah yes, I'm aware of Matt's approach, that's also good as far as I'm concerned. While this is less dangerous, the other way does force people to write their controllers correctly (which is a win for the whole world), lest their stuff gets deleted :)

[jannfis]

@spjmurray Sorry, I forgot to update this issue eventually. So we agreed that we should support the use case when another controller sets an owner reference to a resource synced with Argo CD. I think there was also consensus that the solution you propose in your PoC looks good, and there's an item on my todo list to take it for a test drive. Obviously, I have not come around yet to do it.

[spjmurray]

Cool. If there's anything you want me to do in the mean time just ask e.g. unit tests, technical documentation, etc.

[mkjpryor]

This is where I eventually landed as a solution: argoproj/gitops-engine#503

I’m running this in a custom build now, and it works perfectly for my use case.

It is very similar to @spjmurray’s solution, except that it respects controller references at all times - the annotation only changes behaviour r.e. non-controller references. I think this is probably what we want as conflicting with another controller could be dangerous and cause a sync race/loop.