Skip to content

Dev: bootstrap: Reject to remove qdevice when it will cause quorum lost #2041

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.

Sign up for GitHub

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

Open
liangxin1300 wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Dev: bootstrap: Reject to remove qdevice when it will cause quorum lost #2041

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
574da08
Dev: bootstrap: Reject to remove qdevice when it will cause quorum lost
liangxin1300 Mar 3, 2026
e558b8f
Dev: unittests: Adjust unit test for previous commit
liangxin1300 Mar 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions crmsh/bootstrap.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2425,6 +2425,8 @@ def remove_qdevice() -> None:

utils.check_all_nodes_reachable("removing QDevice from the cluster")
qdevice_reload_policy = qdevice.evaluate_qdevice_quorum_effect(qdevice.QDEVICE_REMOVE)
if qdevice_reload_policy == qdevice.QdevicePolicy.QDEVICE_REMOVE_REJECT:
utils.fatal("Can't remove QDevice since the quorum will be lost for diskless SBD cluster")

@zzhou1 zzhou1 Mar 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
utils.fatal("Can't remove QDevice since the quorum will be lost for diskless SBD cluster")
utils.fatal("operation is rejected since it would result in a malfunctioning cluster lacking quorum")

@gao-yan gao-yan Mar 18, 2026 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It wouldn't necessarily become "malfunctioning" purely based on the logic here :-) It's just that the next cluster startup will require an actual majority or "wait_for_all" to become first quorate.

What I can tell about the case in here is, an user may want to remove qdevice when not all cluster nodes are online, right? But actually it's technically feasible no matter what the quorum status is like, as long as the offline nodes are still reachable through ssh.

The actual problem is, after removal of qdevice, if they/we still want to only bring up the same subset of nodes and expect them to reach the status as before the removal of qdevice right away, it won't be in the same condition any more though. But it's actually about how quorum is first granted on cluster startup, which either requires an actual majority or "wait_for_all" (automatically enabled when "two_node" is enabled). But that's reasonable, even if qdevice is not in place.

So to me, this could be a case where we'd rather tell user "I can remove the on-disk configuration of qdevice for you, but you'll either want to restart cluster later manually by yourself, otherwise I can restart the cluster for you now but bring up all the nodes".

BTW, it's really confusing to still call it QDEVICE_RESTART_LATER when it's leveraging maintenance mode and actually restarting the cluster. The name QDEVICE_RESTART_LATER should probably be preserved for the situation where it really requires users to manually restart their cluster by themselves.

After all, as said, to determine whether a cluster would become malfunctioning, the logic in "warn_diskless_sbd()" and the warning there tell the key:

"Diskless SBD requires cluster with three or more nodes. If you want to use diskless SBD for 2-node cluster, should be combined with QDevice."

if qdevice_reload_policy == qdevice.QdevicePolicy.QDEVICE_RESTART_LATER:
with utils.leverage_maintenance_mode() as enabled:
if not utils.able_to_restart_cluster(enabled):
Expand Down
12 changes: 9 additions & 3 deletions crmsh/qdevice.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ class QdevicePolicy(Enum):
QDEVICE_RELOAD = 0
QDEVICE_RESTART = 1
QDEVICE_RESTART_LATER = 2
QDEVICE_REMOVE_REJECT = 3


def evaluate_qdevice_quorum_effect(mode):
Expand All @@ -44,10 +45,16 @@ def evaluate_qdevice_quorum_effect(mode):
if mode == QDEVICE_ADD:
expected_votes += 1
elif mode == QDEVICE_REMOVE:
actual_votes -= 1
vote = corosync.get_value("quorum.device.votes") or 1
actual_votes -= int(vote)
diskless_sbd = sbd.SBDUtils.is_using_diskless_sbd()

if utils.calculate_quorate_status(expected_votes, actual_votes) and not diskless_sbd:
quorate = utils.calculate_quorate_status(expected_votes, actual_votes)
if not quorate and diskless_sbd and mode == QDEVICE_REMOVE:
# Reject to remove qdevice, as it will lose quorum
# then diskless SBD will self-fence the node
return QdevicePolicy.QDEVICE_REMOVE_REJECT
Comment on lines +52 to +56

@zzhou1 zzhou1 Mar 18, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
quorate = utils.calculate_quorate_status(expected_votes, actual_votes)
if not quorate and diskless_sbd and mode == QDEVICE_REMOVE:
# Reject to remove qdevice, as it will lose quorum
# then diskless SBD will self-fence the node
return QdevicePolicy.QDEVICE_REMOVE_REJECT
qdevice_votes = pseudo.code.to.get.corosync.conf.quorum.device.votes
quorate = utils.calculate_quorate_status(expected_votes-qdevice_votes, actual_votes-qdevice_votes)
if not quorate and diskless_sbd and mode == QDEVICE_REMOVE:
# Rejected since it would result in a malfunctioning cluster lacking quorum
# either the disk-based sbd cluster or the diskless sbd cluster

elif quorate and not diskless_sbd:
# safe to use reload
return QdevicePolicy.QDEVICE_RELOAD
elif xmlutil.CrmMonXmlParser().is_non_stonith_resource_running() and not utils.is_cluster_in_maintenance_mode():
Expand All @@ -60,7 +67,6 @@ def evaluate_qdevice_quorum_effect(mode):
# safe to restart cluster service
return QdevicePolicy.QDEVICE_RESTART


def qnetd_lock_for_same_cluster_name(func):
"""
Decorator to claim lock on qnetd, to avoid the same cluster name added in qnetd
Expand Down
8 changes: 6 additions & 2 deletions test/unittests/test_qdevice.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,25 +22,29 @@ def test_evaluate_qdevice_quorum_effect_reload(mock_get_dict, mock_quorate):
mock_quorate.assert_called_once_with(3, 2)


@mock.patch('crmsh.corosync.get_value')
@mock.patch('crmsh.xmlutil.CrmMonXmlParser')
@mock.patch('crmsh.utils.calculate_quorate_status')
@mock.patch('crmsh.utils.get_quorum_votes_dict')
def test_evaluate_qdevice_quorum_effect_later(mock_get_dict, mock_quorate, mock_parser):
def test_evaluate_qdevice_quorum_effect_later(mock_get_dict, mock_quorate, mock_parser, mock_get_value):
mock_get_dict.return_value = {'Expected': '2', 'Total': '2'}
mock_quorate.return_value = False
mock_get_value.return_value = '1'
mock_parser().is_non_stonith_resource_running.return_value = True
res = qdevice.evaluate_qdevice_quorum_effect(qdevice.QDEVICE_REMOVE)
assert res == qdevice.QdevicePolicy.QDEVICE_RESTART_LATER
mock_get_dict.assert_called_once_with()
mock_quorate.assert_called_once_with(2, 1)


@mock.patch('crmsh.corosync.get_value')
@mock.patch('crmsh.xmlutil.CrmMonXmlParser')
@mock.patch('crmsh.utils.calculate_quorate_status')
@mock.patch('crmsh.utils.get_quorum_votes_dict')
def test_evaluate_qdevice_quorum_effect(mock_get_dict, mock_quorate, mock_parser):
def test_evaluate_qdevice_quorum_effect(mock_get_dict, mock_quorate, mock_parser, mock_get_value):
mock_get_dict.return_value = {'Expected': '2', 'Total': '2'}
mock_quorate.return_value = False
mock_get_value.return_value = '1'
mock_parser().is_non_stonith_resource_running.return_value = False
res = qdevice.evaluate_qdevice_quorum_effect(qdevice.QDEVICE_REMOVE)
assert res == qdevice.QdevicePolicy.QDEVICE_RESTART
Expand Down