feat: Enable server-side-apply on direct applier #399
Conversation
k8s-ci-robot
added
cncf-cla: yes
k8s-ci-robot
added
size/L
| @@ -170,6 +170,7 @@ func (d *DirectApplier) Apply(ctx context.Context, opt ApplierOptions) error { | |||
| applyOpts.PruneResources = append(applyOpts.PruneResources, r...) | |||
| } | |||
|
|
|||
| applyOpts.ServerSideApply = true | |||
There was a problem hiding this comment.
Should this maybe be behind a feature flag, to allow users to choose when to enable this behavior? If so, could probably be done in the set of flags parsed some 20-ish lines above this one.
(I'm all for it, and would probably enable it first thing, but given that transitioning from CSA to SSA is not always straightforward, it might be worth giving the user a choice here; if nothing else, it enables bumping this library for other reasons without being forced to also manage the transition.)
There was a problem hiding this comment.
Thanks for the review @tomasaschan and sorry for the late reply. Changing this to a feature flag makes sense! I changed it as a feature specific to the direct applier rather than another field in ApplyOption (interface level), because we cannot guarantee every implemented applier can support SSA.
Also, since the SSA is a boolean type, to avoid confusing between unset vs default to false, I used a preferred option to distinguish whether it is configured or not.
k8s-ci-robot
added
the
needs-rebase
This will fix the "annotation too long" problem when the managed manifests are too large.
k8s-ci-robot
added
size/S
| inner directApplierSite | ||
| } | ||
|
|
||
| func (d *DirectApplier) UseServerSideApply() { |
There was a problem hiding this comment.
Use a dedicated function to make the flag more discoverable.
k8s-ci-robot
added
the
approved
|
/approve |
k8s-ci-robot
added
the
lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: justinsb, yuwenma The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
This will fix the "annotation too long" problem when the managed manifests are too large.
What this PR does / why we need it:
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Additional documentation: