Dev: bootstrap: Adjust cluster properties including priority-fencing-delay #1017
Conversation
liangxin1300
force-pushed
the
20220809_priority-fencing-delay
branch
5 times, most recently
from
0dd2249 to
4fb26f4
Compare
liangxin1300
force-pushed
the
20220809_priority-fencing-delay
branch
from
4fb26f4 to
b5db8f3
Compare
liangxin1300
changed the title
liangxin1300
force-pushed
the
20220809_priority-fencing-delay
branch
3 times, most recently
from
0441300 to
672e905
Compare
liangxin1300
changed the title
liangxin1300
force-pushed
the
20220809_priority-fencing-delay
branch
3 times, most recently
from
0a43306 to
6347588
Compare
liangxin1300
force-pushed
the
20220809_priority-fencing-delay
branch
2 times, most recently
from
3c5bdf5 to
6f84524
Compare
There was a problem hiding this comment.
Very sensible improvement. Thanks, @liangxin1300 !
Only a couple of nitpickings.
crmsh/utils.py
Outdated
| if origin_value and str(origin_value) == str(property_value): | ||
| return | ||
| if conditional: | ||
| value_from_cib = int(origin_value.strip('s')) if origin_value else 0 |
There was a problem hiding this comment.
AFAICT property_name hence the property_value could be anything, and conditional tends to be used for time values but could be used for other cases. Technically if it's a time value, it should be better parsed with crm_msec() though.
Since it has been always like this, it could be hardened in the future though.
There was a problem hiding this comment.
changed, compare two values after both parsed by crm_msec
crmsh/bootstrap.py
Outdated
| out = utils.get_stdout_or_raise_error("crm configure show related:stonith") | ||
| if not out: | ||
| return | ||
| pcmk_delay_max_v_list = re.findall("pcmk_delay_max=(\d+)s*", out) |
There was a problem hiding this comment.
Similarly about parsing of a time value.
There was a problem hiding this comment.
Changed, use crm_msec
crmsh/bootstrap.py
Outdated
| if is_2node_wo_qdevice: | ||
| utils.set_property("priority", 1, property_type="rsc_defaults", property_id="rsc-options", conditional=True) | ||
| else: | ||
| utils.set_property("priority", 0, property_type="rsc_defaults", property_id="rsc-options") |
There was a problem hiding this comment.
Does it mean we always want it to be put into "rsc-options" meta set, no matter if there's any other existing set? Wondering what is the consideration here.
There was a problem hiding this comment.
Thanks for remind me this.
I changed inside set_property, check if property_id exists, if not, set the property_id as None, to use the default one
There was a problem hiding this comment.
Alright, I understand it now. So basically crm_attribute -t rsc_defaults and crm configure rsc_defaults both have their own conventions for naming the set, and neither of them cares about any sets created and named differently by others :-)
liangxin1300
changed the title
liangxin1300
force-pushed
the
20220809_priority-fencing-delay
branch
3 times, most recently
from
09ea979 to
8fe362a
Compare
crmsh/bootstrap.py
Outdated
| if is_2node_wo_qdevice: | ||
| utils.set_property("priority", 1, property_type="rsc_defaults", property_id="rsc-options", conditional=True) | ||
| else: | ||
| utils.set_property("priority", 0, property_type="rsc_defaults", property_id="rsc-options") |
There was a problem hiding this comment.
Alright, I understand it now. So basically crm_attribute -t rsc_defaults and crm configure rsc_defaults both have their own conventions for naming the set, and neither of them cares about any sets created and named differently by others :-)
crmsh/utils.py
Outdated
| out = get_stdout_or_raise_error("crm configure show") | ||
| if not re.search(" {}:".format(property_id), out): | ||
| logger.warning("No such id \"{}\" exist".format(property_id)) | ||
| property_id = None |
There was a problem hiding this comment.
AFAIU, this way, it'll basically let crm_attribute create the set with its convention. But OTOH, any invocations of crm configure rsc_defaults won't use this set anyways, right? Not sure if we could make everything sensible with this PR, otherwise we could still stick with your previous way. Then a potential improvement in the future could be to make both "crm configure" and "crm bootstrap" consistently follow crm_attribute's convention.
There was a problem hiding this comment.
I think here calling crm configure rsc_defaults is better, since in cibconfig.py, already did a lots of work, if the rsc-option not exist, rsc-option will be created and make sure all rsc_defaults' properties are under this id
There was a problem hiding this comment.
Already changed to use crm configure property|rsc_defaults
liangxin1300
force-pushed
the
20220809_priority-fencing-delay
branch
2 times, most recently
from
975278e to
5a8ac8a
Compare
…delay When join/remove node, add/remove qdevice, add sbd via stage, Set property priority=1 for 2 nodes cluster without qdevice Set priority-fencing-delay=2*pcmk_delay_max
liangxin1300
force-pushed
the
20220809_priority-fencing-delay
branch
from
5a8ac8a to
37d4b63
Compare
When join/remove node, add/remove qdevice, add sbd via stage,
Set property priority=1 for 2 nodes cluster without qdevice
Set priority-fencing-delay=2*pcmk_delay_max