From 02c942a093a28376301b9b4c66d9c712345ff953 Mon Sep 17 00:00:00 2001 From: Sebastian Wagner Date: Mon, 13 Sep 2021 16:03:02 +0200 Subject: [PATCH] mgr/cephadm: Add client.admin keyring when upgrading from older version Signed-off-by: Sebastian Wagner --- .../orch/cephadm/mgr-nfs-upgrade/4-final.yaml | 2 +- src/pybind/mgr/cephadm/migrations.py | 18 ++++++++++++++++-- src/pybind/mgr/cephadm/module.py | 17 +++++++++++++---- src/pybind/mgr/cephadm/tests/fixtures.py | 5 ++--- src/pybind/mgr/cephadm/tests/test_cephadm.py | 13 +++++++++++++ src/pybind/mgr/cephadm/tests/test_migration.py | 16 ++++++++++++++-- 6 files changed, 59 insertions(+), 12 deletions(-) diff --git a/qa/suites/orch/cephadm/mgr-nfs-upgrade/4-final.yaml b/qa/suites/orch/cephadm/mgr-nfs-upgrade/4-final.yaml index 11e8bb3b8d8..b1957d9d628 100644 --- a/qa/suites/orch/cephadm/mgr-nfs-upgrade/4-final.yaml +++ b/qa/suites/orch/cephadm/mgr-nfs-upgrade/4-final.yaml @@ -7,4 +7,4 @@ tasks: - ceph nfs cluster ls | grep foo - ceph nfs export ls foo --detailed - rados -p .nfs --all ls - - - ceph config get mgr mgr/cephadm/migration_current | grep 3 + - ceph config get mgr mgr/cephadm/migration_current | grep 4 diff --git a/src/pybind/mgr/cephadm/migrations.py b/src/pybind/mgr/cephadm/migrations.py index e5a73f30689..b66eedb6907 100644 --- a/src/pybind/mgr/cephadm/migrations.py +++ b/src/pybind/mgr/cephadm/migrations.py @@ -12,7 +12,7 @@ from orchestrator import OrchestratorError, DaemonDescription if TYPE_CHECKING: from .module import CephadmOrchestrator -LAST_MIGRATION = 3 +LAST_MIGRATION = 4 logger = logging.getLogger(__name__) @@ -38,7 +38,7 @@ class Migrations: self.nfs_migration_queue = json.loads(v) if v else [] # for some migrations, we don't need to do anything except for - # setting migration_current = 1. + # incrementing migration_current. # let's try to shortcut things here. self.migrate(True) @@ -68,6 +68,10 @@ class Migrations: if self.migrate_2_3(): self.set(3) + if self.mgr.migration_current == 3: + if self.migrate_3_4(): + self.set(4) + def migrate_0_1(self) -> bool: """ Migration 0 -> 1 @@ -259,6 +263,16 @@ class Migrations: self.mgr.log.warning(f'Failed to migrate export ({ret}): {err}\nExport was:\n{ex}') self.mgr.log.info(f'Done migrating nfs.{service_id}') + def migrate_3_4(self) -> bool: + # We can't set any host with the _admin label, but we're + # going to warn when calling `ceph orch host rm...` + if 'client.admin' not in self.mgr.keys.keys: + self.mgr._client_keyring_set( + entity='client.admin', + placement='label:_admin', + ) + return True + def queue_migrate_nfs_spec(mgr: "CephadmOrchestrator", spec_dict: Dict[Any, Any]) -> None: """ diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py index c5f9d634d96..414013ad5a0 100644 --- a/src/pybind/mgr/cephadm/module.py +++ b/src/pybind/mgr/cephadm/module.py @@ -1402,10 +1402,19 @@ Then run the following: for d in daemons: daemons_table += "{:<20} {:<15}\n".format(d.daemon_type, d.daemon_id) - return "Not allowed to remove %s from cluster. " \ - "The following daemons are running in the host:" \ - "\n%s\nPlease run 'ceph orch host drain %s' to remove daemons from host" % ( - host, daemons_table, host) + raise OrchestratorValidationError("Not allowed to remove %s from cluster. " + "The following daemons are running in the host:" + "\n%s\nPlease run 'ceph orch host drain %s' to remove daemons from host" % ( + host, daemons_table, host)) + + # check, if there we're removing the last _admin host + if not force: + p = PlacementSpec(label='_admin') + admin_hosts = p.filter_matching_hostspecs(self.inventory.all_specs()) + if len(admin_hosts) == 1 and admin_hosts[0] == host: + raise OrchestratorValidationError(f"Host {host} is the last host with the '_admin'" + " label. Please add the '_admin' label to a host" + " or add --force to this command") def run_cmd(cmd_args: dict) -> None: ret, out, err = self.mon_command(cmd_args) diff --git a/src/pybind/mgr/cephadm/tests/fixtures.py b/src/pybind/mgr/cephadm/tests/fixtures.py index 385a93a132d..75ce9900661 100644 --- a/src/pybind/mgr/cephadm/tests/fixtures.py +++ b/src/pybind/mgr/cephadm/tests/fixtures.py @@ -129,15 +129,14 @@ def wait(m, c): @contextmanager -def with_host(m: CephadmOrchestrator, name, addr='1::4', refresh_hosts=True): - # type: (CephadmOrchestrator, str) -> None +def with_host(m: CephadmOrchestrator, name, addr='1::4', refresh_hosts=True, rm_with_force=True): with mock.patch("cephadm.utils.resolve_ip", return_value=addr): wait(m, m.add_host(HostSpec(hostname=name))) if refresh_hosts: CephadmServe(m)._refresh_hosts_and_daemons() receive_agent_metadata(m, name) yield - wait(m, m.remove_host(name)) + wait(m, m.remove_host(name, force=rm_with_force)) def assert_rm_service(cephadm: CephadmOrchestrator, srv_name): diff --git a/src/pybind/mgr/cephadm/tests/test_cephadm.py b/src/pybind/mgr/cephadm/tests/test_cephadm.py index 41513a7fea9..889192a332b 100644 --- a/src/pybind/mgr/cephadm/tests/test_cephadm.py +++ b/src/pybind/mgr/cephadm/tests/test_cephadm.py @@ -1356,6 +1356,8 @@ class TestCephadm(object): assert len(cephadm_module.cache.get_daemons_by_type('mgr')) == 3 assert len(cephadm_module.cache.get_daemons_by_type('crash')) == 1 + cephadm_module.offline_hosts = {} + @mock.patch("cephadm.serve.CephadmServe._run_cephadm") @mock.patch("cephadm.CephadmOrchestrator._host_ok_to_stop") @mock.patch("cephadm.module.HostCache.get_daemon_types") @@ -1663,3 +1665,14 @@ Traceback (most recent call last): with with_osd_daemon(cephadm_module, _run_cephadm, 'test', 1, ceph_volume_lvm_list=_ceph_volume_list): pass + + @mock.patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('[]')) + def test_host_rm_last_admin(self, cephadm_module: CephadmOrchestrator): + with pytest.raises(OrchestratorError): + with with_host(cephadm_module, 'test', refresh_hosts=False, rm_with_force=False): + cephadm_module.inventory.add_label('test', '_admin') + pass + assert False + with with_host(cephadm_module, 'test1', refresh_hosts=False, rm_with_force=True): + with with_host(cephadm_module, 'test2', refresh_hosts=False, rm_with_force=False): + cephadm_module.inventory.add_label('test2', '_admin') diff --git a/src/pybind/mgr/cephadm/tests/test_migration.py b/src/pybind/mgr/cephadm/tests/test_migration.py index ce35672e49d..1c73897cb85 100644 --- a/src/pybind/mgr/cephadm/tests/test_migration.py +++ b/src/pybind/mgr/cephadm/tests/test_migration.py @@ -4,6 +4,7 @@ from ceph.deployment.service_spec import PlacementSpec, ServiceSpec, HostPlaceme from ceph.utils import datetime_to_str, datetime_now from cephadm import CephadmOrchestrator from cephadm.inventory import SPEC_STORE_PREFIX +from cephadm.migrations import LAST_MIGRATION from cephadm.tests.fixtures import _run_cephadm, wait, with_host, receive_agent_metadata_all_hosts from cephadm.serve import CephadmServe from tests import mock @@ -182,7 +183,7 @@ def test_migrate_nfs_initial(cephadm_module: CephadmOrchestrator): assert cephadm_module.migration_current == 2 cephadm_module.migration.migrate() - assert cephadm_module.migration_current == 3 + assert cephadm_module.migration_current == LAST_MIGRATION @mock.patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('[]')) @@ -215,4 +216,15 @@ def test_migrate_nfs_initial_octopus(cephadm_module: CephadmOrchestrator): assert cephadm_module.migration_current == 2 cephadm_module.migration.migrate() - assert cephadm_module.migration_current == 3 + assert cephadm_module.migration_current == LAST_MIGRATION + + +@mock.patch("cephadm.serve.CephadmServe._run_cephadm", _run_cephadm('[]')) +def test_migrate_admin_client_keyring(cephadm_module: CephadmOrchestrator): + assert 'client.admin' not in cephadm_module.keys.keys + + cephadm_module.migration_current = 3 + cephadm_module.migration.migrate() + assert cephadm_module.migration_current == LAST_MIGRATION + + assert cephadm_module.keys.keys['client.admin'].placement.label == '_admin' -- 2.39.5