Fixed incomplete kubectl bash completion. #31333

xingzhou · 2016-08-24T08:18:07Z

Added bash completion for several kubectl commands.

This change is

k8s-bot · 2016-08-24T08:18:30Z

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.

k8s-bot · 2016-08-24T08:18:47Z

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.

k8s-bot · 2016-08-24T08:19:04Z

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.

k8s-bot · 2016-08-24T08:19:21Z

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.

k8s-bot · 2016-08-24T08:19:37Z

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.

k8s-bot · 2016-08-24T08:19:53Z

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.

k8s-bot · 2016-08-24T08:20:25Z

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.

brendandburns · 2016-08-25T00:47:33Z

pkg/kubectl/cmd/taint.go

@@ -76,16 +75,6 @@ var (
 func NewCmdTaint(f *cmdutil.Factory, out io.Writer) *cobra.Command {
 	options := &TaintOptions{}

-	// retrieve a list of handled resources from printer as valid args


@deads2k is this done for a reason here?

@deads2k is this done for a reason here?

I hadn't noticed this code before. It's definitely incorrect for this command and its not a very clean way of getting the data for the completions overall. Removing it here is good.

brendandburns · 2016-08-25T00:48:10Z

Most of this PR looks fine. I'm not sure why you are removing the dynamic code in taint, I want to double check with @deads2k that this won't break anything upstream.

brendandburns · 2016-08-25T00:48:17Z

@k8s-bot ok to test

deads2k · 2016-08-25T12:34:17Z

I want to double check with @deads2k that this won't break anything upstream.

Thanks for the heads up. Generally, yes, we care about being able to substitute the available types, but kubectl taint is a special case that only works against nodes.

deads2k · 2016-08-25T12:36:01Z

pkg/kubectl/cmd/rollout/rollout_history.go

@@ -49,6 +49,9 @@ var (
 func NewCmdRolloutHistory(f *cmdutil.Factory, out io.Writer) *cobra.Command {
 	options := &HistoryOptions{}

+	validArgs := []string{"deployment"}


@fabianofranz You should think of a way for us to refactor this to have these completions work well for us. I don't think it needs to be resolved in this pull, but we'll want some way to add dc and deploymentconfig here.

@deads2k definitely. As a short-term solution could this be an argument in NewCmdRolloutHistory?

@deads2k definitely. As a short-term solution could this be an argument in NewCmdRolloutHistory?

I can't say that I'm a fan of that. I think we should let this merge and you can work it out with @Kargakis since he's trying to integrate these with openshift.

Me neither, I'm ok with addressing it apart of this PR.

We already have ported rollout history. Make sure this is configurable because it needs to work with deploymentconfigs too

@Kargakis, any suggestions to make it configurable? or @fabianofranz can take care of this in a new PR?

Hm, are these changes helping in fixing completion? If not, then let's just drop them. The rollout commands already know their valid resources via the factory:

$ kc rollout history no/127.0.0.1 error: no history viewer has been implemented for {"" "Node"}

The error message should be cleaned up in #31234

Hm, are these changes helping in fixing completion?

Doc in cobra says "yes".

Just noticed it, thanks. Are we going to end up with a factory method that needs to separate between commands?

Just noticed it, thanks. Are we going to end up with a factory method that needs to separate between commands?

Maybe. I see what's been done here as strictly better than the current state of affairs. I'd let this go in and @fabianofranz and team can fix it up in the 1.5 timeframe.

adohe-zz · 2016-08-26T14:53:48Z

We should also support it's alias. @xingzhou

brendandburns · 2016-08-26T21:24:35Z

agree we should support the alias.

xingzhou · 2016-08-28T23:46:40Z

ok, @adohe and @brendandburns, do you think we can open a new ticket for supporting the alias "no" for command taint? In my mind, this can be a separate PR, this one only can be used for fixing command bash completion. If you agree, I can provide a patch later and we can move forward for this PR?

adohe-zz · 2016-08-29T02:27:40Z

@xingzhou anything block you add alias support in this PR?

xingzhou · 2016-08-29T03:13:44Z

@adohe, no, just curious of how to split PRs. I will add one more commit here to resolve taint alias issue.

k8s-bot · 2016-08-30T00:35:41Z

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.

brendandburns · 2016-08-30T02:22:33Z

LGTM, thanks for the PR.

adohe-zz · 2016-08-30T04:04:53Z

@xingzhou @brendandburns please squash before we can merge.

This path added/fixed bash completion for several kubectl commands. Fixes kubernetes#25287

xingzhou · 2016-08-30T05:15:08Z

Rebased and squashed into one commit

k8s-bot · 2016-08-30T19:59:14Z

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.

brendandburns · 2016-09-01T15:32:49Z

LGTM again.

k8s-bot · 2016-09-01T16:16:30Z

GCE e2e build/test passed for commit 80d6cd0.

k8s-github-robot · 2016-09-01T17:09:06Z

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

k8s-bot · 2016-09-01T17:46:53Z

GCE e2e build/test passed for commit 80d6cd0.

k8s-github-robot · 2016-09-01T17:49:50Z

Automatic merge from submit-queue

foxish · 2016-09-08T22:30:50Z

This commit is in 1.4 already.

pwittrock · 2016-09-12T19:40:59Z

git branch --contains 80d6cd0
master

release-1.4

googlebot added the cla: yes label Aug 24, 2016

k8s-github-robot assigned brendandburns Aug 24, 2016

k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. release-note-label-needed labels Aug 24, 2016

brendandburns reviewed Aug 25, 2016
View reviewed changes

brendandburns added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note-label-needed labels Aug 25, 2016

k8s-github-robot mentioned this pull request Aug 25, 2016

[k8s.io] ScheduledJob should replace jobs when ReplaceConcurrent {Kubernetes e2e suite} #30542

Closed

deads2k reviewed Aug 25, 2016
View reviewed changes

0xmichalis added the area/kubectl label Aug 26, 2016

xingzhou force-pushed the kube-25287 branch from c3961bd to e3a9054 Compare August 29, 2016 08:46

brendandburns added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 30, 2016

brendandburns added the cherrypick-candidate label Aug 30, 2016

brendandburns added this to the v1.4 milestone Aug 30, 2016

Added bash completion for several kubectl commands.

80d6cd0

This path added/fixed bash completion for several kubectl commands. Fixes kubernetes#25287

xingzhou force-pushed the kube-25287 branch from e3a9054 to 80d6cd0 Compare August 30, 2016 05:13

k8s-github-robot removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 30, 2016

brendandburns added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Sep 1, 2016

k8s-github-robot merged commit c98a1e2 into kubernetes:master Sep 1, 2016

pwittrock added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Sep 7, 2016

foxish removed the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Sep 8, 2016

pwittrock removed the cherrypick-candidate label Sep 12, 2016

xiangpengzhao mentioned this pull request May 25, 2017

Kubectl bash completion doesn't complete on port-forward #29665

Closed

Fixed incomplete kubectl bash completion. #31333

Fixed incomplete kubectl bash completion. #31333

Conversation

xingzhou commented Aug 24, 2016 • edited by k8s-oncall

k8s-bot commented Aug 24, 2016

k8s-bot commented Aug 24, 2016

k8s-bot commented Aug 24, 2016

k8s-bot commented Aug 24, 2016

k8s-bot commented Aug 24, 2016

k8s-bot commented Aug 24, 2016

k8s-bot commented Aug 24, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brendandburns commented Aug 25, 2016

brendandburns commented Aug 25, 2016

deads2k commented Aug 25, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adohe-zz commented Aug 26, 2016

brendandburns commented Aug 26, 2016

xingzhou commented Aug 28, 2016

adohe-zz commented Aug 29, 2016

xingzhou commented Aug 29, 2016

k8s-bot commented Aug 30, 2016

brendandburns commented Aug 30, 2016

adohe-zz commented Aug 30, 2016

xingzhou commented Aug 30, 2016

k8s-bot commented Aug 30, 2016

brendandburns commented Sep 1, 2016

k8s-bot commented Sep 1, 2016

k8s-github-robot commented Sep 1, 2016

k8s-bot commented Sep 1, 2016

k8s-github-robot commented Sep 1, 2016

foxish commented Sep 8, 2016

pwittrock commented Sep 12, 2016

xingzhou commented Aug 24, 2016 •

edited by k8s-oncall