From 5d640908371f7e647e280863176eaa039e454b84 Mon Sep 17 00:00:00 2001 From: Philip Meulengracht Date: Tue, 19 Nov 2024 14:16:57 +0100 Subject: [PATCH] wrappers: do not reload activation units (#14724) * wrappers: do not reload activation units * t/m/services-socket-activation: replace usage of curl and be less demanding of the MATCH --- .../main/services-socket-activation/task.yaml | 82 +++++++++++-------- wrappers/services.go | 5 +- wrappers/services_test.go | 24 ++++++ 3 files changed, 75 insertions(+), 36 deletions(-) diff --git a/tests/main/services-socket-activation/task.yaml b/tests/main/services-socket-activation/task.yaml index 8508be36e67..5440e5559de 100644 --- a/tests/main/services-socket-activation/task.yaml +++ b/tests/main/services-socket-activation/task.yaml @@ -13,56 +13,68 @@ restore: | execute: | [ -f /etc/systemd/system/snap.socket-activation.sleep-daemon.sock.socket ] [ -S /var/snap/socket-activation/common/socket ] + + verify_status() { + local ENABLED="$1" + local MAIN_ACTIVE="$2" + local ACT_ACTIVE="$3" + + echo "Checking that services are listed correctly" + snap services | cat -n > svcs.txt + MATCH " 1\s+Service\s+Startup\s+Current\s+Notes$" < svcs.txt + MATCH " 2\s+socket-activation.sleep-daemon\s+${ENABLED}\s+${MAIN_ACTIVE}\s+socket-activated$" < svcs.txt + + echo "Check that systemctl for the main unit is as expected" + systemctl show --property=UnitFileState snap.socket-activation.sleep-daemon.service | grep "static" + systemctl show --property=ActiveState snap.socket-activation.sleep-daemon.service | grep "ActiveState=${MAIN_ACTIVE}" + + echo "Check that systemctl for the socket is looking correct too" + systemctl show --property=UnitFileState snap.socket-activation.sleep-daemon.sock.socket | grep "${ENABLED}" + systemctl show --property=ActiveState snap.socket-activation.sleep-daemon.sock.socket | grep "ActiveState=${ACT_ACTIVE}" + } + + # verify default behavior on install is that the main service + # is inactive but enabled, and socket is active + verify_status "enabled" "inactive" "active" + + # this will fail, but still start the service + echo "Start the primary unit, emulate that the trigger has run" + systemctl start snap.socket-activation.sleep-daemon.service + + # verify that the main service is now active + verify_status "enabled" "active" "active" + + # test normal restart + snap restart socket-activation - echo "Checking that services are listed correctly" - snap services | cat -n > svcs.txt - MATCH " 1\s+Service\s+Startup\s+Current\s+Notes$" < svcs.txt - MATCH " 2\s+socket-activation.sleep-daemon\s+enabled\s+inactive\s+socket-activated$" < svcs.txt + verify_status "enabled" "active" "active" - echo "Checking that the service is reported as static" - systemctl show --property=UnitFileState snap.socket-activation.sleep-daemon.service | grep "static" + # test --reload restart, with --reload we expect different behavior + # because of systemd. Verify that systemd is acting like we expect + # as well + snap restart --reload socket-activation - echo "Checking that service activation unit is reported as enabled and running" - systemctl show --property=UnitFileState snap.socket-activation.sleep-daemon.sock.socket | grep "enabled" - systemctl show --property=ActiveState snap.socket-activation.sleep-daemon.sock.socket | grep "ActiveState=active" + verify_status "enabled" "active" "active" + + systemctl reload-or-restart snap.socket-activation.sleep-daemon.sock.socket 2>&1 | MATCH "failed" echo "Testing that we can stop will not disable the service" snap stop socket-activation.sleep-daemon - systemctl show --property=UnitFileState snap.socket-activation.sleep-daemon.sock.socket | grep "enabled" - systemctl show --property=ActiveState snap.socket-activation.sleep-daemon.sock.socket | grep "ActiveState=inactive" + + verify_status "enabled" "inactive" "inactive" echo "Testing that we can correctly disable activations" snap stop --disable socket-activation.sleep-daemon echo "Verifying that service is now listed as disabled" - snap services | cat -n > svcs.txt - MATCH " 1\s+Service\s+Startup\s+Current\s+Notes$" < svcs.txt - MATCH " 2\s+socket-activation.sleep-daemon\s+disabled\s+inactive\s+socket-activated$" < svcs.txt - - echo "Checking that service activation unit is reported as disabled and inactive" - systemctl show --property=UnitFileState snap.socket-activation.sleep-daemon.sock.socket | grep "disabled" - systemctl show --property=ActiveState snap.socket-activation.sleep-daemon.sock.socket | grep "ActiveState=inactive" + verify_status "disabled" "inactive" "inactive" echo "Starting the service will start the socket unit, but not enable" snap start socket-activation.sleep-daemon - - echo "Checking that services are listed as expected" - snap services | cat -n > svcs.txt - MATCH " 1\s+Service\s+Startup\s+Current\s+Notes$" < svcs.txt - MATCH " 2\s+socket-activation.sleep-daemon\s+disabled\s+inactive\s+socket-activated$" < svcs.txt - - echo "Checking that service activation unit is reported as disabled and active" - systemctl show --property=UnitFileState snap.socket-activation.sleep-daemon.sock.socket | grep "disabled" - systemctl show --property=ActiveState snap.socket-activation.sleep-daemon.sock.socket | grep "ActiveState=active" + + verify_status "disabled" "inactive" "active" echo "Enable service and verify its listed as enabled" snap start --enable socket-activation.sleep-daemon - echo "Checking that services are listed correctly" - snap services | cat -n > svcs.txt - MATCH " 1\s+Service\s+Startup\s+Current\s+Notes$" < svcs.txt - MATCH " 2\s+socket-activation.sleep-daemon\s+enabled\s+inactive\s+socket-activated$" < svcs.txt - - echo "Checking that service activation unit is reported as enabled and active again" - systemctl show --property=UnitFileState snap.socket-activation.sleep-daemon.sock.socket | grep "enabled" - systemctl show --property=ActiveState snap.socket-activation.sleep-daemon.sock.socket | grep "ActiveState=active" + verify_status "enabled" "inactive" "active" diff --git a/wrappers/services.go b/wrappers/services.go index 8db7f1a1a67..0d526478f69 100644 --- a/wrappers/services.go +++ b/wrappers/services.go @@ -1231,7 +1231,10 @@ func restartServicesByStatus(svcsSts []*internal.ServiceStatus, explicitServices var unitsToRestart []string // If the service is activated, then we must also consider it's activators - if len(st.ActivatorUnitStatuses()) != 0 { + // as long as we are not requesting a reload. For activated units reload + // is not a supported action. In that case treat it like a non-activated + // service. + if len(st.ActivatorUnitStatuses()) != 0 && !opts.Reload { // Restart any activators first and operate normally on these for _, act := range st.ActivatorUnitStatuses() { // Use the primary name here for shouldRestart, as the caller diff --git a/wrappers/services_test.go b/wrappers/services_test.go index 8f78ecde167..ca5c1eb8bed 100644 --- a/wrappers/services_test.go +++ b/wrappers/services_test.go @@ -5352,6 +5352,19 @@ apps: {"show", "--property=ActiveState", srvFile2Sock2}, {"start", srvFile2Sock1, srvFile2Sock2}, }) + + // Restart but also reload. When reloading we expect to see different behaviour. + // Reloading activated units is not supported by systemd, and for that reason they must + // not appear in the list of systemctl calls. + // The only call we expect to appear here is the primary service of svc1 as + // that one is the only one reported as active here. + s.sysdLog = nil + c.Assert(wrappers.RestartServices(services, nil, &wrappers.RestartServicesOptions{Reload: true}, progress.Null, s.perfTimings), IsNil) + c.Check(s.sysdLog, DeepEquals, [][]string{ + {"show", "--property=Id,ActiveState,UnitFileState,Type,Names,NeedDaemonReload", srvFile1, srvFile2}, + {"show", "--property=Id,ActiveState,UnitFileState,Names", srvFile2Sock1, srvFile2Sock2}, + {"reload-or-restart", srvFile1}, + }) } func (s *servicesTestSuite) TestRestartWithActivatedServicesActive(c *C) { @@ -5428,6 +5441,17 @@ apps: {"show", "--property=ActiveState", srvFile2Sock2}, {"start", srvFile2Sock1, srvFile2Sock2}, }) + + // Restart but also reload. We do not expect any services to be restarted here as + // both the primary units are reported inactive. (Only the activation units are + // reported active). The reason we don't expect to see the activation units restarted + // when reloading, is that these units do not support reloading by systemd. + s.sysdLog = nil + c.Assert(wrappers.RestartServices(services, nil, &wrappers.RestartServicesOptions{Reload: true}, progress.Null, s.perfTimings), IsNil) + c.Check(s.sysdLog, DeepEquals, [][]string{ + {"show", "--property=Id,ActiveState,UnitFileState,Type,Names,NeedDaemonReload", srvFile1, srvFile2}, + {"show", "--property=Id,ActiveState,UnitFileState,Names", srvFile2Sock1, srvFile2Sock2}, + }) } func (s *servicesTestSuite) TestRestartWithActivatedServicesActivePrimaryUnit(c *C) {