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.

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

Fix kubectl explain #15808

Merged

Conversation

caesarxuchao
Copy link
Member

fix #15235
part of #15511
The first commit is #15659. Please only review the second commit.

With this PR, kubectl explain will accept resources in the extensions group, user can either call kubectl explain extensions/hpa or kubectl explain hpa.

Caveats: --api-version is not working properly with non-v1 groupVersion. This is a problem for all kubectl commands, so I don't plan to address it in this PR.

@kubernetes/goog-ux @lavalamp

@k8s-github-robot
Copy link

Labelling this PR as size/L

@k8s-github-robot k8s-github-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Oct 17, 2015
@k8s-bot
Copy link

k8s-bot commented Oct 17, 2015

GCE e2e test build/test passed for commit fe40a7a87cc332d125a2ca73a87fc80e6cc80612.

@smarterclayton
Copy link
Contributor

This is not using a significant chunk of our generic rest client helper, which means it won't benefit from the standard work we do there. This needs to at minimum use a RESTClient and a request.go to fetch the provided resources, instead of performing http.client calls directly.

@caesarxuchao
Copy link
Member Author

@smarterclayton I copy-pasted your comment to #15659

@k8s-github-robot
Copy link

Labelling this PR as size/M

@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Oct 19, 2015
@k8s-bot
Copy link

k8s-bot commented Oct 19, 2015

GCE e2e build/test failed for commit 6f0f87b.

@caesarxuchao
Copy link
Member Author

e2e is a flake:

/go/src/k8s.io/kubernetes/_output/dockerized/go/src/k8s.io/kubernetes/test/e2e/pods.go:861
Oct 19 10:42:11.980: back-off gap is not increasing exponentially pod=pod-back-off-exponentially/back-off delay1=1m31.000000001s delay2=1m27s

@caesarxuchao
Copy link
Member Author

@k8s-bot test this please

@k8s-bot
Copy link

k8s-bot commented Oct 19, 2015

GCE e2e test build/test passed for commit 6f0f87b.

@janetkuo
Copy link
Member

Ref #15874. If we're replacing group/type/name syntax with type.group/name, how do we explain resources in groups?

kubectl explain extensions/hpa.spec becomes kubectl explain hpa.extensions.spec? cc @smarterclayton

@smarterclayton
Copy link
Contributor

Hpa.extensions.spec should probably be type/fieldpath
(hpa.extensions/spec), with some unique separator. I don't think type and
field selector should be combined in a way that overlaps with our other
syntax (more

On Oct 19, 2015, at 6:39 PM, Janet Kuo notifications@github.com wrote:

Ref #15874 #15874. If we're
replacing group/type/name syntax with type.group/name, how do we explain
resources in different groups?

kubectl explain extensions/hpa.spec becomes kubectl explain
hpa.extensions.spec? cc @smarterclayton https://github.com/smarterclayton


Reply to this email directly or view it on GitHub
#15808 (comment)
.

@caesarxuchao
Copy link
Member Author

@janetkuo can we refactor the expected format of the input in another PR? I need this PR to unblock some other work. Thank you.

@janetkuo
Copy link
Member

Sure.

@smarterclayton
Copy link
Contributor

Yes we can

On Tue, Oct 20, 2015 at 2:02 PM, Chao Xu notifications@github.com wrote:

@janetkuo https://github.com/JanetKuo can we refactor the expected
format of the input in another PR? I need this PR to unblock some other
work. Thank you.


Reply to this email directly or view it on GitHub
#15808 (comment)
.

@janetkuo janetkuo added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Oct 20, 2015
@janetkuo
Copy link
Member

Please add some tests in the other PR too.

@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 Oct 21, 2015

GCE e2e build/test failed for commit 6f0f87b.

@caesarxuchao
Copy link
Member Author

@k8s-bot test this please

@k8s-bot
Copy link

k8s-bot commented Oct 21, 2015

GCE e2e test build/test passed for commit 6f0f87b.

@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 Oct 21, 2015

GCE e2e test build/test passed for commit 6f0f87b.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

kubectl explain does not work with experimental resources
7 participants