-
Notifications
You must be signed in to change notification settings - Fork 40k
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
Copy annotations back from RS to Deployment on rollback #23160
Conversation
Labelling this PR as size/M |
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. |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
cc @pwittrock |
@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}. |
There was a problem hiding this comment.
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?)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed
PTAL |
cd95403
to
224eadc
Compare
GCE e2e build/test passed for commit cd95403b1b66c4759257efd289e5ff0843bb8826. |
GCE e2e build/test passed for commit 224eadc630872a38652210ee6017fbc35b6d825b. |
Looks good. Feel free to squash the comments commit. |
224eadc
to
482efba
Compare
Squashed and applying tag. Thanks! |
GCE e2e build/test passed for commit 482efba. |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
GCE e2e build/test passed for commit 482efba. |
@k8s-bot test this [submit-queue is verifying that this PR is safe to merge] |
GCE e2e build/test passed for commit 482efba. |
Automatic merge from submit-queue |
Auto commit by PR queue bot
…ation Auto commit by PR queue bot (cherry picked from commit edc35bf)
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. |
…ation Auto commit by PR queue bot (cherry picked from commit edc35bf)
…ation Auto commit by PR queue bot (cherry picked from commit edc35bf)
…ation Auto commit by PR queue bot (cherry picked from commit edc35bf)
…ation Auto commit by PR queue bot (cherry picked from commit edc35bf)
Increase the wait for the service upgrade e2e test for LB Origin-commit: 2d4a4b9f0b15e8e2c164a312a968795da59ea591
Fixes #23087
@bgrant0607 @Kargakis @kubernetes/sig-config