From 7fcd22203148774824daa92b78fa2b15d9e085e4 Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Thu, 17 Mar 2022 09:38:46 -0400 Subject: [PATCH 01/10] docs: Add server-side apply proposal Signed-off-by: Leonardo Luz Almeida --- docs/proposals/server-side-apply.md | 112 ++++++++++++++++++++++++++++ 1 file changed, 112 insertions(+) create mode 100644 docs/proposals/server-side-apply.md diff --git a/docs/proposals/server-side-apply.md b/docs/proposals/server-side-apply.md new file mode 100644 index 0000000000000..0edd3b4e4d8b1 --- /dev/null +++ b/docs/proposals/server-side-apply.md @@ -0,0 +1,112 @@ +--- +title: Server-Side Apply +authors: + - "@leoluz" +sponsors: + - TBD +reviewers: + - TBD +approvers: + - TBD + +creation-date: 2022-03-17 +--- + +# Server-Side Apply support for ArgoCD + +This proposal describes how ArgoCD could benefit from [Kubernetes Server-Side Apply][ssa] during syncs. + +This is the title of the enhancement. Keep it simple and descriptive. A good title can help +communicate what the enhancement is and should be considered as part of any review. + + +## Open Questions [optional] + +This is where to call out areas of the design that require closure before deciding to implement the +design. + + +## Summary + +The `Summary` is required for producing accurate user-focused documentation +such as release notes or a development roadmap. It should be possible to collect this information +before implementation begins in order to avoid requiring implementors to split their attention +between writing release notes and implementing the feature itself. Before you get started with this document, +please feel free to have a conversation on this with the maintainers/community on Github that would help +drive a more organized thought process for the formal proposal here. + +## Motivation + +This section is for explicitly listing the motivation, goals and non-goals of this proposal. +Describe why the change is important and the benefits to users. + +### Goals + +List the specific goals of the proposal and their measurable results. How will we know that this has succeeded? + +### Non-Goals + +What is out of scope for this proposal? Listing non-goals helps to focus discussion and make +progress. + +## Proposal + +This is where we get down to details of what the proposal is about. + +### Use cases + +Add a list of detailed use cases this enhancement intends to take care of. + +#### Use case 1: +As a user, I would like to understand the drift. (This is an example) + +#### Use case 2: +As a user, I would like to take an action on the deviation/drift. (This is an example) + +### Implementation Details/Notes/Constraints [optional] + +What are the caveats to the implementation? What are some important details that didn't come across +above. Go in to as much detail as necessary here. This might be a good place to talk about core +concepts and how they relate. + +You may have a work-in-progress Pull Request to demonstrate the functioning of the enhancement you are proposing. + +### Detailed examples + +### Security Considerations + +* How does this proposal impact the security aspects of Argo CD workloads ? +* Are there any unresolved follow-ups that need to be done to make the enhancement more robust ? + +### Risks and Mitigations + +What are the risks of this proposal and how do we mitigate. Think broadly. + +For example, consider +both security and how this will impact the larger Kubernetes ecosystem. + +Consider including folks that also work outside your immediate sub-project. + + +### Upgrade / Downgrade Strategy + +If applicable, how will the component be upgraded and downgraded? Make sure this is in the test +plan. + +Consider the following in developing an upgrade/downgrade strategy for this enhancement: + +- What changes (in invocations, configurations, API use, etc.) is an existing cluster required to + make on upgrade in order to keep previous behavior? +- What changes (in invocations, configurations, API use, etc.) is an existing cluster required to + make on upgrade in order to make use of the enhancement? + +## Drawbacks + +The idea is to find the best form of an argument why this enhancement should _not_ be implemented. + +## Alternatives + +Similar to the `Drawbacks` section the `Alternatives` section is used to highlight and record other +possible approaches to delivering the value proposed by an enhancement. + +[ssa]: https://kubernetes.io/docs/reference/using-api/server-side-apply/ From 0bbe8242a2a9977a715a564bcd45e954044077fc Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Thu, 17 Mar 2022 15:53:53 -0400 Subject: [PATCH 02/10] Adds motivation and goals details Signed-off-by: Leonardo Luz Almeida --- docs/proposals/server-side-apply.md | 184 ++++++++++++++++++---------- 1 file changed, 118 insertions(+), 66 deletions(-) diff --git a/docs/proposals/server-side-apply.md b/docs/proposals/server-side-apply.md index 0edd3b4e4d8b1..7d73ba5100f0c 100644 --- a/docs/proposals/server-side-apply.md +++ b/docs/proposals/server-side-apply.md @@ -1,112 +1,164 @@ --- title: Server-Side Apply + authors: - - "@leoluz" +- "@leoluz" + sponsors: - - TBD +- TBD + reviewers: - - TBD +- TBD + approvers: - - TBD +- TBD creation-date: 2022-03-17 + --- # Server-Side Apply support for ArgoCD -This proposal describes how ArgoCD could benefit from [Kubernetes Server-Side Apply][ssa] during syncs. - -This is the title of the enhancement. Keep it simple and descriptive. A good title can help -communicate what the enhancement is and should be considered as part of any review. - +[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 [optional] -This is where to call out areas of the design that require closure before deciding to implement the -design. - +* Should Server-Side Apply support in ArgoCD be implemented allowing multiple + managers for the same controller? ## Summary -The `Summary` is required for producing accurate user-focused documentation -such as release notes or a development roadmap. It should be possible to collect this information -before implementation begins in order to avoid requiring implementors to split their attention -between writing release notes and implementing the feature itself. Before you get started with this document, -please feel free to have a conversation on this with the maintainers/community on Github that would help -drive a more organized thought process for the formal proposal here. +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 +- Better interoperability with different controllers ## Motivation -This section is for explicitly listing the motivation, goals and non-goals of this proposal. -Describe why the change is important and the benefits to users. +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. -### Goals +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. -List the specific goals of the proposal and their measurable results. How will we know that this has succeeded? +Some of the known problems about using client-side approach: -### Non-Goals +### Better interoperability with Admission Controllers -What is out of scope for this proposal? Listing non-goals helps to focus discussion and make -progress. +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. -## Proposal +### Better resource conflict management -This is where we get down to details of what the proposal is about. +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). -### Use cases +### Better resource conflict management -Add a list of detailed use cases this enhancement intends to take care of. +By not having to rely on the `last-applied-configuration` annotation, SSA would +help with failing syncs caused by exceeded annotation size limit when syncing +CRDs with large schemas. -#### Use case 1: -As a user, I would like to understand the drift. (This is an example) +## Goals -#### Use case 2: -As a user, I would like to take an action on the deviation/drift. (This is an example) +- Provide the ability for users to define if they want to use SSA during syncs + ([ISSUE-2267][6]) +- Diffing needs to support strategic merge patch ([ISSUE-2268][7]) +- Allow Admission Controllers to execute even when there is no diff for a + particular resource. (Needs investigation) +- ArgoCD should respect field ownership and provide a configuration to allow + users to define the behavior in case of conflicts -### Implementation Details/Notes/Constraints [optional] +## Non-Goals +What is out of scope for this proposal? +Listing non-goals helps to focus discussion and make progress -What are the caveats to the implementation? What are some important details that didn't come across -above. Go in to as much detail as necessary here. This might be a good place to talk about core -concepts and how they relate. +## Proposal +This is where we get down to details of what the proposal is about. +This is where we get down to details of what the proposal is about. -You may have a work-in-progress Pull Request to demonstrate the functioning of the enhancement you are proposing. +### Use cases +Add a list of detailed use cases this enhancement intends to take +care of. Add a list of detailed use cases this enhancement intends to take care +of. + +#### Use case 1: As a user, I would like to understand the drift. (This is an +example) + +#### Use case 2: As a user, I would like to take an action on the +deviation/drift. (This is an example) + +### Implementation Details/Notes/Constraints [optional] What are the caveats to +the implementation? What are some important details that didn't come across +above. Go in to as much detail as necessary here. This might be a good place to +talk about core concepts and how they relate. concepts and how they relate. You +may have a work-in-progress Pull Request to demonstrate the functioning of the +enhancement you are proposing. You may have a work-in-progress Pull Request to +demonstrate the functioning of the enhancement you are proposing. ### Detailed examples ### Security Considerations - * How does this proposal impact the security aspects of Argo CD workloads ? -* Are there any unresolved follow-ups that need to be done to make the enhancement more robust ? +* Are there any unresolved follow-ups that need to be done to make the + enhancement more robust ? + * Are there any unresolved follow-ups that need to be done to make the + enhancement more robust ? ### Risks and Mitigations - -What are the risks of this proposal and how do we mitigate. Think broadly. - -For example, consider -both security and how this will impact the larger Kubernetes ecosystem. - -Consider including folks that also work outside your immediate sub-project. - - -### Upgrade / Downgrade Strategy - -If applicable, how will the component be upgraded and downgraded? Make sure this is in the test -plan. - -Consider the following in developing an upgrade/downgrade strategy for this enhancement: - -- What changes (in invocations, configurations, API use, etc.) is an existing cluster required to - make on upgrade in order to keep previous behavior? -- What changes (in invocations, configurations, API use, etc.) is an existing cluster required to +What are the risks of this proposal and how do we mitigate. Think broadly. What +are the risks of this proposal and how do we mitigate. Think broadly. For +example, consider both security and how this will impact the larger Kubernetes +ecosystem. both security and how this will impact the larger Kubernetes +ecosystem. Consider including folks that also work outside your immediate +sub-project. Consider including folks that also work outside your immediate +sub-project. + + +### Upgrade / Downgrade +Strategy If applicable, how will the component be upgraded and downgraded? Make +sure this is in the test plan. +Plan. +Consider the following in developing an +upgrade/downgrade strategy for this enhancement: +Consider the following in developing an upgrade/downgrade strategy for this +enhancement: +- What changes (in invocations, configurations, API use, etc.) is an existing + cluster required to make on upgrade in order to keep previous behavior? +- What changes (in invocations, configurations, API use, etc.) is an existing + cluster required to make on upgrade in order to make use of the enhancement? make on upgrade in order to make use of the enhancement? ## Drawbacks - -The idea is to find the best form of an argument why this enhancement should _not_ be implemented. +The idea is to find the best form of an argument why this enhancement should +_not_ be implemented. The idea is to find the best form of an argument why this +enhancement should _not_ be implemented. ## Alternatives - -Similar to the `Drawbacks` section the `Alternatives` section is used to highlight and record other -possible approaches to delivering the value proposed by an enhancement. - -[ssa]: https://kubernetes.io/docs/reference/using-api/server-side-apply/ +Similar to the `Drawbacks` section the `Alternatives` section is used to +highlight and record other possible approaches to delivering the value proposed +by an enhancement. possible approaches to delivering the value proposed by an +enhancement. + +[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 From d3ab338d0e4698c401175ff7a3c7b0b034c8b53c Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Fri, 18 Mar 2022 11:15:09 -0400 Subject: [PATCH 03/10] Add use-case 1 Signed-off-by: Leonardo Luz Almeida --- docs/proposals/server-side-apply.md | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/docs/proposals/server-side-apply.md b/docs/proposals/server-side-apply.md index 7d73ba5100f0c..c4f215c40065e 100644 --- a/docs/proposals/server-side-apply.md +++ b/docs/proposals/server-side-apply.md @@ -78,27 +78,32 @@ CRDs with large schemas. - 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) + - Users should be able to enable SSA for a given Application (via syncOptions) + - Users should be able to enable SSA at resource level (via annotation) - Diffing needs to support strategic merge patch ([ISSUE-2268][7]) - Allow Admission Controllers to execute even when there is no diff for a particular resource. (Needs investigation) - ArgoCD should respect field ownership and provide a configuration to allow users to define the behavior in case of conflicts +- ArgoCD should register itself with a proper manager. ## Non-Goals + What is out of scope for this proposal? Listing non-goals helps to focus discussion and make progress ## Proposal -This is where we get down to details of what the proposal is about. -This is where we get down to details of what the proposal is about. + +Change ArgoCD controller to accept new parameter to enable Server-Side Apply +during syncs. ### Use cases -Add a list of detailed use cases this enhancement intends to take -care of. Add a list of detailed use cases this enhancement intends to take care -of. -#### Use case 1: As a user, I would like to understand the drift. (This is an -example) +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 #### Use case 2: As a user, I would like to take an action on the deviation/drift. (This is an example) @@ -129,7 +134,6 @@ ecosystem. Consider including folks that also work outside your immediate sub-project. Consider including folks that also work outside your immediate sub-project. - ### Upgrade / Downgrade Strategy If applicable, how will the component be upgraded and downgraded? Make sure this is in the test plan. From d8068a938df2ceb0f6fc89f7115b6533b44f4adb Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Fri, 18 Mar 2022 12:27:59 -0400 Subject: [PATCH 04/10] Add TOC Signed-off-by: Leonardo Luz Almeida --- docs/proposals/server-side-apply.md | 101 ++++++++++++++-------------- 1 file changed, 52 insertions(+), 49 deletions(-) diff --git a/docs/proposals/server-side-apply.md b/docs/proposals/server-side-apply.md index c4f215c40065e..d40b74a6f42c9 100644 --- a/docs/proposals/server-side-apply.md +++ b/docs/proposals/server-side-apply.md @@ -23,10 +23,38 @@ creation-date: 2022-03-17 resources in Kubernetes in the server instead of the client. This proposal describes how ArgoCD can leverage SSA during syncs. -## Open Questions [optional] +* Open Questions + * [Q-1] How to handle conflicts? + * [Q-2] Should we support multiple managers? +* Summary +* Motivation + * Better interoperability with Admission Controllers + * Better resource conflict management + * Better resource conflict management +* Goals +* Non-Goals +* Proposal + * Use cases + * [UC-1]: enable SSA at the controller level + * [UC-2]: enable SSA at the Application level + * [UC-3]: enable SSA at the resource level + * Security Considerations + * Risks and Mitigations + * Upgrade / Downgrade +* Drawbacks -* Should Server-Side Apply support in ArgoCD be implemented allowing multiple - managers for the same controller? +--- + +## 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? +Should Server-Side Apply support in ArgoCD be implemented allowing multiple +managers for the same controller? ([more details][10]) ## Summary @@ -96,68 +124,40 @@ Listing non-goals helps to focus discussion and make progress ## Proposal Change ArgoCD controller to accept new parameter to enable Server-Side Apply -during syncs. +during syncs. ArgoCD must register itself with a pre-defined manager +(suggestion: `argocd-controller`). ### 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 +#### [UC-1]: As a user, I would like enable SSA at the controller level so all Application are applied server-side -#### Use case 2: As a user, I would like to take an action on the -deviation/drift. (This is an example) +#### [UC-2]: As a user, I would like enable SSA at the Application level so all resources are applied server-side -### Implementation Details/Notes/Constraints [optional] What are the caveats to -the implementation? What are some important details that didn't come across -above. Go in to as much detail as necessary here. This might be a good place to -talk about core concepts and how they relate. concepts and how they relate. You -may have a work-in-progress Pull Request to demonstrate the functioning of the -enhancement you are proposing. You may have a work-in-progress Pull Request to -demonstrate the functioning of the enhancement you are proposing. +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. -### Detailed examples +#### [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 -* How does this proposal impact the security aspects of Argo CD workloads ? -* Are there any unresolved follow-ups that need to be done to make the - enhancement more robust ? - * Are there any unresolved follow-ups that need to be done to make the - enhancement more robust ? +TBD ### Risks and Mitigations -What are the risks of this proposal and how do we mitigate. Think broadly. What -are the risks of this proposal and how do we mitigate. Think broadly. For -example, consider both security and how this will impact the larger Kubernetes -ecosystem. both security and how this will impact the larger Kubernetes -ecosystem. Consider including folks that also work outside your immediate -sub-project. Consider including folks that also work outside your immediate -sub-project. +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 +enabled and target cluster version < 1.18 ArgoCD should log warning and fallback +to client sync. ### Upgrade / Downgrade -Strategy If applicable, how will the component be upgraded and downgraded? Make -sure this is in the test plan. -Plan. -Consider the following in developing an -upgrade/downgrade strategy for this enhancement: -Consider the following in developing an upgrade/downgrade strategy for this -enhancement: -- What changes (in invocations, configurations, API use, etc.) is an existing - cluster required to make on upgrade in order to keep previous behavior? -- What changes (in invocations, configurations, API use, etc.) is an existing - cluster required to make on upgrade in order to make use of the enhancement? - make on upgrade in order to make use of the enhancement? +No CRD update necessary as `syncOption` field in Application resource is non-typed +(string array). Upgrade will only require ArgoCD controller update. ## Drawbacks -The idea is to find the best form of an argument why this enhancement should -_not_ be implemented. The idea is to find the best form of an argument why this -enhancement should _not_ be implemented. - -## Alternatives -Similar to the `Drawbacks` section the `Alternatives` section is used to -highlight and record other possible approaches to delivering the value proposed -by an enhancement. possible approaches to delivering the value proposed by an -enhancement. +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 @@ -166,3 +166,6 @@ enhancement. [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 From 851af876eefb21f2df24de5c92a070b29069b6e7 Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Fri, 18 Mar 2022 12:38:44 -0400 Subject: [PATCH 05/10] Add TOC links Signed-off-by: Leonardo Luz Almeida --- docs/proposals/server-side-apply.md | 44 ++++++++++++++--------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/docs/proposals/server-side-apply.md b/docs/proposals/server-side-apply.md index d40b74a6f42c9..99c8f11ec8c32 100644 --- a/docs/proposals/server-side-apply.md +++ b/docs/proposals/server-side-apply.md @@ -23,25 +23,25 @@ creation-date: 2022-03-17 resources in Kubernetes in the server instead of the client. This proposal describes how ArgoCD can leverage SSA during syncs. -* Open Questions - * [Q-1] How to handle conflicts? - * [Q-2] Should we support multiple managers? -* Summary -* Motivation - * Better interoperability with Admission Controllers - * Better resource conflict management - * Better resource conflict management -* Goals -* Non-Goals -* Proposal - * Use cases - * [UC-1]: enable SSA at the controller level - * [UC-2]: enable SSA at the Application level - * [UC-3]: enable SSA at the resource level - * Security Considerations - * Risks and Mitigations - * Upgrade / Downgrade -* Drawbacks +* [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) + * [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) --- @@ -96,11 +96,11 @@ 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 resource conflict management +### 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 when syncing -CRDs with large schemas. +help with failing syncs caused by exceeded annotation size limit. This is a +common issue when syncing CRDs with large schemas. ## Goals From 04ade52fa2f6e5e93d86fbe4b0888a2b4eeaf5ef Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Fri, 18 Mar 2022 14:54:14 -0400 Subject: [PATCH 06/10] Conclude proposal first draft Signed-off-by: Leonardo Luz Almeida --- docs/proposals/server-side-apply.md | 43 ++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 10 deletions(-) diff --git a/docs/proposals/server-side-apply.md b/docs/proposals/server-side-apply.md index 99c8f11ec8c32..a5bd573c15650 100644 --- a/docs/proposals/server-side-apply.md +++ b/docs/proposals/server-side-apply.md @@ -34,6 +34,7 @@ describes how ArgoCD can leverage SSA during syncs. * [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) @@ -104,28 +105,39 @@ 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) - - Users should be able to enable SSA at resource level (via annotation) -- Diffing needs to support strategic merge patch ([ISSUE-2268][7]) + (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]) - Allow Admission Controllers to execute even when there is no diff for a - particular resource. (Needs investigation) + 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 -- ArgoCD should register itself with a proper manager. + 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 -What is out of scope for this proposal? -Listing non-goals helps to focus discussion and make progress +TBD ## Proposal Change ArgoCD controller to accept new parameter to enable Server-Side Apply -during syncs. ArgoCD must register itself with a pre-defined manager -(suggestion: `argocd-controller`). +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 @@ -133,15 +145,25 @@ 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. +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 @@ -169,3 +191,4 @@ Slight increase in ArgoCD code base complexity. [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 From ec28729e4864030c97aa2439cf4332140c6a730e Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Thu, 24 Mar 2022 10:11:33 -0400 Subject: [PATCH 07/10] Add KEP-555 reference Signed-off-by: Leonardo Luz Almeida --- docs/proposals/server-side-apply.md | 73 ++++++++++++++++++++++------- 1 file changed, 56 insertions(+), 17 deletions(-) diff --git a/docs/proposals/server-side-apply.md b/docs/proposals/server-side-apply.md index a5bd573c15650..eca8aec5e68c6 100644 --- a/docs/proposals/server-side-apply.md +++ b/docs/proposals/server-side-apply.md @@ -26,12 +26,17 @@ 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) +* [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) + * [[G-1] Fine grained configuration]() + * [[G-2] Strategic merge patch while diffing]() + * [[G-3] Admission Controllers compatibility]() + * [[G-4] Conflict management]() + * [[G-5] Register a proper manager]() * [Non-Goals](#non-goals) * [Proposal](#proposal) * [Non-Functional Requirements](#non-functional-requirements) @@ -41,6 +46,8 @@ describes how ArgoCD can leverage SSA during syncs. * [[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) + * [[R-1] Supported K8s version check]() + * [[R-2] Alternating Server-Side Client-Side syncs]() * [Upgrade / Downgrade](#upgrade--downgrade) * [Drawbacks](#drawbacks) @@ -67,6 +74,8 @@ improvements to consider: - [Fix big CRD][3] sync issues - Better interoperability with different controllers +Kubernetes SSA Proposal ([KEP-555][13]) has more details about how it works. + ## Motivation ArgoCD uses kubectl library while syncing resources in the cluster. Kubectl uses @@ -107,21 +116,37 @@ common issue when syncing CRDs with large schemas. All following goals should be achieve in order to conclude this proposal: +#### [G-1] Fine grained configuration + - 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) +- 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) +- Relates to [ISSUE-2267][6] + +#### [G-2] Strategic merge patch while diffing + - Diffing needs to support strategic merge patch (see [ISSUE-2268][7]) + +#### [G-3] Admission Controllers compatibility + - Allow Admission Controllers to execute even when there is no diff for a particular resource. (Needs investigation) ([more details][2]) + +#### [G-4] Conflict management + - 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)) + users to define the behavior in case of conflicts (see + [Q-1](#q-1-how-to-handle-conflicts) outcome) + +#### [G-5] Register a proper manager + +- 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]) ## Non-Goals @@ -133,12 +158,6 @@ 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: @@ -167,6 +186,9 @@ supports providing multiple options). TBD ### Risks and Mitigations + +#### [R-1] Supported K8s version check + 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 @@ -174,11 +196,26 @@ the target kubernetes cluster is running at least version 1.18. If SSA is enabled and target cluster version < 1.18 ArgoCD should log warning and fallback to client sync. +#### [R-2] Alternating Server-Side Client-Side syncs + +Kubernetes SSA proposal ([KEP-555][13]) mentions about alternating between +server-side and client-side applies in the [Upgrade/Downgrade Strategy][12] +section. It is stated that Kubernetes will verify the incoming apply request +validating if the user-agent is `kubectl` to decide if the +`last-applied-configuration` annotation should be updated. ArgoCD relies on this +annotation and the implementation must make sure that this agent is correctly +informed when changing to server-side apply and specifying a manager different +than `kubectl`. This is mainly to make sure that +[G-5](#g-5-register-a-proper-manager) isn't impacting the +client-side/server-side compatibility. + ### 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/ @@ -192,3 +229,5 @@ Slight increase in ArgoCD code base complexity. [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 +[12]: https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/555-server-side-apply/README.md#upgrade--downgrade-strategy +[13]: https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/555-server-side-apply/README.md From 64fcfb1c8eed7940b6bd05029599fc121a3c51e7 Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Thu, 24 Mar 2022 10:15:29 -0400 Subject: [PATCH 08/10] Update TOC Signed-off-by: Leonardo Luz Almeida --- docs/proposals/server-side-apply.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/docs/proposals/server-side-apply.md b/docs/proposals/server-side-apply.md index eca8aec5e68c6..ac38335128961 100644 --- a/docs/proposals/server-side-apply.md +++ b/docs/proposals/server-side-apply.md @@ -32,11 +32,11 @@ describes how ArgoCD can leverage SSA during syncs. * [Better resource conflict management](#better-resource-conflict-management) * [Better CRD support](#better-crd-support) * [Goals](#goals) - * [[G-1] Fine grained configuration]() - * [[G-2] Strategic merge patch while diffing]() - * [[G-3] Admission Controllers compatibility]() - * [[G-4] Conflict management]() - * [[G-5] Register a proper manager]() + * [[G-1] Fine grained configuration](#g-1-fine-grained-configuration) + * [[G-2] Strategic merge patch while diffing](#g-2-strategic-merge-patch-while-diffing) + * [[G-3] Admission Controllers compatibility](#g-3-admission-controllers-compatibility) + * [[G-4] Conflict management](#g-4-conflict-management) + * [[G-5] Register a proper manager](#g-5-register-a-proper-manager) * [Non-Goals](#non-goals) * [Proposal](#proposal) * [Non-Functional Requirements](#non-functional-requirements) @@ -46,8 +46,8 @@ describes how ArgoCD can leverage SSA during syncs. * [[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) - * [[R-1] Supported K8s version check]() - * [[R-2] Alternating Server-Side Client-Side syncs]() + * [[R-1] Supported K8s version check](#r-1-supported-k8s-version-check) + * [[R-2] Alternating Server-Side Client-Side syncs](#r-2-alternating-server-side-client-side-syncs) * [Upgrade / Downgrade](#upgrade--downgrade) * [Drawbacks](#drawbacks) From 67659804808f70fc0b30c9a55733bb48310b4696 Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Wed, 6 Apr 2022 11:31:29 -0400 Subject: [PATCH 09/10] Add q-1 answer Signed-off-by: Leonardo Luz Almeida --- docs/proposals/server-side-apply.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/proposals/server-side-apply.md b/docs/proposals/server-side-apply.md index ac38335128961..20c5264ea764b 100644 --- a/docs/proposals/server-side-apply.md +++ b/docs/proposals/server-side-apply.md @@ -60,6 +60,10 @@ 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?) +#### Conclusion +The first version should use the force flag and override even if there are +conflicts. We could improve and add other options once there is a use case. + ### [Q-2] Should we support multiple managers? Should Server-Side Apply support in ArgoCD be implemented allowing multiple managers for the same controller? ([more details][10]) From 171769fb723c964d40d3b580e11b51ae226cdcf9 Mon Sep 17 00:00:00 2001 From: Leonardo Luz Almeida Date: Thu, 2 Jun 2022 11:54:56 -0400 Subject: [PATCH 10/10] Add goal to verify correct service patch Signed-off-by: Leonardo Luz Almeida --- docs/proposals/server-side-apply.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/proposals/server-side-apply.md b/docs/proposals/server-side-apply.md index 20c5264ea764b..1fa6c98e313d5 100644 --- a/docs/proposals/server-side-apply.md +++ b/docs/proposals/server-side-apply.md @@ -134,6 +134,7 @@ All following goals should be achieve in order to conclude this proposal: #### [G-2] Strategic merge patch while diffing - Diffing needs to support strategic merge patch (see [ISSUE-2268][7]) +- Make sure Services can be patched correctly ([more details][14]) #### [G-3] Admission Controllers compatibility @@ -235,3 +236,4 @@ Slight increase in ArgoCD code base complexity. [11]: https://github.com/argoproj/gitops-engine/pull/363#issuecomment-1013289982 [12]: https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/555-server-side-apply/README.md#upgrade--downgrade-strategy [13]: https://github.com/kubernetes/enhancements/blob/master/keps/sig-api-machinery/555-server-side-apply/README.md +[14]: https://github.com/argoproj/argo-cd/pull/8812#discussion_r849140565