Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

kubectl rolling-update support for same image #24645

Merged
merged 2 commits into from
May 4, 2016
Merged

kubectl rolling-update support for same image #24645

merged 2 commits into from
May 4, 2016

Conversation

j3ffml
Copy link
Contributor

@j3ffml j3ffml commented Apr 22, 2016

Fixes #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" (#23497 (comment)), or as a forced update when the same tag can mean multiple versions (e.g. :latest). cc @janetkuo @nikhiljindal

@j3ffml j3ffml added area/kubectl release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Apr 22, 2016
@j3ffml j3ffml added this to the v1.2 milestone Apr 22, 2016
@j3ffml
Copy link
Contributor Author

j3ffml commented Apr 22, 2016

ref #23516. This handles that use case safely.

@k8s-github-robot k8s-github-robot added kind/old-docs size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 22, 2016
zhouhaibing089
zhouhaibing089 reviewed Apr 23, 2016
View reviewed changes
pkg/kubectl/cmd/rollingupdate.go
@@ -262,6 +275,7 @@ func RunRollingUpdate(f *cmdutil.Factory, out io.Writer, cmd *cobra.Command, arg
if err != nil {
return err
}
oldHash += "-orig"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe give a comment here on why adding this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done.

@janetkuo
Copy link
Member

janetkuo commented Apr 25, 2016 •
edited
Loading

Suggest adding a test for this.

@nikhiljindal
Copy link
Contributor

Is there a plan to support the same behavior in deployments as well?

@j3ffml
Copy link
Contributor Author

j3ffml commented Apr 25, 2016

Added test. @nikhiljindal, AFAIK, same-image rolling updates are not being supported in deployments. See #23497 (comment)

@k8s-github-robot k8s-github-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Apr 25, 2016
@deads2k deads2k assigned janetkuo and unassigned deads2k Apr 26, 2016
@j3ffml
Copy link
Contributor Author

j3ffml commented Apr 27, 2016

friendly ping

@janetkuo janetkuo added priority/backlog Higher priority than priority/awaiting-more-evidence. lgtm "Looks good to me", indicates that a PR is ready to be merged. labels May 3, 2016
@k8s-github-robot
Copy link

@k8s-bot test this [submit-queue is verifying that this PR is safe to merge]

@k8s-bot
Copy link

k8s-bot commented May 4, 2016

GCE e2e build/test passed for commit 9b91750.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 1642083 into kubernetes:master May 4, 2016
@j3ffml j3ffml mentioned this pull request May 4, 2016
Closed
@roberthbailey roberthbailey added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label May 4, 2016
@roberthbailey roberthbailey removed the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label May 5, 2016
@roberthbailey
Copy link
Contributor

@jlowdermilk Please ping once the flaky test issue has been resolved and I'll re-evaluate the cherry pick status.

@roberthbailey roberthbailey added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label May 23, 2016
@j3ffml j3ffml mentioned this pull request May 23, 2016
Merged
roberthbailey added a commit that referenced this pull request May 23, 2016
@roberthbailey
Merge pull request #26088 from jlowdermilk/automated-cherry-pick-of-#…
672d5a7 
…24645-upstream-release-1.2

Automated cherry pick of #24645
@k8s-cherrypick-bot
Copy link

Commit found in the "release-1.2" branch appears to be this PR. Removing the "cherrypick-candidate" label. If this is an error find help to get your PR picked.

osterman
osterman reviewed Jun 27, 2016
View reviewed changes
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
Copy link

@osterman osterman Jun 27, 2016 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jlowdermilk : I am confused. Does it still need to be distinct? I thought the point of this PR was to allow rolling-updates of the "same" (e.g. non-distinct) image so long as --image-pull-policy=always (as noted below). Should it instead read "Must be distinct from the existing image (either new image or new image tag) unless the --image-pull-policy=always argument is passed."

@j3ffml j3ffml deleted the rolling-update branch October 19, 2016 22:17
shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016
@roberthbailey
Merge pull request kubernetes#26088 from jlowdermilk/automated-cherry…
80c6cb7 
…-pick-of-#24645-upstream-release-1.2

Automated cherry pick of kubernetes#24645
shouhong pushed a commit to shouhong/kubernetes that referenced this pull request Feb 14, 2017
@roberthbailey
Merge pull request kubernetes#26088 from jlowdermilk/automated-cherry…
d8e5cc6 
…-pick-of-#24645-upstream-release-1.2

Automated cherry pick of kubernetes#24645
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@janetkuo janetkuo

Labels
area/kubectl cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. lgtm "Looks good to me", indicates that a PR is ready to be merged. priority/backlog Higher priority than priority/awaiting-more-evidence. release-note Denotes a PR that will be considered when it comes time to generate release notes. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.
Projects
None yet
Milestone
v1.2
Development

Successfully merging this pull request may close these issues.