docs(proposal): server-side apply field manager proposal #17125
Conversation
blakepettersson
changed the title
There was a problem hiding this comment.
Thank you @mattvaughan . Great to see the progress in this proposal.
Please check my comments as I think that there are some important aspects that we need to clarify as part of this proposal.
|
Ok, this might be a dumb reply but this feature basically boils down to resources that we want to have some kind of shared ownership. The example of an ingress is a good one. I want to add entries with an app and other entries with another app. That way each route is self contained. So we could engineer this in Argo CD but maybe we should try to get Kubernetes to support routes as a separate k8s object in ingress. That would actually be a better solution for what we're trying to achieve right? Each app can add its own routes and sync them without worrying about complex merge logic. |
This is actually the way |
There was a problem hiding this comment.
Great progress with the proposal and I think that this is going in a good direction.
There are a few existing Argo CD features that will be likely impacted by this proposal and needs to be considered:
- managedFieldsManagers: There is a feature in Argo CD that allows ignoring differences in state owned by a different managers (See https://argo-cd.readthedocs.io/en/stable/user-guide/diffing/ search by
managedFieldsManagers). We need to validate if the existing feature will be compatible with the changes suggested in this proposal. If not, proper direction needs to be given. - Managed namespaces feature uses SSA behind the scenes. We need to state that this proposal needs to be compatible with the existing feature (see https://argo-cd.readthedocs.io/en/stable/user-guide/sync-options/#namespace-metadata)
- Feature flag: We need to implement this new feature behind a feature flag (Sync Option) that could be enabled at the controller lever as well as at the application level.
We don't need to deep dive in those features during the proposal phase but this is something that absolutely needs to be worked on and well tested during the implementation.
Please add the scenarios above in the use-cases section of this proposal with a brief explanation about how we are going to solve them.
|
|
||
| #### [UC-1]: As a user, I would like to manage specific fields on a Kubernetes object shared by other ArgoCD Applications. | ||
|
|
||
| Change the Server-Side Apply field manager to be unique and deterministic for each Application. |
There was a problem hiding this comment.
We need to add additional use-cases in this section to make sure the proposal is compatible with existing features.
There was a problem hiding this comment.
I added a couple more use cases. Let me know if you had some specific features in mind and I can add some more.
|
@mattvaughan Also.. the feature needs a proper name! How about "Server-Side Apply Dynamic Managers"? Please feel free to suggest something more creative :) |
|
I'm just a fly on the wall but I'm curious how this would work with the Argo CD tracking label/annotations on the resource if multiple applications are updating it? |
@gnunn1. oh! That is an excellent point that I totally forgot about! Tks for chiming in! @mattvaughan Argo CD uses a tracking method logic to define the resources that belong to an specific application. We have 2 different tracking methods, label based (legacy) and annotation based (newer/safer). We need to take this into consideration probably extending the functionality allowing multiple entries in both tracking methods. |
There's a limit of 63 chars for a label value, it might be tough for multiple apps to share the same label. Perhaps this feature should be restricted to annotation (or label+annotation) tracking only? |
Agreed. I'll explicitly call this out in goals/non-goals |
|
|
||
| ## Non-Goals | ||
|
|
||
| 1. We don't intend to give users control of this field manager. Letting users change the field manager could lead to situations where fields get left behind on shared objects. | ||
|
|
||
| 2. We will only support this feature when using annotation resource tracking. Label's 63 character limit will quickly prove too small for multiple Applications. |
There was a problem hiding this comment.
Not sure if we can just ignore this use-case. If this not in scope, the proposal needs to state what will happen when users using label resource tracking and try to use dynamic field manager feature.
There was a problem hiding this comment.
Many ways we could approach this but I'm leaning towards:
- When labels are used, the label is updated to the last Application that changed the resource. Users will need to upgrade to annotation-based tracking to see multiple Applications.
|
I wonder if this proposal could also be useful for |
|
I'm also starting to wonder if this could somehow be further extended (to be clear, this shouldn't be something that would be a part of this initial feature) for implementing The reason for that is that My thinking is that in the future, every field managed by |
|
|
||
| ### [Q-1] What should the behavior be for a server-side applied resource upon Application deletion? | ||
|
|
||
| The current behavior is to delete all objects "owned" by the Application. A user could choose to leave the resource behind with `Prune=false`, but that would also leave behind any config added to the shared object. Should the default delete behavior for server-side applied resources be to remove any fields that match that Application's [field manager][2]? |
There was a problem hiding this comment.
I think this becomes very difficult to get right.
Let's imagine following existing YAML in the cluster:
spec:
foo: bar
bar: baz
baz: fooNow, Argo CD comes to apply a partial patch to that resource, updating the .spec.foo field so that the resource now looks like the following:
spec:
foo: barbar
bar: baz
baz: fooThen, another update will be made by Argo CD so that the resource will look like the following:
spec:
foo: barbarbar
bar: baz
baz: fooHow do we get back to the initial state, i.e. how can we properly unpatch that resource? Any last applied information will have .spec.foo to be barbar, while in reality, it should be bar after deleting the application.
We could just remove .spec.foo completely, but then we could also end up with either a resource that does not look like it should or in the worst case, would be invalid because .spec.foo is a required field.
Do we have any strategies for this case?
There was a problem hiding this comment.
FWIW, my team's use case for this doesn't involve multiple applications fighting for ownership of the same field. Just sibling fields on a shared resource. I imagine that'll be most common. I don't think we should support returning the resource to its original state. We're just exposing more flexibility offered by server-side apply.
If the application that originally wrote foo: bar wants that value back, it would only need to sync again. And it should have shown "Out of Sync" after the second application wrote foo: barbar.
If the system that wrote foo: bar existed outside of ArgoCD, I don't think that's in scope for us to support 🤷♂️ .
There was a problem hiding this comment.
I don't think we should support returning the resource to its original state. We're just exposing more flexibility offered by server-side apply.
I think you're wrong in your assumption here. One of the use cases for server side apply is to be able to patch a preexisting resource on the cluster, no matter who or which system created it in the first place.
The current behavior, for a lack of a better one, is to delete the whole resource (as you wrote). And we probably should change that. Removing a field from the resource could potentially break that resource (if we patched a preexisting one). So returning a resource to the state before Argo CD came around would be a great thing to have.
That being said, I guess removing only the managed fields (unless Prune=false) is better than removing the whole resource in the first place. I was wondering what happens when a mandatory field gets deleted, though - will the delete call fail, or will the field be deleted?
There was a problem hiding this comment.
I think Kubernetes would reject any operation that violated the CRD schema or failed a validating admission webhook.
Removing a field from the resource could potentially break that resource (if we patched a preexisting one). So returning a resource to the state before Argo CD came around would be a great thing to have.
If we take ArgoCD out of the picture, and just server-side patched an existing field on a resource with kubectl, there would be no way to restore it without knowing the original value.
In its current state, can ArgoCD wipe out existing resources? If so, why remember a history of previous state(s) to restore upon app deletion only for server side apply?
Considering that:
We'd need to store the original state in an annotation. This has size limitations, but would cover most cases.
https://argo-cd.readthedocs.io/en/stable/user-guide/sync-options/#replace-resource-instead-of-applying-changes
But above all else, I selfishly want to keep this proposal simple because I'm probably going to end up implementing it 😛
Checklist: