Added api discovery to kubectl run, based on which we decide which generators to use #22849

soltysh · 2016-03-11T10:00:02Z

@bgrant0607 @jlowdermilk @lavalamp @deads2k @soltysh @janetkuo wdyt?

PS If fixes/changes are needed after 4pm UTC take over this PR and resubmit since I'll be out the entire evening.

k8s-github-robot · 2016-03-11T10:06:27Z

Labelling this PR as size/M

k8s-bot · 2016-03-11T10:31:03Z

GCE e2e build/test failed for commit 3077c5d6cc6de9722733bf05818785b076430876.

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.

k8s-bot · 2016-03-11T11:22:12Z

GCE e2e build/test passed for commit daf955834f265373ac7d706888808dbc7f5402f3.

deads2k · 2016-03-11T12:40:18Z

pkg/kubectl/cmd/run.go

 		if restartPolicy == api.RestartPolicyAlways {
-			generatorName = "deployment/v1beta1"
+			if contains(groupList, "extensions/v1beta1") {


use constants for group/versions and use GroupVersion objects.

You're going to need some comments explaining priority decisions here. I know it, but I think its because I just read the issue and a month from now someone is going to be looking through. and wondering.

deads2k · 2016-03-11T12:43:42Z

If we have a decent mock for the discovery client I'd like to see a unit test or two, but this looks fine.

bgrant0607 · 2016-03-11T13:54:55Z

pkg/kubectl/cmd/run.go

+		groupList, err := client.Discovery().ServerGroups()
+		if err != nil {
+			// this cover the cases where old servers do not expose discovery
+			groupList = nil


This is what makes it work against 1.1?

That's my idea, I don't have any 1.1 cluster at hand :( Although I can compile 1.1 version and see what I'm gonna get.

bgrant0607 · 2016-03-11T13:58:05Z

Thanks for jumping on this so quickly!

Could you please update the description text for --generator?

soltysh · 2016-03-11T14:41:14Z

@deads2k @bgrant0607 I've addressed all the comments. Tested that against 1.1 cluster. It nicely falls back all the way back to running pods if jobs are disabled in 1.1 (the default behavior). ptal

bgrant0607 · 2016-03-11T15:00:57Z

Thanks, @soltysh.

Please run hack/update-all.sh.

k8s-bot · 2016-03-11T15:11:35Z

GCE e2e build/test failed for commit aefb94e2224dfa9a92b58241adc3514b86cf8ed4.

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.

soltysh · 2016-03-11T15:11:46Z

@bgrant0607 done, sorry

bgrant0607 · 2016-03-11T15:12:05Z

LGTM, assuming tests pass. Thanks much

deads2k · 2016-03-11T15:30:08Z

pkg/kubectl/cmd/run.go

@@ -254,6 +276,22 @@ func Run(f *cmdutil.Factory, cmdIn io.Reader, cmdOut, cmdErr io.Writer, cmd *cob
 	return nil
 }

+func contains(resourcesList map[string]*unversioned.APIResourceList, gv unversioned.GroupVersion, name string) bool {


GroupVersionResource?

Also, how about a TODO to make this more library-esq. It looks like it would have general utility as a layer on top of a discovery client.

bgrant0607 · 2016-03-11T15:39:23Z

Thanks for the comments @deads2k. Removed lgtm. Please reapply when your comments are addressed.

k8s-bot · 2016-03-11T15:42:15Z

GCE e2e build/test failed for commit d5659eb8a4ae6cdc5730d0c26f4815f5ee02b5ea.

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.

soltysh · 2016-03-11T15:45:30Z

On it...

…nerators to use

soltysh · 2016-03-11T15:59:35Z

@deads2k @bgrant0607 comments addressed ptal.

soltysh · 2016-03-11T16:00:28Z

If more tweaks are needed or tests fail please pick up this PR because I'll be leaving and won't be able to touch it and I do realize this is important to have in as soon as possible.

bgrant0607 · 2016-03-11T16:23:37Z

Thanks @soltysh

bgrant0607 · 2016-03-11T16:24:45Z

There are no unit test results, so I can't see what failed.

@k8s-bot unit test this issue: #IGNORE

deads2k · 2016-03-11T16:27:57Z

lgtm

k8s-bot · 2016-03-11T16:32:18Z

GCE e2e build/test passed for commit f27871e.

bgrant0607 · 2016-03-11T17:04:08Z

All green. Merging @k8s-oncall

Added api discovery to kubectl run, based on which we decide which generators to use

eparis · 2016-03-11T19:03:59Z

This PR was sucessfully cherry picked in PR #22855
please verify that the release-1.2 branch contains these changes as you would expect and contact @eparis if there appear to be problems.

Added api discovery to kubectl run, based on which we decide which generators to use

googlebot added the cla: yes label Mar 11, 2016

k8s-github-robot assigned deads2k Mar 11, 2016

k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 11, 2016

soltysh force-pushed the issue22836 branch from 3077c5d to daf9558 Compare March 11, 2016 10:51

deads2k reviewed Mar 11, 2016
View reviewed changes

bgrant0607 reviewed Mar 11, 2016
View reviewed changes

bgrant0607 added this to the v1.2 milestone Mar 11, 2016

bgrant0607 added priority/critical-urgent Highest priority. Must be actively worked on as someone's top priority right now. cherrypick-candidate labels Mar 11, 2016

soltysh force-pushed the issue22836 branch from daf9558 to aefb94e Compare March 11, 2016 14:39

soltysh force-pushed the issue22836 branch from aefb94e to d5659eb Compare March 11, 2016 15:11

bgrant0607 added lgtm "Looks good to me", indicates that a PR is ready to be merged. cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. labels Mar 11, 2016

k8s-github-robot added the kind/old-docs label Mar 11, 2016

bgrant0607 mentioned this pull request Mar 11, 2016

default kubectl run with kubectl 1.2 is not backwards compatible with 1.1 master #22836

Closed

deads2k reviewed Mar 11, 2016
View reviewed changes

bgrant0607 removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 11, 2016

Added api discovery to kubectl run, based on which we decide which ge…

f27871e

…nerators to use

soltysh force-pushed the issue22836 branch from d5659eb to f27871e Compare March 11, 2016 15:59

deads2k added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 11, 2016

bgrant0607 mentioned this pull request Mar 11, 2016

Cherry-picks for release 1.2 #22855

Merged

bgrant0607 added a commit that referenced this pull request Mar 11, 2016

Merge pull request #22849 from soltysh/issue22836

1c50f18

Added api discovery to kubectl run, based on which we decide which generators to use

bgrant0607 merged commit 1c50f18 into kubernetes:master Mar 11, 2016

eparis pushed a commit to eparis/kubernetes that referenced this pull request Mar 11, 2016

Merge pull request kubernetes#22849 from soltysh/issue22836

7dfc2be

Added api discovery to kubectl run, based on which we decide which generators to use

j3ffml mentioned this pull request Mar 11, 2016

fix typo in kubectl run help string #22862

Merged

eparis removed cherrypick-candidate cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. labels Mar 11, 2016

soltysh deleted the issue22836 branch March 13, 2016 21:28

j3ffml mentioned this pull request Mar 14, 2016

v1.2 Deployment e2e shouldn't run against against masters before v1.2 #21421

Closed

metahertz pushed a commit to metahertz/kubernetes that referenced this pull request Apr 4, 2016

Merge pull request kubernetes#22849 from soltysh/issue22836

a8c6391

Added api discovery to kubectl run, based on which we decide which generators to use

shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016

Merge pull request kubernetes#22849 from soltysh/issue22836

1ae9bee

Added api discovery to kubectl run, based on which we decide which generators to use

shouhong pushed a commit to shouhong/kubernetes that referenced this pull request Feb 14, 2017

Merge pull request kubernetes#22849 from soltysh/issue22836

250c02c

Added api discovery to kubectl run, based on which we decide which generators to use

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added api discovery to kubectl run, based on which we decide which generators to use #22849

Added api discovery to kubectl run, based on which we decide which generators to use #22849

soltysh commented Mar 11, 2016

k8s-github-robot commented Mar 11, 2016

k8s-bot commented Mar 11, 2016

k8s-bot commented Mar 11, 2016

deads2k Mar 11, 2016

deads2k Mar 11, 2016

deads2k commented Mar 11, 2016

bgrant0607 Mar 11, 2016

soltysh Mar 11, 2016

bgrant0607 commented Mar 11, 2016

soltysh commented Mar 11, 2016

bgrant0607 commented Mar 11, 2016

k8s-bot commented Mar 11, 2016

soltysh commented Mar 11, 2016

bgrant0607 commented Mar 11, 2016

deads2k Mar 11, 2016

deads2k Mar 11, 2016

bgrant0607 commented Mar 11, 2016

k8s-bot commented Mar 11, 2016

soltysh commented Mar 11, 2016

soltysh commented Mar 11, 2016

soltysh commented Mar 11, 2016

bgrant0607 commented Mar 11, 2016

bgrant0607 commented Mar 11, 2016

deads2k commented Mar 11, 2016

k8s-bot commented Mar 11, 2016

bgrant0607 commented Mar 11, 2016

eparis commented Mar 11, 2016

Added api discovery to kubectl run, based on which we decide which generators to use #22849

Added api discovery to kubectl run, based on which we decide which generators to use #22849

Conversation

soltysh commented Mar 11, 2016

k8s-github-robot commented Mar 11, 2016

k8s-bot commented Mar 11, 2016

k8s-bot commented Mar 11, 2016

deads2k Mar 11, 2016

Choose a reason for hiding this comment

deads2k Mar 11, 2016

Choose a reason for hiding this comment

deads2k commented Mar 11, 2016

bgrant0607 Mar 11, 2016

Choose a reason for hiding this comment

soltysh Mar 11, 2016

Choose a reason for hiding this comment

bgrant0607 commented Mar 11, 2016

soltysh commented Mar 11, 2016

bgrant0607 commented Mar 11, 2016

k8s-bot commented Mar 11, 2016

soltysh commented Mar 11, 2016

bgrant0607 commented Mar 11, 2016

deads2k Mar 11, 2016

Choose a reason for hiding this comment

deads2k Mar 11, 2016

Choose a reason for hiding this comment

bgrant0607 commented Mar 11, 2016

k8s-bot commented Mar 11, 2016

soltysh commented Mar 11, 2016

soltysh commented Mar 11, 2016

soltysh commented Mar 11, 2016

bgrant0607 commented Mar 11, 2016

bgrant0607 commented Mar 11, 2016

deads2k commented Mar 11, 2016

k8s-bot commented Mar 11, 2016

bgrant0607 commented Mar 11, 2016

eparis commented Mar 11, 2016