Fix `kubectl unlabel` response msg #104372

astraw99 · 2021-08-15T07:00:56Z

What type of PR is this?

/kind bug

What this PR does / why we need it:

To distinguish between label and unlabel msg: labeled/unlabeled, just like tainted/untainted.

Which issue(s) this PR fixes:

Does this PR introduce a user-facing change?

NONE

k8s-ci-robot · 2021-08-15T07:01:04Z

Hi @astraw99. Thanks for your PR.

I'm waiting for a kubernetes member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

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.

afirth · 2021-08-17T11:46:15Z

/sig cli
/ok-to-test

rikatz · 2021-08-17T11:54:25Z

/lgtm
I would just take a look into https://github.com/kubernetes/kubernetes/blob/d98989b62a7005be3b28c75f8a52cf8c4b77e75a/staging/src/k8s.io/kubectl/pkg/cmd/label/label_test.go and check if some additional unit test can be added/needs to be changed

Thanks!

astraw99 · 2021-08-20T07:09:20Z

/lgtm
I would just take a look into https://github.com/kubernetes/kubernetes/blob/d98989b62a7005be3b28c75f8a52cf8c4b77e75a/staging/src/k8s.io/kubectl/pkg/cmd/label/label_test.go and check if some additional unit test can be added/needs to be changed

Thanks!

@rikatz Any update on this? Waiting for your update, thanks.

Pingan2017 · 2021-08-20T09:10:22Z

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

@@ -393,6 +393,9 @@ func updateDataChangeMsg(oldObj []byte, newObj []byte) string {
 	msg := "not labeled"
 	if !reflect.DeepEqual(oldObj, newObj) {
 		msg = "labeled"
+		if len(newObj) < len(oldObj) {
+			msg = "unlabeled"


add the label key?
eg: 'labelkey' unlabeled

Currently the kubectl label response is like node/10.0.x.x labeled/unlabeled, it is the same as node/10.0.x.x tainted/untainted.
So if want to add the label key, there need more code change.

agile6v · 2021-08-22T09:18:33Z

Hello @astraw99 @rikatz I think label command can provide more exact response msg.

Like this:

$ kubectl label pod apple-app foo=bar
pod/apple-app labeled

$ kubectl label pod apple-app foo=bar --overwrite
pod/apple-app not labeled

$ kubectl label pod apple-app foo=bar1 --overwrite
pod/apple-app labeled

$ kubectl label pod apple-app foofoo=barbar foo-
pod/apple-app labeled & unlabeled

$ kubectl label pod apple-app foofoo-
pod/apple-app unlabeled

I submitted a PR for this. Hopefully understand it correctly. Thanks.

astraw99 · 2021-09-01T02:20:48Z

@rikatz PTAL thanks.

pacoxu · 2021-09-10T08:45:17Z

/lgtm
/triage accepted
/assign @soltysh
for approval and btw, there is another fix #104502 which did some refactors meanwhile.

astraw99 · 2021-09-19T03:18:45Z

@soltysh PTAL thanks.

rikatz · 2021-09-19T19:30:43Z

/lgtm
I would just take a look into https://github.com/kubernetes/kubernetes/blob/d98989b62a7005be3b28c75f8a52cf8c4b77e75a/staging/src/k8s.io/kubectl/pkg/cmd/label/label_test.go and check if some additional unit test can be added/needs to be changed
Thanks!

@rikatz Any update on this? Waiting for your update, thanks.

Have you added the unit tests?

Thanks!

astraw99 · 2021-09-23T13:22:09Z

@rikatz Unit tests added and passed, PTAL thanks.

astraw99 · 2021-09-30T02:28:11Z

@soltysh @rikatz PTAL thanks.

astraw99 · 2021-10-08T13:02:47Z

@soltysh @rikatz PTAL thanks.

astraw99 · 2021-10-25T03:27:31Z

@soltysh @rikatz PTAL thanks.

rikatz · 2021-10-25T14:04:38Z

/lgtm

Thank you

soltysh

/priority backlog
/approve

k8s-ci-robot · 2021-11-03T12:49:51Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: astraw99, soltysh

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/kubectl/OWNERS~~ [soltysh]

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

afirth · 2021-11-03T15:40:25Z

This looks like a flake? Don't see an open issue but will make one if it passes

W1103 14:02:20.750] Could not detect Kubernetes master node.  Make sure you've launched a cluster with 'kube-up.sh'
I1103 14:02:20.851] Master not detected. Is the cluster up?

and testgrid
/retest

k8s-triage-robot · 2021-11-03T19:35:03Z

The Kubernetes project has merge-blocking tests that are currently too flaky to consistently pass.

This bot retests PRs for certain kubernetes repos according to the following rules:

The PR does have any do-not-merge/* labels
The PR does not have the needs-ok-to-test label
The PR is mergeable (does not have a needs-rebase label)
The PR is approved (has cncf-cla: yes, lgtm, approved labels)
The PR is failing tests required for merge

You can:

Review the full test history for this PR
Prevent this bot from retesting with /lgtm cancel or /hold
Help make our tests less flaky by following our Flaky Tests Guide

/retest

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

k8s-ci-robot added needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Aug 15, 2021

k8s-ci-robot requested review from rikatz and soltysh August 15, 2021 07:01

k8s-ci-robot added area/kubectl sig/cli Categorizes an issue or PR as relevant to SIG CLI. and removed do-not-merge/needs-sig Indicates an issue or PR lacks a `sig/foo` label and requires one. labels Aug 15, 2021

k8s-ci-robot added ok-to-test Indicates a non-member PR verified by an org member that is safe to test. and removed needs-ok-to-test Indicates a PR that requires an org member to verify it is safe to test. labels Aug 17, 2021

k8s-ci-robot assigned rikatz Aug 17, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 17, 2021

Pingan2017 reviewed Aug 20, 2021

View reviewed changes

pacoxu added this to Needs Approver in SIG CLI Aug 24, 2021

k8s-ci-robot assigned soltysh Sep 10, 2021

k8s-ci-robot added triage/accepted Indicates an issue or PR is ready to be actively worked on. and removed needs-triage Indicates an issue or PR lacks a `triage/foo` label and requires one. labels Sep 10, 2021

k8s-ci-robot assigned pacoxu Sep 10, 2021

pacoxu mentioned this pull request Sep 10, 2021

Make label comamnd's response msg more exact. #104502

Closed

fix kubectl unlabel msg

cec30fd

astraw99 force-pushed the fix_label_msg branch from d98989b to cec30fd Compare September 23, 2021 13:14

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed lgtm "Looks good to me", indicates that a PR is ready to be merged. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Sep 23, 2021

k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 25, 2021

soltysh approved these changes Nov 3, 2021

View reviewed changes

k8s-ci-robot added priority/backlog Higher priority than priority/awaiting-more-evidence. and removed needs-priority Indicates a PR lacks a `priority/foo` label and requires one. labels Nov 3, 2021

k8s-ci-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Nov 3, 2021

k8s-ci-robot merged commit a65bb64 into kubernetes:master Nov 3, 2021

SIG CLI automation moved this from Needs Approver to Done Nov 3, 2021

k8s-ci-robot added this to the v1.23 milestone Nov 3, 2021

zigarn mentioned this pull request Jan 12, 2022

Do not raise an error proposing to use '--overwrite' when labeling with the same value #105936

Merged

soltysh reviewed Jan 19, 2022

View reviewed changes

staging/src/k8s.io/kubectl/pkg/cmd/label/label.go Show resolved Hide resolved

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix `kubectl unlabel` response msg #104372

Fix `kubectl unlabel` response msg #104372

astraw99 commented Aug 15, 2021

k8s-ci-robot commented Aug 15, 2021

afirth commented Aug 17, 2021

rikatz commented Aug 17, 2021

astraw99 commented Aug 20, 2021

Pingan2017 Aug 20, 2021 •

edited

Loading

astraw99 Aug 20, 2021

agile6v commented Aug 22, 2021

astraw99 commented Sep 1, 2021

pacoxu commented Sep 10, 2021 •

edited

Loading

astraw99 commented Sep 19, 2021

rikatz commented Sep 19, 2021

astraw99 commented Sep 23, 2021

astraw99 commented Sep 30, 2021

astraw99 commented Oct 8, 2021

astraw99 commented Oct 25, 2021

rikatz commented Oct 25, 2021

soltysh left a comment

k8s-ci-robot commented Nov 3, 2021

afirth commented Nov 3, 2021

k8s-triage-robot commented Nov 3, 2021

Fix kubectl unlabel response msg #104372

Fix kubectl unlabel response msg #104372

Conversation

astraw99 commented Aug 15, 2021

What type of PR is this?

What this PR does / why we need it:

Which issue(s) this PR fixes:

Does this PR introduce a user-facing change?

k8s-ci-robot commented Aug 15, 2021

afirth commented Aug 17, 2021

rikatz commented Aug 17, 2021

astraw99 commented Aug 20, 2021

Pingan2017 Aug 20, 2021 • edited Loading

Choose a reason for hiding this comment

astraw99 Aug 20, 2021

Choose a reason for hiding this comment

agile6v commented Aug 22, 2021

astraw99 commented Sep 1, 2021

pacoxu commented Sep 10, 2021 • edited Loading

astraw99 commented Sep 19, 2021

rikatz commented Sep 19, 2021

astraw99 commented Sep 23, 2021

astraw99 commented Sep 30, 2021

astraw99 commented Oct 8, 2021

astraw99 commented Oct 25, 2021

rikatz commented Oct 25, 2021

soltysh left a comment

Choose a reason for hiding this comment

k8s-ci-robot commented Nov 3, 2021

afirth commented Nov 3, 2021

k8s-triage-robot commented Nov 3, 2021

Fix `kubectl unlabel` response msg #104372

Fix `kubectl unlabel` response msg #104372

Pingan2017 Aug 20, 2021 •

edited

Loading

pacoxu commented Sep 10, 2021 •

edited

Loading