-
Notifications
You must be signed in to change notification settings - Fork 156
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
Remove explicit copying of replace label between Service instances #1442
Remove explicit copying of replace label between Service instances #1442
Conversation
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.
/retest |
21ba9bd
to
d5da342
Compare
d5da342
to
8f53bc8
Compare
@zimnx the change required some adjustment to unit tests, please check whether the changes to the current behaviour are acceptable. I believe |
b8a84f9
to
dbfde25
Compare
dbfde25
to
1215393
Compare
1215393
to
aa89840
Compare
…eplacement procedure
6659ff5
to
3a37529
Compare
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.
/lgtm
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rzetelskik, tnozicka, zimnx The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description of your changes: Currently the replace label is copied explicitly between Service instances. This can result in the replace label being carried over past the end of the replacement procedure in case the replace label is not removed from the original Service on time, which can in turn break the node, as observed in #1240. This PR removes the superfluous explicit copy of the replacement label. It's necessary in neither the old nor the new replacement procedure. In the old procedure, the label will be picked up from ScyllaCluster status, while the new procedure doesn't terminate services. This change should fix the issue for the new replacement procedure, as the replacement label will never be carried over to a new Service instance. Unfortunately, this PR doesn't fix the deprecated procedure.
Which issue is resolved by this Pull Request:
Resolves #1240