You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
375 lines
16 KiB
375 lines
16 KiB
From a6d45dc165e48e2a463880ebb90f34c2b9d3c4ce Mon Sep 17 00:00:00 2001
|
|
From: Colin Walters <walters@verbum.org>
|
|
Date: Fri, 22 Apr 2022 18:46:28 -0400
|
|
Subject: [PATCH 1/6] Add an `ostree-boot-complete.service` to propagate
|
|
staging failures
|
|
|
|
Quite a while ago we added staged deployments, which solved
|
|
a bunch of issues around the `/etc` merge. However...a persistent
|
|
problem since then is that any failures in that process that
|
|
happened in the *previous* boot are not very visible.
|
|
|
|
We ship custom code in `rpm-ostree status` to query the previous
|
|
journal. But that has a few problems - one is that on systems
|
|
that have been up a while, that failure message may even get
|
|
rotated out. And second, some systems may not even have a persistent
|
|
journal at all.
|
|
|
|
A general thing we do in e.g. Fedora CoreOS testing is to check
|
|
for systemd unit failures. We do that both in our automated tests,
|
|
and we even ship code that displays them on ssh logins. And beyond
|
|
that obviously a lot of other projects do the same; it's easy via
|
|
`systemctl --failed`.
|
|
|
|
So to make failures more visible, change our `ostree-finalize-staged.service`
|
|
to have an internal wrapper around the process that "catches" any
|
|
errors, and copies the error message into a file in `/boot/ostree`.
|
|
|
|
Then, a new `ostree-boot-complete.service` looks for this file on
|
|
startup and re-emits the error message, and fails.
|
|
|
|
It also deletes the file. The rationale is to avoid *continually*
|
|
warning. For example we need to handle the case when an upgrade
|
|
process creates a new staged deployment. Now, we could change the
|
|
ostree core code to delete the warning file when that happens instead,
|
|
but this is trying to be a conservative change.
|
|
|
|
This should make failures here much more visible as is.
|
|
---
|
|
Makefile-boot.am | 2 +
|
|
Makefile-ostree.am | 1 +
|
|
src/boot/ostree-boot-complete.service | 33 +++++++++++
|
|
src/libostree/ostree-cmdprivate.c | 1 +
|
|
src/libostree/ostree-cmdprivate.h | 1 +
|
|
src/libostree/ostree-impl-system-generator.c | 2 +
|
|
src/libostree/ostree-sysroot-deploy.c | 62 ++++++++++++++++++--
|
|
src/libostree/ostree-sysroot-private.h | 7 +++
|
|
src/libostree/ostree-sysroot.c | 2 +
|
|
src/ostree/ot-admin-builtin-boot-complete.c | 58 ++++++++++++++++++
|
|
src/ostree/ot-admin-builtins.h | 1 +
|
|
src/ostree/ot-builtin-admin.c | 3 +
|
|
tests/kolainst/destructive/staged-deploy.sh | 12 ++++
|
|
13 files changed, 181 insertions(+), 4 deletions(-)
|
|
create mode 100644 src/boot/ostree-boot-complete.service
|
|
create mode 100644 src/ostree/ot-admin-builtin-boot-complete.c
|
|
|
|
diff --git a/Makefile-boot.am b/Makefile-boot.am
|
|
index ec10a0d6..e42e5180 100644
|
|
--- a/Makefile-boot.am
|
|
+++ b/Makefile-boot.am
|
|
@@ -38,6 +38,7 @@ endif
|
|
if BUILDOPT_SYSTEMD
|
|
systemdsystemunit_DATA = src/boot/ostree-prepare-root.service \
|
|
src/boot/ostree-remount.service \
|
|
+ src/boot/ostree-boot-complete.service \
|
|
src/boot/ostree-finalize-staged.service \
|
|
src/boot/ostree-finalize-staged.path \
|
|
$(NULL)
|
|
@@ -64,6 +65,7 @@ endif
|
|
EXTRA_DIST += src/boot/dracut/module-setup.sh \
|
|
src/boot/dracut/ostree.conf \
|
|
src/boot/mkinitcpio \
|
|
+ src/boot/ostree-boot-complete.service \
|
|
src/boot/ostree-prepare-root.service \
|
|
src/boot/ostree-finalize-staged.path \
|
|
src/boot/ostree-remount.service \
|
|
diff --git a/Makefile-ostree.am b/Makefile-ostree.am
|
|
index 82af1681..0fe2c5f8 100644
|
|
--- a/Makefile-ostree.am
|
|
+++ b/Makefile-ostree.am
|
|
@@ -70,6 +70,7 @@ ostree_SOURCES += \
|
|
src/ostree/ot-admin-builtin-diff.c \
|
|
src/ostree/ot-admin-builtin-deploy.c \
|
|
src/ostree/ot-admin-builtin-finalize-staged.c \
|
|
+ src/ostree/ot-admin-builtin-boot-complete.c \
|
|
src/ostree/ot-admin-builtin-undeploy.c \
|
|
src/ostree/ot-admin-builtin-instutil.c \
|
|
src/ostree/ot-admin-builtin-cleanup.c \
|
|
diff --git a/src/boot/ostree-boot-complete.service b/src/boot/ostree-boot-complete.service
|
|
new file mode 100644
|
|
index 00000000..5c09fdc9
|
|
--- /dev/null
|
|
+++ b/src/boot/ostree-boot-complete.service
|
|
@@ -0,0 +1,33 @@
|
|
+# Copyright (C) 2022 Red Hat, Inc.
|
|
+#
|
|
+# This library 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 of the License, or (at your option) any later version.
|
|
+#
|
|
+# This library is distributed in the hope that it will be useful,
|
|
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
|
|
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
|
|
+# Lesser General Public License for more details.
|
|
+#
|
|
+# You should have received a copy of the GNU Lesser General Public
|
|
+# License along with this library. If not, see <https://www.gnu.org/licenses/>.
|
|
+
|
|
+[Unit]
|
|
+Description=OSTree Complete Boot
|
|
+Documentation=man:ostree(1)
|
|
+# For now, this is the only condition on which we start, but it's
|
|
+# marked as a triggering condition in case in the future we want
|
|
+# to do something else.
|
|
+ConditionPathExists=|/boot/ostree/finalize-failure.stamp
|
|
+RequiresMountsFor=/boot
|
|
+# Ensure that we propagate the failure into the current boot before
|
|
+# any further finalization attempts.
|
|
+Before=ostree-finalize-staged.service
|
|
+
|
|
+[Service]
|
|
+Type=oneshot
|
|
+# To write to /boot while keeping it read-only
|
|
+MountFlags=slave
|
|
+RemainAfterExit=yes
|
|
+ExecStart=/usr/bin/ostree admin boot-complete
|
|
diff --git a/src/libostree/ostree-cmdprivate.c b/src/libostree/ostree-cmdprivate.c
|
|
index c9a6e2e1..f6c114f4 100644
|
|
--- a/src/libostree/ostree-cmdprivate.c
|
|
+++ b/src/libostree/ostree-cmdprivate.c
|
|
@@ -51,6 +51,7 @@ ostree_cmd__private__ (void)
|
|
_ostree_repo_static_delta_delete,
|
|
_ostree_repo_verify_bindings,
|
|
_ostree_sysroot_finalize_staged,
|
|
+ _ostree_sysroot_boot_complete,
|
|
};
|
|
|
|
return &table;
|
|
diff --git a/src/libostree/ostree-cmdprivate.h b/src/libostree/ostree-cmdprivate.h
|
|
index 46452ebd..17f943c8 100644
|
|
--- a/src/libostree/ostree-cmdprivate.h
|
|
+++ b/src/libostree/ostree-cmdprivate.h
|
|
@@ -33,6 +33,7 @@ typedef struct {
|
|
gboolean (* ostree_static_delta_delete) (OstreeRepo *repo, const char *delta_id, GCancellable *cancellable, GError **error);
|
|
gboolean (* ostree_repo_verify_bindings) (const char *collection_id, const char *ref_name, GVariant *commit, GError **error);
|
|
gboolean (* ostree_finalize_staged) (OstreeSysroot *sysroot, GCancellable *cancellable, GError **error);
|
|
+ gboolean (* ostree_boot_complete) (OstreeSysroot *sysroot, GCancellable *cancellable, GError **error);
|
|
} OstreeCmdPrivateVTable;
|
|
|
|
/* Note this not really "public", we just export the symbol, but not the header */
|
|
diff --git a/src/libostree/ostree-impl-system-generator.c b/src/libostree/ostree-impl-system-generator.c
|
|
index 769f0cbd..92d71605 100644
|
|
--- a/src/libostree/ostree-impl-system-generator.c
|
|
+++ b/src/libostree/ostree-impl-system-generator.c
|
|
@@ -134,6 +134,8 @@ require_internal_units (const char *normal_dir,
|
|
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");
|
|
+ if (symlinkat (SYSTEM_DATA_UNIT_PATH "/ostree-boot-complete.service", normal_dir_dfd, "multi-user.target.wants/ostree-boot-complete.service") < 0)
|
|
+ return glnx_throw_errno_prefix (error, "symlinkat");
|
|
|
|
return TRUE;
|
|
#else
|
|
diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c
|
|
index b7cc232f..fc5916d8 100644
|
|
--- a/src/libostree/ostree-sysroot-deploy.c
|
|
+++ b/src/libostree/ostree-sysroot-deploy.c
|
|
@@ -3255,10 +3255,10 @@ ostree_sysroot_stage_tree_with_options (OstreeSysroot *self,
|
|
}
|
|
|
|
/* Invoked at shutdown time by ostree-finalize-staged.service */
|
|
-gboolean
|
|
-_ostree_sysroot_finalize_staged (OstreeSysroot *self,
|
|
- GCancellable *cancellable,
|
|
- GError **error)
|
|
+static gboolean
|
|
+_ostree_sysroot_finalize_staged_inner (OstreeSysroot *self,
|
|
+ GCancellable *cancellable,
|
|
+ GError **error)
|
|
{
|
|
/* It's totally fine if there's no staged deployment; perhaps down the line
|
|
* though we could teach the ostree cmdline to tell systemd to activate the
|
|
@@ -3355,9 +3355,63 @@ _ostree_sysroot_finalize_staged (OstreeSysroot *self,
|
|
if (!ostree_sysroot_prepare_cleanup (self, cancellable, error))
|
|
return FALSE;
|
|
|
|
+ // Cleanup will have closed some FDs, re-ensure writability
|
|
+ if (!_ostree_sysroot_ensure_writable (self, error))
|
|
+ return FALSE;
|
|
+
|
|
return TRUE;
|
|
}
|
|
|
|
+/* Invoked at shutdown time by ostree-finalize-staged.service */
|
|
+gboolean
|
|
+_ostree_sysroot_finalize_staged (OstreeSysroot *self,
|
|
+ GCancellable *cancellable,
|
|
+ GError **error)
|
|
+{
|
|
+ g_autoptr(GError) finalization_error = NULL;
|
|
+ if (!_ostree_sysroot_ensure_boot_fd (self, error))
|
|
+ return FALSE;
|
|
+ if (!_ostree_sysroot_finalize_staged_inner (self, cancellable, &finalization_error))
|
|
+ {
|
|
+ g_autoptr(GError) writing_error = NULL;
|
|
+ g_assert_cmpint (self->boot_fd, !=, -1);
|
|
+ if (!glnx_file_replace_contents_at (self->boot_fd, _OSTREE_FINALIZE_STAGED_FAILURE_PATH,
|
|
+ (guint8*)finalization_error->message, -1,
|
|
+ 0, cancellable, &writing_error))
|
|
+ {
|
|
+ // We somehow failed to write the failure message...that's not great. Maybe ENOSPC on /boot.
|
|
+ g_printerr ("Failed to write %s: %s\n", _OSTREE_FINALIZE_STAGED_FAILURE_PATH, writing_error->message);
|
|
+ }
|
|
+ g_propagate_error (error, g_steal_pointer (&finalization_error));
|
|
+ return FALSE;
|
|
+ }
|
|
+ return TRUE;
|
|
+}
|
|
+
|
|
+/* Invoked at bootup time by ostree-boot-complete.service */
|
|
+gboolean
|
|
+_ostree_sysroot_boot_complete (OstreeSysroot *self,
|
|
+ GCancellable *cancellable,
|
|
+ GError **error)
|
|
+{
|
|
+ if (!_ostree_sysroot_ensure_boot_fd (self, error))
|
|
+ return FALSE;
|
|
+
|
|
+ glnx_autofd int failure_fd = -1;
|
|
+ if (!ot_openat_ignore_enoent (self->boot_fd, _OSTREE_FINALIZE_STAGED_FAILURE_PATH, &failure_fd, error))
|
|
+ return FALSE;
|
|
+ // If we didn't find a failure log, then there's nothing to do right now.
|
|
+ // (Actually this unit shouldn't even be invoked, but we may do more in the future)
|
|
+ if (failure_fd == -1)
|
|
+ return TRUE;
|
|
+ g_autofree char *failure_data = glnx_fd_readall_utf8 (failure_fd, NULL, cancellable, error);
|
|
+ if (failure_data == NULL)
|
|
+ return glnx_prefix_error (error, "Reading from %s", _OSTREE_FINALIZE_STAGED_FAILURE_PATH);
|
|
+ // Remove the file; we don't want to continually error out.
|
|
+ (void) unlinkat (self->boot_fd, _OSTREE_FINALIZE_STAGED_FAILURE_PATH, 0);
|
|
+ return glnx_throw (error, "ostree-finalize-staged.service failed on previous boot: %s", failure_data);
|
|
+}
|
|
+
|
|
/**
|
|
* ostree_sysroot_deployment_set_kargs:
|
|
* @self: Sysroot
|
|
diff --git a/src/libostree/ostree-sysroot-private.h b/src/libostree/ostree-sysroot-private.h
|
|
index cb34eeb3..a49a406c 100644
|
|
--- a/src/libostree/ostree-sysroot-private.h
|
|
+++ b/src/libostree/ostree-sysroot-private.h
|
|
@@ -96,6 +96,9 @@ struct OstreeSysroot {
|
|
#define _OSTREE_SYSROOT_BOOT_INITRAMFS_OVERLAYS "ostree/initramfs-overlays"
|
|
#define _OSTREE_SYSROOT_INITRAMFS_OVERLAYS "boot/" _OSTREE_SYSROOT_BOOT_INITRAMFS_OVERLAYS
|
|
|
|
+// Relative to /boot, consumed by ostree-boot-complete.service
|
|
+#define _OSTREE_FINALIZE_STAGED_FAILURE_PATH "ostree/finalize-failure.stamp"
|
|
+
|
|
gboolean
|
|
_ostree_sysroot_ensure_writable (OstreeSysroot *self,
|
|
GError **error);
|
|
@@ -142,6 +145,10 @@ gboolean
|
|
_ostree_sysroot_finalize_staged (OstreeSysroot *self,
|
|
GCancellable *cancellable,
|
|
GError **error);
|
|
+gboolean
|
|
+_ostree_sysroot_boot_complete (OstreeSysroot *self,
|
|
+ GCancellable *cancellable,
|
|
+ GError **error);
|
|
|
|
OstreeDeployment *
|
|
_ostree_sysroot_deserialize_deployment_from_variant (GVariant *v,
|
|
diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c
|
|
index 266a2975..f083f950 100644
|
|
--- a/src/libostree/ostree-sysroot.c
|
|
+++ b/src/libostree/ostree-sysroot.c
|
|
@@ -356,6 +356,8 @@ _ostree_sysroot_ensure_writable (OstreeSysroot *self,
|
|
ostree_sysroot_unload (self);
|
|
if (!ensure_sysroot_fd (self, error))
|
|
return FALSE;
|
|
+ if (!_ostree_sysroot_ensure_boot_fd (self, error))
|
|
+ return FALSE;
|
|
|
|
return TRUE;
|
|
}
|
|
diff --git a/src/ostree/ot-admin-builtin-boot-complete.c b/src/ostree/ot-admin-builtin-boot-complete.c
|
|
new file mode 100644
|
|
index 00000000..6e1052f5
|
|
--- /dev/null
|
|
+++ b/src/ostree/ot-admin-builtin-boot-complete.c
|
|
@@ -0,0 +1,58 @@
|
|
+/*
|
|
+ * Copyright (C) 2022 Red Hat, Inc.
|
|
+ *
|
|
+ * SPDX-License-Identifier: LGPL-2.0+
|
|
+ *
|
|
+ * This library 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 of the License, or (at your option) any later version.
|
|
+ *
|
|
+ * This library is distributed in the hope that it will be useful,
|
|
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
|
|
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
|
|
+ * Lesser General Public License for more details.
|
|
+ *
|
|
+ * You should have received a copy of the GNU Lesser General Public
|
|
+ * License along with this library. If not, see <https://www.gnu.org/licenses/>.
|
|
+ */
|
|
+
|
|
+#include "config.h"
|
|
+
|
|
+#include <stdlib.h>
|
|
+
|
|
+#include "ot-main.h"
|
|
+#include "ot-admin-builtins.h"
|
|
+#include "ot-admin-functions.h"
|
|
+#include "ostree.h"
|
|
+#include "otutil.h"
|
|
+
|
|
+#include "ostree-cmdprivate.h"
|
|
+
|
|
+static GOptionEntry options[] = {
|
|
+ { NULL }
|
|
+};
|
|
+
|
|
+gboolean
|
|
+ot_admin_builtin_boot_complete (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error)
|
|
+{
|
|
+ /* Just a sanity check; we shouldn't be called outside of the service though.
|
|
+ */
|
|
+ struct stat stbuf;
|
|
+ if (fstatat (AT_FDCWD, OSTREE_PATH_BOOTED, &stbuf, 0) < 0)
|
|
+ return TRUE;
|
|
+ // We must have been invoked via systemd which should have set up a mount namespace.
|
|
+ g_assert (getenv ("INVOCATION_ID"));
|
|
+
|
|
+ g_autoptr(GOptionContext) context = g_option_context_new ("");
|
|
+ g_autoptr(OstreeSysroot) sysroot = NULL;
|
|
+ if (!ostree_admin_option_context_parse (context, options, &argc, &argv,
|
|
+ OSTREE_ADMIN_BUILTIN_FLAG_SUPERUSER,
|
|
+ invocation, &sysroot, cancellable, error))
|
|
+ return FALSE;
|
|
+
|
|
+ if (!ostree_cmd__private__()->ostree_boot_complete (sysroot, cancellable, error))
|
|
+ return FALSE;
|
|
+
|
|
+ return TRUE;
|
|
+}
|
|
diff --git a/src/ostree/ot-admin-builtins.h b/src/ostree/ot-admin-builtins.h
|
|
index d32b617e..8d9451be 100644
|
|
--- a/src/ostree/ot-admin-builtins.h
|
|
+++ b/src/ostree/ot-admin-builtins.h
|
|
@@ -39,6 +39,7 @@ BUILTINPROTO(deploy);
|
|
BUILTINPROTO(cleanup);
|
|
BUILTINPROTO(pin);
|
|
BUILTINPROTO(finalize_staged);
|
|
+BUILTINPROTO(boot_complete);
|
|
BUILTINPROTO(unlock);
|
|
BUILTINPROTO(status);
|
|
BUILTINPROTO(set_origin);
|
|
diff --git a/src/ostree/ot-builtin-admin.c b/src/ostree/ot-builtin-admin.c
|
|
index e0d2a60c..af09a614 100644
|
|
--- a/src/ostree/ot-builtin-admin.c
|
|
+++ b/src/ostree/ot-builtin-admin.c
|
|
@@ -43,6 +43,9 @@ static OstreeCommand admin_subcommands[] = {
|
|
{ "finalize-staged", OSTREE_BUILTIN_FLAG_NO_REPO | OSTREE_BUILTIN_FLAG_HIDDEN,
|
|
ot_admin_builtin_finalize_staged,
|
|
"Internal command to run at shutdown time" },
|
|
+ { "boot-complete", OSTREE_BUILTIN_FLAG_NO_REPO | OSTREE_BUILTIN_FLAG_HIDDEN,
|
|
+ ot_admin_builtin_boot_complete,
|
|
+ "Internal command to run at boot after an update was applied" },
|
|
{ "init-fs", OSTREE_BUILTIN_FLAG_NO_REPO,
|
|
ot_admin_builtin_init_fs,
|
|
"Initialize a root filesystem" },
|