wrappers: do not reload activation units (canonical#14724)

* wrappers: do not reload activation units * t/m/services-socket-activation: replace usage of curl and be less demanding of the MATCH
snapcore · Nov 19, 2024 · 5d64090 · 5d64090
1 parent eb3064d
commit 5d64090
Show file tree

Hide file tree

Showing 3 changed files with 75 additions and 36 deletions.
diff --git 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
@@ -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
@@ -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) {