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 <sebastian.wagner@suse.com>
(cherry picked from commit 1fcdad17685a8884135a2cda0cdfe3525e325474)

diff --git a/src/pybind/mgr/cephadm/inventory.py b/src/pybind/mgr/cephadm/inventory.py
index 3cff44296a13cfa3e0c4fb422ba0afc910198948..0e43bc607d44799f6fa77c65f0ffd982b4eae7d9 100644 (file)
--- 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 0b4ac9780264a60664dc89579e4a62e2b20984d0..6b02e81e75acc71565b7913e4ffda5b190dce95f 100644 (file)
--- 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 3a830b1cfa1072441a5dfea5dc981e74d7893c58..3e6017e3cd965d33716a0d28712d80deb8954555 100644 (file)
--- 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 53ed10b223b96ec45d3650d703e56a137d6c7d7a..e65a2fff2a97d1892635177b60e5ce7f68b7d46e 100644 (file)
--- 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 95fc14f479d8710711bb0511ad67cbc0ab243942..d9a241a5f040ac52cc97059bdcce3d152531f33e 100644 (file)
--- 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 4a37e006d58b5a256a0685a1df44fef93f9367ee..dfe29bf348a66d82d66130d5e93f5f316dc90e18 100644 (file)
--- 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 98e647bfc9851b66214429dca2ba28170f2e4c30..6f73cfa9e77d98179ef13f065900ccd80801ab60 100644 (file)
--- 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 3a6015982abfa3480d3046aaf921c0cd31a61136..a0e162cc32d7f0c3571f1cdebedeb5d46068942a 100644 (file)
--- 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 4b35440f6cdf6190030b2497761f09574aa5bd80..3a16149ba810f52eb4ed9bd66293e4ad8bb72a80 100644 (file)
--- 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