Merge pull request kubernetes#17111 from munnerz/multi-rolling-update

Allow rolling-update of a single container in multi-container pods
nikhiljindal · Dec 11, 2015 · e53acfe · e53acfe
2 parents 30dc184 + 61306c2
commit e53acfe
Show file tree

Hide file tree

Showing 6 changed files with 194 additions and 8 deletions.
diff --git a/contrib/completions/bash/kubectl b/contrib/completions/bash/kubectl
@@ -836,6 +836,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
@@ -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
@@ -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
 
 <!-- BEGIN MUNGE: GENERATED_ANALYTICS -->
 [![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
@@ -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
@@ -30,6 +30,7 @@ import (
 	"k8s.io/kubernetes/pkg/api/unversioned"
 	client "k8s.io/kubernetes/pkg/client/unversioned"
 	"k8s.io/kubernetes/pkg/labels"
+	"k8s.io/kubernetes/pkg/runtime"
 	"k8s.io/kubernetes/pkg/util/intstr"
 	"k8s.io/kubernetes/pkg/util/wait"
 )
@@ -532,23 +533,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
@@ -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) {