From: Sebastian Wagner Date: Wed, 13 Jan 2021 12:40:16 +0000 (+0100) Subject: mgr/cephadm: add spec_deleted datetime to spec store X-Git-Tag: v17.1.0~2997^2~6 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=1fcdad17685a8884135a2cda0cdfe3525e325474;p=ceph.git mgr/cephadm: add spec_deleted datetime to spec store We'd like to keep the spec in the store, while we're cleaning up all the daemons. Reason is to clean up the config and config-key store when the last daemon of a service is deleted. Signed-off-by: Sebastian Wagner --- diff --git a/src/pybind/mgr/cephadm/inventory.py b/src/pybind/mgr/cephadm/inventory.py index 3cff44296a13..0e43bc607d44 100644 --- a/src/pybind/mgr/cephadm/inventory.py +++ b/src/pybind/mgr/cephadm/inventory.py @@ -2,7 +2,7 @@ import datetime from copy import copy import json import logging -from typing import cast, TYPE_CHECKING, Dict, List, Iterator, Optional, Any, Tuple, Set +from typing import TYPE_CHECKING, Dict, List, Iterator, Optional, Any, Tuple, Set, Mapping, cast import orchestrator from ceph.deployment import inventory @@ -117,10 +117,22 @@ class SpecStore(): def __init__(self, mgr): # type: (CephadmOrchestrator) -> None self.mgr = mgr - self.specs = {} # type: Dict[str, ServiceSpec] + self._specs = {} # type: Dict[str, ServiceSpec] self.spec_created = {} # type: Dict[str, datetime.datetime] + self.spec_deleted = {} # type: Dict[str, datetime.datetime] self.spec_preview = {} # type: Dict[str, ServiceSpec] + @property + def all_specs(self) -> Mapping[str, ServiceSpec]: + """ + returns active and deleted specs. Returns read-only dict. + """ + return self._specs + + @property + def active_specs(self) -> Mapping[str, ServiceSpec]: + return {k: v for k, v in self._specs.items() if k not in self.spec_deleted} + def load(self): # type: () -> None for k, v in self.mgr.get_store_prefix(SPEC_STORE_PREFIX).items(): @@ -129,8 +141,13 @@ class SpecStore(): j = cast(Dict[str, dict], json.loads(v)) spec = ServiceSpec.from_json(j['spec']) created = str_to_datetime(cast(str, j['created'])) - self.specs[service_name] = spec + self._specs[service_name] = spec self.spec_created[service_name] = created + + if 'deleted' in v: + deleted = str_to_datetime(cast(str, j['deleted'])) + self.spec_deleted[service_name] = deleted + self.mgr.log.debug('SpecStore: loaded spec for %s' % ( service_name)) except Exception as e: @@ -138,28 +155,49 @@ class SpecStore(): service_name, e)) pass - def save(self, spec): - # type: (ServiceSpec) -> None + def save(self, spec: ServiceSpec, update_create: bool = True) -> None: + name = spec.service_name() if spec.preview_only: - self.spec_preview[spec.service_name()] = spec + self.spec_preview[name] = spec return None - self.specs[spec.service_name()] = spec - self.spec_created[spec.service_name()] = datetime_now() + self._specs[name] = spec + + if update_create: + self.spec_created[name] = datetime_now() + + data = { + 'spec': spec.to_json(), + 'created': datetime_to_str(self.spec_created[name]), + } + if name in self.spec_deleted: + data['deleted'] = datetime_to_str(self.spec_deleted[name]) + self.mgr.set_store( - SPEC_STORE_PREFIX + spec.service_name(), - json.dumps({ - 'spec': spec.to_json(), - 'created': datetime_to_str(self.spec_created[spec.service_name()]), - }, sort_keys=True), + SPEC_STORE_PREFIX + name, + json.dumps(data, sort_keys=True), ) self.mgr.events.for_service(spec, OrchestratorEvent.INFO, 'service was created') - def rm(self, service_name): + def rm(self, service_name: str) -> bool: + if service_name not in self._specs: + return False + + if self._specs[service_name].preview_only: + self.finally_rm(service_name) + return True + + self.spec_deleted[service_name] = datetime_now() + self.save(self._specs[service_name], update_create=False) + return True + + def finally_rm(self, service_name): # type: (str) -> bool - found = service_name in self.specs + found = service_name in self._specs if found: - del self.specs[service_name] + del self._specs[service_name] del self.spec_created[service_name] + if service_name in self.spec_deleted: + del self.spec_deleted[service_name] self.mgr.set_store(SPEC_STORE_PREFIX + service_name, None) return found @@ -735,7 +773,7 @@ class EventStore(): unknowns: List[str] = [] daemons = self.mgr.cache.get_daemon_names() - specs = self.mgr.spec_store.specs.keys() + specs = self.mgr.spec_store.all_specs.keys() for k_s, v in self.events.items(): kind, subject = k_s.split(':') if kind == 'service': diff --git a/src/pybind/mgr/cephadm/migrations.py b/src/pybind/mgr/cephadm/migrations.py index 0b4ac9780264..6b02e81e75ac 100644 --- a/src/pybind/mgr/cephadm/migrations.py +++ b/src/pybind/mgr/cephadm/migrations.py @@ -76,7 +76,7 @@ class Migrations: """ def interesting_specs() -> Iterator[ServiceSpec]: - for s in self.mgr.spec_store.specs.values(): + for s in self.mgr.spec_store.all_specs.values(): if s.unmanaged: continue p = s.placement @@ -147,17 +147,17 @@ class Migrations: This fixes the data structure consistency """ bad_specs = {} - for name, spec in self.mgr.spec_store.specs.items(): + for name, spec in self.mgr.spec_store.all_specs.items(): if name != spec.service_name(): bad_specs[name] = (spec.service_name(), spec) for old, (new, old_spec) in bad_specs.items(): - if new not in self.mgr.spec_store.specs: + if new not in self.mgr.spec_store.all_specs: spec = old_spec else: - spec = self.mgr.spec_store.specs[new] + spec = self.mgr.spec_store.all_specs[new] spec.unmanaged = True self.mgr.spec_store.save(spec) - self.mgr.spec_store.rm(old) + self.mgr.spec_store.finally_rm(old) return True diff --git a/src/pybind/mgr/cephadm/module.py b/src/pybind/mgr/cephadm/module.py index ae79d89e3784..a838ca04920c 100644 --- a/src/pybind/mgr/cephadm/module.py +++ b/src/pybind/mgr/cephadm/module.py @@ -1486,8 +1486,8 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule, if not dd.osdspec_affinity: # If there is no osdspec_affinity, the spec should suffice for displaying continue - if n in self.spec_store.specs: - spec = self.spec_store.specs[n] + if n in self.spec_store.all_specs: + spec = self.spec_store.all_specs[n] else: spec = ServiceSpec( unmanaged=True, @@ -1505,7 +1505,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule, spec=spec, events=self.events.get_for_service(spec.service_name()), ) - if n in self.spec_store.specs: + if n in self.spec_store.all_specs: if dd.daemon_type == 'osd': """ The osd count can't be determined by the Placement spec. @@ -1535,7 +1535,7 @@ class CephadmOrchestrator(orchestrator.Orchestrator, MgrModule, if dd.daemon_type == 'haproxy' or dd.daemon_type == 'keepalived': # ha-rgw has 2 daemons running per host sm[n].size = sm[n].size * 2 - for n, spec in self.spec_store.specs.items(): + for n, spec in self.spec_store.all_specs.items(): if n in sm: continue if service_type is not None and service_type != spec.service_type: diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py index cad008ab176a..8002a268a324 100644 --- a/src/pybind/mgr/cephadm/serve.py +++ b/src/pybind/mgr/cephadm/serve.py @@ -420,7 +420,7 @@ class CephadmServe: def _apply_all_services(self) -> bool: r = False specs = [] # type: List[ServiceSpec] - for sn, spec in self.mgr.spec_store.specs.items(): + for sn, spec in self.mgr.spec_store.active_specs.items(): specs.append(spec) for spec in specs: try: @@ -607,7 +607,7 @@ class CephadmServe: daemons_post: Dict[str, List[orchestrator.DaemonDescription]] = defaultdict(list) for dd in daemons: # orphan? - spec = self.mgr.spec_store.specs.get(dd.service_name(), None) + spec = self.mgr.spec_store.active_specs.get(dd.service_name(), None) assert dd.hostname is not None assert dd.daemon_type is not None assert dd.daemon_id is not None diff --git a/src/pybind/mgr/cephadm/services/cephadmservice.py b/src/pybind/mgr/cephadm/services/cephadmservice.py index 95fc14f479d8..d9a241a5f040 100644 --- a/src/pybind/mgr/cephadm/services/cephadmservice.py +++ b/src/pybind/mgr/cephadm/services/cephadmservice.py @@ -654,6 +654,7 @@ class RgwService(CephService): 'val': key_data, }) + # TODO: fail, if we don't have a spec logger.info('Saving service %s spec with placement %s' % ( spec.service_name(), spec.placement.pretty_str())) self.mgr.spec_store.save(spec) diff --git a/src/pybind/mgr/cephadm/services/ha_rgw.py b/src/pybind/mgr/cephadm/services/ha_rgw.py index 4a37e006d58b..dfe29bf348a6 100644 --- a/src/pybind/mgr/cephadm/services/ha_rgw.py +++ b/src/pybind/mgr/cephadm/services/ha_rgw.py @@ -63,14 +63,15 @@ class HA_RGWService(CephService): service_name: str = "ha-rgw." + daemon_id.split('.')[0] # if no service spec, return empty config - if not daemon_spec.spec and service_name not in self.mgr.spec_store.specs: + # TODO: fail, if we don't have any spec + if not daemon_spec.spec and service_name not in self.mgr.spec_store.all_specs: config_files = {'files': {}} # type: Dict[str, Any] return config_files, [] elif daemon_spec.spec: spec = daemon_spec.spec else: # service spec is not attached to daemon spec but is in spec store - spec = cast(HA_RGWSpec, self.mgr.spec_store.specs[service_name]) + spec = cast(HA_RGWSpec, self.mgr.spec_store.all_specs[service_name]) rgw_daemons = self.mgr.cache.get_daemons_by_type('rgw') rgw_servers = [] @@ -110,14 +111,15 @@ class HA_RGWService(CephService): service_name: str = "ha-rgw." + daemon_id.split('.')[0] # if no service spec, return empty config - if not daemon_spec.spec and service_name not in self.mgr.spec_store.specs: + # TODO: In case of no spec, fail here + if not daemon_spec.spec and service_name not in self.mgr.spec_store.all_specs: config_file = {'files': {}} # type: Dict[str, Any] return config_file, [] elif daemon_spec.spec: spec = daemon_spec.spec else: # service spec is not attached to daemon spec but is in spec store - spec = cast(HA_RGWSpec, self.mgr.spec_store.specs[service_name]) + spec = cast(HA_RGWSpec, self.mgr.spec_store.all_specs[service_name]) all_hosts = [] for h, network, name in spec.definitive_host_list: diff --git a/src/pybind/mgr/cephadm/services/iscsi.py b/src/pybind/mgr/cephadm/services/iscsi.py index 98e647bfc985..6f73cfa9e77d 100644 --- a/src/pybind/mgr/cephadm/services/iscsi.py +++ b/src/pybind/mgr/cephadm/services/iscsi.py @@ -19,6 +19,7 @@ class IscsiService(CephService): assert spec.pool self.mgr._check_pool_exists(spec.pool, spec.service_name()) + # TODO: remove this: logger.info('Saving service %s spec with placement %s' % ( spec.service_name(), spec.placement.pretty_str())) self.mgr.spec_store.save(spec) @@ -77,8 +78,9 @@ class IscsiService(CephService): def get_set_cmd_dicts(out: str) -> List[dict]: gateways = json.loads(out)['gateways'] cmd_dicts = [] + # TODO: fail, if we don't have a spec spec = cast(IscsiServiceSpec, - self.mgr.spec_store.specs.get(daemon_descrs[0].service_name(), None)) + self.mgr.spec_store.all_specs.get(daemon_descrs[0].service_name(), None)) if spec.api_secure and spec.ssl_cert and spec.ssl_key: cmd_dicts.append({ 'prefix': 'dashboard set-iscsi-api-ssl-verification', @@ -91,8 +93,9 @@ class IscsiService(CephService): }) for dd in daemon_descrs: assert dd.hostname is not None + # todo: this can fail: spec = cast(IscsiServiceSpec, - self.mgr.spec_store.specs.get(dd.service_name(), None)) + self.mgr.spec_store.all_specs.get(dd.service_name(), None)) if not spec: logger.warning('No ServiceSpec found for %s', dd) continue diff --git a/src/pybind/mgr/cephadm/services/nfs.py b/src/pybind/mgr/cephadm/services/nfs.py index 3a6015982abf..a0e162cc32d7 100644 --- a/src/pybind/mgr/cephadm/services/nfs.py +++ b/src/pybind/mgr/cephadm/services/nfs.py @@ -19,6 +19,7 @@ class NFSService(CephService): assert spec.pool self.mgr._check_pool_exists(spec.pool, spec.service_name()) + # TODO: Fail here, in case of no spec logger.info('Saving service %s spec with placement %s' % ( spec.service_name(), spec.placement.pretty_str())) self.mgr.spec_store.save(spec) diff --git a/src/pybind/mgr/cephadm/tests/test_migration.py b/src/pybind/mgr/cephadm/tests/test_migration.py index 4b35440f6cdf..3a16149ba810 100644 --- a/src/pybind/mgr/cephadm/tests/test_migration.py +++ b/src/pybind/mgr/cephadm/tests/test_migration.py @@ -74,15 +74,15 @@ def test_migrate_service_id_mon_one(cephadm_module: CephadmOrchestrator): cephadm_module.spec_store.load() - assert len(cephadm_module.spec_store.specs) == 1 - assert cephadm_module.spec_store.specs['mon.wrong'].service_name() == 'mon' + assert len(cephadm_module.spec_store.all_specs) == 1 + assert cephadm_module.spec_store.all_specs['mon.wrong'].service_name() == 'mon' cephadm_module.migration_current = 1 cephadm_module.migration.migrate() assert cephadm_module.migration_current == 2 - assert len(cephadm_module.spec_store.specs) == 1 - assert cephadm_module.spec_store.specs['mon'] == ServiceSpec( + assert len(cephadm_module.spec_store.all_specs) == 1 + assert cephadm_module.spec_store.all_specs['mon'] == ServiceSpec( service_type='mon', unmanaged=True, placement=PlacementSpec(hosts=['host1']) @@ -116,16 +116,16 @@ def test_migrate_service_id_mon_two(cephadm_module: CephadmOrchestrator): cephadm_module.spec_store.load() - assert len(cephadm_module.spec_store.specs) == 2 - assert cephadm_module.spec_store.specs['mon.wrong'].service_name() == 'mon' - assert cephadm_module.spec_store.specs['mon'].service_name() == 'mon' + assert len(cephadm_module.spec_store.all_specs) == 2 + assert cephadm_module.spec_store.all_specs['mon.wrong'].service_name() == 'mon' + assert cephadm_module.spec_store.all_specs['mon'].service_name() == 'mon' cephadm_module.migration_current = 1 cephadm_module.migration.migrate() assert cephadm_module.migration_current == 2 - assert len(cephadm_module.spec_store.specs) == 1 - assert cephadm_module.spec_store.specs['mon'] == ServiceSpec( + assert len(cephadm_module.spec_store.all_specs) == 1 + assert cephadm_module.spec_store.all_specs['mon'] == ServiceSpec( service_type='mon', unmanaged=True, placement=PlacementSpec(count=5) @@ -149,4 +149,4 @@ def test_migrate_service_id_mds_one(cephadm_module: CephadmOrchestrator): cephadm_module.spec_store.load() # there is nothing to migrate, as the spec is gone now. - assert len(cephadm_module.spec_store.specs) == 0 + assert len(cephadm_module.spec_store.all_specs) == 0