From a8191636aff925b615d957d3591023277f1909a1 Mon Sep 17 00:00:00 2001 From: Dominic Sloan-Murphy Date: Tue, 10 Dec 2024 11:29:23 +0000 Subject: [PATCH] feat(sack): refactor sackd to remove `juju_systemd_notices` --- charms/sackd/build.yaml | 1 - charms/sackd/src/charm.py | 29 +++------------------------ charms/sackd/tests/unit/test_charm.py | 19 ------------------ 3 files changed, 3 insertions(+), 46 deletions(-) diff --git a/charms/sackd/build.yaml b/charms/sackd/build.yaml index acff2da..d2bb328 100644 --- a/charms/sackd/build.yaml +++ b/charms/sackd/build.yaml @@ -1,4 +1,3 @@ external-libraries: - charms.hpc_libs.v0.slurm_ops - charms.operator_libs_linux.v0.apt - - charms.operator_libs_linux.v0.juju_systemd_notices diff --git a/charms/sackd/src/charm.py b/charms/sackd/src/charm.py index 7f6839c..4aa1b4d 100755 --- a/charms/sackd/src/charm.py +++ b/charms/sackd/src/charm.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright 2020-2024 Canonical Ltd. +# Copyright 2024 Canonical Ltd. # See LICENSE file for licensing details. """Sackd Operator Charm.""" @@ -19,11 +19,6 @@ ) from charms.hpc_libs.v0.slurm_ops import SackdManager, SlurmOpsError -from charms.operator_libs_linux.v0.juju_systemd_notices import ( # type: ignore[import-untyped] - ServiceStartedEvent, - ServiceStoppedEvent, - SystemdNotices, -) logger = logging.getLogger(__name__) @@ -46,15 +41,12 @@ def __init__(self, *args, **kwargs): self._sackd = SackdManager(snap=False) self._slurmctld = Slurmctld(self, "slurmctld") - self._systemd_notices = SystemdNotices(self, ["sackd"]) event_handler_bindings = { self.on.install: self._on_install, self.on.update_status: self._on_update_status, self._slurmctld.on.slurmctld_available: self._on_slurmctld_available, self._slurmctld.on.slurmctld_unavailable: self._on_slurmctld_unavailable, - self.on.service_sackd_started: self._on_sackd_started, - self.on.service_sackd_stopped: self._on_sackd_stopped, } for event, handler in event_handler_bindings.items(): self.framework.observe(event, handler) @@ -68,7 +60,6 @@ def _on_install(self, event: InstallEvent) -> None: # Ensure sackd does not start before relation established self._sackd.service.disable() self.unit.set_workload_version(self._sackd.version()) - self._systemd_notices.subscribe() self._stored.sackd_installed = True except SlurmOpsError as e: logger.error(e.message) @@ -109,7 +100,8 @@ def _on_slurmctld_available(self, event: SlurmctldAvailableEvent) -> None: # Restart sackd after we write event data to respective locations. self._sackd.munge.service.restart() # TODO change this once auth/slurm in place self._sackd.service.enable() - self._check_status() + if self._check_status(): + self.unit.status = ActiveStatus() def _on_slurmctld_unavailable(self, _) -> None: """Stop sackd and set slurmctld_available = False when we lose slurmctld.""" @@ -120,14 +112,6 @@ def _on_slurmctld_unavailable(self, _) -> None: self._sackd.service.disable() self._check_status() - def _on_sackd_started(self, _: ServiceStartedEvent) -> None: - """Handle event emitted by systemd after sackd daemon successfully starts.""" - self.unit.status = ActiveStatus() - - def _on_sackd_stopped(self, _: ServiceStoppedEvent) -> None: - """Handle event emitted by systemd after sackd daemon is stopped.""" - self.unit.status = BlockedStatus("sackd not running") - def _check_status(self) -> bool: """Check if we have all needed components. @@ -149,13 +133,6 @@ def _check_status(self) -> bool: self.unit.status = WaitingStatus("Waiting on: slurmctld") return False - # TODO: https://github.com/charmed-hpc/hpc-libs/issues/18 - - # Re-enable auth key validation check check when supported by `slurm_ops` charm library. - # No longer using munge - this is slurm.key now. - # if not self._sackd.check_munged(): - # self.unit.status = BlockedStatus("Error configuring auth key") - # return False - return True diff --git a/charms/sackd/tests/unit/test_charm.py b/charms/sackd/tests/unit/test_charm.py index 415751d..7e976dd 100644 --- a/charms/sackd/tests/unit/test_charm.py +++ b/charms/sackd/tests/unit/test_charm.py @@ -32,7 +32,6 @@ def setUp(self) -> None: """Set up unit test.""" self.ctx = Context(SackdCharm) - @patch("charms.operator_libs_linux.v0.juju_systemd_notices.SystemdNotices.subscribe") @patch("ops.framework.EventBase.defer") def test_install_success(self, defer, *_) -> None: """Test install success behavior.""" @@ -62,24 +61,6 @@ def test_install_fail(self, defer) -> None: defer.assert_called() - def test_service_sackd_start(self) -> None: - """Test service_sackd_started event handler.""" - with self.ctx(self.ctx.on.start(), State()) as manager: - # Run method directly rather than emit a ServiceStartedEvent. - # TODO: Refactor once Scenario has restored support for running custom events. See: - # https://github.com/canonical/operator/issues/1421 - manager.charm._on_sackd_started(None) - self.assertEqual(manager.charm.unit.status, ActiveStatus()) - - def test_service_sackd_stopped(self) -> None: - """Test service_sackd_stopped event handler.""" - with self.ctx(self.ctx.on.stop(), State()) as manager: - # Run method directly rather than emit a ServiceStoppedEvent. - # TODO: Refactor once Scenario has restored support for running custom events. See: - # https://github.com/canonical/operator/issues/1421 - manager.charm._on_sackd_stopped(None) - self.assertEqual(manager.charm.unit.status, BlockedStatus("sackd not running")) - @patch("interface_slurmctld.Slurmctld.is_joined", new_callable=PropertyMock(return_value=True)) def test_update_status_success(self, *_) -> None: """Test `UpdateStateEvent` hook success."""