-
Notifications
You must be signed in to change notification settings - Fork 38
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
pacemaker: remove node on delete (SOC-11240) #376
base: master
Are you sure you want to change the base?
pacemaker: remove node on delete (SOC-11240) #376
Conversation
stonith_mode = proposal["attributes"]["pacemaker"]["stonith"]["mode"] | ||
|
||
# remove node from stonith cfg | ||
[stonith_mode, "per_node"].each do |key| |
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.
Why do we need it for current mode AND "per_node"? I have SBD STONITH in my env and I don't have "per_node" under "stonith".
Even if we decide to keep it, it should be guarded in case the key doesn't exist.
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.
Agreed, I had in it my env, didn't think it might be optional. Thanks
421e108
to
e726e5c
Compare
e726e5c
to
08b5f16
Compare
08b5f16
to
25d4627
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.
Code looks good but I'm not 100% sure if it's safe to commit from inside the transition.
Thanks for pointing out the "Forget" operation. It's behavior does not appear to be affected though: with this patch in place, all occurences of the forgotten node are purged from the Pacemaker proposal upon saving it post-forget. But the same thing happens without this patch in place. So where does this come into play/what is the expected change in behavior? |
@jgrassler the forgotten nodes are removed from the list by UI/CLI and this would be persisted if pacemaker proposal was saved post-forget. The problem here is that if you just "forget" and leave the cloud running, at some point the periodic chef-client runs will start to fail because there is no new founder elected. This change (together with the crowbar-core one) is supposed to make this "save & commit" step trigger automatically after "forget". |
@sandonovsuse I think I know why it didn't work for @jgrassler. I also had similar results in my tests. The problem is that you changed proposal schema but didn't add a migration for this. You probably tested with new proposal (after adding your changes) but it would not work for existing proposals. This should make it usable for existing proposals:
|
On node delete, crowbar doesn't execute pacemaker barclamp due to default pacemaker proposal not having transitions attribute enabled for delete. Next, pacemaker needs to remove the node from the cluster prior to being deleted from crowbar. This change adds said feature. Migartion also added.
25d4627
to
6e1a375
Compare
On node delete, crowbar doesn't execute pacemaker barclamp due to
default pacemaker proposal not having transitions attribute enabled
for delete. This change adds said feature and the delete transition.