From 89eece8c141c4664f4f4ad066455b45afe523bc5 Mon Sep 17 00:00:00 2001 From: Adam King Date: Wed, 30 Sep 2020 09:44:36 -0400 Subject: [PATCH] mgr/cephadm: only deploy 3 mons by default when there are 4 hosts Fixes: https://tracker.ceph.com/issues/47234 Signed-off-by: Adam King --- src/pybind/mgr/cephadm/schedule.py | 22 ++++ .../mgr/cephadm/tests/test_scheduling.py | 112 ++++++++++++++++++ 2 files changed, 134 insertions(+) diff --git a/src/pybind/mgr/cephadm/schedule.py b/src/pybind/mgr/cephadm/schedule.py index 4b42099b293a2..26bee82d9d5d3 100644 --- a/src/pybind/mgr/cephadm/schedule.py +++ b/src/pybind/mgr/cephadm/schedule.py @@ -122,8 +122,30 @@ class HostAssignment(object): # If we don't have the list of candidates is definitive. if count is None: logger.debug('Provided hosts: %s' % candidates) + # if asked to place even number of mons, deploy 1 less + if self.spec.service_type == 'mon' and (len(candidates) % 2) == 0: + logger.info("deploying %s monitor(s) instead of %s so monitors may achieve consensus" % ( + len(candidates) - 1, len(candidates))) + return candidates[0:len(candidates)-1] return candidates + # if asked to place even number of mons, deploy 1 less + if self.spec.service_type == 'mon': + # if count >= number of candidates then number of candidates + # is determining factor in how many mons will be placed + if count >= len(candidates): + if (len(candidates) % 2) == 0: + logger.info("deploying %s monitor(s) instead of %s so monitors may achieve consensus" % ( + len(candidates) - 1, len(candidates))) + count = len(candidates) - 1 + # if count < number of candidates then count is determining + # factor in how many mons will get placed + else: + if (count % 2) == 0: + logger.info( + "deploying %s monitor(s) instead of %s so monitors may achieve consensus" % (count - 1, count)) + count = count - 1 + # prefer hosts that already have services. # this avoids re-assigning to _new_ hosts # and constant re-distribution of hosts when new nodes are diff --git a/src/pybind/mgr/cephadm/tests/test_scheduling.py b/src/pybind/mgr/cephadm/tests/test_scheduling.py index d57b4c08aae7b..efbbb9b99d90b 100644 --- a/src/pybind/mgr/cephadm/tests/test_scheduling.py +++ b/src/pybind/mgr/cephadm/tests/test_scheduling.py @@ -756,3 +756,115 @@ def test_active_assignment(service_type, placement, hosts, daemons, expected): hosts=[HostSpec(h) for h in hosts], get_daemons_func=lambda _: daemons).place() assert sorted([h.hostname for h in hosts]) in expected + +class OddMonsTest(NamedTuple): + service_type: str + placement: PlacementSpec + hosts: List[str] + daemons: List[DaemonDescription] + expected_count: int + + +@pytest.mark.parametrize("service_type,placement,hosts,daemons,expected_count", + [ + OddMonsTest( + 'mon', + PlacementSpec(count=5), + 'host1 host2 host3 host4'.split(), + [], + 3 + ), + OddMonsTest( + 'mon', + PlacementSpec(count=4), + 'host1 host2 host3 host4 host5'.split(), + [], + 3 + ), + OddMonsTest( + 'mon', + PlacementSpec(count=5), + 'host1 host2 host3 host4 host5'.split(), + [], + 5 + ), + OddMonsTest( + 'mon', + PlacementSpec(hosts='host1 host2 host3 host4'.split()), + 'host1 host2 host3 host4 host5'.split(), + [], + 3 + ), + OddMonsTest( + 'mon', + PlacementSpec(hosts='host1 host2 host3 host4 host5'.split()), + 'host1 host2 host3 host4 host5'.split(), + [], + 5 + ), + OddMonsTest( + 'mon', + PlacementSpec(host_pattern='*'), + 'host1 host2 host3 host4'.split(), + [], + 3 + ), + OddMonsTest( + 'mon', + PlacementSpec(count=5, hosts='host1 host2 host3 host4'.split()), + 'host1 host2 host3 host4 host5'.split(), + [], + 3 + ), + OddMonsTest( + 'mon', + PlacementSpec(count=2, hosts='host1 host2 host3'.split()), + 'host1 host2 host3 host4 host5'.split(), + [], + 1 + ), + OddMonsTest( + 'mon', + PlacementSpec(count=5), + 'host1 host2 host3 host4'.split(), + [ + DaemonDescription('mon', 'a', 'host1'), + DaemonDescription('mon', 'b', 'host2'), + DaemonDescription('mon', 'c', 'host3'), + ], + 3 + ), + OddMonsTest( + 'mon', + PlacementSpec(count=5), + 'host1 host2 host3 host4'.split(), + [ + DaemonDescription('mon', 'a', 'host1'), + DaemonDescription('mon', 'b', 'host2'), + ], + 3 + ), + OddMonsTest( + 'mon', + PlacementSpec(hosts='host1 host2 host3 host4'.split()), + 'host1 host2 host3 host4 host5'.split(), + [ + DaemonDescription('mon', 'a', 'host1'), + DaemonDescription('mon', 'b', 'host2'), + DaemonDescription('mon', 'c', 'host3'), + ], + 3 + ), + + ]) +def test_odd_mons(service_type, placement, hosts, daemons, expected_count): + + spec = ServiceSpec(service_type=service_type, + service_id=None, + placement=placement) + + hosts = HostAssignment( + spec=spec, + hosts=[HostSpec(h) for h in hosts], + get_daemons_func=lambda _: daemons).place() + assert len(hosts) == expected_count -- 2.39.5