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.

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

Rolling Updates: Add support for --rollback. #7575

Merged
merged 1 commit into from May 6, 2015

Conversation

Projects
None yet
7 participants

@googlebot googlebot added the cla: yes label Apr 30, 2015

@jlowdermilk jlowdermilk self-assigned this Apr 30, 2015

@jlowdermilk

This comment has been minimized.

Show comment
Hide comment
@jlowdermilk

jlowdermilk Apr 30, 2015

Member

Looks good, though I would prefer --rollback as the flag name. kubectl rolling update --abort doesn't do a good job of conveying "do a rolling update but in reverse". Will see what others think.

Member

jlowdermilk commented Apr 30, 2015

Looks good, though I would prefer --rollback as the flag name. kubectl rolling update --abort doesn't do a good job of conveying "do a rolling update but in reverse". Will see what others think.

@bgrant0607

This comment has been minimized.

Show comment
Hide comment
@bgrant0607

bgrant0607 Apr 30, 2015

Member

I also am not a fan of abort. I believe @thockin feels similarly.

Member

bgrant0607 commented Apr 30, 2015

I also am not a fan of abort. I believe @thockin feels similarly.

@bgrant0607

This comment has been minimized.

Show comment
Hide comment
@bgrant0607
Member

bgrant0607 commented Apr 30, 2015

Fixes #4140

@bgrant0607

This comment has been minimized.

Show comment
Hide comment
@bgrant0607

bgrant0607 Apr 30, 2015

Member

Previous discussion:
#6713 (comment)

Member

bgrant0607 commented Apr 30, 2015

Previous discussion:
#6713 (comment)

@brendandburns

This comment has been minimized.

Show comment
Hide comment
@brendandburns

brendandburns May 1, 2015

Contributor

I don't super care. If the preference is for --rollback I will switch this PR, and update the docs. Please let me know, I'd like to get this merged.

Contributor

brendandburns commented May 1, 2015

I don't super care. If the preference is for --rollback I will switch this PR, and update the docs. Please let me know, I'd like to get this merged.

@bgrant0607

This comment has been minimized.

Show comment
Hide comment
@bgrant0607

bgrant0607 May 1, 2015

Member

I'm fine with either --abort or --rollback, and will accept the consensus. When talking about it, I'm sure I'll continue to say "rollback".

Additionally, FWIW, a reasonably common pattern internally is for teams to leave the old template around until the next rollout, to facilitate easy rollback. I'd like to expose the "preserve" cleanup policy and would like to support rollback/abort in the scenario where the rollout actually completed but the old RC was not deleted. Don't know if that changes your opinion about which is more appropriate or not.

Member

bgrant0607 commented May 1, 2015

I'm fine with either --abort or --rollback, and will accept the consensus. When talking about it, I'm sure I'll continue to say "rollback".

Additionally, FWIW, a reasonably common pattern internally is for teams to leave the old template around until the next rollout, to facilitate easy rollback. I'd like to expose the "preserve" cleanup policy and would like to support rollback/abort in the scenario where the rollout actually completed but the old RC was not deleted. Don't know if that changes your opinion about which is more appropriate or not.

@ironcladlou

This comment has been minimized.

Show comment
Hide comment
@ironcladlou

ironcladlou May 1, 2015

Contributor

Thinking about this feature in the context of #1743, what effect would this feature have on the proposed monotonic revision number for deployments?

Contributor

ironcladlou commented May 1, 2015

Thinking about this feature in the context of #1743, what effect would this feature have on the proposed monotonic revision number for deployments?

@brendandburns

This comment has been minimized.

Show comment
Hide comment
@brendandburns

brendandburns May 1, 2015

Contributor

@ironcladlou I don't think it should have any effect. At the API level, this is actually a roll-forward to the old version.

That said, maybe you actually want the revision number to rewind?

Can we have a vote on --rollback vs. --abort? I'd like to get this PR merged.

Contributor

brendandburns commented May 1, 2015

@ironcladlou I don't think it should have any effect. At the API level, this is actually a roll-forward to the old version.

That said, maybe you actually want the revision number to rewind?

Can we have a vote on --rollback vs. --abort? I'd like to get this PR merged.

@ironcladlou

This comment has been minimized.

Show comment
Hide comment
@ironcladlou

ironcladlou May 1, 2015

Contributor

Allow me to cast my vote for...

--rollback

Contributor

ironcladlou commented May 1, 2015

Allow me to cast my vote for...

--rollback

@derekwaynecarr

This comment has been minimized.

Show comment
Hide comment
@derekwaynecarr

derekwaynecarr May 1, 2015

Member

--rollback

Sent from my iPhone

On May 1, 2015, at 3:41 PM, Dan Mace notifications@github.com wrote:

Allow me to cast my vote for...

--rollback


Reply to this email directly or view it on GitHub.

Member

derekwaynecarr commented May 1, 2015

--rollback

Sent from my iPhone

On May 1, 2015, at 3:41 PM, Dan Mace notifications@github.com wrote:

Allow me to cast my vote for...

--rollback


Reply to this email directly or view it on GitHub.

@pmorie

This comment has been minimized.

Show comment
Hide comment
@pmorie

pmorie May 1, 2015

Member

--rollback (fwiw)

Member

pmorie commented May 1, 2015

--rollback (fwiw)

@brendandburns

This comment has been minimized.

Show comment
Hide comment
@brendandburns

brendandburns May 2, 2015

Contributor

Switched to --rollback by popular opinion. Updated docs to match.

ptal
Thanks!
--brendan

Contributor

brendandburns commented May 2, 2015

Switched to --rollback by popular opinion. Updated docs to match.

ptal
Thanks!
--brendan

@jlowdermilk

This comment has been minimized.

Show comment
Hide comment
@jlowdermilk

jlowdermilk May 2, 2015

Member

lgtm. will merge on green if no objections

Member

jlowdermilk commented May 2, 2015

lgtm. will merge on green if no objections

@brendandburns brendandburns changed the title from Add support for --abort. to Rolling Updates: Add support for --rollback. May 2, 2015

@bgrant0607

This comment has been minimized.

Show comment
Hide comment
@bgrant0607

bgrant0607 May 2, 2015

Member

There's no test. Did you at least try this?

The rename cleanup policy is the default. Does that make sense in this case?

Member

bgrant0607 commented May 2, 2015

There's no test. Did you at least try this?

The rename cleanup policy is the default. Does that make sense in this case?

@brendandburns

This comment has been minimized.

Show comment
Hide comment
@brendandburns

brendandburns May 5, 2015

Contributor

Yes, this has been manually tested. Without a better test framework in place this is tricky to test in an automated way. I could probably get something going in test-cmd.sh if that is required.

Thanks
--brendan

Contributor

brendandburns commented May 5, 2015

Yes, this has been manually tested. Without a better test framework in place this is tricky to test in an automated way. I could probably get something going in test-cmd.sh if that is required.

Thanks
--brendan

@jlowdermilk

This comment has been minimized.

Show comment
Hide comment
@jlowdermilk

jlowdermilk May 6, 2015

Member

lgtm, but you should probably change the commit description

Member

jlowdermilk commented May 6, 2015

lgtm, but you should probably change the commit description

@bgrant0607

This comment has been minimized.

Show comment
Hide comment
@bgrant0607

bgrant0607 May 6, 2015

Member

LGTM

Member

bgrant0607 commented May 6, 2015

LGTM

@bgrant0607

This comment has been minimized.

Show comment
Hide comment
@bgrant0607

bgrant0607 May 6, 2015

Member

The commit description does still refer to --abort

Member

bgrant0607 commented May 6, 2015

The commit description does still refer to --abort

@brendandburns

This comment has been minimized.

Show comment
Hide comment
@brendandburns

brendandburns May 6, 2015

Contributor

Changed the commit description.

Thanks!
--brendan

Contributor

brendandburns commented May 6, 2015

Changed the commit description.

Thanks!
--brendan

bgrant0607 added a commit that referenced this pull request May 6, 2015

Merge pull request #7575 from brendandburns/kubectl
Rolling Updates: Add support for --rollback.

@bgrant0607 bgrant0607 merged commit 32b4b1c into kubernetes:master May 6, 2015

2 of 3 checks passed

continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
Shippable Shippable builds completed
Details
cla/google All necessary CLAs are signed

xingzhou pushed a commit to xingzhou/kubernetes that referenced this pull request Dec 15, 2016

Merge pull request #7575 from brendandburns/kubectl
Rolling Updates: Add support for --rollback.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment