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

Consider moving rollback out of the Deployment controller #46934

Closed
0xmichalis opened this issue Jun 4, 2017 · 16 comments
Closed

Consider moving rollback out of the Deployment controller #46934

0xmichalis opened this issue Jun 4, 2017 · 16 comments
Assignees
@janetkuo @kow3ns
Labels
area/workload-api/deployment kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/apps Categorizes an issue or PR as relevant to SIG Apps.

Comments

@0xmichalis
Copy link
Contributor

Currently, in order for a Deployment to rollback:
i. a user needs to set d.spec.rollbackTo.revision
ii. the controller notices the populated rollback spec and it updates the pod template of the deployment plus it cleans up the rollbackTo field.

Step ii. is breaking the declarative model for the Deployment API. We should consider dropping rollbackTo from the spec and instead either 1. make the deployments/{name}/rollback endpoint perform the rollback or 2. have the rollback endpoint simply return a dry-run of the rolled back Deployment which then clients can PUT.

If we do 1. we need an option for dry-run, 2. is what we do in OpenShift.

@kubernetes/sig-apps-api-reviews

ps. sparked by the decision to have rollback client-side initially for DaemonSets and StatefulSets.
@k8s-ci-robot k8s-ci-robot added sig/apps Categorizes an issue or PR as relevant to SIG Apps. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API labels Jun 4, 2017
@0xmichalis 0xmichalis changed the title Consider moving the rollback out of the Deployment controller Consider moving rollback out of the Deployment controller Jun 4, 2017
@wanghaoran1988
Copy link
Contributor

/cc

@krmayankk
Copy link

how would automatic rollback work in either of these options ?

@0xmichalis
Copy link
Contributor Author

@krmayankk automatic rollback is a different issue and shouldn't be affected by this one.

@0xmichalis
Copy link
Contributor Author

Worth noting user confusion: #47238 (comment)

@janetkuo
Copy link
Member

Server side rollback isn't implement yet in StatefulSet and DaemonSet. The design of deployment rollback will eventually be consistent with other controllers. We should have a decision before GA.

@janetkuo janetkuo added this to the v1.8 milestone Jun 22, 2017
@crimsonfaith91
Copy link
Contributor

/cc

@janetkuo janetkuo self-assigned this Jul 20, 2017
@janetkuo
Copy link
Member

janetkuo commented Jul 20, 2017 •
edited
Loading

Current plan for 1.8:

  • Deprecate this field in previous version of Deployment (extensions/v1beta1, apps/v1beta1)
  • We won't support /rollback endpoint for new version of Deployment (apps/v1beta2)
    • We will add a deprecated.rollback.to=<revision> annotation (tentative name) for conversion between new and old version of Deployment
  • Deployment controller will continue supporting rollback behavior for backward compatibility
  • Update kubectl rollout undo deployment to support rollback without /rollback endpoint
  • Update Deployment doc

@janetkuo janetkuo mentioned this issue Jul 21, 2017
Merged
@kevin-wangzefeng
Copy link
Member

/cc @kubernetes/huawei

@krmayankk
Copy link

@janetkuo which option are we going with for rollback as proposed by @Kargakis

@bgrant0607
Copy link
Member

I am in favor of moving rollback out of the Deployment controller and out of the DeploymentSpec. I'm not in favor of moving it to the apiserver. It can be done in an external controller. I just approved #49340.

k8s-github-robot pushed a commit that referenced this issue Aug 9, 2017
Merge pull request #49340 from janetkuo/apps-v1beta2-dep-rollback
2bd2127 
Automatic merge from submit-queue

Deprecate Deployment .spec.rollbackTo field 

~Depends on #48746~ (merged)
xref: #46934, #49135

1. Deprecate Deployment field `.spec.rollbackTo` in `extensions/v1beta1` and `apps/v1beta1`, and remove the same field and `/rollback` endpoint from `apps/v1beta2` Deployment. 
1. Add an annotation `deprecated.deployment.rollback.to` in `apps/v1beta2` for conversion to/from other versions. 

Note: `apps/v1beta2` is new in 1.8 (and WIP), so it is okay to make breaking changes to it. 

```release-note
Deprecate Deployment .spec.rollbackTo field 
```
@bgrant0607 bgrant0607 added kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. and removed milestone-labels-incomplete labels Sep 1, 2017
@0xmichalis
Copy link
Contributor Author

@janetkuo I guess this falls through to 1.9?

@janetkuo
Copy link
Member

Rollback is deprecated in 1.8. It will eventually be removed along with extensions/v1beta1 Deployment.

@foxish foxish modified the milestones: v1.8, v1.9 Sep 15, 2017
@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Oct 7, 2017
@janetkuo janetkuo removed the kind/bug Categorizes issue or PR as related to a bug. label Oct 9, 2017
@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Oct 9, 2017
@janetkuo janetkuo removed the kind/bug Categorizes issue or PR as related to a bug. label Oct 9, 2017
@k8s-github-robot
Copy link

[MILESTONENOTIFIER] Milestone Issue Needs Approval

@janetkuo @Kargakis @kubernetes/sig-apps-bugs

Action required: This issue must have the status/approved-for-milestone label applied by a SIG maintainer. If the label is not applied within 6 days, the issue will be moved out of the v1.9 milestone.

Issue Labels
  • sig/apps: Issue will be escalated to these SIGs if needed.
  • priority/important-soon: Escalate to the issue owners and SIG owner; move out of milestone after several unsuccessful escalation attempts.
  • kind/cleanup: Adding tests, refactoring, fixing old bugs.
Help

@k8s-ci-robot k8s-ci-robot added the kind/bug Categorizes issue or PR as related to a bug. label Oct 9, 2017
@janetkuo janetkuo removed this from the v1.9 milestone Oct 9, 2017
@janetkuo janetkuo removed kind/bug Categorizes issue or PR as related to a bug. milestone/needs-approval labels Oct 9, 2017
@fejta-bot
Copy link

Issues go stale after 90d of inactivity.
Mark the issue as fresh with /remove-lifecycle stale.
Stale issues rot after an additional 30d of inactivity and eventually close.

Prevent issues from auto-closing with an /lifecycle frozen comment.

If this issue is safe to close now please do so with /close.

Send feedback to sig-testing, kubernetes/test-infra and/or @fejta.
/lifecycle stale

@k8s-ci-robot k8s-ci-robot added the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 7, 2018
@kow3ns
Copy link
Member

kow3ns commented Jan 8, 2018

@janetkou do we want to keep this open until deprecation is complete?

@kow3ns kow3ns removed the lifecycle/stale Denotes an issue or PR has remained open with no activity and has become stale. label Jan 8, 2018
@tnozicka tnozicka mentioned this issue Feb 20, 2018
Closed
@kow3ns
Copy link
Member

kow3ns commented Feb 27, 2018

/close

@lukaszgryglicki lukaszgryglicki mentioned this issue Mar 1, 2018
Closed
@elanv elanv mentioned this issue Mar 27, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@janetkuo janetkuo

@kow3ns kow3ns

Labels
area/workload-api/deployment kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/cleanup Categorizes issue or PR as related to cleaning up code, process, or technical debt. priority/important-soon Must be staffed and worked on either currently, or very soon, ideally in time for the next release. sig/apps Categorizes an issue or PR as relevant to SIG Apps.
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests