Add image replacement support for multicontainer pods in kubectl rolling-update #9868

mhenniges · 2015-06-16T15:26:42Z

Added support to kubectl rolling-update with the --image flag when operating on multi-container pods. The container to update is selected by the new --container-name option.

All existing behavior of kubectl was preserved.

Thanks for reading!

…t a multi-container pod.

googlebot · 2015-06-16T15:26:44Z

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for the commit author(s). If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.

k8s-bot · 2015-06-16T15:27:04Z

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

mhenniges · 2015-06-16T15:32:56Z

Additional CLA was submitted - do I need to do anything to have this rechecked by the system?

Also - I can't view the shippable failure details. If someone can point me to the error, I'll take a look.

Thanks

erictune · 2015-06-16T16:31:26Z

Boilerplate header is wrong for: ./pkg/kubectl/rolling_updater.go

Make sure you did:
cd .git/hooks; ln -s ../../hooks/pre-commit .
as described in docs/devel/development.md

erictune · 2015-06-16T16:33:29Z

Your commits are authored by matthewsimon-ww who has not signed the CLA.
The PR is from mhenniges who has signed the CLA.

erictune · 2015-06-16T16:34:50Z

Looks like this fixes #9868

erictune · 2015-06-16T16:35:14Z

Assigned to davidopp who has previously shown interested in rolling updated. Feel free to punt elsewhere.

…ve hook warnings

googlebot · 2015-06-16T17:15:35Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

mhenniges · 2015-06-16T17:17:33Z

Hi, you are mistaken, they were authored by me.

Thanks.

On Tue, Jun 16, 2015 at 1:16 PM, googlebot notifications@github.com wrote:

We found a Contributor License Agreement for you (the sender of this pull
request) and all commit authors, but as best as we can tell these commits
were authored by someone else. If that's the case, please add them to this
pull request and have them confirm that they're okay with these commits
being contributed to Google. If we're mistaken and you did author these
commits, just reply here to confirm.

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

mhenniges · 2015-06-16T17:21:55Z

Thanks for the feedback erictune, I've resolved the boilerplate and other issues as per the hooks, and also fixed a unit test I missed earlier.

davidopp · 2015-06-16T21:03:14Z

Thanks very much for the PR!

Unfortunately, we're currently in "code freeze" in preparation for our 1.0 release, so we can't merge this right now. However, this feature is obviously useful and we can definitely merge it after the code freeze is over.

In the mean time, it would be great if you could add some tests.

k8s-bot · 2015-07-24T18:11:14Z

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

bgrant0607 · 2015-07-27T06:28:13Z

pkg/kubectl/cmd/rollingupdate.go

@@ -74,6 +74,7 @@ func NewCmdRollingUpdate(f *cmdutil.Factory, out io.Writer) *cobra.Command {
 	cmd.Flags().String("timeout", timeout, `Max time to wait for a controller to update before giving up. Valid time units are "ns", "us" (or "µs"), "ms", "s", "m", "h".`)
 	cmd.Flags().StringP("filename", "f", "", "Filename or URL to file to use to create the new controller.")
 	cmd.Flags().String("image", "", "Image to upgrade the controller to.  Can not be used with --filename/-f")
+	cmd.Flags().String("container-name", "", "Container name which will have its image upgraded. Only relevant when --image is specified, ignored otherwise. Required when using --image on a multi-container pod")


Please conform to the convention of other commands, such as log and exec: --container, -c
https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/kubectl/cmd/log.go#L85
https://github.com/GoogleCloudPlatform/kubernetes/blob/master/pkg/kubectl/cmd/exec.go#L68

bgrant0607 · 2015-07-27T06:29:38Z

We're trying to clear the PR backlog. Please rebase.

cc @nikhiljindal

mhenniges · 2015-07-28T02:41:28Z

What branch should I rebase to? Master? Or the 1.0?

Sent from my iPhone

On Jul 27, 2015, at 2:30 AM, Brian Grant notifications@github.com wrote:

We're trying to clear the PR backlog. Please rebase.

cc @nikhiljindal

—
Reply to this email directly or view it on GitHub.

j3ffml · 2015-07-28T02:51:07Z

Rebase to master. I believe release branches are considered final except
for p0 bugfixes/security patches.
On Jul 27, 2015 7:41 PM, "mhenniges" notifications@github.com wrote:

What branch should I rebase to? Master? Or the 1.0?

Sent from my iPhone

On Jul 27, 2015, at 2:30 AM, Brian Grant notifications@github.com
wrote:

We're trying to clear the PR backlog. Please rebase.

cc @nikhiljindal

—
Reply to this email directly or view it on GitHub.

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

bgrant0607 · 2015-07-28T04:08:24Z

Yes, master, sorry.

k8s-bot · 2015-08-12T11:25:04Z

Can one of the admins verify that this patch is reasonable to test? (reply "ok to test", or if you trust the user, reply "add to whitelist")

If this message is too spammy, please complain to ixdy.

bgrant0607 · 2015-08-13T07:39:03Z

cc @kubernetes/kubectl

bgrant0607 · 2015-08-13T16:15:26Z

docs/kubectl_rolling-update.md

@@ -36,6 +36,7 @@ $ kubectl rolling-update frontend --image=image:v2
 ### Options

 ```
+      --container-name="": Container name which will have its image upgraded. Only relevant when --image is specified, ignored otherwise. Required when using --image on a multi-container pod


Please be consistent with other commands that have container-name flags. For instance:
https://github.com/kubernetes/kubernetes/blob/master/pkg/kubectl/cmd/exec.go#L68

bgrant0607 · 2015-08-13T16:17:26Z

Please rebase.

janetkuo · 2015-08-24T09:02:00Z

Ping @mhenniges could you rebase?

@k8s-bot ok to test

k8s-bot · 2015-08-24T09:06:03Z

GCE e2e build/test failed for commit 5c55120.

k8s-github-robot · 2015-08-27T22:12:31Z

Labelling this PR as size/M

bgrant0607 · 2015-10-21T18:02:03Z

This PR appears to be inactive since July. Closing.

coleca · 2015-10-21T18:03:53Z

So this will never be added to K8S?

janetkuo · 2015-10-21T18:06:32Z

@coleca please see #13483 (comment)

Matthew Simon added 2 commits June 16, 2015 11:07

Add support to kubectl rolling-update for direct image replace agains…

d04e72f

…t a multi-container pod.

Correcting formatting

762df4c

googlebot added the cla: no label Jun 16, 2015

erictune assigned davidopp Jun 16, 2015

Update unit test to reflect new signature of validateArguments, resol…

5c55120

…ve hook warnings

bgrant0607 assigned j3ffml and unassigned davidopp Jul 13, 2015

bgrant0607 added the area/kubectl label Jul 27, 2015

bgrant0607 reviewed Jul 27, 2015
View reviewed changes

mhenniges unassigned j3ffml Aug 12, 2015

mikedanese assigned j3ffml Aug 12, 2015

bgrant0607 reviewed Aug 13, 2015
View reviewed changes

bgrant0607 assigned janetkuo and unassigned j3ffml Aug 13, 2015

k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Aug 27, 2015

JeanMertz mentioned this pull request Aug 28, 2015

Declarative rolling-update #8827

Closed

janetkuo mentioned this pull request Sep 3, 2015

Enhance Rolling-Updates to support updating image with multi-container pods #13483

Closed

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

bgrant0607 closed this Oct 21, 2015

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add image replacement support for multicontainer pods in kubectl rolling-update #9868

Add image replacement support for multicontainer pods in kubectl rolling-update #9868

mhenniges commented Jun 16, 2015

googlebot commented Jun 16, 2015

k8s-bot commented Jun 16, 2015

mhenniges commented Jun 16, 2015

erictune commented Jun 16, 2015

erictune commented Jun 16, 2015

erictune commented Jun 16, 2015

erictune commented Jun 16, 2015

googlebot commented Jun 16, 2015

mhenniges commented Jun 16, 2015

mhenniges commented Jun 16, 2015

davidopp commented Jun 16, 2015

k8s-bot commented Jul 24, 2015

bgrant0607 Jul 27, 2015

bgrant0607 commented Jul 27, 2015

mhenniges commented Jul 28, 2015

j3ffml commented Jul 28, 2015

bgrant0607 commented Jul 28, 2015

k8s-bot commented Aug 12, 2015

bgrant0607 commented Aug 13, 2015

bgrant0607 Aug 13, 2015

bgrant0607 commented Aug 13, 2015

janetkuo commented Aug 24, 2015

k8s-bot commented Aug 24, 2015

k8s-github-robot commented Aug 27, 2015

bgrant0607 commented Oct 21, 2015

coleca commented Oct 21, 2015

janetkuo commented Oct 21, 2015

Add image replacement support for multicontainer pods in kubectl rolling-update #9868

Add image replacement support for multicontainer pods in kubectl rolling-update #9868

Conversation

mhenniges commented Jun 16, 2015

googlebot commented Jun 16, 2015

k8s-bot commented Jun 16, 2015

mhenniges commented Jun 16, 2015

erictune commented Jun 16, 2015

erictune commented Jun 16, 2015

erictune commented Jun 16, 2015

erictune commented Jun 16, 2015

googlebot commented Jun 16, 2015

mhenniges commented Jun 16, 2015

mhenniges commented Jun 16, 2015

davidopp commented Jun 16, 2015

k8s-bot commented Jul 24, 2015

bgrant0607 Jul 27, 2015

Choose a reason for hiding this comment

bgrant0607 commented Jul 27, 2015

mhenniges commented Jul 28, 2015

j3ffml commented Jul 28, 2015

bgrant0607 commented Jul 28, 2015

k8s-bot commented Aug 12, 2015

bgrant0607 commented Aug 13, 2015

bgrant0607 Aug 13, 2015

Choose a reason for hiding this comment

bgrant0607 commented Aug 13, 2015

janetkuo commented Aug 24, 2015

k8s-bot commented Aug 24, 2015

k8s-github-robot commented Aug 27, 2015

bgrant0607 commented Oct 21, 2015

coleca commented Oct 21, 2015

janetkuo commented Oct 21, 2015