From: Adam King Date: Mon, 28 Nov 2022 19:14:58 +0000 (-0500) Subject: mgr/cephadm: redo service level config when spec is updated X-Git-Tag: v17.2.7~370^2~4 X-Git-Url: http://git.apps.os.sepia.ceph.com/?a=commitdiff_plain;h=757f8e959d233d773c04b2719d7ab7ca77e8571a;p=ceph.git mgr/cephadm: redo service level config when spec is updated Previously, the service config function was only called when we deploy a new daemon for that service. That meant that updates to the spec such as changing a cert that don't affect the daemon placement wouldn't trigger the service level config to happen again. With this change, we now mark the service as needing its config function ran if a daemon for the service is added/removed or if the spec is updated. Fixes: https://tracker.ceph.com/issues/58100 Signed-off-by: Adam King (cherry picked from commit 1d50a2cdfb32b60b352cc03a83f8d1c35f2ae178) --- diff --git a/src/pybind/mgr/cephadm/inventory.py b/src/pybind/mgr/cephadm/inventory.py index fcbad42b7d929..e29b43e76e8fc 100644 --- a/src/pybind/mgr/cephadm/inventory.py +++ b/src/pybind/mgr/cephadm/inventory.py @@ -212,6 +212,7 @@ class SpecStore(): self.spec_created = {} # type: Dict[str, datetime.datetime] self.spec_deleted = {} # type: Dict[str, datetime.datetime] self.spec_preview = {} # type: Dict[str, ServiceSpec] + self._needs_configuration: Dict[str, bool] = {} @property def all_specs(self) -> Mapping[str, ServiceSpec]: @@ -256,6 +257,9 @@ class SpecStore(): deleted = str_to_datetime(cast(str, j['deleted'])) self.spec_deleted[service_name] = deleted + if 'needs_configuration' in j: + self._needs_configuration[service_name] = cast(bool, j['needs_configuration']) + if 'rank_map' in j and isinstance(j['rank_map'], dict): self._rank_maps[service_name] = {} for rank_str, m in j['rank_map'].items(): @@ -292,6 +296,7 @@ class SpecStore(): self.spec_preview[name] = spec return None self._specs[name] = spec + self._needs_configuration[name] = True if update_create: self.spec_created[name] = datetime_now() @@ -306,12 +311,15 @@ class SpecStore(): def _save(self, name: str) -> None: data: Dict[str, Any] = { 'spec': self._specs[name].to_json(), - 'created': datetime_to_str(self.spec_created[name]), } + if name in self.spec_created: + data['created'] = datetime_to_str(self.spec_created[name]) if name in self._rank_maps: data['rank_map'] = self._rank_maps[name] if name in self.spec_deleted: data['deleted'] = datetime_to_str(self.spec_deleted[name]) + if name in self._needs_configuration: + data['needs_configuration'] = self._needs_configuration[name] self.mgr.set_store( SPEC_STORE_PREFIX + name, @@ -343,6 +351,8 @@ class SpecStore(): del self.spec_created[service_name] if service_name in self.spec_deleted: del self.spec_deleted[service_name] + if service_name in self._needs_configuration: + del self._needs_configuration[service_name] self.mgr.set_store(SPEC_STORE_PREFIX + service_name, None) return found @@ -358,6 +368,23 @@ class SpecStore(): self.save(self._specs[service_name]) return f'Set unmanaged to {str(value)} for service {service_name}' + def needs_configuration(self, name: str) -> bool: + return self._needs_configuration.get(name, False) + + def mark_needs_configuration(self, name: str) -> None: + if name in self._specs: + self._needs_configuration[name] = True + self._save(name) + else: + self.mgr.log.warning(f'Attempted to mark unknown service "{name}" as needing configuration') + + def mark_configured(self, name: str) -> None: + if name in self._specs: + self._needs_configuration[name] = False + self._save(name) + else: + self.mgr.log.warning(f'Attempted to mark unknown service "{name}" as having been configured') + class ClientKeyringSpec(object): """ diff --git a/src/pybind/mgr/cephadm/serve.py b/src/pybind/mgr/cephadm/serve.py index 0c6a0b208d32e..951f68d19310b 100644 --- a/src/pybind/mgr/cephadm/serve.py +++ b/src/pybind/mgr/cephadm/serve.py @@ -725,9 +725,6 @@ class CephadmServe: if progress_total: update_progress() - # add any? - did_config = False - self.log.debug('Hosts that will receive new daemons: %s' % slots_to_add) self.log.debug('Daemons that will be removed: %s' % daemons_to_remove) @@ -794,9 +791,6 @@ class CephadmServe: # deploy new daemon daemon_id = slot.name - if not did_config: - svc.config(spec) - did_config = True daemon_spec = svc.make_daemon_spec( slot.hostname, daemon_id, slot.network, spec, @@ -816,6 +810,7 @@ class CephadmServe: progress_done += 1 update_progress() hosts_altered.add(daemon_spec.host) + self.mgr.spec_store.mark_needs_configuration(spec.service_name()) except (RuntimeError, OrchestratorError) as e: msg = (f"Failed while placing {slot.daemon_type}.{daemon_id} " f"on {slot.hostname}: {e}") @@ -872,12 +867,16 @@ class CephadmServe: progress_done += 1 update_progress() hosts_altered.add(d.hostname) + self.mgr.spec_store.mark_needs_configuration(spec.service_name()) self.mgr.remote('progress', 'complete', progress_id) except Exception as e: self.mgr.remote('progress', 'fail', progress_id, str(e)) raise finally: + if self.mgr.spec_store.needs_configuration(spec.service_name()): + svc.config(spec) + self.mgr.spec_store.mark_configured(spec.service_name()) if self.mgr.use_agent: # can only send ack to agents if we know for sure port they bound to hosts_altered = set([h for h in hosts_altered if (h in self.mgr.agent_cache.agent_ports and h in [