From 1baa473ef2f39ff0d9ada1be3ad0f20a96aa0c53 Mon Sep 17 00:00:00 2001 From: Jeff Lowdermilk Date: Thu, 21 Apr 2016 15:20:32 -0700 Subject: [PATCH 1/2] kubectl rolling-update support for same image --- contrib/completions/bash/kubectl | 1 + docs/man/man1/kubectl-rolling-update.1 | 4 +++ .../kubectl/kubectl_rolling-update.md | 3 +- docs/yaml/kubectl/kubectl_rolling-update.yaml | 3 ++ hack/verify-flags/known-flags.txt | 1 + pkg/kubectl/cmd/rollingupdate.go | 25 ++++++++++--- pkg/kubectl/rolling_updater.go | 36 ++++++++++++------- pkg/kubectl/rolling_updater_test.go | 11 ++++-- 8 files changed, 64 insertions(+), 20 deletions(-) diff --git a/contrib/completions/bash/kubectl b/contrib/completions/bash/kubectl index 3125cd294ec..e68560dbfd3 100644 --- a/contrib/completions/bash/kubectl +++ b/contrib/completions/bash/kubectl @@ -1668,6 +1668,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=") diff --git a/docs/man/man1/kubectl-rolling-update.1 b/docs/man/man1/kubectl-rolling-update.1 index e99bdcdcbf8..9ac6140b280 100644 --- a/docs/man/man1/kubectl-rolling-update.1 +++ b/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] diff --git a/docs/user-guide/kubectl/kubectl_rolling-update.md b/docs/user-guide/kubectl/kubectl_rolling-update.md index cd5551b07e9..172a30d59cb 100644 --- a/docs/user-guide/kubectl/kubectl_rolling-update.md +++ b/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 [![Analytics](https://kubernetes-site.appspot.com/UA-36037335-10/GitHub/docs/user-guide/kubectl/kubectl_rolling-update.md?pixel)]() diff --git a/docs/yaml/kubectl/kubectl_rolling-update.yaml b/docs/yaml/kubectl/kubectl_rolling-update.yaml index 9dc739e96b1..28d2bfa9d5b 100644 --- a/docs/yaml/kubectl/kubectl_rolling-update.yaml +++ b/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: | diff --git a/hack/verify-flags/known-flags.txt b/hack/verify-flags/known-flags.txt index a70d09ba190..2865a422c75 100644 --- a/hack/verify-flags/known-flags.txt +++ b/hack/verify-flags/known-flags.txt @@ -164,6 +164,7 @@ ignore-daemonsets ignore-not-found image-gc-high-threshold image-gc-low-threshold +image-pull-policy include-extended-apis input-dirs insecure-bind-address diff --git a/pkg/kubectl/cmd/rollingupdate.go b/pkg/kubectl/cmd/rollingupdate.go index cdc8b44d86c..8f745e06d86 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.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 { - if oldRc.Spec.Template.Spec.Containers[0].Image == image { - return cmdutil.UsageError(cmd, "Specified --image must be distinct from existing container image") + config := &kubectl.NewControllerConfig{ + Namespace: cmdNamespace, + OldName: oldName, + NewName: newName, + Image: image, + Container: container, + DeploymentKey: deploymentKey, } - newRc, err = kubectl.CreateNewControllerFromCurrentController(client, codec, cmdNamespace, oldName, newName, image, container, deploymentKey) + if oldRc.Spec.Template.Spec.Containers[0].Image == 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, 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 diff --git a/pkg/kubectl/rolling_updater.go b/pkg/kubectl/rolling_updater.go index e69d889a73f..5306f3f88ac 100644 --- a/pkg/kubectl/rolling_updater.go +++ b/pkg/kubectl/rolling_updater.go @@ -504,19 +504,28 @@ 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 @@ -524,31 +533,34 @@ func CreateNewControllerFromCurrentController(c client.Interface, codec runtime. } 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 diff --git a/pkg/kubectl/rolling_updater_test.go b/pkg/kubectl/rolling_updater_test.go index 727e66abcad..88f20ead3ec 100644 --- a/pkg/kubectl/rolling_updater_test.go +++ b/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) } } } From 9b91750284fcc290c365d3d490e722265d87e74c Mon Sep 17 00:00:00 2001 From: Jeff Lowdermilk Date: Mon, 25 Apr 2016 13:38:45 -0700 Subject: [PATCH 2/2] add e2e test for same-image rolling-update --- test/e2e/framework/util.go | 6 +++--- test/e2e/kubectl.go | 36 ++++++++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/test/e2e/framework/util.go b/test/e2e/framework/util.go index ff87ec4ff3d..f799bcf626f 100644 --- a/test/e2e/framework/util.go +++ b/test/e2e/framework/util.go @@ -1416,7 +1416,7 @@ 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)) @@ -1424,13 +1424,13 @@ waitLoop: } 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 diff --git a/test/e2e/kubectl.go b/test/e2e/kubectl.go index 8dfb6911951..788e764bf26 100644 --- a/test/e2e/kubectl.go +++ b/test/e2e/kubectl.go @@ -1020,6 +1020,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 @@ -1444,6 +1478,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. //