Merge pull request kubernetes#24645 from jlowdermilk/rolling-update

Automatic merge from submit-queue

kubectl rolling-update support for same image

Fixes kubernetes#23497.

Enables `kubectl rolling-update --image` to the same image, adding a `--image-pull-policy` flag to remove ambiguity. This allows rolling-update to behave as an "update and/or restart" (kubernetes#23497 (comment)), or as a forced update when the same tag can mean multiple versions (e.g. `:latest`). cc @janetkuo @nikhiljindal

contrib/completions/bash/kubectl

@@ -1650,6 +1650,7 @@ _kubectl_rolling-update()
     flags_with_completion+=("-f")
     flags_completion+=("__handle_filename_extension_flag json|yaml|yml")
     flags+=("--image=")
+    flags+=("--image-pull-policy=")
     flags+=("--include-extended-apis")
     flags+=("--no-headers")
     flags+=("--output=")

docs/man/man1/kubectl-rolling-update.1

@@ -42,6 +42,10 @@ existing replication controller and overwrite at least one (common) label in its
 \fB\-\-image\fP=""
     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
 
+.PP
+\fB\-\-image\-pull\-policy\fP=""
+    Explicit policy for when to pull container images. Required when \-\-image is same as existing image, ignored otherwise.
+
 .PP
 \fB\-\-include\-extended\-apis\fP=true
     If true, include definitions of new APIs via calls to the API server. [default true]

docs/user-guide/kubectl/kubectl_rolling-update.md

@@ -78,6 +78,7 @@ kubectl rolling-update frontend-v1 frontend-v2 --rollback
       --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.
       --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
+      --image-pull-policy="": Explicit policy for when to pull container images. Required when --image is same as existing image, ignored otherwise.
       --include-extended-apis[=true]: If true, include definitions of new APIs via calls to the API server. [default true]
       --no-headers[=false]: When using the default output, don't print headers.
   -o, --output="": Output format. One of: json|yaml|wide|name|go-template=...|go-template-file=...|jsonpath=...|jsonpath-file=... See golang template [http://golang.org/pkg/text/template/#pkg-overview] and jsonpath template [http://releases.k8s.io/HEAD/docs/user-guide/jsonpath.md].
@@ -126,7 +127,7 @@ kubectl rolling-update frontend-v1 frontend-v2 --rollback
 
 * [kubectl](kubectl.md)	 - kubectl controls the Kubernetes cluster manager
 
-###### Auto generated by spf13/cobra on 5-Apr-2016
+###### Auto generated by spf13/cobra on 21-Apr-2016
 
 <!-- BEGIN MUNGE: GENERATED_ANALYTICS -->
 [![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/docs/user-guide/kubectl/kubectl_rolling-update.md?pixel)]()

docs/yaml/kubectl/kubectl_rolling-update.yaml

@@ -26,6 +26,9 @@ options:
 - name: image
   usage: |
     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
+- name: image-pull-policy
+  usage: |
+    Explicit policy for when to pull container images. Required when --image is same as existing image, ignored otherwise.
 - name: include-extended-apis
   default_value: "true"
   usage: |

hack/verify-flags/known-flags.txt

@@ -167,6 +167,7 @@ ignore-daemonsets
 ignore-not-found
 image-gc-high-threshold
 image-gc-low-threshold
+image-pull-policy
 include-extended-apis
 input-base
 input-dirs

pkg/kubectl/cmd/rollingupdate.go

@@ -97,6 +97,7 @@ func NewCmdRollingUpdate(f *cmdutil.Factory, out io.Writer) *cobra.Command {
 	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().String("image-pull-policy", "", "Explicit policy for when to pull container images. Required when --image is same as existing image, ignored otherwise.")
 	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)
@@ -150,6 +151,7 @@ func RunRollingUpdate(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, arg
 	deploymentKey := cmdutil.GetFlagString(cmd, "deployment-label-key")
 	filename := ""
 	image := cmdutil.GetFlagString(cmd, "image")
+	pullPolicy := cmdutil.GetFlagString(cmd, "image-pull-policy")
 	oldName := args[0]
 	rollback := cmdutil.GetFlagBool(cmd, "rollback")
 	period := cmdutil.GetFlagDuration(cmd, "update-period")
@@ -233,8 +235,8 @@ func RunRollingUpdate(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, arg
 		}
 	}
 	// If the --image option is specified, we need to create a new rc with at least one different selector
-	// than the old rc. This selector is the hash of the rc, which will differ because the new rc has a
-	// different image.
+	// than the old rc. This selector is the hash of the rc, with a suffix to provide uniqueness for
+	// same-image updates.
 	if len(image) != 0 {
 		codec := api.Codecs.LegacyCodec(client.APIVersion())
 		keepOldName = len(args) == 1
@@ -248,10 +250,21 @@ func RunRollingUpdate(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, arg
 			}
 			fmt.Fprintf(out, "Found existing update in progress (%s), resuming.\n", newRc.Name)
 		} else {
+			config := &kubectl.NewControllerConfig{
+				Namespace:     cmdNamespace,
+				OldName:       oldName,
+				NewName:       newName,
+				Image:         image,
+				Container:     container,
+				DeploymentKey: deploymentKey,
+			}
 			if oldRc.Spec.Template.Spec.Containers[0].Image == image {
-				return cmdutil.UsageError(cmd, "Specified --image must be distinct from existing container image")
+				if len(pullPolicy) == 0 {
+					return cmdutil.UsageError(cmd, "--image-pull-policy (Always|Never|IfNotPresent) must be provided when --image is the same as existing container image")
+				}
+				config.PullPolicy = api.PullPolicy(pullPolicy)
 			}
-			newRc, err = kubectl.CreateNewControllerFromCurrentController(client, codec, cmdNamespace, oldName, newName, image, container, deploymentKey)
+			newRc, err = kubectl.CreateNewControllerFromCurrentController(client, codec, config)
 			if err != nil {
 				return err
 			}
@@ -262,6 +275,8 @@ func RunRollingUpdate(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, arg
 		if err != nil {
 			return err
 		}
+		// If new image is same as old, the hash may not be distinct, so add a suffix.
+		oldHash += "-orig"
 		oldRc, err = kubectl.UpdateExistingReplicationController(client, oldRc, cmdNamespace, newRc.Name, deploymentKey, oldHash, out)
 		if err != nil {
 			return err

pkg/kubectl/rolling_updater.go

@@ -505,51 +505,63 @@ func LoadExistingNextReplicationController(c client.ReplicationControllersNamesp
 	return newRc, err
 }
 
-func CreateNewControllerFromCurrentController(c client.Interface, codec runtime.Codec, namespace, oldName, newName, image, container, deploymentKey string) (*api.ReplicationController, error) {
+type NewControllerConfig struct {
+	Namespace        string
+	OldName, NewName string
+	Image            string
+	Container        string
+	DeploymentKey    string
+	PullPolicy       api.PullPolicy
+}
+
+func CreateNewControllerFromCurrentController(c client.Interface, codec runtime.Codec, cfg *NewControllerConfig) (*api.ReplicationController, error) {
 	containerIndex := 0
 	// load the old RC into the "new" RC
-	newRc, err := c.ReplicationControllers(namespace).Get(oldName)
+	newRc, err := c.ReplicationControllers(cfg.Namespace).Get(cfg.OldName)
 	if err != nil {
 		return nil, err
 	}
 
-	if len(container) != 0 {
+	if len(cfg.Container) != 0 {
 		containerFound := false
 
 		for i, c := range newRc.Spec.Template.Spec.Containers {
-			if c.Name == container {
+			if c.Name == cfg.Container {
 				containerIndex = i
 				containerFound = true
 				break
 			}
 		}
 
 		if !containerFound {
-			return nil, fmt.Errorf("container %s not found in pod", container)
+			return nil, fmt.Errorf("container %s not found in pod", cfg.Container)
 		}
 	}
 
-	if len(newRc.Spec.Template.Spec.Containers) > 1 && len(container) == 0 {
+	if len(newRc.Spec.Template.Spec.Containers) > 1 && len(cfg.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[containerIndex].Image = image
+	newRc.Spec.Template.Spec.Containers[containerIndex].Image = cfg.Image
+	if len(cfg.PullPolicy) != 0 {
+		newRc.Spec.Template.Spec.Containers[containerIndex].ImagePullPolicy = cfg.PullPolicy
+	}
 
 	newHash, err := api.HashObject(newRc, codec)
 	if err != nil {
 		return nil, err
 	}
 
-	if len(newName) == 0 {
-		newName = fmt.Sprintf("%s-%s", newRc.Name, newHash)
+	if len(cfg.NewName) == 0 {
+		cfg.NewName = fmt.Sprintf("%s-%s", newRc.Name, newHash)
 	}
-	newRc.Name = newName
+	newRc.Name = cfg.NewName
 
-	newRc.Spec.Selector[deploymentKey] = newHash
-	newRc.Spec.Template.Labels[deploymentKey] = newHash
+	newRc.Spec.Selector[cfg.DeploymentKey] = newHash
+	newRc.Spec.Template.Labels[cfg.DeploymentKey] = newHash
 	// Clear resource version after hashing so that identical updates get different hashes.
 	newRc.ResourceVersion = ""
 	return newRc, nil

pkg/kubectl/rolling_updater_test.go

@@ -1090,12 +1090,19 @@ func TestRollingUpdater_multipleContainersInPod(t *testing.T) {
 		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)
+		config := &NewControllerConfig{
+			OldName:       test.oldRc.ObjectMeta.Name,
+			NewName:       test.newRc.ObjectMeta.Name,
+			Image:         test.image,
+			Container:     test.container,
+			DeploymentKey: test.deploymentKey,
+		}
+		updatedRc, err := CreateNewControllerFromCurrentController(fake, codec, config)
 		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)
+			t.Errorf("expected:\n%#v\ngot:\n%#v\n", test.newRc, updatedRc)
 		}
 	}
 }

test/e2e/framework/util.go

@@ -1419,21 +1419,21 @@ func ValidateController(c *client.Client, containerImage string, replicas int, c
 	By(fmt.Sprintf("waiting for all containers in %s pods to come up.", testname)) //testname should be selector
 waitLoop:
 	for start := time.Now(); time.Since(start) < PodStartTimeout; time.Sleep(5 * time.Second) {
-		getPodsOutput := RunKubectlOrDie("get", "pods", "-o", "template", getPodsTemplate, "--api-version=v1", "-l", testname, fmt.Sprintf("--namespace=%v", ns))
+		getPodsOutput := RunKubectlOrDie("get", "pods", "-o", "template", getPodsTemplate, "-l", testname, fmt.Sprintf("--namespace=%v", ns))
 		pods := strings.Fields(getPodsOutput)
 		if numPods := len(pods); numPods != replicas {
 			By(fmt.Sprintf("Replicas for %s: expected=%d actual=%d", testname, replicas, numPods))
 			continue
 		}
 		var runningPods []string
 		for _, podID := range pods {
-			running := RunKubectlOrDie("get", "pods", podID, "-o", "template", getContainerStateTemplate, "--api-version=v1", fmt.Sprintf("--namespace=%v", ns))
+			running := RunKubectlOrDie("get", "pods", podID, "-o", "template", getContainerStateTemplate, fmt.Sprintf("--namespace=%v", ns))
 			if running != "true" {
 				Logf("%s is created but not running", podID)
 				continue waitLoop
 			}
 
-			currentImage := RunKubectlOrDie("get", "pods", podID, "-o", "template", getImageTemplate, "--api-version=v1", fmt.Sprintf("--namespace=%v", ns))
+			currentImage := RunKubectlOrDie("get", "pods", podID, "-o", "template", getImageTemplate, fmt.Sprintf("--namespace=%v", ns))
 			if currentImage != containerImage {
 				Logf("%s is created but running wrong image; expected: %s, actual: %s", podID, containerImage, currentImage)
 				continue waitLoop

test/e2e/kubectl.go

@@ -1025,6 +1025,40 @@ var _ = framework.KubeDescribe("Kubectl client", func() {
 		})
 	})
 
+	framework.KubeDescribe("Kubectl rolling-update", func() {
+		var nsFlag string
+		var rcName string
+		var c *client.Client
+
+		BeforeEach(func() {
+			c = f.Client
+			nsFlag = fmt.Sprintf("--namespace=%v", ns)
+			rcName = "e2e-test-nginx-rc"
+		})
+
+		AfterEach(func() {
+			framework.RunKubectlOrDie("delete", "rc", rcName, nsFlag)
+		})
+
+		It("should support rolling-update to same image [Conformance]", func() {
+			By("running the image " + nginxImage)
+			framework.RunKubectlOrDie("run", rcName, "--image="+nginxImage, "--generator=run/v1", nsFlag)
+			By("verifying the rc " + rcName + " was created")
+			rc, err := c.ReplicationControllers(ns).Get(rcName)
+			if err != nil {
+				framework.Failf("Failed getting rc %s: %v", rcName, err)
+			}
+			containers := rc.Spec.Template.Spec.Containers
+			if containers == nil || len(containers) != 1 || containers[0].Image != nginxImage {
+				framework.Failf("Failed creating rc %s for 1 pod with expected image %s", rcName, nginxImage)
+			}
+
+			By("rolling-update to same image controller")
+			framework.RunKubectlOrDie("rolling-update", rcName, "--update-period=1s", "--image="+nginxImage, "--image-pull-policy="+string(api.PullIfNotPresent), nsFlag)
+			framework.ValidateController(c, nginxImage, 1, rcName, "run="+rcName, noOpValidatorFn, ns)
+		})
+	})
+
 	framework.KubeDescribe("Kubectl run deployment", func() {
 		var nsFlag string
 		var dName string
@@ -1449,6 +1483,8 @@ func getUDData(jpgExpected string, ns string) func(*client.Client, string) error
 	}
 }
 
+func noOpValidatorFn(c *client.Client, podID string) error { return nil }
+
 // newBlockingReader returns a reader that allows reading the given string,
 // then blocks until Close() is called on the returned closer.
 //