Improve kubectl field manager names for clearer conflicts #88885

julianvmodesto · 2020-03-06T02:20:33Z

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

This change improves the kubectl field manager names for clearer conflicts, instead of the default .manager=kubectl.

When an object is managed by server-side apply and has field managers, any kubectl commands will use the same field manager: kubectl. When there is a conflict, then this is confusing because different operations will have the same field manager name. Instead, we can set kubectl subcommands as the field manager name e.g. the manager for kubectl annotate should be kubectl-annotate. This provides a better conflict hint.

For kubectl commands, this change sets the field manager accordingly:

kubectl apply has .manager=kubectl-client-side-apply
kubectl apply --server-side has .manager=kubectl (remains kubectl instead of kubectl-server-side-apply for backwards compatibility)
kubectl annotate has .manager=kubectl-annotate

List of kubectl subcommands updated:

Which issue(s) this PR fixes:

Related to #80916

Does this PR introduce a user-facing change?:

Improve server-side apply conflict errors by setting dedicated kubectl subcommand field managers

julianvmodesto · 2020-03-06T02:21:32Z

/wg apply
cc @seans3
/assign @apelisse
/priority important-soon

julianvmodesto · 2020-03-06T02:21:48Z

/sig cli

julianvmodesto · 2020-03-06T02:22:52Z

How does this look? We would add --field-manager to more of the imperative commands where this will be helpful

apelisse

All of this looks great to me (I haven't looked in details). I think this is the right approach, but I suspect we should get approval of sig-cli, maybe through a apply kep update?

staging/src/k8s.io/cli-runtime/pkg/resource/helper.go

staging/src/k8s.io/kubectl/pkg/cmd/util/helpers.go

liggitt · 2020-03-08T03:15:33Z

staging/src/k8s.io/kubectl/pkg/cmd/apply/apply.go

@@ -223,6 +223,13 @@ func (o *ApplyOptions) Complete(f cmdutil.Factory, cmd *cobra.Command) error {
 	}
 	o.DryRunVerifier = resource.NewDryRunVerifier(o.DynamicClient, discoveryClient)
 	o.FieldManager = cmdutil.GetFieldManagerFlag(cmd)
+	if !cmd.Flag("field-manager").Changed {
+		if o.ServerSideApply {
+			o.FieldManager = "kubectl-server-side-apply"


does changing the default mean the same kubectl serverside apply operation in subsequent releases would report a conflict with itself?

Yes it does, that's a good point. We probably can't change the "server-side apply" default manager now, kubectl is still a fine name though.

Ohh I see. Updated to kubectl and added a note about why

julianvmodesto · 2020-03-19T22:49:09Z

Okay i'll make a small update to the KEP so we can get feedback from CLI too, and proceed to inventory all of the commands that need to be updated

apelisse · 2020-05-11T15:58:07Z

/lgtm
/assign @seans3

apelisse · 2020-05-11T16:03:32Z

/retest

seans3 · 2020-05-13T20:31:25Z

/retest

seans3 · 2020-05-13T20:33:19Z

/lgtm
/approve

k8s-ci-robot · 2020-05-13T20:34:56Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: julianvmodesto, seans3

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:

~~staging/src/k8s.io/cli-runtime/OWNERS~~ [seans3]
~~staging/src/k8s.io/kubectl/OWNERS~~ [seans3]
~~test/cmd/OWNERS~~ [seans3]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

fejta-bot · 2020-05-14T01:12:42Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2020-05-14T06:48:43Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2020-05-14T10:18:44Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2020-05-14T13:48:40Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

julianvmodesto · 2020-05-14T14:51:41Z

Looks like a flake: #91075

/retest

fejta-bot · 2020-05-14T18:00:40Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2020-05-14T21:09:30Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2020-05-15T00:18:32Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

fejta-bot · 2020-05-15T03:48:48Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

k8s-ci-robot · 2020-05-15T04:45:21Z

@julianvmodesto: The following test failed, say /retest to rerun all failed tests:

Test name	Commit	Details	Rerun command
pull-kubernetes-node-e2e-containerd	d5cc92abfd40ddc63981ed32a173019f46a83f56	link	`/test pull-kubernetes-node-e2e-containerd`

Full PR test history. Your PR dashboard. Please help us cut down on flakes by linking to an open issue when you hit one in your PR.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here.

fejta-bot · 2020-05-15T06:57:23Z

/retest
This bot automatically retries jobs that failed/flaked on approved PRs (send feedback to fejta).

Review the full test history for this PR.

Silence the bot with an /lgtm cancel or /hold comment for consistent failures.

remove unnecessary function after PR #88885 merged

k8s-ci-robot assigned apelisse Mar 6, 2020

k8s-ci-robot requested review from adohe-zz and deads2k March 6, 2020 02:21

k8s-ci-robot added sig/cli Categorizes an issue or PR as relevant to SIG CLI. area/kubectl area/test sig/testing Categorizes an issue or PR as relevant to SIG Testing. labels Mar 6, 2020

apelisse reviewed Mar 6, 2020

View reviewed changes

staging/src/k8s.io/cli-runtime/pkg/resource/helper.go Show resolved Hide resolved

staging/src/k8s.io/cli-runtime/pkg/resource/helper.go Show resolved Hide resolved

staging/src/k8s.io/kubectl/pkg/cmd/util/helpers.go Outdated Show resolved Hide resolved

liggitt reviewed Mar 8, 2020

View reviewed changes

julianvmodesto force-pushed the kubectl-field-managers branch 2 times, most recently from de80201 to 0d5af2b Compare March 19, 2020 22:47

julianvmodesto force-pushed the kubectl-field-managers branch from 96250e3 to 3435626 Compare March 25, 2020 19:40

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Mar 25, 2020

julianvmodesto force-pushed the kubectl-field-managers branch 2 times, most recently from f97e078 to d035f84 Compare March 25, 2020 19:42

julianvmodesto force-pushed the kubectl-field-managers branch from e1c2f12 to 360c348 Compare May 9, 2020 17:24

k8s-ci-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 9, 2020

k8s-ci-robot assigned seans3 May 11, 2020

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 11, 2020

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label May 13, 2020

k8s-ci-robot merged commit 53a707d into kubernetes:master May 15, 2020

k8s-ci-robot added this to the v1.19 milestone May 15, 2020

github-actions bot mentioned this pull request May 27, 2020

Week Ending May 17, 2020 dev-obs/actus#156

Closed

CKchen0726 added a commit to CKchen0726/kubernetes that referenced this pull request Jul 12, 2021

remove unnecessary function after PR kubernetes#88885 merged

d4a0152

CKchen0726 mentioned this pull request Jul 12, 2021

remove unnecessary function after PR #88885 merged #103634

Merged

k8s-ci-robot added a commit that referenced this pull request Nov 8, 2021

Merge pull request #103634 from CKchen0726/remove_unnecessary_code

67a7d40

remove unnecessary function after PR #88885 merged

alexzielenski mentioned this pull request Dec 12, 2022

Server-side apply: migration from client-side apply leaves stuck fields in the object #99003

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve kubectl field manager names for clearer conflicts #88885

Improve kubectl field manager names for clearer conflicts #88885

julianvmodesto commented Mar 6, 2020 •

edited

julianvmodesto commented Mar 6, 2020

julianvmodesto commented Mar 6, 2020

julianvmodesto commented Mar 6, 2020

apelisse left a comment

liggitt Mar 8, 2020

apelisse Mar 9, 2020

julianvmodesto Mar 19, 2020

julianvmodesto commented Mar 19, 2020

apelisse commented May 11, 2020

apelisse commented May 11, 2020

seans3 commented May 13, 2020

seans3 commented May 13, 2020

k8s-ci-robot commented May 13, 2020

fejta-bot commented May 14, 2020

fejta-bot commented May 14, 2020

fejta-bot commented May 14, 2020

fejta-bot commented May 14, 2020

julianvmodesto commented May 14, 2020

fejta-bot commented May 14, 2020

fejta-bot commented May 14, 2020

fejta-bot commented May 15, 2020

fejta-bot commented May 15, 2020

k8s-ci-robot commented May 15, 2020 •

edited

fejta-bot commented May 15, 2020

Improve kubectl field manager names for clearer conflicts #88885

Improve kubectl field manager names for clearer conflicts #88885

Conversation

julianvmodesto commented Mar 6, 2020 • edited

julianvmodesto commented Mar 6, 2020

julianvmodesto commented Mar 6, 2020

julianvmodesto commented Mar 6, 2020

apelisse left a comment

Choose a reason for hiding this comment

liggitt Mar 8, 2020

Choose a reason for hiding this comment

apelisse Mar 9, 2020

Choose a reason for hiding this comment

julianvmodesto Mar 19, 2020

Choose a reason for hiding this comment

julianvmodesto commented Mar 19, 2020

apelisse commented May 11, 2020

apelisse commented May 11, 2020

seans3 commented May 13, 2020

seans3 commented May 13, 2020

k8s-ci-robot commented May 13, 2020

fejta-bot commented May 14, 2020

fejta-bot commented May 14, 2020

fejta-bot commented May 14, 2020

fejta-bot commented May 14, 2020

julianvmodesto commented May 14, 2020

fejta-bot commented May 14, 2020

fejta-bot commented May 14, 2020

fejta-bot commented May 15, 2020

fejta-bot commented May 15, 2020

k8s-ci-robot commented May 15, 2020 • edited

fejta-bot commented May 15, 2020

julianvmodesto commented Mar 6, 2020 •

edited

k8s-ci-robot commented May 15, 2020 •

edited