Conversation
Use salt-cloud to terminate an instance when a minion is to be removed. Added an event handler for cloud specific events; currently only used when an instance is terminated, to remove the associated minion from the database.
This reverts commit 377e4fe. Node removal is now supported in Public Clouds.
|
This requires SUSE/caasp-container-manifests#208 to work properly. |
flavio
left a comment
flavio
left a comment
There was a problem hiding this comment.
I didn't test the code with these changes. I'm not sure our CI has automated tests in place simulating node removal, that's something we should test to ensure no regression is introduced.
lib/velum/salt.rb
Outdated
| data: { client: "local_async", | ||
| tgt: "admin", | ||
| fun: "cloud.destroy", | ||
| arg: [ minion_id ] }) |
There was a problem hiding this comment.
I looked at the salt documentation and it's not clear to me whether this API call is going to drop the minion key as well.
I wonder if we shouldn't always execute the other branch of this if/else statement.
There was a problem hiding this comment.
cloud.destroy does remove the minion key.
| fun: "key.delete", | ||
| match: minion_id }) | ||
| end | ||
| JSON.parse(res.body) |
There was a problem hiding this comment.
This changed from parsing the res.code to parsing the whole body of the response. Doesn't it have side effects in other parts of the code?
There was a problem hiding this comment.
I don't think the return code is used at the moment, and actually even couldn't, as the existing code JSON.parse(res.code) throws an exception since res.code is not JSON (see also https://bugzilla.suse.com/show_bug.cgi?id=1117152).
|
This GitHub PR is unactive since 30. Is this GitHub PR still needed? Please close or update it accordingly. |
3 participants
This adds support for removing cluster nodes in the public cloud. It uses salt-cloud to terminate the instance associated with the given minion, and uses a salt event processor to react on the salt-cloud destroyed event to remove the minion from the database.
This fixes https://bugzilla.suse.com/show_bug.cgi?id=1117152.