From 61306c2a0f6f03be36b42abf066bd9b1500972d4 Mon Sep 17 00:00:00 2001 From: James Munnelly Date: Wed, 11 Nov 2015 12:38:56 +0000 Subject: [PATCH] Allow rolling-update of a single container in multi-container pods Accept codec as parameter to CreateNewControllerFromCurrentController function. Add tests for performing a rolling update on a container in a multi-container pod. --- contrib/completions/bash/kubectl | 1 + docs/man/man1/kubectl-rolling-update.1 | 4 + .../kubectl/kubectl_rolling-update.md | 3 +- pkg/kubectl/cmd/rollingupdate.go | 4 +- pkg/kubectl/rolling_updater.go | 30 +++- pkg/kubectl/rolling_updater_test.go | 160 ++++++++++++++++++ 6 files changed, 194 insertions(+), 8 deletions(-) diff --git a/contrib/completions/bash/kubectl b/contrib/completions/bash/kubectl index 22cfcfc5a3ba5..19bce1420c13e 100644 --- a/contrib/completions/bash/kubectl +++ b/contrib/completions/bash/kubectl @@ -587,6 +587,7 @@ _kubectl_rolling-update() flags_with_completion=() flags_completion=() + flags+=("--container=") flags+=("--deployment-label-key=") flags+=("--dry-run") flags+=("--filename=") diff --git a/docs/man/man1/kubectl-rolling-update.1 b/docs/man/man1/kubectl-rolling-update.1 index bcbcf39f27d2c..f492d35fbe5ab 100644 --- a/docs/man/man1/kubectl-rolling-update.1 +++ b/docs/man/man1/kubectl-rolling-update.1 @@ -22,6 +22,10 @@ existing replication controller and overwrite at least one (common) label in its .SH OPTIONS +.PP +\fB\-\-container\fP="" + 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 + .PP \fB\-\-deployment\-label\-key\fP="deployment" The key to use to differentiate between two different controllers, default 'deployment'. Only relevant when \-\-image is specified, ignored otherwise diff --git a/docs/user-guide/kubectl/kubectl_rolling-update.md b/docs/user-guide/kubectl/kubectl_rolling-update.md index f9d32c0a15a52..5d3082d7c8b28 100644 --- a/docs/user-guide/kubectl/kubectl_rolling-update.md +++ b/docs/user-guide/kubectl/kubectl_rolling-update.md @@ -72,6 +72,7 @@ $ kubectl rolling-update frontend-v1 frontend-v2 --rollback ### Options ``` + --container="": 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 --deployment-label-key="deployment": The key to use to differentiate between two different controllers, default 'deployment'. Only relevant when --image is specified, ignored otherwise --dry-run[=false]: If true, print out the changes that would be made, but don't actually make them. -f, --filename=[]: Filename or URL to file to use to create the new replication controller. @@ -122,7 +123,7 @@ $ kubectl rolling-update frontend-v1 frontend-v2 --rollback * [kubectl](kubectl.md) - kubectl controls the Kubernetes cluster manager -###### Auto generated by spf13/cobra on 24-Nov-2015 +###### Auto generated by spf13/cobra on 26-Nov-2015 [![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/docs/user-guide/kubectl/kubectl_rolling-update.md?pixel)]() diff --git a/pkg/kubectl/cmd/rollingupdate.go b/pkg/kubectl/cmd/rollingupdate.go index d462b9285988f..3edee352b547a 100644 --- a/pkg/kubectl/cmd/rollingupdate.go +++ b/pkg/kubectl/cmd/rollingupdate.go @@ -97,6 +97,7 @@ func NewCmdRollingUpdate(f *cmdutil.Factory, out io.Writer) *cobra.Command { cmd.Flags().String("image", "", "Image to use for upgrading the replication controller. Must be distinct from the existing image (either new image or new image tag). Can not be used with --filename/-f") cmd.MarkFlagRequired("image") cmd.Flags().String("deployment-label-key", "deployment", "The key to use to differentiate between two different controllers, default 'deployment'. Only relevant when --image is specified, ignored otherwise") + cmd.Flags().String("container", "", "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") cmd.Flags().Bool("dry-run", false, "If true, print out the changes that would be made, but don't actually make them.") cmd.Flags().Bool("rollback", false, "If true, this is a request to abort an existing rollout that is partially rolled out. It effectively reverses current and next and runs a rollout") cmdutil.AddValidateFlags(cmd) @@ -155,6 +156,7 @@ func RunRollingUpdate(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, arg timeout := cmdutil.GetFlagDuration(cmd, "timeout") dryrun := cmdutil.GetFlagBool(cmd, "dry-run") outputFormat := cmdutil.GetFlagString(cmd, "output") + container := cmdutil.GetFlagString(cmd, "container") if len(options.Filenames) > 0 { filename = options.Filenames[0] @@ -247,7 +249,7 @@ func RunRollingUpdate(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, arg if oldRc.Spec.Template.Spec.Containers[0].Image == image { return cmdutil.UsageError(cmd, "Specified --image must be distinct from existing container image") } - newRc, err = kubectl.CreateNewControllerFromCurrentController(client, cmdNamespace, oldName, newName, image, deploymentKey) + newRc, err = kubectl.CreateNewControllerFromCurrentController(client, client.Codec, cmdNamespace, oldName, newName, image, container, deploymentKey) if err != nil { return err } diff --git a/pkg/kubectl/rolling_updater.go b/pkg/kubectl/rolling_updater.go index 7639cf981a992..b2049a96fd4a2 100644 --- a/pkg/kubectl/rolling_updater.go +++ b/pkg/kubectl/rolling_updater.go @@ -30,6 +30,7 @@ import ( client "k8s.io/kubernetes/pkg/client/unversioned" "k8s.io/kubernetes/pkg/fields" "k8s.io/kubernetes/pkg/labels" + "k8s.io/kubernetes/pkg/runtime" "k8s.io/kubernetes/pkg/util/intstr" "k8s.io/kubernetes/pkg/util/wait" ) @@ -531,23 +532,40 @@ func LoadExistingNextReplicationController(c client.ReplicationControllersNamesp return newRc, err } -func CreateNewControllerFromCurrentController(c *client.Client, namespace, oldName, newName, image, deploymentKey string) (*api.ReplicationController, error) { +func CreateNewControllerFromCurrentController(c client.Interface, codec runtime.Codec, namespace, oldName, newName, image, container, deploymentKey string) (*api.ReplicationController, error) { + containerIndex := 0 // load the old RC into the "new" RC newRc, err := c.ReplicationControllers(namespace).Get(oldName) if err != nil { return nil, err } - if len(newRc.Spec.Template.Spec.Containers) > 1 { - // TODO: support multi-container image update. - return nil, goerrors.New("Image update is not supported for multi-container pods") + if len(container) != 0 { + containerFound := false + + for i, c := range newRc.Spec.Template.Spec.Containers { + if c.Name == container { + containerIndex = i + containerFound = true + break + } + } + + if !containerFound { + return nil, fmt.Errorf("container %s not found in pod", container) + } + } + + if len(newRc.Spec.Template.Spec.Containers) > 1 && len(container) == 0 { + return nil, goerrors.New("Must specify container to update when updating a multi-container pod") } + if len(newRc.Spec.Template.Spec.Containers) == 0 { return nil, goerrors.New(fmt.Sprintf("Pod has no containers! (%v)", newRc)) } - newRc.Spec.Template.Spec.Containers[0].Image = image + newRc.Spec.Template.Spec.Containers[containerIndex].Image = image - newHash, err := api.HashObject(newRc, c.Codec) + newHash, err := api.HashObject(newRc, codec) if err != nil { return nil, err } diff --git a/pkg/kubectl/rolling_updater_test.go b/pkg/kubectl/rolling_updater_test.go index 796f387732617..fbeec5d086066 100644 --- a/pkg/kubectl/rolling_updater_test.go +++ b/pkg/kubectl/rolling_updater_test.go @@ -747,6 +747,166 @@ func TestUpdate_assignOriginalAnnotation(t *testing.T) { } } +func TestRollingUpdater_multipleContainersInPod(t *testing.T) { + tests := []struct { + oldRc *api.ReplicationController + newRc *api.ReplicationController + + container string + image string + deploymentKey string + }{ + { + oldRc: &api.ReplicationController{ + ObjectMeta: api.ObjectMeta{ + Name: "foo", + }, + Spec: api.ReplicationControllerSpec{ + Selector: map[string]string{ + "dk": "old", + }, + Template: &api.PodTemplateSpec{ + ObjectMeta: api.ObjectMeta{ + Labels: map[string]string{ + "dk": "old", + }, + }, + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: "container1", + Image: "image1", + }, + { + Name: "container2", + Image: "image2", + }, + }, + }, + }, + }, + }, + newRc: &api.ReplicationController{ + ObjectMeta: api.ObjectMeta{ + Name: "foo", + }, + Spec: api.ReplicationControllerSpec{ + Selector: map[string]string{ + "dk": "old", + }, + Template: &api.PodTemplateSpec{ + ObjectMeta: api.ObjectMeta{ + Labels: map[string]string{ + "dk": "old", + }, + }, + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: "container1", + Image: "newimage", + }, + { + Name: "container2", + Image: "image2", + }, + }, + }, + }, + }, + }, + container: "container1", + image: "newimage", + deploymentKey: "dk", + }, + { + oldRc: &api.ReplicationController{ + ObjectMeta: api.ObjectMeta{ + Name: "bar", + }, + Spec: api.ReplicationControllerSpec{ + Selector: map[string]string{ + "dk": "old", + }, + Template: &api.PodTemplateSpec{ + ObjectMeta: api.ObjectMeta{ + Labels: map[string]string{ + "dk": "old", + }, + }, + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: "container1", + Image: "image1", + }, + }, + }, + }, + }, + }, + newRc: &api.ReplicationController{ + ObjectMeta: api.ObjectMeta{ + Name: "bar", + }, + Spec: api.ReplicationControllerSpec{ + Selector: map[string]string{ + "dk": "old", + }, + Template: &api.PodTemplateSpec{ + ObjectMeta: api.ObjectMeta{ + Labels: map[string]string{ + "dk": "old", + }, + }, + Spec: api.PodSpec{ + Containers: []api.Container{ + { + Name: "container1", + Image: "newimage", + }, + }, + }, + }, + }, + }, + container: "container1", + image: "newimage", + deploymentKey: "dk", + }, + } + + for _, test := range tests { + fake := &testclient.Fake{} + fake.AddReactor("*", "*", func(action testclient.Action) (handled bool, ret runtime.Object, err error) { + switch action.(type) { + case testclient.GetAction: + return true, test.oldRc, nil + } + return false, nil, nil + }) + + codec := testapi.Default.Codec() + + deploymentHash, err := api.HashObject(test.newRc, codec) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + + test.newRc.Spec.Selector[test.deploymentKey] = deploymentHash + test.newRc.Spec.Template.Labels[test.deploymentKey] = deploymentHash + test.newRc.Name = fmt.Sprintf("%s-%s", test.newRc.Name, deploymentHash) + + updatedRc, err := CreateNewControllerFromCurrentController(fake, codec, "", test.oldRc.ObjectMeta.Name, test.newRc.ObjectMeta.Name, test.image, test.container, test.deploymentKey) + if err != nil { + t.Errorf("unexpected error: %v", err) + } + if !reflect.DeepEqual(updatedRc, test.newRc) { + t.Errorf("expected:\n%v\ngot:\n%v\n", test.newRc, updatedRc) + } + } +} + // TestRollingUpdater_cleanupWithClients ensures that the cleanup policy is // correctly implemented. func TestRollingUpdater_cleanupWithClients(t *testing.T) {