Merge pull request #4763 from keszybz/offline-update-loop

A fix for offline update loop

6 files changed, 79 insertions, 25 deletions

diff --git a/Makefile.am b/Makefile.am
index 5cec19fbb8..6c350b0ec4 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -529,6 +529,7 @@ nodist_systemunit_DATA = \
 	units/serial-getty@.service \
 	units/console-getty.service \
 	units/container-getty@.service \
+	units/system-update-cleanup.service \
 	units/systemd-initctl.service \
 	units/systemd-remount-fs.service \
 	units/systemd-ask-password-wall.service \
@@ -592,6 +593,7 @@ EXTRA_DIST += \
 	units/console-getty.service.m4.in \
 	units/container-getty@.service.m4.in \
 	units/rescue.service.in \
+	units/system-update-cleanup.service.in \
 	units/systemd-initctl.service.in \
 	units/systemd-remount-fs.service.in \
 	units/systemd-update-utmp.service.in \
diff --git a/man/systemd.offline-updates.xml b/man/systemd.offline-updates.xml
index 07a5225512..d673cf5db8 100644
--- a/man/systemd.offline-updates.xml
+++ b/man/systemd.offline-updates.xml
@@ -86,34 +86,44 @@
       </listitem>
 
       <listitem>
-        <para>The system now continues to boot into <filename>default.target</filename>, and thus
-        into <filename>system-update.target</filename>. This target pulls in the system update unit,
-        which starts the system update script after all file systems have been mounted.</para>
+        <para>The system now continues to boot into <filename>default.target</filename>, and
+        thus into <filename>system-update.target</filename>. This target pulls in all system
+        update units. Only one service should perform an update (see the next point), and all
+        the other ones should exit cleanly with a "success" return code and without doing
+        anything. Update services should be ordered after <filename>sysinit.target</filename>
+        so that the update starts after after all file systems have been mounted.</para>
       </listitem>
 
       <listitem>
-        <para>As the first step, the update script should check if the
+        <para>As the first step, an update service should check if the
         <filename>/system-update</filename> symlink points to the location used by that update
-        script. In case it does not exists or points to a different location, the script must exit
+        service. In case it does not exist or points to a different location, the service must exit
         without error. It is possible for multiple update services to be installed, and for multiple
-        update scripts to be launched in parallel, and only the one that corresponds to the tool
+        update services to be launched in parallel, and only the one that corresponds to the tool
         that <emphasis>created</emphasis> the symlink before reboot should perform any actions. It
         is unsafe to run multiple updates in parallel.</para>
       </listitem>
 
       <listitem>
-        <para>The update script should now do its job. If applicable and possible, it should
-        create a file system snapshot, then install all packages.
-        After completion (regardless whether the update succeeded or failed) the machine
-        must be rebooted, for example by calling <command>systemctl reboot</command>.
-        In addition, on failure the script should revert to the old file system snapshot
-        (without the symlink).</para>
+        <para>The update service should now do its job. If applicable and possible, it should
+        create a file system snapshot, then install all packages.  After completion (regardless
+        whether the update succeeded or failed) the machine must be rebooted, for example by
+        calling <command>systemctl reboot</command>. In addition, on failure the script should
+        revert to the old file system snapshot (without the symlink).</para>
       </listitem>
 
       <listitem>
-        <para>The system is rebooted. Since the <filename>/system-update</filename> symlink is gone,
-        the generator won't redirect <filename>default.target</filename> after reboot and the
-        system now boots into the default target again.</para>
+        <para>The upgrade scripts should exit only after the update is finished. It is expected
+        that the service which performs the upgrade will cause the machine to reboot after it
+        is done. If the <filename>system-update.target</filename> is successfully reached, i.e.
+        all update services have run, and the <filename>/system-update</filename> symlink still
+        exists, it will be removed and the machine rebooted as a safety measure.</para>
+      </listitem>
+
+      <listitem>
+        <para>After a reboot, now that the <filename>/system-update</filename> symlink is gone,
+        the generator won't redirect <filename>default.target</filename> anymore and the system
+        now boots into the default target again.</para>
       </listitem>
     </orderedlist>
   </refsect1>
@@ -150,7 +160,8 @@
 
       <listitem>
         <para>The update service should declare <varname>DefaultDependencies=false</varname>,
-        and pull in any services it requires explicitly.</para>
+        <varname>Requires=sysinit.target</varname>, <varname>After=sysinit.target</varname>,
+        and explicitly pull in any other services it requires.</para>
       </listitem>
     </orderedlist>
   </refsect1>
diff --git a/man/systemd.special.xml b/man/systemd.special.xml
index d977298cd8..b513a13b5a 100644
--- a/man/systemd.special.xml
+++ b/man/systemd.special.xml
@@ -102,6 +102,7 @@
     <filename>sysinit.target</filename>,
     <filename>syslog.socket</filename>,
     <filename>system-update.target</filename>,
+    <filename>system-update-cleanup.service</filename>,
     <filename>time-sync.target</filename>,
     <filename>timers.target</filename>,
     <filename>umount.target</filename>,
@@ -608,15 +609,21 @@
       </varlistentry>
       <varlistentry>
         <term><filename>system-update.target</filename></term>
+        <term><filename>system-update-cleanup.service</filename></term>
         <listitem>
-          <para>A special target unit that is used for off-line system
-          updates.
+          <para>A special target unit that is used for offline system updates.
           <citerefentry><refentrytitle>systemd-system-update-generator</refentrytitle><manvolnum>8</manvolnum></citerefentry>
-          will redirect the boot process to this target if
-          <filename>/system-update</filename> exists. For more
-          information see the <ulink
-          url="http://freedesktop.org/wiki/Software/systemd/SystemUpdates">System
-          Updates Specification</ulink>.</para>
+          will redirect the boot process to this target if <filename>/system-update</filename>
+          exists. For more information see
+          <citerefentry><refentrytitle>systemd.offline-updates</refentrytitle><manvolnum>7</manvolnum></citerefentry>.
+          </para>
+
+          <para>Updates should happen before the <filename>system-update.target</filename> is
+          reached, and the services which implement them should cause the machine to reboot. As
+          a safety measure, if this does not happen, and <filename>/system-update</filename>
+          still exists after <filename>system-update.target</filename> is reached,
+          <filename>system-update-cleanup.service</filename> will remove this symlink and
+          reboot the machine.</para>
         </listitem>
       </varlistentry>
       <varlistentry>
diff --git a/units/.gitignore b/units/.gitignore
index 8f4949258e..8fdb6e9ab5 100644
--- a/units/.gitignore
+++ b/units/.gitignore
@@ -16,6 +16,7 @@
 /rc-local.service
 /rescue.service
 /serial-getty@.service
+/system-update-cleanup.service
 /systemd-ask-password-console.service
 /systemd-ask-password-wall.service
 /systemd-backlight@.service
diff --git a/units/system-update-cleanup.service.in b/units/system-update-cleanup.service.in
new file mode 100644
index 0000000000..116be8bc2d
--- /dev/null
+++ b/units/system-update-cleanup.service.in
@@ -0,0 +1,32 @@
+#  This file is part of systemd.
+#
+#  systemd is free software; you can redistribute it and/or modify it
+#  under the terms of the GNU Lesser General Public License as published by
+#  the Free Software Foundation; either version 2.1 of the License, or
+#  (at your option) any later version.
+
+[Unit]
+Description=Remove the Offline System Updates symlink
+Documentation=man:systemd.special(5) man:systemd.offline-updates(7)
+After=system-update.target
+DefaultDependencies=no
+Conflicts=shutdown.target
+
+# system-update-generator uses laccess("/system-update"), while a plain
+# ConditionPathExists=/system-update uses access("/system-update"), so
+# we need an alternate condition to cover the case of a dangling symlink.
+#
+# This service is only invoked if /system-update exists, i.e. if the
+# condition tested by system-update-generator remains true and the system
+# would be diverted into system-update.target again after reboot. This way
+# we guard against being diverted into system-update.target again, which
+# works as a safety measure, but we will not step on the toes of the
+# update script if it successfully removed the symlink and scheduled a
+# reboot or some other action on its own.
+ConditionPathExists=|/system-update
+ConditionPathIsSymbolicLink=|/system-update
+
+[Service]
+Type=oneshot
+ExecStart=/bin/rm -fv /system-update
+ExecStart=@SYSTEMCTL@ reboot
diff --git a/units/system-update.target b/units/system-update.target
index 48d46fcbda..3542879706 100644
--- a/units/system-update.target
+++ b/units/system-update.target
@@ -6,11 +6,12 @@
 #  (at your option) any later version.
 
 [Unit]
-Description=System Update
-Documentation=http://freedesktop.org/wiki/Software/systemd/SystemUpdates
+Description=Offline System Update
+Documentation=man:systemd.offline-updates(7)
 Documentation=man:systemd.special(7) man:systemd-system-update-generator(8)
 Requires=sysinit.target
 Conflicts=shutdown.target
 After=sysinit.target
 Before=shutdown.target
 AllowIsolate=yes
+Wants=system-update-cleanup.service