Bug 1273816 - Incorrect message when delete pod's label
Summary: Incorrect message when delete pod's label
Keywords:
Status: CLOSED CURRENTRELEASE
Alias: None
Product: OKD
Classification: Red Hat
Component: oc
Version: 3.x
Hardware: Unspecified
OS: Unspecified
medium
low
Target Milestone: ---
: ---
Assignee: Jakub Hadvig
QA Contact: Xingxing Xia
URL:
Whiteboard:
Depends On:
Blocks:
TreeView+ depends on / blocked
 
Reported: 2015-10-21 10:04 UTC by Qixuan Wang
Modified: 2017-05-30 12:51 UTC (History)
6 users (show)

Fixed In Version:
Doc Type: Bug Fix
Doc Text:
Clone Of:
Environment:
Last Closed: 2017-05-30 12:51:03 UTC
Target Upstream Version:
Embargoed:


Attachments (Terms of Use)

Description Qixuan Wang 2015-10-21 10:04:59 UTC
Description of problem:
When delete a label with command "oc label pod/$pod_name $label-", the message shows "pod xxx labeled". It should be "label deleted" or "unmarked"... 

Version-Release number of selected component (if applicable):
devenv-fedora_2505(openshift v1.0.6-803-g7c12c7a, kubernetes v1.2.0-alpha.1-1107-g4c8e6f4, etcd 2.1.2)

How reproducible:
Always

Steps to Reproduce:
1. Create a pod with labels
# cat dapi-volume.yaml
apiVersion: v1
kind: Pod
metadata:
  name: kubernetes-downwardapi-volume-example
  labels:
    zone: us-est-coast
    cluster: test-cluster1
  annotations:
    build: two
    builder: john-doe
spec:
  containers:
    - name: client-container
      image: gcr.io/google_containers/busybox
      command: ["sh", "-c", "while true; do if [[ -e /etc/labels ]]; then cat /etc/labels; fi; if [[ -e /etc/annotations ]]; then cat /etc/annotations; fi; sleep 5; done"]
      volumeMounts:
        - name: podinfo
          mountPath: /etc
          readOnly: false
  volumes:
    - name: podinfo
      downwardAPI:
        items:
          - path: "labels"
            fieldRef:
              fieldPath: metadata.labels
          - path: "annotations"
            fieldRef:
              fieldPath: metadata.annotations

# oc create -f dapi-volume.yaml

2. Check the label
# oc describe pod kubernetes-downwardapi-volume-example | grep Labels

3. Delete the lable
# oc label pod/kubernetes-downwardapi-volume-example zone-

4. Check the label again


Actual results:
3. [root@ip-172-18-3-115 work]# oc label pod/kubernetes-downwardapi-volume-example zone-
pod "kubernetes-downwardapi-volume-example" labeled


Expected results:
3. It should show pod "xxx" unmarked or something else.

Additional info:

Comment 1 Jakub Hadvig 2015-12-02 13:21:42 UTC
The proposed fix for this bug was:
When removing label thats not present in the pod, kubectl will respond with:
  $ cluster/kubectl.sh pod/redis-slave-0ooo8 test1-
  label test1 not found.
  pod redis-slave-0ooo8 not labeled

This issue has been resolved and merged into upstream:
https://github.com/kubernetes/kubernetes/pull/17946

Comment 2 Fabiano Franz 2016-01-08 20:18:55 UTC
Was the upstream commit also added to Origin Godeps? If not we need to do so, and once it's merged in Origin put this bug ON_QA so they can test it, thanks.

Comment 3 Jakub Hadvig 2016-01-11 09:04:16 UTC
The change is incorporated in the k8s rebase. Will track the rebase and once it lands will put this issue on ON_QA.

Comment 4 Qixuan Wang 2016-02-16 05:06:01 UTC
Tested on devenv-rhel_3433 (openshift v1.1.2-274-g6187dc3, kubernetes v1.2.0-origin, etcd 2.2.2+git)

"Deleting undefined labels from the pod" has been fixed, but there are two things still misleading.
1. Deleting defined labels, the return message is "xxx labeled". Does it mean the pod has labels before or the process is labeling the pod and then it's done?
2. If the pod has more than one label and after deleting one, there are other labels left, the return message should not be "pod xxx not labeled"

Some steps:
1. Create pods with labels
# oc create -f origin/examples/deployment/recreate-example.yaml

2. Check labels
# oc describe pod recreate-example-1-8i81m | grep Label
Labels:		deployment=recreate-example-1,deploymentconfig=recreate-example

3. Delete undefined labels
# oc label pod/recreate-example-1-8i81m d-
label "d" not found.  (That's correct)
pod "recreate-example-1-8i81m" not labeled   (Is it appropriate? Labels "deployment" and "deploymentconfig" exist)

4. Delete defined labels
# oc label pod/recreate-example-1-8i81m deployment-
pod "recreate-example-1-8i81m" labeled   (Is it appropriate?)

5. Check labels again, one label is deleted
# oc describe pod recreate-example-1-8i81m | grep Label
Labels:		deploymentconfig=recreate-example

6. Delete the same label
# oc label pod/recreate-example-1-8i81m deployment-
label "deployment" not found.   (That's correct)
pod "recreate-example-1-8i81m" not labeled    (Pod has a label "deploymentconfig")

Comment 5 Paul Weil 2017-02-16 21:50:41 UTC
I am going to make the case that this is the correct behavior.  The label command allows you to pass multiple labels and in that scenario some may be deletions, some may be additions, and some may be modifications.  In all cases the pod is "labeled" correctly and trying to differentiate only works on a single label case.

example: oc label pod/kubernetes-downwardapi-volume-example zone- cluster=foo --overwrite

Comment 6 Qixuan Wang 2017-02-20 10:24:45 UTC
Tested on OpenShift Origin build devenv-rhel7_5940(openshift v1.5.0-alpha.2+cf7e336-633, kubernetes v1.5.2+43a9be4, etcd 3.1.0)

As a non-native speaker, I was puzzled by "pod "xxx" not labeled". From comment 5, I think "labeled" here is a passive voice of phrasal verb instead of adjective. The following steps I commented my understanding and doubt, could you help review? Thanks.
 
# oc create -f https://raw.githubusercontent.com/openshift/origin/master/examples/deployment/recreate-example.yaml

# oc describe pod recreate-example-1-bdjhj
Name:			recreate-example-1-bdjhj
Namespace:		test
Security Policy:	restricted 
Node:			ip-172-18-4-148.ec2.internal/172.18.4.148
Start Time: 		Mon, 20 Feb 2017 16:40:17 +0800
Labels: 		deployment=recreate-example-1
			deploymentconfig=recreate-example
Status:			Running

1. Delete a nonexistent label and keep other labels.
# oc label pod/recreate-example-1-bdjhj d-
label "d" not found.   (Correct)
pod "recreate-example-1-bdjhj" not labeled   (Not labeled by "d" before. But it's labeled by others. Deletion is for a single label "d", so the message is "not labeled".)

2. Delete one of the existing labels.
# oc label pod/recreate-example-1-bdjhj deployment-
pod "recreate-example-1-bdjhj" labeled   (labeled by "deployment" before. Now it's deleted or not? Label "deploymentconfig" is kept. Deletion is for a single label "deployment".)

3. Delete one existing label. Meanwhile add a new one.
# oc label pod/recreate-example-1-bdjhj deploymentconfig- cluster=foo --overwrite
pod "recreate-example-1-bdjhj" labeled    (Labeled by "deploymentconfig" before. Now it's deleted or not? New label "cluster" is added. Miscellaneous operation, so "labeled" in short.)

4. Delete a nonexistent label. Meanwhile add a new one.
# oc label pod/recreate-example-1-bdjhj d- zone=bar --overwrite
label "d" not found.   (Correct)
pod "recreate-example-1-bdjhj" labeled    (Labeled by "cluster" and not labeled by "d" before. New label "zone" is added. Miscellaneous operation, so "labeled" in short.)

5. Delete all of the existing label.
# oc label pod/recreate-example-1-bdjhj cluster- zone-
pod "recreate-example-1-bdjhj" labeled   (labeled by those labels before. Multiple deletion, so "labeled" in short. However, there is no label left, the message is sort of weird if users don't know the logic rules.)

6. Delete a nonexistent label
# oc describe pod recreate-example-1-bdjhj | grep Label
Labels:			<none>
# oc label pod/recreate-example-1-bdjhj d-
label "d" not found.   (Correct)
pod "recreate-example-1-bdjhj" not labeled   (Correct. Not labeled by "d". It's a single label deletion.)

Comment 7 Paul Weil 2017-02-20 14:04:07 UTC
I think the only thing to really make it more clear would be a message based on each label that is passed in.  

Fabiano, do you want to make a call here on if it's reasonable for usability or if this current behavior is as far as we want to go?

Comment 8 Fabiano Franz 2017-02-20 20:52:03 UTC
The "labeled" or "not labeled" message does not refer to the object's initial or final state, but to the labeling action being performed or not. 

So if any change in the object's labels (not matter if inclusion, change, or deletion) was performed then it was labeled, otherwise it wasn't. I think that, along with the "not found" message we added as part of the fix here, is good enough.


Note You need to log in before you can comment on or make changes to this bug.