From 10533cece3034b3a3039e514aed0c0ddf503284f Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Tue, 16 Jul 2024 13:18:28 -0400 Subject: [PATCH 01/15] fix: `ruff` -> `ruff check` Original `ruff $@` is no longer supported by latest versions of ruff Signed-off-by: Jason C. Nucciarone --- .../juju_systemd_notices/notices-charm/actions.yaml | 5 ----- .../juju_systemd_notices/notices-charm/metadata.yaml | 8 -------- tox.ini | 4 ++-- 3 files changed, 2 insertions(+), 15 deletions(-) delete mode 100644 tests/integration/juju_systemd_notices/notices-charm/actions.yaml delete mode 100644 tests/integration/juju_systemd_notices/notices-charm/metadata.yaml diff --git a/tests/integration/juju_systemd_notices/notices-charm/actions.yaml b/tests/integration/juju_systemd_notices/notices-charm/actions.yaml deleted file mode 100644 index 243f52a3..00000000 --- a/tests/integration/juju_systemd_notices/notices-charm/actions.yaml +++ /dev/null @@ -1,5 +0,0 @@ -# Copyright 2023 Canonical Ltd. -# See LICENSE file for licensing details. - -stop-service: - description: Stop internal test service inside charm diff --git a/tests/integration/juju_systemd_notices/notices-charm/metadata.yaml b/tests/integration/juju_systemd_notices/notices-charm/metadata.yaml deleted file mode 100644 index c5a88502..00000000 --- a/tests/integration/juju_systemd_notices/notices-charm/metadata.yaml +++ /dev/null @@ -1,8 +0,0 @@ -# Copyright 2023 Canonical Ltd. -# See LICENSE file for licensing details. - -name: juju-systemd-notices -description: | - Test charm used for the juju_systemd_notices charm library integration tests. -summary: | - A charm with a minimal daemon for testing the juju-systemd-notices charm library. diff --git a/tox.ini b/tox.ini index 9d33c65b..ba817594 100644 --- a/tox.ini +++ b/tox.ini @@ -32,7 +32,7 @@ deps = black ruff commands = - ruff --fix {[vars]all_dir} + ruff check --fix {[vars]all_dir} black {[vars]all_dir} [testenv:lint] @@ -43,7 +43,7 @@ deps = codespell commands = codespell {toxinidir} - ruff {[vars]all_dir} + ruff check {[vars]all_dir} black --check --diff {[vars]all_dir} [testenv:unit] From ab3394d797dcde6d9fa524139a43ffa19b7c0aad Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Tue, 16 Jul 2024 13:19:52 -0400 Subject: [PATCH 02/15] fix!: add support for observing snap services Adds the `watch.yaml` file for configuring how the notices daemon observes services. The daemon will get the service alias from this file and use the alias to determine the proper hook to fire when the observed service is started or stopped. BREAKING CHANGES: __init__ for SystemdNotices now takes the Service data class instead of a generic list. The Service dataclass was added so that it would be easier to specify an alias for an observed service. Signed-off-by: Jason C. Nucciarone --- .../v0/juju_systemd_notices.py | 124 +++++++++++------- 1 file changed, 80 insertions(+), 44 deletions(-) diff --git a/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py b/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py index 08157c9d..679f6b9e 100644 --- a/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py +++ b/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py @@ -79,19 +79,20 @@ def _on_slurmd_stopped(self, _: ServiceStoppedEvent) -> None: ``` """ -__all__ = ["ServiceStartedEvent", "ServiceStoppedEvent", "SystemdNotices"] +__all__ = ["Service", "ServiceStartedEvent", "ServiceStoppedEvent", "SystemdNotices"] import argparse import asyncio import functools import logging -import re import signal import subprocess import textwrap +from dataclasses import dataclass from pathlib import Path -from typing import List +from typing import Mapping, Optional +import yaml from dbus_fast.aio import MessageBus from dbus_fast.constants import BusType, MessageType from dbus_fast.errors import DBusError @@ -111,12 +112,11 @@ def _on_slurmd_stopped(self, _: ServiceStoppedEvent) -> None: # juju-systemd-notices charm library dependencies. # Charm library dependencies are installed when the consuming charm is packed. -PYDEPS = ["dbus-fast>=1.90.2"] +PYDEPS = ["dbus-fast>=1.90.2", "pyyaml>=6.0.1"] _logger = logging.getLogger(__name__) _juju_unit = None _service_states = {} -_service_hook_regex_filter = re.compile(r"service-(?P[\w\\:-]*)-(?:started|stopped)") _DBUS_CHAR_MAPPINGS = { "_5f": "_", # _ must be first since char mappings contain _. "_40": "@", @@ -148,6 +148,22 @@ def _systemctl(*args) -> None: _disable_service = functools.partial(_systemctl, "disable") +@dataclass +class Service: + """Systemd service to observe. + + Args: + name: Name of systemd service to observe on dbus. + alias: Event name alias for service. + """ + + name: str + alias: Optional[str] = None + + def __post_init__(self) -> None: # noqa D105 + self.alias = self.alias or self.name + + class ServiceStartedEvent(EventBase): """Event emitted when service has started.""" @@ -159,7 +175,7 @@ class ServiceStoppedEvent(EventBase): class SystemdNotices: """Observe systemd services on your machine base.""" - def __init__(self, charm: CharmBase, services: List[str]) -> None: + def __init__(self, charm: CharmBase, *services: Service) -> None: """Instantiate systemd notices service.""" self._charm = charm self._services = services @@ -170,39 +186,65 @@ def __init__(self, charm: CharmBase, services: List[str]) -> None: "Attaching systemd notice events to charm %s", self._charm.__class__.__name__ ) for service in self._services: - self._charm.on.define_event(f"service_{service}_started", ServiceStartedEvent) - self._charm.on.define_event(f"service_{service}_stopped", ServiceStoppedEvent) + self._charm.on.define_event(f"service_{service.alias}_started", ServiceStartedEvent) + self._charm.on.define_event(f"service_{service.alias}_stopped", ServiceStoppedEvent) def subscribe(self) -> None: """Subscribe charmed operator to observe status of systemd services.""" + self._generate_hooks() + self._generate_config() + self._start() + + def stop(self) -> None: + """Stop charmed operator from observing the status of subscribed services.""" + _stop_service(self._service_file.name) + # Notices daemon is disabled so that the service will not restart after machine reboot. + _disable_service(self._service_file.name) + + def _generate_hooks(self) -> None: + """Generate legacy event hooks for observed systemd services.""" _logger.debug("Generating systemd notice hooks for %s", self._services) - start_hooks = [Path(f"hooks/service-{service}-started") for service in self._services] - stop_hooks = [Path(f"hooks/service-{service}-stopped") for service in self._services] + start_hooks = [Path(f"hooks/service-{s.alias}-started") for s in self._services] + stop_hooks = [Path(f"hooks/service-{s.alias}-stopped") for s in self._services] for hook in start_hooks + stop_hooks: if hook.exists(): _logger.debug("Hook %s already exists. Skipping...", hook.name) else: hook.symlink_to(self._charm.framework.charm_dir / "dispatch") + def _generate_config(self) -> None: + """Generate watch file for systemd notices daemon.""" + _logger.debug("Generating watch file for %s", self._services) + config = {"services": {s.name: s.alias for s in self._services}} + + config_file = self._charm.framework.charm_dir / "watch.yaml" + if config_file.exists(): + _logger.debug("Overwriting existing watch file %s", config_file.name) + with config_file.open("wt") as fout: + yaml.dump(config, fout) + config_file.chmod(0o600) + + def _start(self) -> None: + """Start systemd notices daemon to observe subscribed services.""" _logger.debug("Starting %s daemon", self._service_file.name) if self._service_file.exists(): _logger.debug("Overwriting existing service file %s", self._service_file.name) self._service_file.write_text( textwrap.dedent( f""" - [Unit] - Description=Juju systemd notices daemon - After=multi-user.target - - [Service] - Type=simple - Restart=always - WorkingDirectory={self._charm.framework.charm_dir} - Environment="PYTHONPATH={self._charm.framework.charm_dir / "venv"}" - ExecStart=/usr/bin/python3 {__file__} {self._charm.unit.name} - - [Install] - WantedBy=multi-user.target + [Unit] + Description=Juju systemd notices daemon + After=multi-user.target + + [Service] + Type=simple + Restart=always + WorkingDirectory={self._charm.framework.charm_dir} + Environment="PYTHONPATH={self._charm.framework.charm_dir / "venv"}" + ExecStart=/usr/bin/python3 {__file__} {self._charm.unit.name} + + [Install] + WantedBy=multi-user.target """ ).strip() ) @@ -214,12 +256,6 @@ def subscribe(self) -> None: _start_service(self._service_file.name) _logger.debug("Started %s daemon", self._service_file.name) - def stop(self) -> None: - """Stop charmed operator from observing the status of subscribed services.""" - _stop_service(self._service_file.name) - # Notices daemon is disabled so that the service will not restart after machine reboot. - _disable_service(self._service_file.name) - def _name_to_dbus_path(name: str) -> str: """Convert the specified name into an org.freedesktop.systemd1.Unit path handle. @@ -256,6 +292,16 @@ def _dbus_path_to_name(path: str) -> str: return name +@functools.lru_cache(maxsize=32) +def _read_config() -> Mapping[str, str]: + """Read systemd notices daemon configuration to service names and aliases.""" + config_file = Path.cwd() / "watch.yaml" + _logger.debug("Loading observed services from configuration file %s", config_file) + + with config_file.open("rt") as fin: + return yaml.safe_load(fin)["services"] + + def _systemd_unit_changed(msg: Message) -> bool: """Send Juju notification if systemd unit state changes on the DBus bus. @@ -310,8 +356,10 @@ async def _send_juju_notification(service: str, state: str) -> None: if service.endswith(".service"): service = service[0:-len(".service")] # fmt: skip + watched_services = _read_config() + alias = watched_services[service] event_name = "started" if state == "active" else "stopped" - hook = f"service-{service}-{event_name}" + hook = f"service-{alias}-{event_name}" cmd = ["/usr/bin/juju-exec", _juju_unit, f"hooks/{hook}"] _logger.debug("Invoking hook %s with command: %s", hook, " ".join(cmd)) @@ -364,20 +412,8 @@ async def _async_load_services() -> None: will be queried from systemd to determine it's initial state. """ global _juju_unit - hooks_dir = Path.cwd() / "hooks" - _logger.info("Loading services from hooks in %s", hooks_dir) - - if not hooks_dir.exists(): - _logger.warning("Hooks dir %s does not exist.", hooks_dir) - return - - watched_services = [] - # Get service-{service}-(started|stopped) hooks defined by the charm. - for hook in hooks_dir.iterdir(): - match = _service_hook_regex_filter.match(hook.name) - if match: - watched_services.append(match.group("service")) + watched_services = _read_config() _logger.info("Services from hooks are %s", watched_services) if not watched_services: return @@ -386,7 +422,7 @@ async def _async_load_services() -> None: # Loop through all the services and be sure that a new watcher is # started for new ones. - for service in watched_services: + for service in watched_services.keys(): # The .service suffix is necessary and will cause lookup failures of the # service unit when readying the watcher if absent from the service name. service = f"{service}.service" From d80f05efc8c54b5f537dcf65c9bb9dc9c1922910 Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Tue, 16 Jul 2024 13:23:28 -0400 Subject: [PATCH 03/15] tests: update unit tests to reflect new notices API Signed-off-by: Jason C. Nucciarone --- tests/unit/test_juju_systemd_notices.py | 93 +++++++++++++++++-------- 1 file changed, 63 insertions(+), 30 deletions(-) diff --git a/tests/unit/test_juju_systemd_notices.py b/tests/unit/test_juju_systemd_notices.py index 59f79872..207ec586 100644 --- a/tests/unit/test_juju_systemd_notices.py +++ b/tests/unit/test_juju_systemd_notices.py @@ -11,6 +11,7 @@ from unittest.mock import AsyncMock, patch from charms.operator_libs_linux.v0.juju_systemd_notices import ( + Service, ServiceStartedEvent, ServiceStoppedEvent, SystemdNotices, @@ -29,6 +30,14 @@ from ops.charm import CharmBase, InstallEvent, StopEvent from ops.model import ActiveStatus, BlockedStatus from ops.testing import Harness +from pyfakefs.fake_filesystem_unittest import Patcher + +mock_watch_config = """ +--- +services: + foobar: foobar + snap.test.service: service +""" class MockNoticesCharm(CharmBase): @@ -37,7 +46,7 @@ class MockNoticesCharm(CharmBase): def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) - self._systemd_notices = SystemdNotices(self, ["foobar"]) + self.notices = SystemdNotices(self, Service("foobar"), Service("snap.test.service", alias="service")) event_handler_bindings = { self.on.install: self._on_install, self.on.stop: self._on_stop, @@ -49,11 +58,11 @@ def __init__(self, *args, **kwargs) -> None: def _on_install(self, _: InstallEvent) -> None: """Subscribe to foobar service to watch for events.""" - self._systemd_notices.subscribe() + self.notices.subscribe() def _on_stop(self, _: StopEvent) -> None: """Stop watching foobar service as machine is removed.""" - self._systemd_notices.stop() + self.notices.stop() def _on_foobar_started(self, _: ServiceStartedEvent) -> None: """Set status to active after systemctl marks service as active.""" @@ -72,23 +81,57 @@ def setUp(self) -> None: self.addCleanup(self.harness.cleanup) self.harness.begin() + @patch("pathlib.Path.symlink_to") + @patch("pathlib.Path.exists") + def test_generate_hooks(self, mock_exists, _) -> None: + """Test that legacy event hooks for observed services are created correctly.""" + # Scenario 1 - Generate success but no pre-existing hooks. + mock_exists.return_value = False + self.harness.charm.notices._generate_hooks() + + # Scenario 2 - Generate success but pre-existing hooks. + mock_exists.return_value = True + self.harness.charm.notices._generate_hooks() + + @patch("pathlib.Path.exists") + def test_generate_config(self, mock_exists) -> None: + """Test that watch configuration file is generated correctly.""" + with Patcher() as patcher: + patcher.fs.create_file("no-disk-path/watch.yaml") + + # Scenario 1 - Generate success but no pre-existing watch configuration. + mock_exists.return_value = False + self.harness.charm.notices._generate_config() + + # Scenario 2 - Generate success but pre-existing watch configuration. + mock_exists.return_value = True + self.harness.charm.notices._generate_config() + @patch("pathlib.Path.write_text") @patch("pathlib.Path.symlink_to") @patch("subprocess.check_output") @patch("pathlib.Path.exists") - def test_subscribe(self, mock_exists, mock_subp, *_) -> None: + def test_start(self, mock_exists, mock_subp, *_) -> None: + """Test that start method correctly starts notices daemon.""" # Scenario 1 - Subscribe success but no pre-existing service file. mock_exists.return_value = False - self.harness.charm.on.install.emit() + self.harness.charm.notices._start() # Scenario 2 - Subscribe success and pre-existing service file. mock_exists.return_value = True - self.harness.charm.on.install.emit() + self.harness.charm.notices._start() # Scenario 3 - Subscribe success but systemctl fails to start notices daemon. mock_subp.side_effect = subprocess.CalledProcessError(1, "systemctl start foobar") with self.assertRaises(subprocess.CalledProcessError): - self.harness.charm.on.install.emit() + self.harness.charm.notices._start() + + @patch("charms.operator_libs_linux.v0.juju_systemd_notices.SystemdNotices._generate_hooks") + @patch("charms.operator_libs_linux.v0.juju_systemd_notices.SystemdNotices._generate_config") + @patch("charms.operator_libs_linux.v0.juju_systemd_notices.SystemdNotices._start") + def test_subscribe(self, *_) -> None: + """Test `subscribe` method.""" + self.harness.charm.on.install.emit() @patch("subprocess.check_output") def test_stop(self, *_) -> None: @@ -170,21 +213,22 @@ async def test_systemd_unit_changed(self, mock_message, mock_variant, *_) -> Non ): self.assertTrue(_systemd_unit_changed(mock_message)) + @patch("charms.operator_libs_linux.v0.juju_systemd_notices._read_config", return_value={"foobar": "foobar"}) @patch("charms.operator_libs_linux.v0.juju_systemd_notices._juju_unit", "foobar/0") @patch("asyncio.create_subprocess_exec") - async def test_send_juju_notification(self, mock_subp, *_) -> None: + async def test_send_juju_notification(self, mock_subcmd, *_) -> None: # Scenario 1 - .service in service name and notification succeeds. mock_p = AsyncMock() mock_p.wait.return_value = None mock_p.returncode = 0 - mock_subp.return_value = mock_p + mock_subcmd.return_value = mock_p await _send_juju_notification("foobar.service", "active") # Scenario 2 - No .service in name and state is stopped but notification fails. mock_p = AsyncMock() mock_p.wait.return_value = None mock_p.returncode = 1 - mock_subp.return_value = mock_p + mock_subcmd.return_value = mock_p await _send_juju_notification("foobar", "inactive") async def test_get_service_state(self) -> None: @@ -206,33 +250,22 @@ async def test_get_service_state(self) -> None: self.assertEqual(state, "unknown") @patch("charms.operator_libs_linux.v0.juju_systemd_notices._get_service_state") - @patch("pathlib.Path.iterdir") - @patch("pathlib.Path.exists") - async def test_async_load_services(self, mock_exists, mock_iterdir, mock_state) -> None: - # Scenario 1 - Hooks dir does not exist. - mock_exists.return_value = False - self.assertIsNone(await _async_load_services()) + @patch("charms.operator_libs_linux.v0.juju_systemd_notices._read_config") + async def test_async_load_services(self, mock_config, mock_state) -> None: + mock_config.return_value = {} + await _async_load_services() - # Scenario 2 - There are no services to watch. - mock_exists.return_value = True - mock_iterdir.return_value = [] - self.assertIsNone(await _async_load_services()) - - # Scenario 3 - Desired outcome (services are subscribed to for watching). - mock_exists.return_value = True - mock_iterdir.return_value = [ - Path("service-foobar-started"), - Path("service-foobar-stopped"), - Path("dispatch"), # Ensure that unmatched hooks are ignored/not registered. - ] + mock_config.return_value = {"foobar": "foobar"} mock_state.return_value = "active" - self.assertIsNone(await _async_load_services()) + await _async_load_services() @patch("pathlib.Path.exists", return_value=False) @patch("asyncio.Event.wait") async def test_juju_systemd_notices_daemon(self, *_) -> None: # Desired outcome is that _juju_systemd_notices does not fail to start. - await _juju_systemd_notices_daemon() + with Patcher() as patcher: + patcher.fs.create_file("watch.yaml", contents=mock_watch_config) + await _juju_systemd_notices_daemon() @patch("charms.operator_libs_linux.v0.juju_systemd_notices._juju_systemd_notices_daemon") @patch("argparse.ArgumentParser.parse_args") From 5ebe2a8ecd73d69c43ef0e856900f836f28d0813 Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Tue, 16 Jul 2024 13:24:08 -0400 Subject: [PATCH 04/15] tests: update integration tests * Consolidates metadata.yaml and actions.yaml into charmcraft.yaml for test charm. Less files to worry about. Signed-off-by: Jason C. Nucciarone --- .../notices-charm/charmcraft.yaml | 13 ++++++++++++- .../juju_systemd_notices/notices-charm/src/charm.py | 5 +++-- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/tests/integration/juju_systemd_notices/notices-charm/charmcraft.yaml b/tests/integration/juju_systemd_notices/notices-charm/charmcraft.yaml index e543430c..4fc0954c 100644 --- a/tests/integration/juju_systemd_notices/notices-charm/charmcraft.yaml +++ b/tests/integration/juju_systemd_notices/notices-charm/charmcraft.yaml @@ -1,6 +1,12 @@ -# Copyright 2023 Canonical Ltd. +# Copyright 2023-2024 Canonical Ltd. # See LICENSE file for licensing details. +name: juju-systemd-notices +description: | + Test charm used for the juju_systemd_notices charm library integration tests. +summary: | + A charm with a minimal daemon for testing the juju-systemd-notices charm library. + type: charm bases: - build-on: @@ -9,3 +15,8 @@ bases: run-on: - name: ubuntu channel: "22.04" + +actions: + stop-service: + description: Stop internal test service inside charm + diff --git a/tests/integration/juju_systemd_notices/notices-charm/src/charm.py b/tests/integration/juju_systemd_notices/notices-charm/src/charm.py index ff6cf87d..26c15ae6 100755 --- a/tests/integration/juju_systemd_notices/notices-charm/src/charm.py +++ b/tests/integration/juju_systemd_notices/notices-charm/src/charm.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright 2023 Canonical Ltd. +# Copyright 2023-2024 Canonical Ltd. # See LICENSE file for licensing details. """Minimal charm for testing the juju_systemd_notices charm library. @@ -12,6 +12,7 @@ import charms.operator_libs_linux.v1.systemd as systemd import daemon from charms.operator_libs_linux.v0.juju_systemd_notices import ( + Service, ServiceStartedEvent, ServiceStoppedEvent, SystemdNotices, @@ -29,7 +30,7 @@ class NoticesCharm(CharmBase): def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) - self._systemd_notices = SystemdNotices(self, ["test"]) + self._systemd_notices = SystemdNotices(self, Service("test")) event_handler_bindings = { self.on.install: self._on_install, self.on.start: self._on_start, From c0309feead76a2eee0f89d2f5210351220682c06 Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Tue, 16 Jul 2024 13:28:11 -0400 Subject: [PATCH 05/15] docs: update lib documentation Signed-off-by: Jason C. Nucciarone --- lib/charms/operator_libs_linux/v0/juju_systemd_notices.py | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py b/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py index 679f6b9e..c3d8fca2 100644 --- a/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py +++ b/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py @@ -28,6 +28,7 @@ ```python from charms.operator_libs_linux.v0.juju_systemd_notices import ( + Service, ServiceStartedEvent, ServiceStoppedEvent, SystemdNotices, @@ -41,7 +42,7 @@ def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) # Register services with charm. This adds the events to observe. - self._systemd_notices = SystemdNotices(self, ["slurmd"]) + self._systemd_notices = SystemdNotices(self, Service("snap.slurm.slurmd", alias="slurmd")) self.framework.observe(self.on.install, self._on_install) self.framework.observe(self.on.stop, self._on_stop) self.framework.observe(self.on.service_slurmd_started, self._on_slurmd_started) @@ -58,7 +59,7 @@ def _on_install(self, _: InstallEvent) -> None: def _on_start(self, _: StartEvent) -> None: # This will trigger the juju-systemd-notices daemon to # emit a `service-slurmd-started` event. - systemd.service_start("slurmd") + snap.slurmd.enable() def _on_stop(self, _: StopEvent) -> None: # To stop the juju-systemd-notices service running in the background. @@ -72,7 +73,7 @@ def _on_slurmd_started(self, _: ServiceStartedEvent) -> None: # This will trigger the juju-systemd-notices daemon to # emit a `service-slurmd-stopped` event. - systemd.service_stop("slurmd") + snap.slurmd.stop() def _on_slurmd_stopped(self, _: ServiceStoppedEvent) -> None: self.unit.status = BlockedStatus("slurmd not running") From 6f8e7db651a78b16b23acd851ea44324585e3e40 Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Tue, 16 Jul 2024 13:28:33 -0400 Subject: [PATCH 06/15] chore(fmt): apply formatting rules Signed-off-by: Jason C. Nucciarone --- tests/unit/test_juju_systemd_notices.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/unit/test_juju_systemd_notices.py b/tests/unit/test_juju_systemd_notices.py index 207ec586..86bb32a6 100644 --- a/tests/unit/test_juju_systemd_notices.py +++ b/tests/unit/test_juju_systemd_notices.py @@ -7,7 +7,6 @@ import argparse import subprocess import unittest -from pathlib import Path from unittest.mock import AsyncMock, patch from charms.operator_libs_linux.v0.juju_systemd_notices import ( @@ -46,7 +45,9 @@ class MockNoticesCharm(CharmBase): def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) - self.notices = SystemdNotices(self, Service("foobar"), Service("snap.test.service", alias="service")) + self.notices = SystemdNotices( + self, Service("foobar"), Service("snap.test.service", alias="service") + ) event_handler_bindings = { self.on.install: self._on_install, self.on.stop: self._on_stop, @@ -213,7 +214,10 @@ async def test_systemd_unit_changed(self, mock_message, mock_variant, *_) -> Non ): self.assertTrue(_systemd_unit_changed(mock_message)) - @patch("charms.operator_libs_linux.v0.juju_systemd_notices._read_config", return_value={"foobar": "foobar"}) + @patch( + "charms.operator_libs_linux.v0.juju_systemd_notices._read_config", + return_value={"foobar": "foobar"}, + ) @patch("charms.operator_libs_linux.v0.juju_systemd_notices._juju_unit", "foobar/0") @patch("asyncio.create_subprocess_exec") async def test_send_juju_notification(self, mock_subcmd, *_) -> None: From f27fdc473a0760166a959d32d119da08889b7bbc Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Tue, 16 Jul 2024 13:30:59 -0400 Subject: [PATCH 07/15] chore(lint): update copyright year Signed-off-by: Jason C. Nucciarone --- lib/charms/operator_libs_linux/v0/juju_systemd_notices.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py b/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py index c3d8fca2..d4c60bef 100644 --- a/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py +++ b/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py @@ -1,5 +1,5 @@ #!/usr/bin/python3 -# Copyright 2023 Canonical Ltd. +# Copyright 2023-2024 Canonical Ltd. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. From 1e15df38ee1137fc441d13c2c62557452b2fd20f Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Tue, 16 Jul 2024 15:14:33 -0400 Subject: [PATCH 08/15] refactor: `open` -> `write_text` `yaml.dump()` returns a string if steam is none, so it is easier to just pass the content off to `write_text` instead of opening the file with a context manager. Signed-off-by: Jason C. Nucciarone --- lib/charms/operator_libs_linux/v0/juju_systemd_notices.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py b/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py index d4c60bef..5b3185c2 100644 --- a/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py +++ b/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py @@ -221,8 +221,7 @@ def _generate_config(self) -> None: config_file = self._charm.framework.charm_dir / "watch.yaml" if config_file.exists(): _logger.debug("Overwriting existing watch file %s", config_file.name) - with config_file.open("wt") as fout: - yaml.dump(config, fout) + config_file.write_text(yaml.dump(config)) config_file.chmod(0o600) def _start(self) -> None: From 1367df2d55719c958d3b1cbfd0a7e3d4c0b99afd Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Tue, 16 Jul 2024 15:17:57 -0400 Subject: [PATCH 09/15] fix(tests): correctly patch filesystem on python 3.8 Signed-off-by: Jason C. Nucciarone --- tests/unit/test_juju_systemd_notices.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tests/unit/test_juju_systemd_notices.py b/tests/unit/test_juju_systemd_notices.py index 86bb32a6..4ce2dfb1 100644 --- a/tests/unit/test_juju_systemd_notices.py +++ b/tests/unit/test_juju_systemd_notices.py @@ -7,6 +7,7 @@ import argparse import subprocess import unittest +from pathlib import Path from unittest.mock import AsyncMock, patch from charms.operator_libs_linux.v0.juju_systemd_notices import ( @@ -98,7 +99,12 @@ def test_generate_hooks(self, mock_exists, _) -> None: def test_generate_config(self, mock_exists) -> None: """Test that watch configuration file is generated correctly.""" with Patcher() as patcher: - patcher.fs.create_file("no-disk-path/watch.yaml") + # Set charm_dir to "/" because mocked charm_dir default + # `no-disk-path` does not exist within the fake filesystem. + # Tried creating `no-disk-path` in the fake filesystem, but + # the mocked objects could not find it. + self.harness.charm.framework.charm_dir = Path("/") + patcher.fs.create_file("watch.yaml") # Scenario 1 - Generate success but no pre-existing watch configuration. mock_exists.return_value = False From c8f0767563b3e317b91c0406c0872db930a09830 Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Thu, 18 Jul 2024 13:42:10 -0400 Subject: [PATCH 10/15] reactor: remove watch.yaml mechanism - Switch to passing the service names and aliases via argparse rather than using the pyyaml external dependency to create the watch.yaml file. - Use global map to track the observed service rather than caching the output of a watch.yaml read. Signed-off-by: Jason C. Nucciarone --- .../v0/juju_systemd_notices.py | 121 ++++++++---------- 1 file changed, 56 insertions(+), 65 deletions(-) diff --git a/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py b/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py index 5b3185c2..b90ba9e3 100644 --- a/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py +++ b/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py @@ -91,9 +91,8 @@ def _on_slurmd_stopped(self, _: ServiceStoppedEvent) -> None: import textwrap from dataclasses import dataclass from pathlib import Path -from typing import Mapping, Optional +from typing import List, Optional, Union -import yaml from dbus_fast.aio import MessageBus from dbus_fast.constants import BusType, MessageType from dbus_fast.errors import DBusError @@ -113,10 +112,11 @@ def _on_slurmd_stopped(self, _: ServiceStoppedEvent) -> None: # juju-systemd-notices charm library dependencies. # Charm library dependencies are installed when the consuming charm is packed. -PYDEPS = ["dbus-fast>=1.90.2", "pyyaml>=6.0.1"] +PYDEPS = ["dbus-fast>=1.90.2"] _logger = logging.getLogger(__name__) _juju_unit = None +_observed_services = {} _service_states = {} _DBUS_CHAR_MAPPINGS = { "_5f": "_", # _ must be first since char mappings contain _. @@ -161,9 +161,6 @@ class Service: name: str alias: Optional[str] = None - def __post_init__(self) -> None: # noqa D105 - self.alias = self.alias or self.name - class ServiceStartedEvent(EventBase): """Event emitted when service has started.""" @@ -176,24 +173,25 @@ class ServiceStoppedEvent(EventBase): class SystemdNotices: """Observe systemd services on your machine base.""" - def __init__(self, charm: CharmBase, *services: Service) -> None: + def __init__(self, charm: CharmBase, services: List[Union[str, Service]]) -> None: """Instantiate systemd notices service.""" self._charm = charm - self._services = services + self._services = [Service(s) if isinstance(s, str) else s for s in services] unit_name = self._charm.unit.name.replace("/", "-") self._service_file = Path(f"/etc/systemd/system/juju-{unit_name}-systemd-notices.service") _logger.debug( "Attaching systemd notice events to charm %s", self._charm.__class__.__name__ ) - for service in self._services: - self._charm.on.define_event(f"service_{service.alias}_started", ServiceStartedEvent) - self._charm.on.define_event(f"service_{service.alias}_stopped", ServiceStoppedEvent) + for s in self._services: + event = s.alias or s.name + self._charm.on.define_event(f"service_{event}_started", ServiceStartedEvent) + self._charm.on.define_event(f"service_{event}_stopped", ServiceStoppedEvent) def subscribe(self) -> None: """Subscribe charmed operator to observe status of systemd services.""" self._generate_hooks() - self._generate_config() + self._generate_service() self._start() def stop(self) -> None: @@ -205,53 +203,55 @@ def stop(self) -> None: def _generate_hooks(self) -> None: """Generate legacy event hooks for observed systemd services.""" _logger.debug("Generating systemd notice hooks for %s", self._services) - start_hooks = [Path(f"hooks/service-{s.alias}-started") for s in self._services] - stop_hooks = [Path(f"hooks/service-{s.alias}-stopped") for s in self._services] + events = [s.alias or s.name for s in self._services] + start_hooks = [Path(f"hooks/service-{e}-started") for e in events] + stop_hooks = [Path(f"hooks/service-{e}-stopped") for e in events] for hook in start_hooks + stop_hooks: if hook.exists(): _logger.debug("Hook %s already exists. Skipping...", hook.name) else: hook.symlink_to(self._charm.framework.charm_dir / "dispatch") - def _generate_config(self) -> None: - """Generate watch file for systemd notices daemon.""" - _logger.debug("Generating watch file for %s", self._services) - config = {"services": {s.name: s.alias for s in self._services}} - - config_file = self._charm.framework.charm_dir / "watch.yaml" - if config_file.exists(): - _logger.debug("Overwriting existing watch file %s", config_file.name) - config_file.write_text(yaml.dump(config)) - config_file.chmod(0o600) - - def _start(self) -> None: - """Start systemd notices daemon to observe subscribed services.""" - _logger.debug("Starting %s daemon", self._service_file.name) + def _generate_service(self) -> None: + """Generate systemd service file for notices daemon.""" + _logger.debug("Generating service file %s", self._service_file.name) if self._service_file.exists(): _logger.debug("Overwriting existing service file %s", self._service_file.name) + + services = [f"{s.name}={s.alias or s.name}" for s in self._services] self._service_file.write_text( textwrap.dedent( f""" - [Unit] - Description=Juju systemd notices daemon - After=multi-user.target - - [Service] - Type=simple - Restart=always - WorkingDirectory={self._charm.framework.charm_dir} - Environment="PYTHONPATH={self._charm.framework.charm_dir / "venv"}" - ExecStart=/usr/bin/python3 {__file__} {self._charm.unit.name} - - [Install] - WantedBy=multi-user.target + [Unit] + Description=Juju systemd notices daemon + After=multi-user.target + + [Service] + Type=simple + Restart=always + WorkingDirectory={self._charm.framework.charm_dir} + Environment="PYTHONPATH={self._charm.framework.charm_dir / "venv"}" + ExecStart=/usr/bin/python3 {__file__} --unit {self._charm.unit.name} {' '.join(services)} + + [Install] + WantedBy=multi-user.target """ ).strip() ) - _logger.debug("Service file %s written. Reloading systemd", self._service_file.name) + + _logger.debug( + "Service file %s written. Reloading systemd manager configuration", + self._service_file.name, + ) + + def _start(self) -> None: + """Start systemd notices daemon to observe subscribed services.""" + _logger.debug("Starting %s daemon", self._service_file.name) + + # Reload systemd manager configuration so that it will pick up notices daemon. _daemon_reload() - # Notices daemon is enabled so that the service will start even after machine reboot. - # This functionality is needed in the event that a charm is rebooted to apply updates. + + # Enable notices daemon to start after machine reboots. _enable_service(self._service_file.name) _start_service(self._service_file.name) _logger.debug("Started %s daemon", self._service_file.name) @@ -292,16 +292,6 @@ def _dbus_path_to_name(path: str) -> str: return name -@functools.lru_cache(maxsize=32) -def _read_config() -> Mapping[str, str]: - """Read systemd notices daemon configuration to service names and aliases.""" - config_file = Path.cwd() / "watch.yaml" - _logger.debug("Loading observed services from configuration file %s", config_file) - - with config_file.open("rt") as fin: - return yaml.safe_load(fin)["services"] - - def _systemd_unit_changed(msg: Message) -> bool: """Send Juju notification if systemd unit state changes on the DBus bus. @@ -356,8 +346,8 @@ async def _send_juju_notification(service: str, state: str) -> None: if service.endswith(".service"): service = service[0:-len(".service")] # fmt: skip - watched_services = _read_config() - alias = watched_services[service] + global _observed_services + alias = _observed_services[service] event_name = "started" if state == "active" else "stopped" hook = f"service-{alias}-{event_name}" cmd = ["/usr/bin/juju-exec", _juju_unit, f"hooks/{hook}"] @@ -411,18 +401,13 @@ async def _async_load_services() -> None: that should be watched. Upon finding a service hook it's current ActiveState will be queried from systemd to determine it's initial state. """ - global _juju_unit - - watched_services = _read_config() - _logger.info("Services from hooks are %s", watched_services) - if not watched_services: - return - bus = await MessageBus(bus_type=BusType.SYSTEM).connect() # Loop through all the services and be sure that a new watcher is # started for new ones. - for service in watched_services.keys(): + global _observed_services + _logger.info("Services to observe are %s", _observed_services) + for service in _observed_services: # The .service suffix is necessary and will cause lookup failures of the # service unit when readying the watcher if absent from the service name. service = f"{service}.service" @@ -498,13 +483,19 @@ def _main(): """ parser = argparse.ArgumentParser() parser.add_argument("-d", "--debug", action="store_true") - parser.add_argument("unit", type=str) + parser.add_argument("--unit", type=str) + parser.add_argument("services", nargs="*") args = parser.parse_args() # Intentionally set as global. global _juju_unit _juju_unit = args.unit + global _observed_services + for s in args.services: + service, alias = s.split("=") + _observed_services[service] = alias + console_handler = logging.StreamHandler() if args.debug: _logger.setLevel(logging.DEBUG) From 59e78a182ceaeb35eb6201e2ae768a3a9115c8ce Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Thu, 18 Jul 2024 13:44:55 -0400 Subject: [PATCH 11/15] tests: update tests to not use watch.yaml - Reverted SystemdNotices in test charm back to the original constructor API. Signed-off-by: Jason C. Nucciarone --- .../notices-charm/src/charm.py | 3 +- tests/unit/test_juju_systemd_notices.py | 62 +++++++++---------- 2 files changed, 29 insertions(+), 36 deletions(-) diff --git a/tests/integration/juju_systemd_notices/notices-charm/src/charm.py b/tests/integration/juju_systemd_notices/notices-charm/src/charm.py index 26c15ae6..56426d22 100755 --- a/tests/integration/juju_systemd_notices/notices-charm/src/charm.py +++ b/tests/integration/juju_systemd_notices/notices-charm/src/charm.py @@ -12,7 +12,6 @@ import charms.operator_libs_linux.v1.systemd as systemd import daemon from charms.operator_libs_linux.v0.juju_systemd_notices import ( - Service, ServiceStartedEvent, ServiceStoppedEvent, SystemdNotices, @@ -30,7 +29,7 @@ class NoticesCharm(CharmBase): def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) - self._systemd_notices = SystemdNotices(self, Service("test")) + self._systemd_notices = SystemdNotices(self, ["test"]) event_handler_bindings = { self.on.install: self._on_install, self.on.start: self._on_start, diff --git a/tests/unit/test_juju_systemd_notices.py b/tests/unit/test_juju_systemd_notices.py index 4ce2dfb1..074d9074 100644 --- a/tests/unit/test_juju_systemd_notices.py +++ b/tests/unit/test_juju_systemd_notices.py @@ -7,7 +7,6 @@ import argparse import subprocess import unittest -from pathlib import Path from unittest.mock import AsyncMock, patch from charms.operator_libs_linux.v0.juju_systemd_notices import ( @@ -47,7 +46,7 @@ def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) self.notices = SystemdNotices( - self, Service("foobar"), Service("snap.test.service", alias="service") + self, ["foobar", Service("snap.test.service", alias="service")] ) event_handler_bindings = { self.on.install: self._on_install, @@ -96,45 +95,33 @@ def test_generate_hooks(self, mock_exists, _) -> None: self.harness.charm.notices._generate_hooks() @patch("pathlib.Path.exists") - def test_generate_config(self, mock_exists) -> None: + def test_generate_service(self, mock_exists) -> None: """Test that watch configuration file is generated correctly.""" with Patcher() as patcher: - # Set charm_dir to "/" because mocked charm_dir default - # `no-disk-path` does not exist within the fake filesystem. - # Tried creating `no-disk-path` in the fake filesystem, but - # the mocked objects could not find it. - self.harness.charm.framework.charm_dir = Path("/") - patcher.fs.create_file("watch.yaml") + name = self.harness.charm.unit.name.replace("/", "-") + patcher.fs.create_file(f"/etc/systemd/system/juju-{name}-systemd-notices.service") # Scenario 1 - Generate success but no pre-existing watch configuration. mock_exists.return_value = False - self.harness.charm.notices._generate_config() + self.harness.charm.notices._generate_service() # Scenario 2 - Generate success but pre-existing watch configuration. mock_exists.return_value = True - self.harness.charm.notices._generate_config() + self.harness.charm.notices._generate_service() - @patch("pathlib.Path.write_text") - @patch("pathlib.Path.symlink_to") @patch("subprocess.check_output") - @patch("pathlib.Path.exists") - def test_start(self, mock_exists, mock_subp, *_) -> None: + def test_start(self, mock_subp) -> None: """Test that start method correctly starts notices daemon.""" - # Scenario 1 - Subscribe success but no pre-existing service file. - mock_exists.return_value = False + # Scenario 1 - systemctl successfully starts notices daemon. self.harness.charm.notices._start() - # Scenario 2 - Subscribe success and pre-existing service file. - mock_exists.return_value = True - self.harness.charm.notices._start() - - # Scenario 3 - Subscribe success but systemctl fails to start notices daemon. + # Scenario 2 - systemctl fails to start notices daemon. mock_subp.side_effect = subprocess.CalledProcessError(1, "systemctl start foobar") with self.assertRaises(subprocess.CalledProcessError): self.harness.charm.notices._start() @patch("charms.operator_libs_linux.v0.juju_systemd_notices.SystemdNotices._generate_hooks") - @patch("charms.operator_libs_linux.v0.juju_systemd_notices.SystemdNotices._generate_config") + @patch("charms.operator_libs_linux.v0.juju_systemd_notices.SystemdNotices._generate_service") @patch("charms.operator_libs_linux.v0.juju_systemd_notices.SystemdNotices._start") def test_subscribe(self, *_) -> None: """Test `subscribe` method.""" @@ -221,7 +208,7 @@ async def test_systemd_unit_changed(self, mock_message, mock_variant, *_) -> Non self.assertTrue(_systemd_unit_changed(mock_message)) @patch( - "charms.operator_libs_linux.v0.juju_systemd_notices._read_config", + "charms.operator_libs_linux.v0.juju_systemd_notices._observed_services", return_value={"foobar": "foobar"}, ) @patch("charms.operator_libs_linux.v0.juju_systemd_notices._juju_unit", "foobar/0") @@ -259,35 +246,42 @@ async def test_get_service_state(self) -> None: state = await _get_service_state(mock_sysbus, "foobar") self.assertEqual(state, "unknown") + @patch( + "charms.operator_libs_linux.v0.juju_systemd_notices._observed_services", + {"foobar": "foobar", "snap.test.service": "service"}, + ) @patch("charms.operator_libs_linux.v0.juju_systemd_notices._get_service_state") - @patch("charms.operator_libs_linux.v0.juju_systemd_notices._read_config") - async def test_async_load_services(self, mock_config, mock_state) -> None: - mock_config.return_value = {} + async def test_async_load_services(self, mock_state) -> None: + mock_state.return_value = "active" await _async_load_services() - mock_config.return_value = {"foobar": "foobar"} - mock_state.return_value = "active" + # Somehow calling `_async_load_services()` fully + # covers the for-loop without any additional changes to + # the `_observed_services` global ¯\_(ツ)_/¯ + mock_state.return_value = "stopped" await _async_load_services() @patch("pathlib.Path.exists", return_value=False) @patch("asyncio.Event.wait") async def test_juju_systemd_notices_daemon(self, *_) -> None: # Desired outcome is that _juju_systemd_notices does not fail to start. - with Patcher() as patcher: - patcher.fs.create_file("watch.yaml", contents=mock_watch_config) - await _juju_systemd_notices_daemon() + await _juju_systemd_notices_daemon() @patch("charms.operator_libs_linux.v0.juju_systemd_notices._juju_systemd_notices_daemon") @patch("argparse.ArgumentParser.parse_args") def test_main(self, mocked_args, *_) -> None: # Scenario 1 - Desired outcome (juju-systemd-notices daemon starts successfully) # and debug is set to True. - mocked_args.return_value = argparse.Namespace(debug=True, unit="foobar/0") + mocked_args.return_value = argparse.Namespace( + debug=True, unit="foobar/0", services=["foobar=foobar", "snap.test.service=service"] + ) _main() # Scenario 2 - Desired outcome (juju-systemd-notices daemon starts successfully) # and debug is set to False - mocked_args.return_value = argparse.Namespace(debug=False, unit="foobar/0") + mocked_args.return_value = argparse.Namespace( + debug=False, unit="foobar/0", services=["foobar=foobar", "snap.test.service=service"] + ) _main() # Scenario 3 - Debug flag is passed to script but no unit name. From a39406d70569a862cfcb14dddaceec0e31c5cfac Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Thu, 18 Jul 2024 14:06:11 -0400 Subject: [PATCH 12/15] fix(tests): remove pyfakefs as test dependency - Fixes unit tests failing on Python 3.10 due to incorrect patching. Signed-off-by: Jason C. Nucciarone --- tests/unit/test_juju_systemd_notices.py | 27 ++++++++----------------- 1 file changed, 8 insertions(+), 19 deletions(-) diff --git a/tests/unit/test_juju_systemd_notices.py b/tests/unit/test_juju_systemd_notices.py index 074d9074..2e252c9d 100644 --- a/tests/unit/test_juju_systemd_notices.py +++ b/tests/unit/test_juju_systemd_notices.py @@ -29,14 +29,6 @@ from ops.charm import CharmBase, InstallEvent, StopEvent from ops.model import ActiveStatus, BlockedStatus from ops.testing import Harness -from pyfakefs.fake_filesystem_unittest import Patcher - -mock_watch_config = """ ---- -services: - foobar: foobar - snap.test.service: service -""" class MockNoticesCharm(CharmBase): @@ -94,20 +86,17 @@ def test_generate_hooks(self, mock_exists, _) -> None: mock_exists.return_value = True self.harness.charm.notices._generate_hooks() + @patch("pathlib.Path.write_text") @patch("pathlib.Path.exists") - def test_generate_service(self, mock_exists) -> None: + def test_generate_service(self, mock_exists, _) -> None: """Test that watch configuration file is generated correctly.""" - with Patcher() as patcher: - name = self.harness.charm.unit.name.replace("/", "-") - patcher.fs.create_file(f"/etc/systemd/system/juju-{name}-systemd-notices.service") - - # Scenario 1 - Generate success but no pre-existing watch configuration. - mock_exists.return_value = False - self.harness.charm.notices._generate_service() + # Scenario 1 - Generate success but no pre-existing watch configuration. + mock_exists.return_value = False + self.harness.charm.notices._generate_service() - # Scenario 2 - Generate success but pre-existing watch configuration. - mock_exists.return_value = True - self.harness.charm.notices._generate_service() + # Scenario 2 - Generate success but pre-existing watch configuration. + mock_exists.return_value = True + self.harness.charm.notices._generate_service() @patch("subprocess.check_output") def test_start(self, mock_subp) -> None: From e3ab7193bffcdc5b04efb6fcd37f4b6e960ea7fb Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Fri, 19 Jul 2024 18:06:36 -0400 Subject: [PATCH 13/15] refactor: remove explicit global references Signed-off-by: Jason C. Nucciarone --- lib/charms/operator_libs_linux/v0/juju_systemd_notices.py | 4 ---- 1 file changed, 4 deletions(-) diff --git a/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py b/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py index b90ba9e3..6c109e8f 100644 --- a/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py +++ b/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py @@ -316,7 +316,6 @@ def _systemd_unit_changed(msg: Message) -> bool: if "ActiveState" not in properties: return False - global _service_states if service not in _service_states: _logger.debug("Dropping event for unwatched service: %s", service) return False @@ -346,7 +345,6 @@ async def _send_juju_notification(service: str, state: str) -> None: if service.endswith(".service"): service = service[0:-len(".service")] # fmt: skip - global _observed_services alias = _observed_services[service] event_name = "started" if state == "active" else "stopped" hook = f"service-{alias}-{event_name}" @@ -405,7 +403,6 @@ async def _async_load_services() -> None: # Loop through all the services and be sure that a new watcher is # started for new ones. - global _observed_services _logger.info("Services to observe are %s", _observed_services) for service in _observed_services: # The .service suffix is necessary and will cause lookup failures of the @@ -491,7 +488,6 @@ def _main(): global _juju_unit _juju_unit = args.unit - global _observed_services for s in args.services: service, alias = s.split("=") _observed_services[service] = alias From cd8292e505e6e41ac8b854a00e7ca7f2b8e47d67 Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Fri, 19 Jul 2024 18:09:18 -0400 Subject: [PATCH 14/15] docs: update example to use correct constructor arguments Signed-off-by: Jason C. Nucciarone --- lib/charms/operator_libs_linux/v0/juju_systemd_notices.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py b/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py index 6c109e8f..71926734 100644 --- a/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py +++ b/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py @@ -42,7 +42,7 @@ def __init__(self, *args, **kwargs) -> None: super().__init__(*args, **kwargs) # Register services with charm. This adds the events to observe. - self._systemd_notices = SystemdNotices(self, Service("snap.slurm.slurmd", alias="slurmd")) + self._systemd_notices = SystemdNotices(self, [Service("snap.slurm.slurmd", alias="slurmd")]) self.framework.observe(self.on.install, self._on_install) self.framework.observe(self.on.stop, self._on_stop) self.framework.observe(self.on.service_slurmd_started, self._on_slurmd_started) From 60b588649b372f32ef236755c45dbcfc64563828 Mon Sep 17 00:00:00 2001 From: "Jason C. Nucciarone" Date: Fri, 19 Jul 2024 18:10:03 -0400 Subject: [PATCH 15/15] chore: bump libpatch version Signed-off-by: Jason C. Nucciarone --- lib/charms/operator_libs_linux/v0/juju_systemd_notices.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py b/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py index 71926734..6a12463a 100644 --- a/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py +++ b/lib/charms/operator_libs_linux/v0/juju_systemd_notices.py @@ -108,7 +108,7 @@ def _on_slurmd_stopped(self, _: ServiceStoppedEvent) -> None: # Increment this PATCH version before using `charmcraft publish-lib` or reset # to 0 if you are raising the major API version. -LIBPATCH = 1 +LIBPATCH = 2 # juju-systemd-notices charm library dependencies. # Charm library dependencies are installed when the consuming charm is packed.