nixos/switch-to-configuration: Document and test handling of socket-activated services

nixos/doc/manual/development/unit-handling.section.md

@@ -41,17 +41,18 @@ checks:
     `RefuseManualStop` in the `[Unit]` section, and `X-OnlyManualStart` in the
     `[Unit]` section.
 
-  - The rest of the behavior is decided whether the unit has `X-StopIfChanged`
-    in the `[Service]` section set (exposed via
+  - Further behavior depends on the unit having `X-StopIfChanged` in the
+    `[Service]` section set to `true` (exposed via
     [systemd.services.\<name\>.stopIfChanged](#opt-systemd.services)). This is
     set to `true` by default and must be explicitly turned off if not wanted.
     If the flag is enabled, the unit is **stop**ped and then **start**ed. If
     not, the unit is **restart**ed. The goal of the flag is to make sure that
     the new unit never runs in the old environment which is still in place
-    before the activation script is run.
+    before the activation script is run. This behavior is different when the
+    service is socket-activated, as outlined in the following steps.
 
   - The last thing that is taken into account is whether the unit is a service
-    and socket-activated. Due to a bug, this is currently only done when
-    `X-StopIfChanged` is set. If the unit is socket-activated, the socket is
-    stopped and started, and the service is stopped and to be started by socket
-    activation.
+    and socket-activated. If `X-StopIfChanged` is **not** set, the service
+    is **restart**ed with the others. If it is set, both the service and the
+    socket are **stop**ped and the socket is **start**ed, leaving socket
+    activation to start the service when it's needed.

nixos/doc/manual/from_md/development/unit-handling.section.xml

@@ -88,9 +88,10 @@
         </listitem>
         <listitem>
           <para>
-            The rest of the behavior is decided whether the unit has
+            Further behavior depends on the unit having
             <literal>X-StopIfChanged</literal> in the
-            <literal>[Service]</literal> section set (exposed via
+            <literal>[Service]</literal> section set to
+            <literal>true</literal> (exposed via
             <link linkend="opt-systemd.services">systemd.services.&lt;name&gt;.stopIfChanged</link>).
             This is set to <literal>true</literal> by default and must
             be explicitly turned off if not wanted. If the flag is
@@ -100,17 +101,22 @@
             is <emphasis role="strong">restart</emphasis>ed. The goal of
             the flag is to make sure that the new unit never runs in the
             old environment which is still in place before the
-            activation script is run.
+            activation script is run. This behavior is different when
+            the service is socket-activated, as outlined in the
+            following steps.
           </para>
         </listitem>
         <listitem>
           <para>
             The last thing that is taken into account is whether the
-            unit is a service and socket-activated. Due to a bug, this
-            is currently only done when
-            <literal>X-StopIfChanged</literal> is set. If the unit is
-            socket-activated, the socket is stopped and started, and the
-            service is stopped and to be started by socket activation.
+            unit is a service and socket-activated. If
+            <literal>X-StopIfChanged</literal> is
+            <emphasis role="strong">not</emphasis> set, the service is
+            <emphasis role="strong">restart</emphasis>ed with the
+            others. If it is set, both the service and the socket are
+            <emphasis role="strong">stop</emphasis>ped and the socket is
+            <emphasis role="strong">start</emphasis>ed, leaving socket
+            activation to start the service when it’s needed.
           </para>
         </listitem>
       </itemizedlist>

nixos/modules/system/activation/switch-to-configuration.pl

@@ -307,6 +307,7 @@ sub handleModifiedUnit {
         # seem to get applied on daemon-reload.
     } elsif ($unit =~ /\.mount$/) {
         # Reload the changed mount unit to force a remount.
+        # FIXME: only reload when Options= changed, restart otherwise
         $unitsToReload->{$unit} = 1;
         recordUnit($reloadListFile, $unit);
     } elsif ($unit =~ /\.socket$/) {
@@ -339,21 +340,23 @@ sub handleModifiedUnit {
                 # If this unit is socket-activated, then stop the
                 # socket unit(s) as well, and restart the
                 # socket(s) instead of the service.
-                my $socketActivated = 0;
+                my $socket_activated = 0;
                 if ($unit =~ /\.service$/) {
                     my @sockets = split(/ /, join(" ", @{$unitInfo{Service}{Sockets} // []}));
                     if (scalar @sockets == 0) {
                         @sockets = ("$baseName.socket");
                     }
                     foreach my $socket (@sockets) {
                         if (defined $activePrev->{$socket}) {
+                            # We can now be sure this is a socket-activate unit
+
                             $unitsToStop->{$socket} = 1;
                             # Only restart sockets that actually
                             # exist in new configuration:
                             if (-e "$out/etc/systemd/system/$socket") {
                                 $unitsToStart->{$socket} = 1;
                                 recordUnit($startListFile, $socket);
-                                $socketActivated = 1;
+                                $socket_activated = 1;
                             }
                             # Remove from units to reload so we don't restart and reload
                             if ($unitsToReload->{$unit}) {
@@ -368,7 +371,7 @@ sub handleModifiedUnit {
                 # that this unit needs to be started below.
                 # We write this to a file to ensure that the
                 # service gets restarted if we're interrupted.
-                if (!$socketActivated) {
+                if (!$socket_activated) {
                     $unitsToStart->{$unit} = 1;
                     recordUnit($startListFile, $unit);
                 }