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.

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

Copy annotations back from RS to Deployment on rollback #23160

Merged

Conversation

janetkuo
Copy link
Member

Fixes #23087

@bgrant0607 @Kargakis @kubernetes/sig-config

@janetkuo janetkuo added kind/bug Categorizes issue or PR as related to a bug. area/app-lifecycle team/ux labels Mar 17, 2016
@k8s-github-robot
Copy link

Labelling this PR as size/M

@k8s-github-robot k8s-github-robot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Mar 17, 2016
@k8s-bot
Copy link

k8s-bot commented Mar 17, 2016

GCE e2e build/test passed for commit 10539a084dcdc83e5670d94b245811112bc92693.

@@ -922,6 +927,21 @@ func copyDeploymentAnnotationsToReplicaSet(deployment *extensions.Deployment, rs
return rsAnnotationsChanged
}

// copyReplicaSetAnnotationsToDeployment copies RS's annotations to deployment's annotations.
// This action should be done if and only if the deployment is rolling back to this rs.
// Note that apply and revision annotations are not copied.
Copy link
Contributor

Choose a reason for hiding this comment

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

So the new revision is N+1?

Copy link
Member Author

Choose a reason for hiding this comment

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

Revision is only updated (to max old revision + 1, if it's larger) in getNewReplicaSet and then copied to deployment (revision in deployment is for reference only and is always copied from new RS to deployment). We don't touch it here.

@bgrant0607 bgrant0607 assigned bgrant0607 and unassigned fejta Mar 18, 2016
@bgrant0607
Copy link
Member

cc @pwittrock

@bgrant0607 bgrant0607 assigned pwittrock and unassigned bgrant0607 Mar 18, 2016
@bgrant0607
Copy link
Member

@pwittrock PTAL. Good time to become familiar with Deployment. :-)

// otherwise, the deployment's current annotations (should be the same as current new RS) will be copied to the RS after the rollback.
//
// For example,
// A Deployment has old RS1 with annotation {change-cause:create}, and new RS2 {change-cause:edit}.
Copy link
Member

Choose a reason for hiding this comment

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

How are annotations that do not directly override each other handled in this situation? e.g.
{rs1-annotation:foo}
{rs2-annotation:bar}

When we rollback to rs1 from rs2, this will add back {rs1-annotation:foo}, but does {rs2-annotation:bar} need to be explicitly removed (or is that handled by other code?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, yes we should remove those annotations.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed

@janetkuo
Copy link
Member Author

PTAL

@k8s-bot
Copy link

k8s-bot commented Mar 19, 2016

GCE e2e build/test passed for commit cd95403b1b66c4759257efd289e5ff0843bb8826.

@k8s-bot
Copy link

k8s-bot commented Mar 19, 2016

GCE e2e build/test passed for commit 224eadc630872a38652210ee6017fbc35b6d825b.

@pwittrock
Copy link
Member

Looks good. Feel free to squash the comments commit.

@janetkuo
Copy link
Member Author

Squashed and applying tag. Thanks!

@janetkuo janetkuo added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label Mar 21, 2016
@k8s-bot
Copy link

k8s-bot commented Mar 21, 2016

GCE e2e build/test passed for commit 482efba.

@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 Mar 21, 2016

GCE e2e build/test passed for commit 482efba.

@janetkuo janetkuo added this to the v1.2 milestone Mar 21, 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 Mar 21, 2016

GCE e2e build/test passed for commit 482efba.

@k8s-github-robot
Copy link

Automatic merge from submit-queue

k8s-github-robot pushed a commit that referenced this pull request Mar 21, 2016
@k8s-github-robot k8s-github-robot merged commit edc35bf into kubernetes:master Mar 21, 2016
@bgrant0607 bgrant0607 added the cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. label Mar 23, 2016
eparis pushed a commit to eparis/kubernetes that referenced this pull request Mar 24, 2016
…ation

Auto commit by PR queue bot
(cherry picked from commit edc35bf)
@bgrant0607 bgrant0607 added release-note Denotes a PR that will be considered when it comes time to generate release notes. and removed release-note Denotes a PR that will be considered when it comes time to generate release notes. labels Mar 24, 2016
@k8s-cherrypick-bot
Copy link

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

AlainRoy pushed a commit to vmware-archive/kubernetes-archived that referenced this pull request Mar 29, 2016
…ation

Auto commit by PR queue bot
(cherry picked from commit edc35bf)
alena1108 pushed a commit to rancher/kubernetes that referenced this pull request May 20, 2016
…ation

Auto commit by PR queue bot
(cherry picked from commit edc35bf)
shyamjvs pushed a commit to shyamjvs/kubernetes that referenced this pull request Dec 1, 2016
…ation

Auto commit by PR queue bot
(cherry picked from commit edc35bf)
shouhong pushed a commit to shouhong/kubernetes that referenced this pull request Feb 14, 2017
…ation

Auto commit by PR queue bot
(cherry picked from commit edc35bf)
openshift-publish-robot pushed a commit to openshift/kubernetes that referenced this pull request Jun 14, 2019
Increase the wait for the service upgrade e2e test for LB

Origin-commit: 2d4a4b9f0b15e8e2c164a312a968795da59ea591
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/app-lifecycle cherry-pick-approved Indicates a cherry-pick PR into a release branch has been approved by the release branch manager. kind/bug Categorizes issue or PR as related to a bug. 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. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

10 participants