docs(proposal): server-side apply field manager proposal

[mattvaughan]

Checklist:

• Either (a) I've created an enhancement proposal and discussed it with the community, (b) this is a bug fix, or (c) this does not need to be in the release notes.
• The title of the PR states what changed and the related issues number (used for the release note).
• The title of the PR conforms to the Toolchain Guide
• I've included "Closes [ISSUE #]" or "Fixes [ISSUE #]" in the description to automatically close the associated issue.
• I've updated both the CLI and UI to expose my feature, or I plan to submit a second PR with them.
• Does this PR require documentation updates?
• I've updated documentation as required by this PR.
• I have signed off all my commits as required by DCO
• I have written unit and/or e2e tests for my change. PRs without these are unlikely to be merged.
• My build is green (troubleshooting builds).
• My new feature complies with the feature status guidelines.
• I have added a brief description of why this PR is necessary and/or what this PR solves.
• Optional. My organization is added to USERS.md.
• Optional. For bug fixes, I've indicated what older releases this fix should be cherry-picked into (this may or may not happen depending on risk/complexity).

[leoluz]

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.

[todaywasawesome]

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.

[mattvaughan]

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 Gateway and HTTPRoute work. Consul implements the Kubernetes Gateway API, but for internal compatibility reasons, we're still using Consul's IngressGateway.

[leoluz]

Tks for your work in this proposal.
I added a few more questions.

[leoluz]

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.

[leoluz]

We need to add additional use-cases in this section to make sure the proposal is compatible with existing features.

[mattvaughan]

I added a couple more use cases. Let me know if you had some specific features in mind and I can add some more.

[leoluz]

@mattvaughan Also.. the feature needs a proper name! How about "Server-Side Apply Dynamic Managers"? Please feel free to suggest something more creative :)

[gnunn1]

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?

[leoluz]

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.

[leoluz]

Please update the proposal adding the requirement to make the resource tracking logic for labels and annotation support multiple Apps owning the same resource.

[blakepettersson]

Please update the proposal adding the requirement to make the resource tracking logic for labels and annotation support multiple Apps owning the same resource.

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?

[mattvaughan]

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

[leoluz]

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.

[mattvaughan]

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.

[blakepettersson]

I wonder if this proposal could also be useful for managedNamespaceMetadata (see argoproj/gitops-engine#537). Currently, existing namespaces that already have pre-existing labels and annotations will get clobbered by MNM. In the linked PR, we needed to (re-)apply the namespace with a separate field manager in order to preserve the existing metadata. Perhaps this issue could be solved more elegantly with this proposal?

[blakepettersson]

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 ignoreDifferences as a separate field manager.

The reason for that is that ignoreDifferences + RespectIgnoreDifferences=true doesn't currently work for arrays, which is getting a lot of people bitten in #9678. From what I have been able to see, there's not really a practical way of fixing this with the current implementation.

My thinking is that in the future, every field managed by ignoreDifferences could be managed by a separate "ignore differences" field manager, which a "default" field manager would not touch. Would that be something that could be implemented with this proposal?

[jannfis]

I think this becomes very difficult to get right.

Let's imagine following existing YAML in the cluster:

spec:
  foo: bar
  bar: baz
  baz: foo

Now, 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: foo

Then, another update will be made by Argo CD so that the resource will look like the following:

spec:
  foo: barbarbar
  bar: baz
  baz: foo

How 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?

[mattvaughan]

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 🤷‍♂️ .

[jannfis]

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?

[mattvaughan]

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:

Argo CD is largely stateless.

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 😛