-
Notifications
You must be signed in to change notification settings - Fork 89
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
Adds a node delete
command
#3716
Conversation
Important Auto Review SkippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the To trigger a single review, invoke the TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
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 other than the one question about reconnecting
pkg/publicapi/apimodels/node.go
Outdated
@@ -67,10 +68,12 @@ func (n NodeAction) Description() string { | |||
return "Approve a node whose membership is pending" | |||
case NodeActionReject: | |||
return "Reject a node whose membership is pending" | |||
case NodeActionDelete: | |||
return "Delete a node from the cluster. Note this does not stop them re-connecting." |
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.
I don't think we need this warning. If a node disconnects, does it have to go pending again? (it should)
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.
Yes, would go back to Pending, unless whatever criteria we're using for auto-approve (tbd) puts them in that state.
94c3c82
to
d667849
Compare
Allows for nodes to be delete from the node info store. If the node is found in the store, it is deleted, and then will not re-appear when the requester node is re-started. If however the compute node restarts, it will re-register so this command is only useful if you know that nodes will not be coming back. In future if we want to block the node-id from ever being an entry, we should add tombstone entries to the node info store that blog it's registration.
d667849
to
8dac9fa
Compare
Allows for nodes to be delete from the node info store. If the node is found in the store, it is deleted, and then will not re-appear when the requester node is re-started. If however the compute node restarts, it will re-register so this command is only useful if you know that nodes will not be coming back. In future if we want to block the node-id from ever being an entry, we should add tombstone entries to the node info store that blog it's registration. Closes #3715
Allows for nodes to be delete from the node info store. If the node is found in the store, it is deleted, and then will not re-appear when the requester node is re-started. If however the compute node restarts, it will re-register so this command is only useful if you know that nodes will not be coming back.
In future if we want to block the node-id from ever being an entry, we should add tombstone entries to the node info store that blog it's registration.
Closes #3715