Skip to content

Commit 0aabe7c

Browse files
committed
When containerd is installed by resource, don't restart service on apt install
1 parent ba8d8dc commit 0aabe7c

File tree

4 files changed

+65
-5
lines changed

4 files changed

+65
-5
lines changed

src/reactive/containerd.py

Lines changed: 23 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -468,7 +468,10 @@ def upgrade_charm():
468468
if is_state("containerd.resource.installed"):
469469
# if a resource is currently overriding the deb,
470470
# upgrade containerd from the apt packages
471-
reinstall_containerd()
471+
# to make sure we get latest systemd services, latest configs,
472+
# and dependencies like runc if needed -- but don't restart into
473+
# those services
474+
reinstall_containerd(restart=False)
472475

473476
# Re-render config in case the template has changed in the new charm.
474477
config_changed()
@@ -518,11 +521,28 @@ def install_containerd():
518521
config_changed()
519522

520523

521-
def reinstall_containerd():
524+
@contextlib.contextmanager
525+
def _apt_restart_services(restart: bool):
526+
"""
527+
Context manager to conditionally restart services after apt operations.
528+
529+
:param restart: whether to restart services after apt operations
530+
"""
531+
restore = os.environ.pop("NEEDRESTART_SUSPEND", None)
532+
if not restart:
533+
log("Services will be not restarted after apt operations.")
534+
os.environ.update({"NEEDRESTART_SUSPEND": "1"})
535+
yield
536+
if restore is not None:
537+
os.environ["NEEDRESTART_SUSPEND"] = restore
538+
539+
540+
def reinstall_containerd(restart: bool = True):
522541
"""Install and hold containerd with apt."""
523542
apt_update(fatal=True)
524543
apt_unhold(CONTAINERD_PACKAGE)
525-
apt_install([CONTAINERD_PACKAGE, "--reinstall"], fatal=True)
544+
with _apt_restart_services(restart):
545+
apt_install([CONTAINERD_PACKAGE, "--reinstall"], fatal=True)
526546
apt_hold(CONTAINERD_PACKAGE)
527547
set_state("containerd.installed")
528548
remove_state("containerd.resource.evaluated")

tests/integration/test_containerd_integration.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@
1414
from utils import JujuRun
1515
import yaml
1616

17-
1817
log = logging.getLogger(__name__)
1918

2019

tests/unit/conftest.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,4 @@
11
import charms.unit_test
22

3-
43
charms.unit_test.patch_reactive()
54
charms.unit_test.patch_module("requests")

tests/unit/test_containerd_reactive.py

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -407,3 +407,45 @@ def test_needs_gpu_reboot_true(check_output, is_state, remove_state, set_state):
407407
check_output.assert_called_once_with(["nvidia-smi"], stderr=STDOUT)
408408
set_state.assert_called_once_with("containerd.nvidia.needs_reboot")
409409
remove_state.assert_not_called()
410+
411+
412+
@pytest.mark.parametrize(
413+
"restart,initial_env,expected_during,expected_after,log_called",
414+
[
415+
(True, {}, None, None, False),
416+
(False, {}, "1", "1", True),
417+
(True, {"NEEDRESTART_SUSPEND": "original_value"}, None, "original_value", False),
418+
(False, {"NEEDRESTART_SUSPEND": "original_value"}, "1", "original_value", True),
419+
],
420+
ids=[
421+
"restart=True, no initial env",
422+
"restart=False, no initial env",
423+
"restart=True, with initial env",
424+
"restart=False, with initial env",
425+
],
426+
)
427+
@mock.patch.object(containerd, "log")
428+
@mock.patch.dict(os.environ, {}, clear=True)
429+
def test_apt_restart_services(mock_log, restart, initial_env, expected_during, expected_after, log_called):
430+
"""Verify _apt_restart_services behavior with various configurations."""
431+
# Setup initial environment from parameters
432+
os.environ.update(initial_env)
433+
434+
with containerd._apt_restart_services(restart=restart):
435+
# Check value during context
436+
if expected_during is None:
437+
assert "NEEDRESTART_SUSPEND" not in os.environ
438+
else:
439+
assert os.environ["NEEDRESTART_SUSPEND"] == expected_during
440+
441+
# Check value after context
442+
if expected_after is None:
443+
assert "NEEDRESTART_SUSPEND" not in os.environ
444+
else:
445+
assert os.environ["NEEDRESTART_SUSPEND"] == expected_after
446+
447+
# Check log call
448+
if log_called:
449+
mock_log.assert_called_once_with("Services will be not restarted after apt operations.")
450+
else:
451+
mock_log.assert_not_called()

0 commit comments

Comments
 (0)