-
Notifications
You must be signed in to change notification settings - Fork 7.5k
docs: Add server-side apply proposal #8812
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
Merged
Merged
Changes from 6 commits
Commits
Show all changes
11 commits
Select commit
Hold shift + click to select a range
7fcd222
docs: Add server-side apply proposal
leoluz 0bbe824
Adds motivation and goals details
leoluz d3ab338
Add use-case 1
leoluz d8068a9
Add TOC
leoluz 851af87
Add TOC links
leoluz 04ade52
Conclude proposal first draft
leoluz ec28729
Add KEP-555 reference
leoluz 64fcfb1
Update TOC
leoluz 6765980
Add q-1 answer
leoluz 171769f
Add goal to verify correct service patch
leoluz 7b7c093
Merge remote-tracking branch 'upstream/master' into ssa-proposal
leoluz File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,194 @@ | ||
| --- | ||
| title: Server-Side Apply | ||
|
|
||
| authors: | ||
| - "@leoluz" | ||
|
|
||
| sponsors: | ||
| - TBD | ||
|
|
||
| reviewers: | ||
| - TBD | ||
|
|
||
| approvers: | ||
| - TBD | ||
|
|
||
| creation-date: 2022-03-17 | ||
|
|
||
| --- | ||
|
|
||
| # Server-Side Apply support for ArgoCD | ||
|
|
||
| [Server-Side Apply (SSA)][1] allows calculating the final patch to update | ||
| resources in Kubernetes in the server instead of the client. This proposal | ||
| describes how ArgoCD can leverage SSA during syncs. | ||
|
|
||
| * [Open Questions](#open-questions) | ||
| * [[Q-1] How to handle conflicts?](#q-1-how-to-handle-conflicts) | ||
| * [[Q-2] Should we support multiple managers?](#q-2-should-we-support-multiple-managers) | ||
| * [ Summary ](#summary) | ||
| * [Motivation](#motivation) | ||
| * [Better interoperability with Admission Controllers](#better-interoperability-with-admission-controllers) | ||
| * [Better resource conflict management](#better-resource-conflict-management) | ||
| * [Better CRD support](#better-crd-support) | ||
| * [Goals](#goals) | ||
| * [Non-Goals](#non-goals) | ||
| * [Proposal](#proposal) | ||
| * [Non-Functional Requirements](#non-functional-requirements) | ||
| * [Use cases](#use-cases) | ||
| * [[UC-1]: enable SSA at the controller level](#uc-1-as-a-user-i-would-like-enable-ssa-at-the-controller-level-so-all-application-are-applied-server-side) | ||
| * [[UC-2]: enable SSA at the Application level](#uc-2-as-a-user-i-would-like-enable-ssa-at-the-application-level-so-all-resources-are-applied-server-side) | ||
| * [[UC-3]: enable SSA at the resource level](#uc-3-as-a-user-i-would-like-enable-ssa-at-the-resource-level-so-only-a-single-manifest-is-applied-server-side) | ||
| * [Security Considerations](#security-considerations) | ||
| * [Risks and Mitigations](#risks-and-mitigations) | ||
| * [Upgrade / Downgrade](#upgrade--downgrade) | ||
| * [Drawbacks](#drawbacks) | ||
|
|
||
| --- | ||
|
|
||
| ## Open Questions | ||
|
|
||
| ### [Q-1] How to handle conflicts? | ||
| When SSA is enabled, the server may return field conflicts with other managers. | ||
| What ArgoCD controller should do in case of conflict? Just force the sync and | ||
| log warnings (like some other controllers do?) | ||
|
|
||
| ### [Q-2] Should we support multiple managers? | ||
|
leoluz marked this conversation as resolved.
|
||
| Should Server-Side Apply support in ArgoCD be implemented allowing multiple | ||
| managers for the same controller? ([more details][10]) | ||
|
|
||
| ## Summary | ||
|
|
||
| ArgoCD can benefit from [Server-Side Apply][1] during syncs. A few | ||
| improvements to consider: | ||
|
|
||
| - More reliable dry-runs (as admission controller is executed) ([ISSUE-804][5]) | ||
| - [Syncs always run][2] mutating webhooks (even without diff) | ||
| - [Fix big CRD][3] sync issues | ||
|
leoluz marked this conversation as resolved.
|
||
| - Better interoperability with different controllers | ||
|
|
||
| ## Motivation | ||
|
|
||
| ArgoCD uses kubectl library while syncing resources in the cluster. Kubectl uses | ||
| by default a 3-way-merge logic between the live state (in k8s), desired state | ||
| (in git) and the previous state (`last-applied-configuration` annotation) to | ||
| calculate diffs and patch resources in the cluster. This logic is executed in | ||
| the client (ArgoCD) and once the patch is calculated it is then sent to the | ||
| server. | ||
|
|
||
| This strategy works well in the majority of the use cases. However, there are | ||
| some scenarios where calculating patches in the client side can cause problems. | ||
|
|
||
| Some of the known problems about using client-side approach: | ||
|
|
||
| ### Better interoperability with Admission Controllers | ||
|
|
||
| More and more users are adopting and configuring [Admission Controllers][4] in | ||
| Kubernetes with different flavors of Validating Webhooks and Mutating Webhooks. | ||
| Admission Controllers will only execute in server-side. In cases where users | ||
| rely on dry-run executions to decide if they should proceed with a deployment, | ||
| having the patch calculated at the client side might provide undesired results. | ||
| Having SSA enabled syncs also guaranties that Admission Controllers are always | ||
| executed, even when there is no diff in the resource. | ||
|
|
||
| ### Better resource conflict management | ||
|
|
||
| Server-Side Apply will better handle and identify conflicts during syncs by | ||
| analyzing the `managedFields` metadata available in all Kubernetes resources | ||
| (since kubernetes 1.18). | ||
|
|
||
| ### Better CRD support | ||
|
|
||
| By not having to rely on the `last-applied-configuration` annotation, SSA would | ||
| help with failing syncs caused by exceeded annotation size limit. This is a | ||
| common issue when syncing CRDs with large schemas. | ||
|
|
||
| ## Goals | ||
|
|
||
| All following goals should be achieve in order to conclude this proposal: | ||
|
|
||
| - Provide the ability for users to define if they want to use SSA during syncs | ||
| ([ISSUE-2267][6]) | ||
| - Users should be able to enable SSA at the controller level (via binary flag) | ||
| (see [UC-1](#uc-1-as-a-user-i-would-like-enable-ssa-at-the-controller-level-so-all-application-are-applied-server-side)) | ||
| - Users should be able to enable SSA for a given Application (via syncOptions) | ||
| (see [UC-2](#uc-2-as-a-user-i-would-like-enable-ssa-at-the-application-level-so-all-resources-are-applied-server-side)) | ||
| - Users should be able to enable SSA at resource level (via annotation) (see | ||
| [UC-3](#uc-3-as-a-user-i-would-like-enable-ssa-at-the-resource-level-so-only-a-single-manifest-is-applied-server-side) | ||
| - Diffing needs to support strategic merge patch (see [ISSUE-2268][7]) | ||
|
leoluz marked this conversation as resolved.
|
||
| - Allow Admission Controllers to execute even when there is no diff for a | ||
| particular resource. (Needs investigation) ([more details][2]) | ||
| - ArgoCD should respect field ownership and provide a configuration to allow | ||
| users to define the behavior in case of conflicts (see [Q-1](#q-1-how-to-handle-conflicts) outcome) | ||
| - ArgoCD should register itself with a proper manager (see [non-functional | ||
| requirements](#non-functional-requirements)) | ||
|
|
||
| ## Non-Goals | ||
|
|
||
| TBD | ||
|
|
||
| ## Proposal | ||
|
|
||
| Change ArgoCD controller to accept new parameter to enable Server-Side Apply | ||
| during syncs. Changes are necessary in ArgoCD as well as in | ||
| gitops-engine library. | ||
|
|
||
| ### Non-Functional Requirements | ||
|
|
||
| - ArgoCD must register itself with a pre-defined manager (suggestion: | ||
| `argocd-controller`). It shouldn't rely on the default value defined in the | ||
| kubectl code. ([more details][11]) | ||
|
|
||
| ### Use cases | ||
|
|
||
| The following use cases should be implemented: | ||
|
|
||
| #### [UC-1]: As a user, I would like enable SSA at the controller level so all Application are applied server-side | ||
|
|
||
| Implement a binary flag to configure ArgoCD to run all syncs using SSA. | ||
| (suggestion: `--server-side-apply=true`). Default value should be `false`. | ||
|
|
||
| #### [UC-2]: As a user, I would like enable SSA at the Application level so all resources are applied server-side | ||
|
|
||
| Implement a new syncOption to allow users to enable SSA at the application | ||
| level (Suggestion `ServerSideApply=true`). UI needs to be updated to support | ||
| this new sync option. If not informed, the controller should keep the current | ||
| behaviour (client-side). | ||
|
|
||
| #### [UC-3]: As a user, I would like enable SSA at the resource level so only a single manifest is applied server-side | ||
|
|
||
| Leverage the existing `argocd.argoproj.io/sync-options` annotation allowing the | ||
| `ServerSideApply=true` to be informed at the resource level. Must not impact | ||
| other sync-options informed in the annotation (make sure this annotation | ||
| supports providing multiple options). | ||
|
|
||
| ### Security Considerations | ||
|
|
||
| TBD | ||
|
|
||
| ### Risks and Mitigations | ||
| ArgoCD must check if the target Kubernetes cluster has full support for SSA. The | ||
| feature turned [GA in Kubernetes 1.22][8]. Full support for managed fields was | ||
| introduced as [beta in Kubernetes 1.18][9]. The implementation must check that | ||
| the target kubernetes cluster is running at least version 1.18. If SSA is | ||
|
leoluz marked this conversation as resolved.
|
||
| enabled and target cluster version < 1.18 ArgoCD should log warning and fallback | ||
| to client sync. | ||
|
|
||
| ### Upgrade / Downgrade | ||
| No CRD update necessary as `syncOption` field in Application resource is non-typed | ||
| (string array). Upgrade will only require ArgoCD controller update. | ||
|
|
||
| ## Drawbacks | ||
| Slight increase in ArgoCD code base complexity. | ||
|
|
||
| [1]: https://kubernetes.io/docs/reference/using-api/server-side-apply/ | ||
| [2]: https://github.com/argoproj/argo-cd/issues/2267#issuecomment-920445236 | ||
| [3]: https://github.com/prometheus-community/helm-charts/issues/1500#issuecomment-1017961377 | ||
| [4]: https://kubernetes.io/docs/reference/access-authn-authz/admission-controllers/ | ||
| [5]: https://github.com/argoproj/argo-cd/issues/804 | ||
| [6]: https://github.com/argoproj/argo-cd/issues/2267 | ||
| [7]: https://github.com/argoproj/argo-cd/issues/2268 | ||
| [8]: https://kubernetes.io/blog/2021/08/06/server-side-apply-ga/ | ||
| [9]: https://kubernetes.io/blog/2020/04/01/kubernetes-1.18-feature-server-side-apply-beta-2/ | ||
| [10]: https://github.com/argoproj/gitops-engine/pull/363#issuecomment-1013641708 | ||
| [11]: https://github.com/argoproj/gitops-engine/pull/363#issuecomment-1013289982 | ||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.