enable recursive processing in kubectl edit #25085

metral · 2016-05-03T16:58:08Z

This PR was split out of #23673 per @deads2k's suggestion: #23673 (comment)

It makes use of the recursive processing of a directory in kubectl edit

k8s-bot · 2016-05-03T16:58:31Z

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 hack/jenkins/job-configs/kubernetes-jenkins-pull/ instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

k8s-bot · 2016-05-03T16:59:13Z

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 hack/jenkins/job-configs/kubernetes-jenkins-pull/ instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

k8s-bot · 2016-05-03T17:00:23Z

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 hack/jenkins/job-configs/kubernetes-jenkins-pull/ instead.)

This message may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

metral · 2016-05-03T20:11:02Z

@janetkuo can this PR get an "ok to test"?

janetkuo · 2016-05-04T00:05:53Z

@k8s-bot ok to test

janetkuo · 2016-05-04T00:06:35Z

@kubernetes/kubectl

metral · 2016-05-06T20:22:49Z

@janetkuo anything else missing from this PR?

metral · 2016-05-10T17:31:03Z

@janetkuo Do you happen to have an estimate as to when you can get to this PR? Thanks!

janetkuo · 2016-05-11T00:39:20Z

@metral sorry for the late reply, reviewing this now.

janetkuo · 2016-05-11T01:01:24Z

pkg/kubectl/cmd/edit.go

-	clientConfig, err := f.ClientConfig()
+	infos := []*resource.Info{}
+	allErrs := []error{}
+	err = r.Visit(func(info *resource.Info, err error) error {


Why not use r.Infos() here?

Good catch - I was being creative on a previous attempt at getting things to work and I didn't realize I left the code to basically recreate r.Infos(), rather than enhancing it through a direct usage of Visit() like I've done with other portions of the recursive feature.

I'll update it to use r.Infos(). Thanks

janetkuo · 2016-05-11T01:22:17Z

cc @Kargakis who's also familiar with edit

metral · 2016-05-12T15:57:41Z

the e2e test seems to be having issues with one of its images not being able to be deleted on GCE: log details

k8s-bot · 2016-05-12T18:15:19Z

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 may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

metral · 2016-05-19T17:49:58Z

@Kargakis I cherry-picked #20918 and refactored my changes on top of it, but #20918 is stuck in the queue as its been having trouble, so it merging into master to then push my changes out has been a waiting game.

What do you recommend I do to ensure this gets into v1.3 as its the last piece of the original --recursive feature request per #19767?

deads2k · 2016-05-19T17:56:30Z

@metral I haven't tagged #20918 as 1.3 because I could find a reference that someone thought it was required for 1.3. The issue you linked was a candidate, but not explicitly 1.3. If you have something that indicates this is a 1.3 requirement I can mark a milestone on the other edit pull since it's a logical prereq.

metral · 2016-05-19T18:05:24Z

@deads2k That makes sense. I don't have an indication that this PR is a requirement other than me wanting to get it merged into v1.3 for the sake of completeness wrt the rest of the kubectl subcommands that already support it because their PR's were merged.

I'll just follow your lead on #20918, as its a dep of this PR

metral · 2016-05-22T17:53:35Z

@Kargakis I've gone ahead and rebased on top of #20918

Just need an LGTM, thanks!

k8s-bot · 2016-05-23T10:36:17Z

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 may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

0xmichalis · 2016-05-24T13:06:24Z

@k8s-bot test this issue: #IGNORE

metral · 2016-05-25T15:12:53Z

@Kargakis Is there anything else missing?

Also, ?w=1 is your friend here: https://github.com/kubernetes/kubernetes/pull/25085/files?w=1

0xmichalis · 2016-05-25T16:22:25Z

LGTM, thanks!

k8s-bot · 2016-05-27T17:12:55Z

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 may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

0xmichalis · 2016-05-28T16:09:44Z

@k8s-bot test this issue #IGNORE

k8s-bot · 2016-06-15T00:35:10Z

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 may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

k8s-bot · 2016-06-15T21:49:16Z

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 may repeat a few times in short succession due to jenkinsci/ghprb-plugin#292. Sorry.

Otherwise, if this message is too spammy, please complain to ixdy.

k8s-bot · 2016-06-21T02:56:23Z

GCE e2e build/test passed for commit 4913481.

k8s-bot · 2016-06-25T03:10:13Z

GCE e2e build/test passed for commit 4913481.

k8s-github-robot · 2016-06-25T06:25:13Z

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

k8s-bot · 2016-06-25T06:57:20Z

GCE e2e build/test passed for commit 4913481.

k8s-github-robot · 2016-06-25T06:59:54Z

Automatic merge from submit-queue

googlebot added the cla: yes label May 3, 2016

k8s-github-robot assigned janetkuo May 3, 2016

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

janetkuo added release-note Denotes a PR that will be considered when it comes time to generate release notes. area/kubectl and removed release-note-label-needed labels May 4, 2016

k8s-github-robot added the needs-ok-to-merge label May 4, 2016

janetkuo added ok-to-merge and removed needs-ok-to-merge labels May 4, 2016

janetkuo reviewed May 11, 2016
View reviewed changes

metral closed this May 12, 2016

metral reopened this May 12, 2016

metral closed this May 12, 2016

metral reopened this May 12, 2016

k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label May 22, 2016

enable recursive processing in kubectl edit

4913481

metral force-pushed the recursive-edit branch from 33efa2b to 4913481 Compare May 22, 2016 16:51

k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels May 22, 2016

0xmichalis assigned 0xmichalis and unassigned janetkuo May 22, 2016

0xmichalis added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 25, 2016

k8s-github-robot merged commit 7c355e1 into kubernetes:master Jun 25, 2016

metral deleted the recursive-edit branch June 25, 2016 15:47

This was referenced Nov 1, 2016

kubectl edit <list> opens editor multiple times #20519

Closed

Make edit work with lists again openshift/origin#11704

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

enable recursive processing in kubectl edit #25085

enable recursive processing in kubectl edit #25085

metral commented May 3, 2016

k8s-bot commented May 3, 2016

k8s-bot commented May 3, 2016

k8s-bot commented May 3, 2016

metral commented May 3, 2016

janetkuo commented May 4, 2016

janetkuo commented May 4, 2016

metral commented May 6, 2016

metral commented May 10, 2016

janetkuo commented May 11, 2016

janetkuo May 11, 2016

metral May 11, 2016 •

edited

Loading

janetkuo commented May 11, 2016

metral commented May 12, 2016 •

edited

Loading

k8s-bot commented May 12, 2016

metral commented May 19, 2016

deads2k commented May 19, 2016

metral commented May 19, 2016

metral commented May 22, 2016

k8s-bot commented May 23, 2016

0xmichalis commented May 24, 2016

metral commented May 25, 2016

0xmichalis commented May 25, 2016

k8s-bot commented May 27, 2016

0xmichalis commented May 28, 2016

k8s-bot commented Jun 15, 2016

k8s-bot commented Jun 15, 2016

k8s-bot commented Jun 21, 2016

k8s-bot commented Jun 25, 2016

k8s-github-robot commented Jun 25, 2016

k8s-bot commented Jun 25, 2016

k8s-github-robot commented Jun 25, 2016

enable recursive processing in kubectl edit #25085

enable recursive processing in kubectl edit #25085

Conversation

metral commented May 3, 2016

k8s-bot commented May 3, 2016

k8s-bot commented May 3, 2016

k8s-bot commented May 3, 2016

metral commented May 3, 2016

janetkuo commented May 4, 2016

janetkuo commented May 4, 2016

metral commented May 6, 2016

metral commented May 10, 2016

janetkuo commented May 11, 2016

janetkuo May 11, 2016

Choose a reason for hiding this comment

metral May 11, 2016 • edited Loading

Choose a reason for hiding this comment

janetkuo commented May 11, 2016

metral commented May 12, 2016 • edited Loading

k8s-bot commented May 12, 2016

metral commented May 19, 2016

deads2k commented May 19, 2016

metral commented May 19, 2016

metral commented May 22, 2016

k8s-bot commented May 23, 2016

0xmichalis commented May 24, 2016

metral commented May 25, 2016

0xmichalis commented May 25, 2016

k8s-bot commented May 27, 2016

0xmichalis commented May 28, 2016

k8s-bot commented Jun 15, 2016

k8s-bot commented Jun 15, 2016

k8s-bot commented Jun 21, 2016

k8s-bot commented Jun 25, 2016

k8s-github-robot commented Jun 25, 2016

k8s-bot commented Jun 25, 2016

k8s-github-robot commented Jun 25, 2016

metral May 11, 2016 •

edited

Loading

metral commented May 12, 2016 •

edited

Loading