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

Deprecate Deployment .spec.rollbackTo field #49340

Merged
merged 4 commits into from
Aug 9, 2017
Merged

Deprecate Deployment .spec.rollbackTo field #49340

merged 4 commits into from
Aug 9, 2017

Conversation

janetkuo
Copy link
Member

@janetkuo janetkuo commented Jul 21, 2017 •
edited
Loading

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.
  2. 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.

Deprecate Deployment .spec.rollbackTo field

@janetkuo janetkuo added this to the v1.8 milestone Jul 21, 2017
@janetkuo janetkuo self-assigned this Jul 21, 2017
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Jul 21, 2017
@k8s-github-robot k8s-github-robot added kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API kind/new-api size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Jul 21, 2017
@kow3ns kow3ns mentioned this pull request Jul 21, 2017
Closed
11 tasks
@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 21, 2017
@janetkuo janetkuo force-pushed the apps-v1beta2-dep-rollback branch from 8830b2f to 2e2bb14 Compare July 21, 2017 20:58
@k8s-github-robot k8s-github-robot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. kind/new-api size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. labels Jul 21, 2017
@janetkuo janetkuo force-pushed the apps-v1beta2-dep-rollback branch from 2e2bb14 to c281dea Compare July 26, 2017 00:36
@k8s-github-robot k8s-github-robot added size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Jul 26, 2017
@janetkuo janetkuo assigned 0xmichalis and unassigned janetkuo Jul 26, 2017
@janetkuo
Copy link
Member Author

@kubernetes/sig-apps-api-reviews

@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 Jul 26, 2017
@janetkuo janetkuo force-pushed the apps-v1beta2-dep-rollback branch from c281dea to e8b731c Compare July 26, 2017 02:19
@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 26, 2017
@janetkuo janetkuo force-pushed the apps-v1beta2-dep-rollback branch from e8b731c to 5580af1 Compare July 26, 2017 23:16
@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jul 26, 2017
@janetkuo janetkuo force-pushed the apps-v1beta2-dep-rollback branch from 5580af1 to a6625e5 Compare July 26, 2017 23:56
@janetkuo
Copy link
Member Author

Seems a test-infra issue

Something went wrong: failed to acquire k8s binaries: error during /go/src/k8s.io/release/push-build.sh --nomock --verbose --bucket=kubernetes-release-pull --ci --gcs-suffix=/pull-kubernetes-e2e-kops-aws: exit status 2

/test pull-kubernetes-e2e-kops-aws

@janetkuo
Copy link
Member Author

janetkuo commented Jul 27, 2017 •
edited
Loading

The pull-kubernetes-e2e-gce-etcd3 test failure is #49672

@k8s-github-robot k8s-github-robot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 6, 2017
@janetkuo janetkuo force-pushed the apps-v1beta2-dep-rollback branch 2 times, most recently from de0835b to 8565fc7 Compare August 6, 2017 13:21
@janetkuo
Copy link
Member Author

janetkuo commented Aug 6, 2017

/retest

1 similar comment
@janetkuo
Copy link
Member Author

janetkuo commented Aug 6, 2017

/retest

kow3ns
kow3ns approved these changes Aug 7, 2017
View reviewed changes
staging/src/k8s.io/api/apps/v1beta2/types.go
@@ -25,6 +25,7 @@ import (
const (
ControllerRevisionHashLabelKey = "controller-revision-hash"
StatefulSetRevisionLabel = ControllerRevisionHashLabelKey
DeprecatedRollbackTo = "deprecated.deployment.rollback.to"
Copy link
Member

Choose a reason for hiding this comment

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

Is this the right naming convention?

@kow3ns
Copy link
Member

kow3ns commented Aug 7, 2017

/lgtm

@k8s-ci-robot k8s-ci-robot added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 7, 2017
@erictune
Copy link
Member

erictune commented Aug 7, 2017

/approve

approved for API change

@k8s-github-robot k8s-github-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Aug 8, 2017
janetkuo added 4 commits August 7, 2017 18:35
@janetkuo
Deprecate Deployment rollbackTo field and remove rollback endpoint
e793052 
1. Deprecate `.spec.rollbackTo` field in extensions/v1beta1 and
   apps/v1beta1 Deployments
2. Remove the same field from apps/v1beta2 Deployment, and remove
   its rollback subresource and endpoint
@janetkuo
Conversion code for apps/v1beta2 Deployment
0d23960 
Need to convert deprecated .spec.rollbackTo field into
an annotation in apps/v1beta2 Deployment for roundTrip
@janetkuo
Remove some apps/v1beta2 generated files so that codegen works
5473d56
@janetkuo
Autogen
e245fbc
@janetkuo janetkuo force-pushed the apps-v1beta2-dep-rollback branch from 8565fc7 to e245fbc Compare August 8, 2017 02:11
@k8s-github-robot k8s-github-robot removed lgtm "Looks good to me", indicates that a PR is ready to be merged. needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Aug 8, 2017
@janetkuo janetkuo added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Aug 8, 2017
@janetkuo
Copy link
Member Author

janetkuo commented Aug 8, 2017

Just another rebase

@janetkuo
Copy link
Member Author

janetkuo commented Aug 8, 2017

/test pull-kubernetes-e2e-gce-etcd3

@bgrant0607
Copy link
Member

This is fine. It could be implemented in an external controller using annotations, and consistency across the workload controllers is more important.

/approve

@k8s-github-robot
Copy link

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: bgrant0607, erictune, janetkuo, kargakis, kow3ns

Associated issue: 48746

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these OWNERS Files:

You can indicate your approval by writing /approve in a comment
You can cancel your approval by writing /approve cancel in a comment

@k8s-github-robot k8s-github-robot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. label Aug 8, 2017
@bgrant0607 bgrant0607 mentioned this pull request Aug 8, 2017
Closed
@kow3ns
Copy link
Member

kow3ns commented Aug 8, 2017

/retest

@k8s-github-robot
Copy link

/test all [submit-queue is verifying that this PR is safe to merge]

@k8s-github-robot
Copy link

Automatic merge from submit-queue

@k8s-github-robot k8s-github-robot merged commit 2bd2127 into kubernetes:master Aug 9, 2017
@tnozicka tnozicka mentioned this pull request Feb 19, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kow3ns kow3ns kow3ns approved these changes

Assignees

@0xmichalis 0xmichalis

@kow3ns kow3ns

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. kind/api-change Categorizes issue or PR as related to adding, removing, or otherwise changing an API lgtm "Looks good to me", indicates that a PR is ready to be merged. release-note Denotes a PR that will be considered when it comes time to generate release notes. sig/apps Categorizes an issue or PR as relevant to SIG Apps. size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Milestone
v1.8
Development

Successfully merging this pull request may close these issues.
7 participants
@janetkuo @0xmichalis @kow3ns @erictune @bgrant0607 @k8s-github-robot @k8s-ci-robot