diff --git a/src/reactive/containerd.py b/src/reactive/containerd.py index a280064..a1b4deb 100644 --- a/src/reactive/containerd.py +++ b/src/reactive/containerd.py @@ -52,6 +52,7 @@ from charmhelpers.fetch.ubuntu_apt_pkg import Package NVIDIA_SOURCES_FILE = "/etc/apt/sources.list.d/nvidia.list" +NEEDRESTART_SUSPEND = "NEEDRESTART_SUSPEND" def apt_packages(packages: typing.Set[str]) -> typing.Mapping[str, Package]: @@ -468,7 +469,10 @@ def upgrade_charm(): if is_state("containerd.resource.installed"): # if a resource is currently overriding the deb, # upgrade containerd from the apt packages - reinstall_containerd() + # to make sure we get latest systemd services, latest configs, + # and dependencies like runc if needed -- but don't restart into + # those services + reinstall_containerd(restart=False) # Re-render config in case the template has changed in the new charm. config_changed() @@ -518,11 +522,34 @@ def install_containerd(): config_changed() -def reinstall_containerd(): +@contextlib.contextmanager +def _apt_restart_services(restart: bool): + """ + Context manager to conditionally restart services after apt operations. + + :param bool restart: whether to restart services after apt operations + """ + env = NEEDRESTART_SUSPEND + original = os.environ.pop(env, None) + log(f"Services will {'' if restart else 'not '}be restarted after apt operations.") + if not restart: + os.environ.update({env: "1"}) + + try: + yield + finally: + if original is None: + os.environ.pop(env, None) + else: + os.environ[env] = original + + +def reinstall_containerd(restart: bool = True): """Install and hold containerd with apt.""" apt_update(fatal=True) apt_unhold(CONTAINERD_PACKAGE) - apt_install([CONTAINERD_PACKAGE, "--reinstall"], fatal=True) + with _apt_restart_services(restart): + apt_install([CONTAINERD_PACKAGE, "--reinstall"], fatal=True) apt_hold(CONTAINERD_PACKAGE) set_state("containerd.installed") remove_state("containerd.resource.evaluated") diff --git a/tests/integration/test_containerd_integration.py b/tests/integration/test_containerd_integration.py index b6bd871..b43e985 100644 --- a/tests/integration/test_containerd_integration.py +++ b/tests/integration/test_containerd_integration.py @@ -14,7 +14,6 @@ from utils import JujuRun import yaml - log = logging.getLogger(__name__) diff --git a/tests/unit/conftest.py b/tests/unit/conftest.py index 6d913a3..c34575e 100644 --- a/tests/unit/conftest.py +++ b/tests/unit/conftest.py @@ -1,5 +1,4 @@ import charms.unit_test - charms.unit_test.patch_reactive() charms.unit_test.patch_module("requests") diff --git a/tests/unit/test_containerd_reactive.py b/tests/unit/test_containerd_reactive.py index 1af13ae..481946d 100644 --- a/tests/unit/test_containerd_reactive.py +++ b/tests/unit/test_containerd_reactive.py @@ -407,3 +407,44 @@ def test_needs_gpu_reboot_true(check_output, is_state, remove_state, set_state): check_output.assert_called_once_with(["nvidia-smi"], stderr=STDOUT) set_state.assert_called_once_with("containerd.nvidia.needs_reboot") remove_state.assert_not_called() + + +@pytest.mark.parametrize( + "restart,initial_env,expected_during,expected_after", + [ + (True, {}, None, None), + (False, {}, "1", None), + (True, {containerd.NEEDRESTART_SUSPEND: "original"}, None, "original"), + (False, {containerd.NEEDRESTART_SUSPEND: "original"}, "1", "original"), + ], + ids=[ + "restart=True, no initial env", + "restart=False, no initial env", + "restart=True, with initial env", + "restart=False, with initial env", + ], +) +@mock.patch.object(containerd, "log") +@mock.patch.dict(os.environ, {}, clear=True) +def test_apt_restart_services(mock_log, restart, initial_env, expected_during, expected_after): + """Verify _apt_restart_services behavior with various configurations.""" + # Setup initial environment from parameters + os.environ.update(initial_env) + env_var = containerd.NEEDRESTART_SUSPEND + + with containerd._apt_restart_services(restart=restart): + # Check value during context + if expected_during is None: + assert env_var not in os.environ + else: + assert os.environ[env_var] == expected_during + + # Check value after context + if expected_after is None: + assert env_var not in os.environ + else: + assert os.environ[env_var] == expected_after + + # Check log call + log_fmt = "" if restart else "not " + mock_log.assert_called_once_with(f"Services will {log_fmt}be restarted after apt operations.")