Use generator to enable ostree-remount.service and ostree-finalize-staged.path

We struggled for a long time with enablement of our "internal units",
trying to follow the philosophy that units should only be enabled
by explicit preset.

See https://bugzilla.redhat.com/show_bug.cgi?id=1451458
and https://github.com/coreos/rpm-ostree/pull/1482
etc.

And I just saw chat (RH internal on a proprietary system sadly) where
someone hit `ostree-remount.service` not being enabled in CentOS8.

Thinking about this more, I realized we've shipped a systemd generator
for a long time and while its only role until now was to generate `var.mount`,
but by using it to force on our internal units, we don't require
people to deal with presets anymore.

Basically we're inverting things so that "if ostree= is on the kernel
cmdline, then enable our units" and not "enable our units, but have
them use ConditionKernelCmdline=ostree to skip".

Drop the weird gyrations we were doing around `ostree-finalize-staged.path`
too; forking `systemctl start` is just asking for bugs.

So after this, hopefully we won't ever again have to think about
distribution presets and our units.

configure.ac

@@ -499,6 +499,7 @@ AS_IF([test "x$with_libsystemd" = "xyes"], [
               [with_systemdsystemunitdir=$($PKG_CONFIG --variable=systemdsystemunitdir systemd)])
   AS_IF([test "x$with_systemdsystemunitdir" != "xno"], [
     AC_SUBST([systemdsystemunitdir], [$with_systemdsystemunitdir])
+    AC_DEFINE_UNQUOTED([SYSTEM_DATA_UNIT_PATH], ["$with_systemdsystemunitdir"], ["unit path"])
   ])
   AC_ARG_WITH([systemdsystemgeneratordir],
               AS_HELP_STRING([--with-systemdsystemgeneratordir=DIR], [Directory for systemd generators]),

src/libostree/ostree-impl-system-generator.c

@@ -113,9 +113,35 @@ stateroot_from_ostree_cmdline (const char *ostree_cmdline,
 }
 #endif
 
-/* Implementation of ostree-system-generator */
+/* Forcibly enable our internal units, since we detected ostree= on the kernel cmdline */
-gboolean
+static gboolean
-_ostree_impl_system_generator (const char *ostree_cmdline,
+require_internal_units (const char *normal_dir,
+                        const char *early_dir,
+                        const char *late_dir,
+                        GError    **error)
+{
+  GCancellable *cancellable = NULL;
+
+  glnx_autofd int normal_dir_dfd = -1;
+  if (!glnx_opendirat (AT_FDCWD, normal_dir, TRUE, &normal_dir_dfd, error))
+    return FALSE;
+
+  if (!glnx_shutil_mkdir_p_at (normal_dir_dfd, "local-fs.target.requires", 0755, cancellable, error))
+    return FALSE;
+  if (symlinkat (SYSTEM_DATA_UNIT_PATH "/ostree-remount.service", normal_dir_dfd, "local-fs.target.requires/ostree-remount.service") < 0)
+    return glnx_throw_errno_prefix (error, "symlinkat");
+
+  if (!glnx_shutil_mkdir_p_at (normal_dir_dfd, "multi-user.target.wants", 0755, cancellable, error))
+    return FALSE;
+  if (symlinkat (SYSTEM_DATA_UNIT_PATH "/ostree-finalize-staged.path", normal_dir_dfd, "multi-user.target.wants/ostree-finalize-staged.path") < 0)
+    return glnx_throw_errno_prefix (error, "symlinkat");
+
+  return TRUE;
+}
+
+/* Generate var.mount */
+static gboolean
+fstab_generator (const char *ostree_cmdline,
                  const char *normal_dir,
                  const char *early_dir,
                  const char *late_dir,
@@ -225,3 +251,19 @@ _ostree_impl_system_generator (const char *ostree_cmdline,
   return glnx_throw (error, "Not implemented");
 #endif
 }
+
+/* Implementation of ostree-system-generator */
+gboolean
+_ostree_impl_system_generator (const char *ostree_cmdline,
+                               const char *normal_dir,
+                               const char *early_dir,
+                               const char *late_dir,
+                               GError    **error)
+{
+  if (!require_internal_units (normal_dir, early_dir, late_dir, error))
+    return FALSE;
+  if (!fstab_generator (ostree_cmdline, normal_dir, early_dir, late_dir, error))
+    return FALSE;
+
+  return TRUE;
+}

src/libostree/ostree-sysroot-deploy.c

@@ -3106,28 +3106,6 @@ ostree_sysroot_stage_tree_with_options (OstreeSysroot     *self,
   if (booted_deployment == NULL)
     return glnx_prefix_error (error, "Cannot stage deployment");
 
-  /* This is used by the testsuite to exercise the path unit, until it becomes the default
-   * (which is pending on the preset making it everywhere). */
-  if ((self->debug_flags & OSTREE_SYSROOT_DEBUG_TEST_STAGED_PATH) == 0)
-    {
-  /* This is a bit of a hack.  When adding a new service we have to end up getting
-   * into the presets for downstream distros; see e.g. https://src.fedoraproject.org/rpms/ostree/pull-request/7
-   *
-   * Then again, it's perhaps a bit nicer to only start the service on-demand anyways.
-   */
-  const char *const systemctl_argv[] = {"systemctl", "start", "ostree-finalize-staged.service", NULL};
-  int estatus;
-  if (!g_spawn_sync (NULL, (char**)systemctl_argv, NULL, G_SPAWN_SEARCH_PATH,
-                     NULL, NULL, NULL, NULL, &estatus, error))
-    return FALSE;
-  if (!g_spawn_check_exit_status (estatus, error))
-    return FALSE;
-    }
-  else
-    {
-      g_print ("test-staged-path: Not running `systemctl start`\n");
-    } /* OSTREE_SYSROOT_DEBUG_TEST_STAGED_PATH */
-
   g_autoptr(OstreeDeployment) deployment = NULL;
   if (!sysroot_initialize_deployment (self, osname, revision, origin, opts, &deployment,
                                       cancellable, error))

src/libostree/ostree-sysroot-private.h

@@ -37,8 +37,7 @@ typedef enum {
   OSTREE_SYSROOT_DEBUG_TEST_FIFREEZE = 1 << 2,
   /* This is a temporary flag until we fully drop the explicit `systemctl start
    * ostree-finalize-staged.service` so that tests can exercise the new path unit. */
-  OSTREE_SYSROOT_DEBUG_TEST_STAGED_PATH = 1 << 3,
+  OSTREE_SYSROOT_DEBUG_TEST_NO_DTB = 1 << 3, /* https://github.com/ostreedev/ostree/issues/2154 */
-  OSTREE_SYSROOT_DEBUG_TEST_NO_DTB = 1 << 4, /* https://github.com/ostreedev/ostree/issues/2154 */
 } OstreeSysrootDebugFlags;
 
 typedef enum {

src/libostree/ostree-sysroot.c

@@ -190,7 +190,6 @@ ostree_sysroot_init (OstreeSysroot *self)
     { "mutable-deployments", OSTREE_SYSROOT_DEBUG_MUTABLE_DEPLOYMENTS },
     { "test-fifreeze", OSTREE_SYSROOT_DEBUG_TEST_FIFREEZE },
     { "no-xattrs", OSTREE_SYSROOT_DEBUG_NO_XATTRS },
-    { "test-staged-path", OSTREE_SYSROOT_DEBUG_TEST_STAGED_PATH },
     { "no-dtb", OSTREE_SYSROOT_DEBUG_TEST_NO_DTB },
   };
 

tests/kolainst/destructive/staged-deploy.sh

@@ -8,6 +8,10 @@ prepare_tmpdir
 
 case "${AUTOPKGTEST_REBOOT_MARK:-}" in
   "")
+  # Test our generator
+  test -f /run/systemd/generator/multi-user.target.wants/ostree-finalize-staged.path
+  test -f /run/systemd/generator/local-fs.target.requires/ostree-remount.service
+
   # Initial cleanup to handle the cosa fast-build case
     ## TODO remove workaround for https://github.com/coreos/rpm-ostree/pull/2021
     mkdir -p /var/lib/rpm-ostree/history
@@ -19,7 +23,6 @@ case "${AUTOPKGTEST_REBOOT_MARK:-}" in
   # Start up path unit
     systemctl enable --now ostree-finalize-staged.path
   # Write staged-deploy commit
-    export OSTREE_SYSROOT_DEBUG="test-staged-path"
     cd /ostree/repo/tmp
     # https://github.com/ostreedev/ostree/issues/1569
     ostree checkout -H ${commit} t
@@ -29,8 +32,7 @@ case "${AUTOPKGTEST_REBOOT_MARK:-}" in
     systemctl show -p SubState ostree-finalize-staged.path | grep -q waiting
     systemctl show -p ActiveState ostree-finalize-staged.service | grep -q inactive
     systemctl show -p TriggeredBy ostree-finalize-staged.service | grep -q path
-    ostree admin deploy --stage staged-deploy | tee out.txt
+    ostree admin deploy --stage staged-deploy
-    assert_file_has_content out.txt 'test-staged-path: Not running'
     systemctl show -p SubState ostree-finalize-staged.path | grep running
     systemctl show -p ActiveState ostree-finalize-staged.service | grep active
     new_mtime=$(stat -c '%.Y' /sysroot/ostree/deploy)