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

Adding clusters to the list of valid resources printed by kubectl help #31719

Merged
merged 1 commit into from
Sep 1, 2016
Merged

Adding clusters to the list of valid resources printed by kubectl help #31719

merged 1 commit into from
Sep 1, 2016

Conversation

nikhiljindal
Copy link
Contributor

@nikhiljindal nikhiljindal commented Aug 30, 2016 •
edited by k8s-oncall
Loading

Ref #25592

Adding clusters to the list of valid resources printed by kubectl help with a clear message that it only works when talking to federation apiserver.
In future, we should replace the hard coded list with a dynamic list generated using APIServer's discovery API.

Adding clusters to the list of valid resources printed by kubectl help

cc @kubernetes/kubectl @kubernetes/sig-cluster-federation

This change is Reviewable

@nikhiljindal nikhiljindal added this to the v1.4 milestone Aug 30, 2016
@k8s-bot
Copy link

k8s-bot commented Aug 30, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message will repeat several times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

1 similar comment
@k8s-bot
Copy link

k8s-bot commented Aug 30, 2016

Can one of the admins verify that this patch is reasonable to test? If so, please reply "ok to test".
(Note: "add to whitelist" is no longer supported. Please update configurations in kubernetes/test-infra/jenkins/job-configs/kubernetes-jenkins-pull instead.)

This message will repeat several times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

@nikhiljindal nikhiljindal mentioned this pull request Aug 30, 2016
Closed
@nikhiljindal nikhiljindal added the release-note Denotes a PR that will be considered when it comes time to generate release notes. label Aug 30, 2016
@nikhiljindal
Copy link
Contributor Author

@k8s-bot ok to test

@k8s-github-robot k8s-github-robot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Aug 30, 2016
@smarterclayton
Copy link
Contributor

Is this for 1.4? I only ask because it would be great if we just made the
transition now to read this from API discovery, rather than wait longer (if
it's targeted for 1.5)

On Tue, Aug 30, 2016 at 2:05 PM, Kubernetes Submit Queue <
notifications@github.com> wrote:

Labelling this PR as size/XS

—
You are receiving this because you are on a team that was mentioned.
Reply to this email directly, view it on GitHub
#31719 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ABG_p6J6Ug7AowULHII7u7YXLInWbE0Wks5qlHDTgaJpZM4JwzLe
.

@nikhiljindal
Copy link
Contributor Author

Yes, I could not get to the transition to API discovery in 1.4, so sent this for 1.4.

smarterclayton
smarterclayton reviewed Aug 30, 2016
View reviewed changes
pkg/kubectl/cmd/cmd.go
valid_resources = `Valid resource types include:
* clusters (valid only when talking to federation apiserver)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(valid only for federation apiservers)

reads a bit better. "talking" is awkward in some contexts.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

@ghost ghost added kind/documentation Categorizes issue or PR as related to documentation. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. labels Aug 30, 2016
@nikhiljindal
Copy link
Contributor Author

Updated code as per comment, PTAL

@j3ffml
Copy link
Contributor

j3ffml commented Aug 31, 2016

:lgtm:

Reviewed 1 of 1 files at r2.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

Comments from Reviewable

@nikhiljindal
Copy link
Contributor Author

Adding the LGTM label as per comment from @jlowdermilk above.

@nikhiljindal nikhiljindal added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 31, 2016
@k8s-bot
Copy link

k8s-bot commented Aug 31, 2016

GCE e2e build/test passed for commit 0fcbde5.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 3d30787 into kubernetes:master Sep 1, 2016
@irfanurrehman irfanurrehman mentioned this pull request Oct 30, 2017
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@j3ffml j3ffml

Labels
area/kubectl kind/documentation Categorizes issue or PR as related to documentation. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Milestone
v1.4
Development

Successfully merging this pull request may close these issues.
6 participants
@nikhiljindal @k8s-bot @smarterclayton @j3ffml @k8s-github-robot @googlebot