Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

kubectl: don't display an empty list when trying to get a single resource that isn't found #28294

Merged
merged 1 commit into from
Jul 1, 2016
Merged

kubectl: don't display an empty list when trying to get a single resource that isn't found #28294

merged 1 commit into from
Jul 1, 2016

Conversation

ncdc
Copy link
Member

@ncdc ncdc commented Jun 30, 2016

Return immediately when attempting to get a singular resource that isn't found, so that we avoid
printing out a List if the output format is something like json or yaml.

Before:

$ kubectl get pod/foo -o yaml
apiVersion: v1
items: []
kind: List
metadata: {}
pods "foo" not found

After:

$ kubectl get pod/foo -o yaml
pods "foo" not found

Fixes #28243

@kubernetes/kubectl @kubernetes/rh-ux @smarterclayton @liggitt @deads2k @metral

Return immediately for singular get on error
67d929a 
Return immediately when attempting to get a singular resource that isn't found, so that we avoid
printing out a List if the output format is something like json or yaml.

Before:

```
$ kubectl get pod/foo -o yaml
apiVersion: v1
items: []
kind: List
metadata: {}
pods "foo" not found
```

After:

```
$ kubectl get pod/foo -o yaml
pods "foo" not found
```
@ncdc
Copy link
Member Author

ncdc commented Jun 30, 2016

This restores the v1.2 behavior (v1.3 changed it). I'm guessing this warrants a release note?

@k8s-github-robot k8s-github-robot added size/S Denotes a PR that changes 10-29 lines, ignoring generated files. release-note-label-needed labels Jun 30, 2016
@metral
Copy link
Contributor

metral commented Jun 30, 2016

I submitted a fix yesterday, in PR #28251

The fix can go either way, by using singular like @ncdc has done, or checking infos as in #28251

This PR does have the missing test cases - I'm fine with either, just so long as the fix makes it in

@metral metral mentioned this pull request Jun 30, 2016
Closed
@ncdc
Copy link
Member Author

ncdc commented Jun 30, 2016

Sorry @metral I didn't realize you had opened a fix for this already

@k8s-bot
Copy link

k8s-bot commented Jun 30, 2016

GCE e2e build/test passed for commit 67d929a.

@ncdc
Copy link
Member Author

ncdc commented Jun 30, 2016

Jenkins verification failed b/c it was unable to install etcd.

@k8s-bot test this issue: #IGNORE

@k8s-bot
Copy link

k8s-bot commented Jun 30, 2016

GCE e2e build/test failed for commit 67d929a.

Please reference the list of currently known flakes when examining this failure. If you request a re-test, you must reference the issue describing the flake.

@ncdc
Copy link
Member Author

ncdc commented Jun 30, 2016

FATAL: Could not checkout 372e4b98ad72ea1b3051dff471c1c011b40007f8 ???

@k8s-bot e2e test this issue: #IGNORE

@metral metral mentioned this pull request Jun 30, 2016
Closed
@k8s-bot
Copy link

k8s-bot commented Jun 30, 2016

GCE e2e build/test passed for commit 67d929a.

@smarterclayton
Copy link
Contributor

Singular is supposed to represent the check (infos may return a single
item).

A user who asks for kubectl get pod/foo and no foo exists should get an
error. A user who asks for kubectl get pods and gets no pods should not
get an error.

On Thu, Jun 30, 2016 at 2:39 PM, Kubernetes Bot notifications@github.com
wrote:

GCE e2e build/test passed for commit 67d929a
67d929a
.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#28294 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ABG_p_c-MM1rVx-c7tCqK54YnT_ou5evks5qRDefgaJpZM4JCWMC
.

@metral
Copy link
Contributor

metral commented Jul 1, 2016

Per @smarterclayton's advice above and in #28243 (comment), this PR should be used for the fix.

I'll go ahead and close out the dup over at #28251

@ncdc
Copy link
Member Author

ncdc commented Jul 1, 2016

If this makes it in before 1.3.0 is cut, we don't need a release note. If it goes in 1.3.1, then we probably need one.

@ncdc ncdc added this to the v1.3 milestone Jul 1, 2016
@ncdc
Copy link
Member Author

ncdc commented Jul 1, 2016

@smarterclayton @erictune @bgrant0607 @janetkuo @goltermann bump for review & decision if this is 1.3.0 or 1.3.1. Thanks!

@erictune
Copy link
Member

erictune commented Jul 1, 2016

This is not 1.3.0

On Jul 1, 2016 7:00 AM, "Andy Goldstein" notifications@github.com wrote:

@smarterclayton https://github.com/smarterclayton @erictune
https://github.com/erictune @bgrant0607 https://github.com/bgrant0607
@janetkuo https://github.com/janetkuo @goltermann
https://github.com/goltermann bump for review & decision if this is
1.3.0 or 1.3.1. Thanks!

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
#28294 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AHuudrIhxJUjggGIf-BfjWsgpQJHd__pks5qRR1YgaJpZM4JCWMC
.

@goltermann
Copy link
Contributor

Yes, agreed, and let's definitely release note it for 1.3.0.

@ncdc
Copy link
Member Author

ncdc commented Jul 1, 2016

K, typing up a release note now.

@ncdc ncdc changed the title Return immediately for singular get on error Don't display an empty list when trying to get a single resource that isn't found Jul 1, 2016
@ncdc ncdc added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Jul 1, 2016
@ncdc ncdc changed the title Don't display an empty list when trying to get a single resource that isn't found kubectl: don't display an empty list when trying to get a single resource that isn't found Jul 1, 2016
@ncdc
Copy link
Member Author

ncdc commented Jul 1, 2016

I updated the title of the PR and set the release-note label

@janetkuo
Copy link
Member

janetkuo commented Jul 1, 2016

I tested it locally and it still gives me:

$ kubectl get pod/abc -o yaml 
apiVersion: v1
items: []
kind: List
metadata: {}
pods "abc" not found

Would you confirm it?

@ncdc
Copy link
Member Author

ncdc commented Jul 1, 2016

Testing now

@ncdc
Copy link
Member Author

ncdc commented Jul 1, 2016

Weird, this worked when I wrote it... But I'm now seeing the same thing as you. Looking into it.

@ncdc
Copy link
Member Author

ncdc commented Jul 1, 2016

Oh wait, I was running the wrong kubectl binary. Make sure you're running the kubectl build from this branch 😄 It works fine for me.

@janetkuo
Copy link
Member

janetkuo commented Jul 1, 2016

@ncdc ah-ha, yes, I was running the wrong kubectl binary. This LGTM now.

@janetkuo janetkuo added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Jul 1, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented Jul 1, 2016

GCE e2e build/test passed for commit 67d929a.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 1e33d11 into kubernetes:master Jul 1, 2016
@smarterclayton smarterclayton mentioned this pull request Jul 13, 2016
Closed
@fabioy
Copy link
Contributor

fabioy commented Jul 19, 2016

@ncdc Cherrypick approved. Please create a cherrypick PR. Thanks.

@fabioy fabioy added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Jul 19, 2016
@ncdc ncdc mentioned this pull request Jul 21, 2016
Merged
k8s-github-robot pushed a commit that referenced this pull request Jul 21, 2016
Merge pull request #29382 from ncdc/automated-cherry-pick-of-#28294-u…
8aa9a0d 
…pstream-release-1.3

Automatic merge from submit-queue

Automated cherry pick of #28294

Cherry pick of #28294 on release-1.3.
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.3" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016
Merge pull request kubernetes#29382 from ncdc/automated-cherry-pick-o…
527483f 
…f-#28294-upstream-release-1.3

Automatic merge from submit-queue

Automated cherry pick of kubernetes#28294

Cherry pick of kubernetes#28294 on release-1.3.
@ncdc ncdc deleted the return-error-for-kubectl-get-singluar branch February 13, 2017 17:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@janetkuo janetkuo

Labels
cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/S Denotes a PR that changes 10-29 lines, ignoring generated files.
Projects
None yet
Milestone
v1.3
Development

Successfully merging this pull request may close these issues.