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
Replace ScyllaCluster nodes using Host ID #1312
Conversation
1600cec
to
795b36b
Compare
Adding the "do-not-merge/release-note-label-needed" label because no release-note block was detected, please follow our release note process to remove it. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
795b36b
to
6865ac6
Compare
6865ac6
to
f7dfbe3
Compare
Should #1313 be added as a prerequisite? |
f7dfbe3
to
4cf813e
Compare
3142881
to
5621f54
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.
few nits, rest lgtm
thanks for the updates!
5621f54
to
93e1d53
Compare
except for #1312 (comment) , I think this is ready to land |
93e1d53
to
9574fe7
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.
/approve
/lgtm
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
23a9e8e
to
11d6fd6
Compare
11d6fd6
to
6d08585
Compare
/lgtm |
6d08585
to
866c15d
Compare
04d286e
to
da1ad93
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 NOT APPROVED This pull-request has been approved by: tnozicka, zimnx The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Existing procedure of replacing nodes using their broadcasted IP address is deprecated since 5.2. This change brings support for new procedure where nodes are identified using their HostID. HostID is better because it's constanct across entire node lifetime, where IP address is not, which may cause ambiguity when node IP changes, and node requires a replacement. From user point of view, nothing changes, as the same label is used to mark node to be replaced. Operator validates whether deployed ScyllaDB version supports new procedure, and uses it when it does. In other case, old procedure is executed.
da1ad93
to
98f603f
Compare
New changes are detected. LGTM label has been removed. |
@zimnx: The following test failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. I understand the commands that are listed here. |
Existing procedure of replacing nodes using their broadcasted IP address
is deprecated since 5.2. This change brings support for new procedure
where nodes are identified using their HostID.
HostID is better because it's constanct across entire node lifetime,
where IP address is not, which may cause ambiguity when node IP changes,
and node requires a replacement.
From user point of view, nothing changes, as the same label is used to
mark node to be replaced. Operator validates whether deployed ScyllaDB
version supports new procedure, and uses it when it does. In other case,
old procedure is executed.
Fixes #1311
Prerequisites:
scyllaclient
package #1309