From 0d91206a62b9bc1957c49e23fbd0631ab926d99d Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 21 Jul 2020 21:46:43 +0000 Subject: [PATCH 01/23] Post-release version bump --- Makefile-libostree.am | 6 +++--- configure.ac | 4 ++-- src/libostree/libostree-devel.sym | 7 +++++++ 3 files changed, 12 insertions(+), 5 deletions(-) diff --git a/Makefile-libostree.am b/Makefile-libostree.am index 96b9249b..a180e86b 100644 --- a/Makefile-libostree.am +++ b/Makefile-libostree.am @@ -182,9 +182,9 @@ libostree_1_la_SOURCES += \ endif # USE_GPGME symbol_files = $(top_srcdir)/src/libostree/libostree-released.sym -if BUILDOPT_IS_DEVEL_BUILD -symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym -endif +#if BUILDOPT_IS_DEVEL_BUILD +#symbol_files += $(top_srcdir)/src/libostree/libostree-devel.sym +#endif # http://blog.jgc.org/2007/06/escaping-comma-and-space-in-gnu-make.html wl_versionscript_arg = -Wl,--version-script= EXTRA_DIST += \ diff --git a/configure.ac b/configure.ac index 855528c0..50646f1e 100644 --- a/configure.ac +++ b/configure.ac @@ -7,10 +7,10 @@ dnl Seed the release notes with `git-shortlog-with-prs ..`. Th dnl `git-evtag` to create the tag and push it. Finally, create a GitHub release and attach dnl the tarball from `make dist`. m4_define([year_version], [2020]) -m4_define([release_version], [4]) +m4_define([release_version], [5]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=yes +is_release_build=no AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux]) diff --git a/src/libostree/libostree-devel.sym b/src/libostree/libostree-devel.sym index 2ed658c4..21669c4a 100644 --- a/src/libostree/libostree-devel.sym +++ b/src/libostree/libostree-devel.sym @@ -17,6 +17,13 @@ Boston, MA 02111-1307, USA. ***/ +LIBOSTREE_2020.5 { +global: + /* Add symbols here, and uncomment the bits in + * Makefile-libostree.am to enable this too. + */ +} LIBOSTREE_2020.4; + /* Stub section for the stable release *after* this development one; don't * edit this other than to update the year. This is just a copy/paste * source. Replace $LASTSTABLE with the last stable version, and $NEWVERSION From 999f9a2b2de35c89d500badc288f2237c1bfedc2 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Fri, 31 Jul 2020 11:49:38 +0200 Subject: [PATCH 02/23] man: add glossary to main man page Add glossary to define some commonly used literals throughout the ostree man pages. Signed-off-by: Stefan Agner --- man/ostree.xml | 64 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 64 insertions(+) diff --git a/man/ostree.xml b/man/ostree.xml index 8c08bae6..e280a024 100644 --- a/man/ostree.xml +++ b/man/ostree.xml @@ -483,6 +483,70 @@ Boston, MA 02111-1307, USA. + + Terminology + + The following terms are commonly used throughout the man pages. Terms in upper case letters + are literals used in command line arguments. + + + BRANCH + + + Branch name. Part of a REF. + + + + CHECKSUM + + + A SHA256 hash of a object stored in the OSTree repository. This can be a content, + a dirtree, a dirmeta or a commit object. If the SHA256 hash of a commit object is + meant, the term COMMIT is used. + + + + COMMIT + + + A SHA256 hash of a commit object. + + + + REF + + + A reference to a particular commit. References are text files stored in + refs/ that name (refer to) a particular commit. A + reference can only be the branch name part, in which case a local reference + is used (e.g. mybranch/stable). If a remote branch + is referred to, the remote name followed by a colon and the branch name + needs to be used (e.g. myremote:mybranch/stable). + + + + REV REFSPEC + + + A specific revision, a commit. This can be anything which can be resolved to a + commit, e.g. a REF or a + COMMIT. + + + + SHA256 + + + A cryptographic hash function used to store objects in the OSTree + repository. The hashes have a length of 256 bites and are typically + shown and passed to ostree in its 64 ASCII character long hexadecimal + representation + (e.g. 0fc70ed33cfd7d26fe99ae29afb7682ddd0e2157a4898bd8cfcdc8a03565b870). + + + + + See Also From b94c3ae79f2ffa3670e4c3f215a2d0d39b551ca6 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Fri, 31 Jul 2020 11:52:39 +0200 Subject: [PATCH 03/23] man: add missing options to the ostree-commit man page Add missing parameter to the ostree-commit man page. Signed-off-by: Stefan Agner --- man/ostree-commit.xml | 34 +++++++++++++++++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/man/ostree-commit.xml b/man/ostree-commit.xml index b0c5b335..ab5d3415 100644 --- a/man/ostree-commit.xml +++ b/man/ostree-commit.xml @@ -82,6 +82,14 @@ Boston, MA 02111-1307, USA. + + , ="FILE" + + + Full commit description from a file. (optional) + + + , @@ -98,6 +106,14 @@ Boston, MA 02111-1307, USA. + + ="COMMIT" + + + Parent checksum or "none" to explicitly use no parent. If not specified, BRANCH is used as parent (no parent in case BRANCH does not exist). + + + ="dir=PATH" or "tar=TARFILE" or "ref=COMMIT" @@ -119,7 +135,23 @@ Boston, MA 02111-1307, USA. ="KEY=VALUE" - Add a key/value pair to metadata. + Add a key/value pair to metadata. Can be specified multiple times. + + + + + ="KEY=VALUE" + + + Add a key/value pair to metadata, where the KEY is a string, and VALUE is g_variant_parse() formatted. Can be specified multiple times. + + + + + ="KEY" + + + Keep metadata KEY and its associated VALUE from parent. Can be specified multiple times. From 5d7f897908dbf7f471ddfdbd6c29a84ac6bc0bda Mon Sep 17 00:00:00 2001 From: Benjamin Gilbert Date: Fri, 31 Jul 2020 06:57:58 -0400 Subject: [PATCH 04/23] ci: test FCOS PXE and ISO install Make sure we don't break the FCOS live image. PXE is probably sufficient, but also test the ISO image for good measure. --- .cci.jenkinsfile | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/.cci.jenkinsfile b/.cci.jenkinsfile index 2a4c7288..1670ede2 100644 --- a/.cci.jenkinsfile +++ b/.cci.jenkinsfile @@ -77,11 +77,25 @@ parallel fcos: { rmdir insttree coreos-assembler fetch coreos-assembler build + coreos-assembler buildextend-metal + coreos-assembler buildextend-metal4k + coreos-assembler buildextend-live # Install the tests make -C tests/kolainst install """) } - fcosKola(cosaDir: "${env.WORKSPACE}") + stage("Test") { + parallel metal: { + try { + shwrap("cd /srv/fcos && kola testiso -S --scenarios pxe-install,iso-offline-install --output-dir tmp/kola-testiso-metal") + } finally { + shwrap("cd /srv/fcos && tar -cf - tmp/kola-testiso-metal/ | xz -c9 > ${env.WORKSPACE}/kola-testiso-metal.tar.xz") + archiveArtifacts allowEmptyArchive: true, artifacts: 'kola-testiso*.tar.xz' + } + }, kola: { + fcosKola(cosaDir: "${env.WORKSPACE}") + } + } } }, buildopts: { From af140266d5285b8f0a9b15f53ef2176c241b9dbe Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Thu, 30 Jul 2020 17:35:20 -0400 Subject: [PATCH 05/23] app: Fix various CLI metavariable names - Use `REV` instead of `REF` in places where we meant it. - Fix `commit --parent` actually taking a commit checksum and not a ref. - Fix `ostree admin switch` using `REF` instead of `REFSPEC`. --- src/ostree/ot-admin-builtin-switch.c | 4 ++-- src/ostree/ot-builtin-admin.c | 2 +- src/ostree/ot-builtin-commit.c | 4 ++-- src/ostree/ot-builtin-log.c | 4 ++-- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/src/ostree/ot-admin-builtin-switch.c b/src/ostree/ot-admin-builtin-switch.c index 2f12ef1d..b94be767 100644 --- a/src/ostree/ot-admin-builtin-switch.c +++ b/src/ostree/ot-admin-builtin-switch.c @@ -44,7 +44,7 @@ gboolean ot_admin_builtin_switch (int argc, char **argv, OstreeCommandInvocation *invocation, GCancellable *cancellable, GError **error) { g_autoptr(GOptionContext) context = - g_option_context_new ("REF"); + g_option_context_new ("REFSPEC"); g_autoptr(OstreeSysroot) sysroot = NULL; if (!ostree_admin_option_context_parse (context, options, &argc, &argv, OSTREE_ADMIN_BUILTIN_FLAG_SUPERUSER, @@ -53,7 +53,7 @@ ot_admin_builtin_switch (int argc, char **argv, OstreeCommandInvocation *invocat if (argc < 2) { - ot_util_usage_error (context, "REF must be specified", error); + ot_util_usage_error (context, "REFSPEC must be specified", error); return FALSE; } diff --git a/src/ostree/ot-builtin-admin.c b/src/ostree/ot-builtin-admin.c index 9f1a6156..834a271b 100644 --- a/src/ostree/ot-builtin-admin.c +++ b/src/ostree/ot-builtin-admin.c @@ -65,7 +65,7 @@ static OstreeCommand admin_subcommands[] = { "List deployments" }, { "switch", OSTREE_BUILTIN_FLAG_NO_REPO, ot_admin_builtin_switch, - "Construct new tree from REF and deploy it" }, + "Construct new tree from REFSPEC and deploy it" }, { "undeploy", OSTREE_BUILTIN_FLAG_NO_REPO, ot_admin_builtin_undeploy, "Delete deployment INDEX" }, diff --git a/src/ostree/ot-builtin-commit.c b/src/ostree/ot-builtin-commit.c index 20409fc2..e2fcf103 100644 --- a/src/ostree/ot-builtin-commit.c +++ b/src/ostree/ot-builtin-commit.c @@ -94,7 +94,7 @@ parse_fsync_cb (const char *option_name, */ static GOptionEntry options[] = { - { "parent", 0, 0, G_OPTION_ARG_STRING, &opt_parent, "Parent ref, or \"none\"", "REF" }, + { "parent", 0, 0, G_OPTION_ARG_STRING, &opt_parent, "Parent commit checksum, or \"none\"", "COMMIT" }, { "subject", 's', 0, G_OPTION_ARG_STRING, &opt_subject, "One line subject", "SUBJECT" }, { "body", 'm', 0, G_OPTION_ARG_STRING, &opt_body, "Full description", "BODY" }, { "body-file", 'F', 0, G_OPTION_ARG_FILENAME, &opt_body_file, "Commit message from FILE path", "FILE" }, @@ -103,7 +103,7 @@ static GOptionEntry options[] = { { "orphan", 0, 0, G_OPTION_ARG_NONE, &opt_orphan, "Create a commit without writing a ref", NULL }, { "no-bindings", 0, 0, G_OPTION_ARG_NONE, &opt_no_bindings, "Do not write any ref bindings", NULL }, { "bind-ref", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_bind_refs, "Add a ref to ref binding commit metadata", "BRANCH" }, - { "base", 0, 0, G_OPTION_ARG_STRING, &opt_base, "Start from the given commit as a base (no modifiers apply)", "REF" }, + { "base", 0, 0, G_OPTION_ARG_STRING, &opt_base, "Start from the given commit as a base (no modifiers apply)", "REV" }, { "tree", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_trees, "Overlay the given argument as a tree", "dir=PATH or tar=TARFILE or ref=COMMIT" }, { "add-metadata-string", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_metadata_strings, "Add a key/value pair to metadata", "KEY=VALUE" }, { "add-metadata", 0, 0, G_OPTION_ARG_STRING_ARRAY, &opt_metadata_variants, "Add a key/value pair to metadata, where the KEY is a string, an VALUE is g_variant_parse() formatted", "KEY=VALUE" }, diff --git a/src/ostree/ot-builtin-log.c b/src/ostree/ot-builtin-log.c index 306f177b..0a1d408b 100644 --- a/src/ostree/ot-builtin-log.c +++ b/src/ostree/ot-builtin-log.c @@ -95,7 +95,7 @@ ostree_builtin_log (int argc, g_autofree char *checksum = NULL; OstreeDumpFlags flags = OSTREE_DUMP_NONE; - context = g_option_context_new ("REF"); + context = g_option_context_new ("REV"); if (!ostree_option_context_parse (context, options, &argc, &argv, invocation, &repo, cancellable, error)) goto out; @@ -105,7 +105,7 @@ ostree_builtin_log (int argc, if (argc <= 1) { - ot_util_usage_error (context, "A ref argument is required", error); + ot_util_usage_error (context, "A rev argument is required", error); goto out; } rev = argv[1]; From 512b4e63132869f62c007954ee6beee98f4d7228 Mon Sep 17 00:00:00 2001 From: Stefan Agner Date: Fri, 31 Jul 2020 21:43:09 +0200 Subject: [PATCH 06/23] Show commit checksum of parent, if present This is useful for ostree log on client side where often not the full history of a branch is available. It is also helpful for ostree show to show if a particular commit has a parent. --- src/ostree/ot-dump.c | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/ostree/ot-dump.c b/src/ostree/ot-dump.c index 38f3730b..225d1845 100644 --- a/src/ostree/ot-dump.c +++ b/src/ostree/ot-dump.c @@ -114,6 +114,7 @@ dump_commit (GVariant *variant, const gchar *subject; const gchar *body; guint64 timestamp; + g_autofree char *parent = NULL; g_autofree char *str = NULL; g_autofree char *version = NULL; g_autoptr(GError) local_error = NULL; @@ -129,6 +130,12 @@ dump_commit (GVariant *variant, g_assert (local_error); /* Pacify static analysis */ errx (1, "Failed to read commit: %s", local_error->message); } + + if ((parent = ostree_commit_get_parent(variant))) + { + g_print ("Parent: %s\n", parent); + } + g_autofree char *contents = ostree_commit_get_content_checksum (variant) ?: ""; g_print ("ContentChecksum: %s\n", contents); g_print ("Date: %s\n", str); From 33eeb7b9ebd858c0246a9155b7a64b9f8a258583 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 1 Aug 2020 17:24:28 +0000 Subject: [PATCH 07/23] remount: Still remount /sysroot writable if not configured ro Regression from https://github.com/ostreedev/ostree/pull/2113/commits/35642259175973617da937f3cab6ce5f13c95077 BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1862568 We still need to remount writable if it's not configured on; because it may need OS adjustments it needs to be opt-in. --- src/switchroot/ostree-remount.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/switchroot/ostree-remount.c b/src/switchroot/ostree-remount.c index 5c313c87..cfd270bb 100644 --- a/src/switchroot/ostree-remount.c +++ b/src/switchroot/ostree-remount.c @@ -106,11 +106,11 @@ main(int argc, char *argv[]) exit (EXIT_SUCCESS); } - /* Handle remounting /sysroot read-only now */ - if (unlink (_OSTREE_SYSROOT_READONLY_STAMP) == 0) - { - do_remount ("/sysroot", false); - } + /* Handle remounting /sysroot; if it's explicitly marked as read-only (opt in) + * then ensure it's readonly, otherwise mount writable, the same as / + */ + bool sysroot_configured_readonly = unlink (_OSTREE_SYSROOT_READONLY_STAMP) == 0; + do_remount ("/sysroot", !sysroot_configured_readonly); /* If /var was created as as an OSTree default bind mount (instead of being a separate filesystem) * then remounting the root mount read-only also remounted it. From f3c7834f1e41e07990528e951d3d98c1e150db2b Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 1 Aug 2020 18:47:36 +0000 Subject: [PATCH 08/23] tests/repo-finder: Explicitly commit empty dir We were committing the whole tempdir, which seems to fail in Travis because the GPG agent Unix domain socket ends up there too, and ostree refuses to commit sockets. --- tests/test-repo-finder-config.c | 5 ++++- tests/test-repo-finder-mount.c | 5 ++++- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/tests/test-repo-finder-config.c b/tests/test-repo-finder-config.c index 0a2e9e60..ead9e907 100644 --- a/tests/test-repo-finder-config.c +++ b/tests/test-repo-finder-config.c @@ -173,6 +173,9 @@ assert_create_remote (Fixture *fixture, glnx_shutil_mkdir_p_at (fixture->tmpdir.fd, repo_name, 0700, NULL, &error); g_assert_no_error (error); + glnx_shutil_mkdir_p_at (fixture->tmpdir.fd, "empty", 0700, NULL, &error); + g_assert_no_error (error); + g_autoptr(GFile) repo_path = g_file_get_child (fixture->working_dir, repo_name); g_autoptr(OstreeRepo) repo = ostree_repo_new (repo_path); ostree_repo_set_collection_id (repo, collection_id, &error); @@ -193,7 +196,7 @@ assert_create_remote (Fixture *fixture, g_autoptr(OstreeRepoFile) repo_file = NULL; mtree = ostree_mutable_tree_new (); - ostree_repo_write_dfd_to_mtree (repo, AT_FDCWD, ".", mtree, NULL, NULL, &error); + ostree_repo_write_dfd_to_mtree (repo, fixture->tmpdir.fd, "empty", mtree, NULL, NULL, &error); g_assert_no_error (error); ostree_repo_write_mtree (repo, mtree, (GFile **) &repo_file, NULL, &error); g_assert_no_error (error); diff --git a/tests/test-repo-finder-mount.c b/tests/test-repo-finder-mount.c index af2f5e08..e4130921 100644 --- a/tests/test-repo-finder-mount.c +++ b/tests/test-repo-finder-mount.c @@ -190,6 +190,9 @@ assert_create_remote_va (Fixture *fixture, ostree_repo_create (repo, OSTREE_REPO_MODE_ARCHIVE, NULL, &error); g_assert_no_error (error); + glnx_shutil_mkdir_p_at (fixture->tmpdir.fd, "empty", 0700, NULL, &error); + g_assert_no_error (error); + /* Set up the refs from @.... */ for (const OstreeCollectionRef *ref = va_arg (args, const OstreeCollectionRef *); ref != NULL; @@ -201,7 +204,7 @@ assert_create_remote_va (Fixture *fixture, gchar **out_checksum = va_arg (args, gchar **); mtree = ostree_mutable_tree_new (); - ostree_repo_write_dfd_to_mtree (repo, AT_FDCWD, ".", mtree, NULL, NULL, &error); + ostree_repo_write_dfd_to_mtree (repo, fixture->tmpdir.fd, "empty", mtree, NULL, NULL, &error); g_assert_no_error (error); ostree_repo_write_mtree (repo, mtree, (GFile **) &repo_file, NULL, &error); g_assert_no_error (error); From 98137403aa1250f2dd2620f034d447ab2039b702 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 1 Aug 2020 18:14:30 +0000 Subject: [PATCH 09/23] ci: Fix ISO testing Regression from https://github.com/ostreedev/ostree/pull/2158/commits/5d7f897908dbf7f471ddfdbd6c29a84ac6bc0bda I'm not sure how (or if) this passed before, the job logs have been GC'd. This is a bit confusing but basically right now ostree/rpm-ostree's CI jobs don't use `/srv/fcos` - it might make sense to port these to `fcosBuild` but that needs investigation. --- .cci.jenkinsfile | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/.cci.jenkinsfile b/.cci.jenkinsfile index 1670ede2..79425a49 100644 --- a/.cci.jenkinsfile +++ b/.cci.jenkinsfile @@ -79,7 +79,7 @@ parallel fcos: { coreos-assembler build coreos-assembler buildextend-metal coreos-assembler buildextend-metal4k - coreos-assembler buildextend-live + coreos-assembler buildextend-live --fast # Install the tests make -C tests/kolainst install """) @@ -87,9 +87,9 @@ parallel fcos: { stage("Test") { parallel metal: { try { - shwrap("cd /srv/fcos && kola testiso -S --scenarios pxe-install,iso-offline-install --output-dir tmp/kola-testiso-metal") + shwrap("kola testiso -S --scenarios pxe-install,iso-offline-install --output-dir tmp/kola-testiso-metal") } finally { - shwrap("cd /srv/fcos && tar -cf - tmp/kola-testiso-metal/ | xz -c9 > ${env.WORKSPACE}/kola-testiso-metal.tar.xz") + shwrap("tar -cf - tmp/kola-testiso-metal/ | xz -c9 > ${env.WORKSPACE}/kola-testiso-metal.tar.xz") archiveArtifacts allowEmptyArchive: true, artifacts: 'kola-testiso*.tar.xz' } }, kola: { From 1e127f2dcc83dd39fee1f40fdb98429f101ba8e3 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Wed, 5 Aug 2020 14:17:53 +0000 Subject: [PATCH 10/23] ci: Barf on unset umask Since it's just not a sane thing to do and will cause various failures in our test suite. --- ci/build.sh | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/ci/build.sh b/ci/build.sh index 4ff6eaa5..2afcd018 100755 --- a/ci/build.sh +++ b/ci/build.sh @@ -6,6 +6,11 @@ set -xeuo pipefail dn=$(dirname $0) . ${dn}/libbuild.sh +if [ `umask` = 0000 ]; then + echo 'Your umask is broken, please use e.g. `umask 0022`' 1>&2 + exit 1 +fi + ${dn}/installdeps.sh # Default libcurl on by default in fedora unless libsoup is enabled From bd68c7dfd7985a88cac41d060b46ce7ac6e9b052 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Wed, 15 Jul 2020 16:24:32 +0100 Subject: [PATCH 11/23] pull: Improve formatting of pull options in documentation Backticks improve all things. Signed-off-by: Philip Withnall --- src/libostree/ostree-repo-pull.c | 54 ++++++++++++++++---------------- 1 file changed, 27 insertions(+), 27 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 5fdbeabc..18fe9d27 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -3273,37 +3273,37 @@ initiate_request (OtPullData *pull_data, * Like ostree_repo_pull(), but supports an extensible set of flags. * The following are currently defined: * - * * refs (as): Array of string refs - * * collection-refs (a(sss)): Array of (collection ID, ref name, checksum) tuples to pull; + * * `refs` (`as`): Array of string refs + * * `collection-refs` (`a(sss)`): Array of (collection ID, ref name, checksum) tuples to pull; * mutually exclusive with `refs` and `override-commit-ids`. Checksums may be the empty * string to pull the latest commit for that ref - * * flags (i): An instance of #OstreeRepoPullFlags - * * subdir (s): Pull just this subdirectory - * * subdirs (as): Pull just these subdirectories - * * override-remote-name (s): If local, add this remote to refspec - * * gpg-verify (b): GPG verify commits - * * gpg-verify-summary (b): GPG verify summary - * * disable-sign-verify (b): Disable signapi verification of commits - * * disable-sign-verify-summary (b): Disable signapi verification of the summary - * * depth (i): How far in the history to traverse; default is 0, -1 means infinite - * * per-object-fsync (b): Perform disk writes more slowly, avoiding a single large I/O sync - * * disable-static-deltas (b): Do not use static deltas - * * require-static-deltas (b): Require static deltas - * * override-commit-ids (as): Array of specific commit IDs to fetch for refs - * * timestamp-check (b): Verify commit timestamps are newer than current (when pulling via ref); Since: 2017.11 - * * timestamp-check-from-rev (s): Verify that all fetched commit timestamps are newer than timestamp of given rev; Since: 2020.4 - * * metadata-size-restriction (t): Restrict metadata objects to a maximum number of bytes; 0 to disable. Since: 2018.9 - * * dry-run (b): Only print information on what will be downloaded (requires static deltas) - * * override-url (s): Fetch objects from this URL if remote specifies no metalink in options - * * inherit-transaction (b): Don't initiate, finish or abort a transaction, useful to do multiple pulls in one transaction. - * * http-headers (a(ss)): Additional headers to add to all HTTP requests - * * update-frequency (u): Frequency to call the async progress callback in milliseconds, if any; only values higher than 0 are valid - * * localcache-repos (as): File paths for local repos to use as caches when doing remote fetches - * * append-user-agent (s): Additional string to append to the user agent - * * n-network-retries (u): Number of times to retry each download on receiving + * * `flags` (`i`): An instance of #OstreeRepoPullFlags + * * `subdir` (`s`): Pull just this subdirectory + * * `subdirs` (`as`): Pull just these subdirectories + * * `override-remote-name` (`s`): If local, add this remote to refspec + * * `gpg-verify` (`b`): GPG verify commits + * * `gpg-verify-summary` (`b`): GPG verify summary + * * `disable-sign-verify` (`b`): Disable signapi verification of commits + * * `disable-sign-verify-summary` (`b`): Disable signapi verification of the summary + * * `depth` (`i`): How far in the history to traverse; default is 0, -1 means infinite + * * `per-object-fsync` (`b`): Perform disk writes more slowly, avoiding a single large I/O sync + * * `disable-static-deltas` (`b`): Do not use static deltas + * * `require-static-deltas` (`b`): Require static deltas + * * `override-commit-ids` (`as`): Array of specific commit IDs to fetch for refs + * * `timestamp-check` (`b`): Verify commit timestamps are newer than current (when pulling via ref); Since: 2017.11 + * * `timestamp-check-from-rev` (`s`): Verify that all fetched commit timestamps are newer than timestamp of given rev; Since: 2020.4 + * * `metadata-size-restriction` (`t`): Restrict metadata objects to a maximum number of bytes; 0 to disable. Since: 2018.9 + * * `dry-run` (`b`): Only print information on what will be downloaded (requires static deltas) + * * `override-url` (`s`): Fetch objects from this URL if remote specifies no metalink in options + * * `inherit-transaction` (`b`): Don't initiate, finish or abort a transaction, useful to do multiple pulls in one transaction. + * * `http-headers` (`a(ss)`): Additional headers to add to all HTTP requests + * * `update-frequency` (`u`): Frequency to call the async progress callback in milliseconds, if any; only values higher than 0 are valid + * * `localcache-repos` (`as`): File paths for local repos to use as caches when doing remote fetches + * * `append-user-agent` (`s`): Additional string to append to the user agent + * * `n-network-retries` (`u`): Number of times to retry each download on receiving * a transient network error, such as a socket timeout; default is 5, 0 * means return errors without retrying. Since: 2018.6 - * * ref-keyring-map (a(sss)): Array of (collection ID, ref name, keyring + * * `ref-keyring-map` (`a(sss)`): Array of (collection ID, ref name, keyring * remote name) tuples specifying which remote's keyring should be used when * doing GPG verification of each collection-ref. This is useful to prevent a * remote from serving malicious updates to refs which did not originate from From f5da67d78a0e64117169762d102a706b941c5945 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Thu, 16 Jul 2020 16:16:37 +0100 Subject: [PATCH 12/23] pull: Add summary-{,sig-}bytes options to ostree_repo_pull() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These allow the `summary` and `summary.sig` files to be cached at a higher layer (for example, flatpak) between related pull operations (for example, within a single flatpak transaction). This avoids re-downloading `summary.sig` multiple times throughout a transaction, which increases the transaction’s latency and introduces the possibility for inconsistency between parts of the transaction if the server changes its `summary` file part-way through. In particular, this should speed up flatpak transactions on machines with high latency network connections, where network round trips have a high impact on the latency of an overall operation. Signed-off-by: Philip Withnall --- src/libostree/ostree-repo-pull.c | 43 ++++++++++++++++++++++++++++++-- 1 file changed, 41 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-repo-pull.c b/src/libostree/ostree-repo-pull.c index 18fe9d27..894e4b1f 100644 --- a/src/libostree/ostree-repo-pull.c +++ b/src/libostree/ostree-repo-pull.c @@ -3311,6 +3311,14 @@ initiate_request (OtPullData *pull_data, * not being pulled will be ignored and any ref without a keyring remote * will be verified with the keyring of the remote being pulled from. * Since: 2019.2 + * * `summary-bytes` (`ay'): Contents of the `summary` file to use. If this is + * specified, `summary-sig-bytes` must also be specified. This is + * useful if doing multiple pull operations in a transaction, using + * ostree_repo_remote_fetch_summary_with_options() beforehand to download + * the `summary` and `summary.sig` once for the entire transaction. If not + * specified, the `summary` will be downloaded from the remote. Since: 2020.5 + * * `summary-sig-bytes` (`ay`): Contents of the `summary.sig` file. If this + * is specified, `summary-bytes` must also be specified. Since: 2020.5 */ gboolean ostree_repo_pull_with_options (OstreeRepo *self, @@ -3356,6 +3364,8 @@ ostree_repo_pull_with_options (OstreeRepo *self, int i; g_autofree char **opt_localcache_repos = NULL; g_autoptr(GVariantIter) ref_keyring_map_iter = NULL; + g_autoptr(GVariant) summary_bytes_v = NULL; + g_autoptr(GVariant) summary_sig_bytes_v = NULL; /* If refs or collection-refs has exactly one value, this will point to that * value, otherwise NULL. Used for logging. */ @@ -3402,6 +3412,8 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_variant_lookup (options, "n-network-retries", "u", &pull_data->n_network_retries); opt_ref_keyring_map_set = g_variant_lookup (options, "ref-keyring-map", "a(sss)", &ref_keyring_map_iter); + (void) g_variant_lookup (options, "summary-bytes", "@ay", &summary_bytes_v); + (void) g_variant_lookup (options, "summary-sig-bytes", "@ay", &summary_sig_bytes_v); if (pull_data->remote_refspec_name != NULL) pull_data->remote_name = g_strdup (pull_data->remote_refspec_name); @@ -3439,6 +3451,10 @@ ostree_repo_pull_with_options (OstreeRepo *self, */ g_return_val_if_fail (!pull_data->dry_run || pull_data->require_static_deltas, FALSE); + /* summary-bytes and summary-sig-bytes must both be specified, or neither be + * specified, so we know they’re consistent */ + g_return_val_if_fail ((summary_bytes_v == NULL) == (summary_sig_bytes_v == NULL), FALSE); + pull_data->is_mirror = (flags & OSTREE_REPO_PULL_FLAGS_MIRROR) > 0; pull_data->is_commit_only = (flags & OSTREE_REPO_PULL_FLAGS_COMMIT_ONLY) > 0; /* See our processing of OSTREE_REPO_PULL_FLAGS_UNTRUSTED below */ @@ -3651,6 +3667,10 @@ ostree_repo_pull_with_options (OstreeRepo *self, if (!metalink_uri) goto out; + /* FIXME: Use summary_bytes_v/summary_sig_bytes_v to avoid unnecessary + * re-downloads here. Would require additional support for caching the + * metalink file or mirror list. */ + metalink = _ostree_metalink_new (pull_data->fetcher, "summary", OSTREE_MAX_METADATA_SIZE, metalink_uri, pull_data->n_network_retries); @@ -3841,7 +3861,25 @@ ostree_repo_pull_with_options (OstreeRepo *self, g_autoptr(GVariant) additional_metadata = NULL; gboolean summary_from_cache = FALSE; - if (!pull_data->summary_data_sig) + if (summary_sig_bytes_v) + { + /* Must both be specified */ + g_assert (summary_bytes_v); + + bytes_sig = g_variant_get_data_as_bytes (summary_sig_bytes_v); + bytes_summary = g_variant_get_data_as_bytes (summary_bytes_v); + + if (!bytes_sig || !bytes_summary) + { + g_set_error (error, G_IO_ERROR, G_IO_ERROR_FAILED, + "summary-bytes or summary-sig-bytes set to invalid value"); + goto out; + } + + g_debug ("Loaded %s summary from options", remote_name_or_baseurl); + } + + if (!bytes_sig) { if (!_ostree_fetcher_mirrored_request_to_membuf (pull_data->fetcher, pull_data->meta_mirrorlist, @@ -3854,6 +3892,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, } if (bytes_sig && + !bytes_summary && !pull_data->remote_repo_local && !_ostree_repo_load_cache_summary_if_same_sig (self, remote_name_or_baseurl, @@ -3863,7 +3902,7 @@ ostree_repo_pull_with_options (OstreeRepo *self, error)) goto out; - if (bytes_summary) + if (bytes_summary && !summary_bytes_v) { g_debug ("Loaded %s summary from cache", remote_name_or_baseurl); summary_from_cache = TRUE; From f2773c1b55cdcc7eea0558e4f2505d4ecbd53d62 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 17 May 2020 18:17:37 +0000 Subject: [PATCH 13/23] Add "transient" unlock I was thinking a bit more recently about the "live" changes stuff https://github.com/coreos/rpm-ostree/issues/639 (particularly since https://github.com/coreos/rpm-ostree/pull/2060 ) and I realized reading the last debates in that issue that there's really a much simpler solution; do exactly the same thing we do for `ostree admin unlock`, except mount it read-only by default. Then, anything that wants to modify it does the same thing libostree does for `/sysroot` and `/boot` as of recently; create a new mount namespace and do the modifications there. The advantages of this are numerous. First, we already have all of the code, it's basically just plumbing through a new entry in the state enumeration and passing `MS_RDONLY` into the `mount()` system call. "live" changes here also naturally don't persist, unlike what we are currently doing in rpm-ostree. --- src/libostree/ostree-deployment.c | 2 ++ src/libostree/ostree-deployment.h | 3 +- src/libostree/ostree-sysroot-private.h | 1 + src/libostree/ostree-sysroot.c | 28 +++++++++++---- src/ostree/ot-admin-builtin-unlock.c | 18 +++++++++- .../kolainst/destructive/unlock-transient.sh | 34 +++++++++++++++++++ 6 files changed, 77 insertions(+), 9 deletions(-) create mode 100755 tests/kolainst/destructive/unlock-transient.sh diff --git a/src/libostree/ostree-deployment.c b/src/libostree/ostree-deployment.c index 6532a973..70e1bc49 100644 --- a/src/libostree/ostree-deployment.c +++ b/src/libostree/ostree-deployment.c @@ -318,6 +318,8 @@ ostree_deployment_unlocked_state_to_string (OstreeDeploymentUnlockedState state) return "hotfix"; case OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT: return "development"; + case OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT: + return "transient"; } g_assert_not_reached (); } diff --git a/src/libostree/ostree-deployment.h b/src/libostree/ostree-deployment.h index 756e39d2..dcfa25ec 100644 --- a/src/libostree/ostree-deployment.h +++ b/src/libostree/ostree-deployment.h @@ -99,7 +99,8 @@ char *ostree_deployment_get_origin_relpath (OstreeDeployment *self); typedef enum { OSTREE_DEPLOYMENT_UNLOCKED_NONE, OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT, - OSTREE_DEPLOYMENT_UNLOCKED_HOTFIX + OSTREE_DEPLOYMENT_UNLOCKED_HOTFIX, + OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT, } OstreeDeploymentUnlockedState; _OSTREE_PUBLIC diff --git a/src/libostree/ostree-sysroot-private.h b/src/libostree/ostree-sysroot-private.h index 96670b13..fa1e5336 100644 --- a/src/libostree/ostree-sysroot-private.h +++ b/src/libostree/ostree-sysroot-private.h @@ -85,6 +85,7 @@ struct OstreeSysroot { #define _OSTREE_SYSROOT_RUNSTATE_STAGED_LOCKED "/run/ostree/staged-deployment-locked" #define _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_DIR "/run/ostree/deployment-state/" #define _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_DEVELOPMENT "unlocked-development" +#define _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_TRANSIENT "unlocked-transient" gboolean _ostree_sysroot_ensure_writable (OstreeSysroot *self, diff --git a/src/libostree/ostree-sysroot.c b/src/libostree/ostree-sysroot.c index 0dc7a392..b211fea7 100644 --- a/src/libostree/ostree-sysroot.c +++ b/src/libostree/ostree-sysroot.c @@ -747,9 +747,13 @@ parse_deployment (OstreeSysroot *self, ret_deployment->unlocked = OSTREE_DEPLOYMENT_UNLOCKED_NONE; g_autofree char *unlocked_development_path = _ostree_sysroot_get_runstate_path (ret_deployment, _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_DEVELOPMENT); + g_autofree char *unlocked_transient_path = + _ostree_sysroot_get_runstate_path (ret_deployment, _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_TRANSIENT); struct stat stbuf; if (lstat (unlocked_development_path, &stbuf) == 0) ret_deployment->unlocked = OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT; + else if (lstat (unlocked_transient_path, &stbuf) == 0) + ret_deployment->unlocked = OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT; else { GKeyFile *origin = ostree_deployment_get_origin (ret_deployment); @@ -1932,6 +1936,8 @@ ostree_sysroot_deployment_unlock (OstreeSysroot *self, const char *ovl_options = NULL; static const char hotfix_ovl_options[] = "lowerdir=usr,upperdir=.usr-ovl-upper,workdir=.usr-ovl-work"; + g_autofree char *unlock_ovldir = NULL; + switch (unlocked_state) { case OSTREE_DEPLOYMENT_UNLOCKED_NONE: @@ -1951,11 +1957,12 @@ ostree_sysroot_deployment_unlock (OstreeSysroot *self, } break; case OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT: + case OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT: { + unlock_ovldir = g_strdup ("/var/tmp/ostree-unlock-ovl.XXXXXX"); /* We're just doing transient development/hacking? Okay, * stick the overlayfs bits in /var/tmp. */ - char *development_ovldir = strdupa ("/var/tmp/ostree-unlock-ovl.XXXXXX"); const char *development_ovl_upper; const char *development_ovl_work; @@ -1966,14 +1973,14 @@ ostree_sysroot_deployment_unlock (OstreeSysroot *self, "/usr", usr_mode, error)) return FALSE; - if (g_mkdtemp_full (development_ovldir, 0755) == NULL) + if (g_mkdtemp_full (unlock_ovldir, 0755) == NULL) return glnx_throw_errno_prefix (error, "mkdtemp"); } - development_ovl_upper = glnx_strjoina (development_ovldir, "/upper"); + development_ovl_upper = glnx_strjoina (unlock_ovldir, "/upper"); if (!mkdir_unmasked (AT_FDCWD, development_ovl_upper, usr_mode, cancellable, error)) return FALSE; - development_ovl_work = glnx_strjoina (development_ovldir, "/work"); + development_ovl_work = glnx_strjoina (unlock_ovldir, "/work"); if (!mkdir_unmasked (AT_FDCWD, development_ovl_work, usr_mode, cancellable, error)) return FALSE; ovl_options = glnx_strjoina ("lowerdir=usr,upperdir=", development_ovl_upper, @@ -1996,6 +2003,9 @@ ostree_sysroot_deployment_unlock (OstreeSysroot *self, return glnx_throw_errno_prefix (error, "fork"); else if (mount_child == 0) { + int mountflags = 0; + if (unlocked_state == OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT) + mountflags |= MS_RDONLY; /* Child process. Do NOT use any GLib API here; it's not generally fork() safe. * * TODO: report errors across a pipe (or use the journal?) rather than @@ -2003,7 +2013,7 @@ ostree_sysroot_deployment_unlock (OstreeSysroot *self, */ if (fchdir (deployment_dfd) < 0) err (1, "fchdir"); - if (mount ("overlay", "/usr", "overlay", 0, ovl_options) < 0) + if (mount ("overlay", "/usr", "overlay", mountflags, ovl_options) < 0) err (1, "mount"); exit (EXIT_SUCCESS); } @@ -2036,15 +2046,19 @@ ostree_sysroot_deployment_unlock (OstreeSysroot *self, return FALSE; break; case OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT: + case OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT: { g_autofree char *devpath = - _ostree_sysroot_get_runstate_path (deployment, _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_DEVELOPMENT); + unlocked_state == OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT ? + _ostree_sysroot_get_runstate_path (deployment, _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_DEVELOPMENT) + : + _ostree_sysroot_get_runstate_path (deployment, _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_TRANSIENT); g_autofree char *devpath_parent = dirname (g_strdup (devpath)); if (!glnx_shutil_mkdir_p_at (AT_FDCWD, devpath_parent, 0755, cancellable, error)) return FALSE; - if (!g_file_set_contents (devpath, "", 0, error)) + if (!g_file_set_contents (devpath, unlock_ovldir, -1, error)) return FALSE; } } diff --git a/src/ostree/ot-admin-builtin-unlock.c b/src/ostree/ot-admin-builtin-unlock.c index cd466183..6c265f54 100644 --- a/src/ostree/ot-admin-builtin-unlock.c +++ b/src/ostree/ot-admin-builtin-unlock.c @@ -31,9 +31,11 @@ #include static gboolean opt_hotfix; +static gboolean opt_transient; static GOptionEntry options[] = { { "hotfix", 0, 0, G_OPTION_ARG_NONE, &opt_hotfix, "Retain changes across reboots", NULL }, + { "transient", 0, 0, G_OPTION_ARG_NONE, &opt_transient, "Mount overlayfs read-only by default", NULL }, { NULL } }; @@ -67,7 +69,17 @@ ot_admin_builtin_unlock (int argc, char **argv, OstreeCommandInvocation *invocat goto out; } - target_state = opt_hotfix ? OSTREE_DEPLOYMENT_UNLOCKED_HOTFIX : OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT; + if (opt_hotfix && opt_transient) + { + glnx_throw (error, "Cannot specify both --hotfix and --transient"); + goto out; + } + else if (opt_hotfix) + target_state = OSTREE_DEPLOYMENT_UNLOCKED_HOTFIX; + else if (opt_transient) + target_state = OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT; + else + target_state = OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT; if (!ostree_sysroot_deployment_unlock (sysroot, booted_deployment, target_state, cancellable, error)) @@ -87,6 +99,10 @@ ot_admin_builtin_unlock (int argc, char **argv, OstreeCommandInvocation *invocat g_print ("Development mode enabled. A writable overlayfs is now mounted on /usr.\n" "All changes there will be discarded on reboot.\n"); break; + case OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT: + g_print ("A writable overlayfs is prepared for /usr, but is mounted read-only by default.\n" + "All changes there will be discarded on reboot.\n"); + break; } ret = TRUE; diff --git a/tests/kolainst/destructive/unlock-transient.sh b/tests/kolainst/destructive/unlock-transient.sh new file mode 100755 index 00000000..8dce2224 --- /dev/null +++ b/tests/kolainst/destructive/unlock-transient.sh @@ -0,0 +1,34 @@ +#!/bin/bash +# Test unlock --transient +set -xeuo pipefail + +. ${KOLA_EXT_DATA}/libinsttest.sh + +testfile=/usr/share/writable-usr-test + +case "${AUTOPKGTEST_REBOOT_MARK:-}" in + "") + require_writable_sysroot + assert_not_has_file "${testfile}" + ostree admin unlock --transient + # It's still read-only + if touch ${testfile}; then + fatal "modified /usr" + fi + # But, we can affect it in a new mount namespace + unshare -m -- /bin/sh -c 'mount -o remount,rw /usr && echo hello from transient unlock >'"${testfile}" + assert_file_has_content "${testfile}" "hello from transient unlock" + # Still can't write to it from the outer namespace + if touch ${testfile} || rm -v "${testfile}" 2>/dev/null; then + fatal "modified ${testfile}" + fi + /tmp/autopkgtest-reboot 2 + ;; + "2") + if test -f "${testfile}"; then + fatal "${testfile} persisted across reboot?" + fi + echo "ok unlock transient" + ;; + *) fatal "Unexpected boot mark ${AUTOPKGTEST_REBOOT_MARK}" +esac From 82c0b4a8b8738e7d72cf74a97aead73aa7ef3c72 Mon Sep 17 00:00:00 2001 From: Stephen Lowrie Date: Tue, 11 Aug 2020 00:02:12 -0500 Subject: [PATCH 14/23] ci: add pxe-offline-install testiso scenario --- .cci.jenkinsfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.cci.jenkinsfile b/.cci.jenkinsfile index 79425a49..add8c35a 100644 --- a/.cci.jenkinsfile +++ b/.cci.jenkinsfile @@ -87,7 +87,7 @@ parallel fcos: { stage("Test") { parallel metal: { try { - shwrap("kola testiso -S --scenarios pxe-install,iso-offline-install --output-dir tmp/kola-testiso-metal") + shwrap("kola testiso -S --scenarios pxe-install,iso-offline-install,pxe-offline-install --output-dir tmp/kola-testiso-metal") } finally { shwrap("tar -cf - tmp/kola-testiso-metal/ | xz -c9 > ${env.WORKSPACE}/kola-testiso-metal.tar.xz") archiveArtifacts allowEmptyArchive: true, artifacts: 'kola-testiso*.tar.xz' From 7cf1fb38b0a75166f7b1cc3f920b054a7e59b046 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Paulo=20Rechi=20Vita?= Date: Fri, 26 Jun 2020 18:28:48 -0700 Subject: [PATCH 15/23] dracut: Create reproducible images Without reproducible images, a rebuild of the initrd will create a different image file (due to things like creation time of the files in the cpio archive) even if the actual contents in it are exactly the same, adding an unnecessary download during updates. Adding 'reproducible=yes' avoids this and creates the same image files for the same content. --- src/boot/dracut/ostree.conf | 1 + 1 file changed, 1 insertion(+) diff --git a/src/boot/dracut/ostree.conf b/src/boot/dracut/ostree.conf index 612bb435..ac70494b 100755 --- a/src/boot/dracut/ostree.conf +++ b/src/boot/dracut/ostree.conf @@ -16,3 +16,4 @@ # Boston, MA 02111-1307, USA. add_dracutmodules+=" ostree systemd " +reproducible=yes From 52a6224606e4a057b806225d93a2d0239a004723 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 17 Aug 2020 09:48:09 -0400 Subject: [PATCH 16/23] lib/deploy: Clean up kargs override handling Tighten up how we handle kargs here so it's more clear. When we call `sysroot_finalize_deployment`, any karg overrides have already been set on the bootconfig object of the deployment. So re-setting it here is redundant and confusing. --- src/libostree/ostree-sysroot-deploy.c | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index cb593020..89e3b8e8 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -2770,7 +2770,6 @@ get_var_dfd (OstreeSysroot *self, static gboolean sysroot_finalize_deployment (OstreeSysroot *self, OstreeDeployment *deployment, - char **override_kernel_argv, OstreeDeployment *merge_deployment, GCancellable *cancellable, GError **error) @@ -2780,15 +2779,18 @@ sysroot_finalize_deployment (OstreeSysroot *self, if (!glnx_opendirat (self->sysroot_fd, deployment_path, TRUE, &deployment_dfd, error)) return FALSE; - /* Only use the merge if we didn't get an override */ - if (!override_kernel_argv && merge_deployment) + OstreeBootconfigParser *bootconfig = ostree_deployment_get_bootconfig (deployment); + + /* If the kargs weren't set yet, then just pick it up from the merge deployment. In the + * deploy path, overrides are set as part of sysroot_initialize_deployment(). In the + * finalize-staged path, they're set by OstreeSysroot when reading the staged GVariant. */ + if (merge_deployment && ostree_bootconfig_parser_get (bootconfig, "options") == NULL) { - /* Override the bootloader arguments */ OstreeBootconfigParser *merge_bootconfig = ostree_deployment_get_bootconfig (merge_deployment); if (merge_bootconfig) { - const char *opts = ostree_bootconfig_parser_get (merge_bootconfig, "options"); - ostree_bootconfig_parser_set (ostree_deployment_get_bootconfig (deployment), "options", opts); + const char *kargs = ostree_bootconfig_parser_get (merge_bootconfig, "options"); + ostree_bootconfig_parser_set (bootconfig, "options", kargs); } } @@ -2882,8 +2884,7 @@ ostree_sysroot_deploy_tree (OstreeSysroot *self, &deployment, cancellable, error)) return FALSE; - if (!sysroot_finalize_deployment (self, deployment, override_kernel_argv, - provided_merge_deployment, + if (!sysroot_finalize_deployment (self, deployment, provided_merge_deployment, cancellable, error)) return FALSE; @@ -3149,8 +3150,6 @@ _ostree_sysroot_finalize_staged (OstreeSysroot *self, ostree_deployment_get_csum (merge_deployment_stub), ostree_deployment_get_deployserial (merge_deployment_stub)); } - g_autofree char **kargs = NULL; - g_variant_lookup (self->staged_deployment_data, "kargs", "^a&s", &kargs); /* Unlink the staged state now; if we're interrupted in the middle, * we don't want e.g. deal with the partially written /etc merge. @@ -3158,7 +3157,7 @@ _ostree_sysroot_finalize_staged (OstreeSysroot *self, if (!glnx_unlinkat (AT_FDCWD, _OSTREE_SYSROOT_RUNSTATE_STAGED, 0, error)) return FALSE; - if (!sysroot_finalize_deployment (self, self->staged_deployment, kargs, merge_deployment, + if (!sysroot_finalize_deployment (self, self->staged_deployment, merge_deployment, cancellable, error)) return FALSE; From 61c544df1bceeaa81e4ce98b587fad1e5198046d Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 17 Aug 2020 09:48:10 -0400 Subject: [PATCH 17/23] lib/deploy: Avoid shadowing variable There's already a `boot_relpath` variable in the outside scope. --- src/libostree/ostree-sysroot-deploy.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 89e3b8e8..0179873b 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -1935,8 +1935,9 @@ install_deployment_kernel (OstreeSysroot *sysroot, if (kernel_layout->initramfs_namever) { - g_autofree char * boot_relpath = g_strconcat ("/", bootcsumdir, "/", kernel_layout->initramfs_namever, NULL); - ostree_bootconfig_parser_set (bootconfig, "initrd", boot_relpath); + g_autofree char * initrd_boot_relpath = + g_strconcat ("/", bootcsumdir, "/", kernel_layout->initramfs_namever, NULL); + ostree_bootconfig_parser_set (bootconfig, "initrd", initrd_boot_relpath); } else { From 74bd1362861ed66200094b469ef40d7a1f818c9f Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 17 Aug 2020 09:48:11 -0400 Subject: [PATCH 18/23] lib/deploy: Simplify deployment creation Minor cleanup; we were declaring a superfluous variable. --- src/libostree/ostree-sysroot-deploy.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 0179873b..4b678890 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -2699,10 +2699,8 @@ sysroot_initialize_deployment (OstreeSysroot *self, cancellable, error)) return FALSE; - g_autofree char *new_bootcsum = NULL; g_autoptr(OstreeDeployment) new_deployment = - ostree_deployment_new (0, osname, revision, new_deployserial, - new_bootcsum, -1); + ostree_deployment_new (0, osname, revision, new_deployserial, NULL, -1); ostree_deployment_set_origin (new_deployment, origin); /* Check out the userspace tree onto the filesystem */ From e4fb7d3bb152f70ec82c33bf19089e9ac0bcc9c4 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 17 Aug 2020 09:48:12 -0400 Subject: [PATCH 19/23] lib/cleanup: Drop unnecessary GEqualFunc cast --- src/libostree/ostree-sysroot-cleanup.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-sysroot-cleanup.c b/src/libostree/ostree-sysroot-cleanup.c index 71d978e0..ffad4130 100644 --- a/src/libostree/ostree-sysroot-cleanup.c +++ b/src/libostree/ostree-sysroot-cleanup.c @@ -295,9 +295,9 @@ cleanup_old_deployments (OstreeSysroot *self, /* Load all active deployments referenced by bootloader configuration. */ g_autoptr(GHashTable) active_deployment_dirs = - g_hash_table_new_full (g_str_hash, (GEqualFunc)g_str_equal, g_free, NULL); + g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL); g_autoptr(GHashTable) active_boot_checksums = - g_hash_table_new_full (g_str_hash, (GEqualFunc)g_str_equal, g_free, NULL); + g_hash_table_new_full (g_str_hash, g_str_equal, g_free, NULL); for (guint i = 0; i < self->deployments->len; i++) { OstreeDeployment *deployment = self->deployments->pdata[i]; From 5de3a9759fc4a89c3c5ae6c4720bf984a2688374 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 17 Aug 2020 09:48:13 -0400 Subject: [PATCH 20/23] lib/deploy: Drop unneccessary function arg --- src/libostree/ostree-sysroot-deploy.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index 4b678890..c4a0ca92 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -111,7 +111,6 @@ install_into_boot (OstreeRepo *repo, const char *src_subpath, int dest_dfd, const char *dest_subpath, - OstreeSysrootDebugFlags flags, GCancellable *cancellable, GError **error) { @@ -1798,7 +1797,6 @@ install_deployment_kernel (OstreeSysroot *sysroot, { if (!install_into_boot (repo, sepolicy, kernel_layout->boot_dfd, kernel_layout->kernel_srcpath, bootcsum_dfd, kernel_layout->kernel_namever, - sysroot->debug_flags, cancellable, error)) return FALSE; } @@ -1815,7 +1813,6 @@ install_deployment_kernel (OstreeSysroot *sysroot, { if (!install_into_boot (repo, sepolicy, kernel_layout->boot_dfd, kernel_layout->initramfs_srcpath, bootcsum_dfd, kernel_layout->initramfs_namever, - sysroot->debug_flags, cancellable, error)) return FALSE; } @@ -1832,7 +1829,6 @@ install_deployment_kernel (OstreeSysroot *sysroot, { if (!install_into_boot (repo, sepolicy, kernel_layout->boot_dfd, kernel_layout->devicetree_srcpath, bootcsum_dfd, kernel_layout->devicetree_namever, - sysroot->debug_flags, cancellable, error)) return FALSE; } @@ -1853,7 +1849,6 @@ install_deployment_kernel (OstreeSysroot *sysroot, { if (!install_into_boot (repo, sepolicy, kernel_layout->boot_dfd, kernel_layout->kernel_hmac_srcpath, bootcsum_dfd, kernel_layout->kernel_hmac_namever, - sysroot->debug_flags, cancellable, error)) return FALSE; } From 10a68cd26b82489898d81cb5c6e5121e0de31222 Mon Sep 17 00:00:00 2001 From: Jonathan Lebon Date: Mon, 17 Aug 2020 09:48:15 -0400 Subject: [PATCH 21/23] lib/deploy: Clarify comment re. staging API Don't mention deprecation in the description for `ostree_sysroot_deploy_tree` since there are legitimate use cases for it (e.g. to create the first deployment via `ostree admin deploy`). Instead, make the comment clearly redirect to the staging API when booted into the sysroot. --- src/libostree/ostree-sysroot-deploy.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/libostree/ostree-sysroot-deploy.c b/src/libostree/ostree-sysroot-deploy.c index c4a0ca92..8488111d 100644 --- a/src/libostree/ostree-sysroot-deploy.c +++ b/src/libostree/ostree-sysroot-deploy.c @@ -2856,8 +2856,8 @@ sysroot_finalize_deployment (OstreeSysroot *self, * Check out deployment tree with revision @revision, performing a 3 * way merge with @provided_merge_deployment for configuration. * - * While this API is not deprecated, you most likely want to use the - * ostree_sysroot_stage_tree() API. + * When booted into the sysroot, you should use the + * ostree_sysroot_stage_tree() API instead. */ gboolean ostree_sysroot_deploy_tree (OstreeSysroot *self, From 1101c02c2a0eca8c73b7f0c25ea187f74ab6834a Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sun, 31 May 2020 17:57:22 +0000 Subject: [PATCH 22/23] tests/inst: Add destructive test framework This adds infrastructure to the Rust test suite for destructive tests, and adds a new `transactionality` test which runs rpm-ostree in a loop (along with `ostree-finalize-staged`) and repeatedly uses either `kill -9`, `reboot` and `reboot -ff`. The main goal here is to flush out any "logic errors". So far I've validated that this passes a lot of cycles using ``` $ kola run --qemu-image=fastbuild-fedora-coreos-ostree-qemu.qcow2 ext.ostree.destructive-rs.transactionality --debug --multiply 8 --parallel 4 ``` a number of times. --- .cci.jenkinsfile | 2 + Makefile.am | 3 + tests/inst/Cargo.toml | 30 +- tests/inst/itest-macro/src/itest-macro.rs | 45 +- tests/inst/src/destructive.rs | 624 ++++++++++++++++++++++ tests/inst/src/insttest.rs | 46 -- tests/inst/src/insttestmain.rs | 113 ++++ tests/inst/src/repobin.rs | 2 +- tests/inst/src/rpmostree.rs | 33 ++ tests/inst/src/test.rs | 78 ++- tests/inst/src/treegen.rs | 148 +++++ tests/kolainst/.gitignore | 1 + tests/kolainst/Makefile | 6 +- tests/kolainst/destructive-stamp.ign | 17 + tests/kolainst/install-wrappers.sh | 17 + 15 files changed, 1079 insertions(+), 86 deletions(-) create mode 100644 tests/inst/src/destructive.rs delete mode 100644 tests/inst/src/insttest.rs create mode 100644 tests/inst/src/insttestmain.rs create mode 100644 tests/inst/src/rpmostree.rs create mode 100644 tests/inst/src/treegen.rs create mode 100644 tests/kolainst/.gitignore create mode 100644 tests/kolainst/destructive-stamp.ign create mode 100755 tests/kolainst/install-wrappers.sh diff --git a/.cci.jenkinsfile b/.cci.jenkinsfile index add8c35a..ac65b9c8 100644 --- a/.cci.jenkinsfile +++ b/.cci.jenkinsfile @@ -81,6 +81,8 @@ parallel fcos: { coreos-assembler buildextend-metal4k coreos-assembler buildextend-live --fast # Install the tests + # Build and install the tests + make -C tests/kolainst make -C tests/kolainst install """) } diff --git a/Makefile.am b/Makefile.am index cd04a055..87a705cc 100644 --- a/Makefile.am +++ b/Makefile.am @@ -43,6 +43,9 @@ AM_DISTCHECK_CONFIGURE_FLAGS += \ GITIGNOREFILES = aclocal.m4 build-aux/ buildutil/*.m4 config.h.in gtk-doc.make +# Generated by coreos-assembler build-fast and kola +GITIGNOREFILES += fastbuild-*.qcow2 _kola_temp/ + SUBDIRS += . if ENABLE_GTK_DOC diff --git a/tests/inst/Cargo.toml b/tests/inst/Cargo.toml index a3838922..d47db53c 100644 --- a/tests/inst/Cargo.toml +++ b/tests/inst/Cargo.toml @@ -6,17 +6,21 @@ edition = "2018" [[bin]] name = "ostree-test" -path = "src/insttest.rs" +path = "src/insttestmain.rs" [dependencies] clap = "2.32.0" -structopt = "0.2" +structopt = "0.3" +serde = "1.0.111" +serde_derive = "1.0.111" +serde_json = "1.0" commandspec = "0.12.2" anyhow = "1.0" tempfile = "3.1.0" +glib = "0.9.1" gio = "0.8" ostree = { version = "0.7.1", features = ["v2020_1"] } -libtest-mimic = "0.2.0" +libtest-mimic = "0.3.0" twoway = "0.2.1" hyper = "0.13" futures = "0.3.4" @@ -26,17 +30,21 @@ tokio = { version = "0.2", features = ["full"] } futures-util = "0.3.1" base64 = "0.12.0" procspawn = "0.8" -proc-macro2 = "0.4" -quote = "0.6" -syn = "0.15" +rand = "0.7.3" linkme = "0.2" +strum = "0.18.0" +strum_macros = "0.18.0" +openat = "0.1.19" +openat-ext = "0.1.4" +nix = "0.17.0" +# This one I might publish to crates.io, not sure yet +with-procspawn-tempdir = { git = "https://github.com/cgwalters/with-procspawn-tempdir" } + +# Internal crate for the test macro itest-macro = { path = "itest-macro" } -with-procspawn-tempdir = { git = "https://github.com/cgwalters/with-procspawn-tempdir" } -#with-procspawn-tempdir = { path = "/var/srv/walters/src/github/cgwalters/with-procspawn-tempdir" } - -# See https://github.com/tcr/commandspec/pulls?q=is%3Apr+author%3Acgwalters+ [patch.crates-io] +# See https://github.com/tcr/commandspec/pulls?q=is%3Apr+author%3Acgwalters+ +# If patches don't get reviewed I'll probably fork it. commandspec = { git = "https://github.com/cgwalters/commandspec", branch = 'walters-master' } -#commandspec = { path = "/var/srv/walters/src/github/tcr/commandspec" } diff --git a/tests/inst/itest-macro/src/itest-macro.rs b/tests/inst/itest-macro/src/itest-macro.rs index 42b99581..34d35a1a 100644 --- a/tests/inst/itest-macro/src/itest-macro.rs +++ b/tests/inst/itest-macro/src/itest-macro.rs @@ -9,18 +9,57 @@ use quote::quote; #[proc_macro_attribute] pub fn itest(attrs: TokenStream, input: TokenStream) -> TokenStream { let attrs = syn::parse_macro_input!(attrs as syn::AttributeArgs); - if attrs.len() > 0 { - return syn::Error::new_spanned(&attrs[0], "itest takes no attributes") + if attrs.len() > 1 { + return syn::Error::new_spanned(&attrs[1], "itest takes 0 or 1 attributes") .to_compile_error() .into(); } + let destructive = match attrs.get(0) { + Some(syn::NestedMeta::Meta(syn::Meta::NameValue(namevalue))) => { + if let Some(name) = namevalue.path.get_ident().map(|i| i.to_string()) { + if name == "destructive" { + match &namevalue.lit { + syn::Lit::Bool(v) => v.value, + _ => { + return syn::Error::new_spanned( + &attrs[1], + format!("destructive must be bool {}", name), + ) + .to_compile_error() + .into(); + } + } + } else { + return syn::Error::new_spanned( + &attrs[1], + format!("Unknown argument {}", name), + ) + .to_compile_error() + .into(); + } + } else { + false + } + } + Some(v) => { + return syn::Error::new_spanned(&v, "Unexpected argument") + .to_compile_error() + .into() + } + None => false, + }; let func = syn::parse_macro_input!(input as syn::ItemFn); let fident = func.sig.ident.clone(); let varident = quote::format_ident!("ITEST_{}", fident); let fidentstrbuf = format!(r#"{}"#, fident); let fidentstr = syn::LitStr::new(&fidentstrbuf, Span::call_site()); + let testident = if destructive { + quote::format_ident!("{}", "DESTRUCTIVE_TESTS") + } else { + quote::format_ident!("{}", "NONDESTRUCTIVE_TESTS") + }; let output = quote! { - #[linkme::distributed_slice(TESTS)] + #[linkme::distributed_slice(#testident)] #[allow(non_upper_case_globals)] static #varident : Test = Test { name: #fidentstr, diff --git a/tests/inst/src/destructive.rs b/tests/inst/src/destructive.rs new file mode 100644 index 00000000..4d22ea83 --- /dev/null +++ b/tests/inst/src/destructive.rs @@ -0,0 +1,624 @@ +//! Test that interrupting an upgrade is safe. +//! +//! This test builds on coreos-assembler's "external tests": +//! https://github.com/coreos/coreos-assembler/blob/master/mantle/kola/README-kola-ext.md +//! Key to this in particular is coreos-assembler implementing the Debian autopkgtest reboot API. +//! +//! The basic model of this test is: +//! +//! Copy the OS content in to an archive repository, and generate a "synthetic" +//! update for it by randomly mutating ELF files. Time how long upgrading +//! to that takes, to use as a baseline in a range of time we will target +//! for interrupt. +//! +//! Start a webserver, pointing rpm-ostree at the updated content. We +//! alternate between a few "interrupt strategies", from `kill -9` on +//! rpm-ostreed, or rebooting normally, or an immediate forced reboot +//! (with no filesystem sync). +//! +//! The state of the tests is passed by serializing JSON into the +//! AUTOPKGTEST_REBOOT_MARK. + +use anyhow::{Context, Result}; +use commandspec::sh_execute; +use rand::seq::SliceRandom; +use rand::Rng; +use serde::{Deserialize, Serialize}; +use std::collections::BTreeMap; +use std::io::Write; +use std::path::Path; +use std::time; +use strum::IntoEnumIterator; +use strum_macros::EnumIter; + +use crate::rpmostree; +use crate::test::*; + +const ORIGREF: &'static str = "orig-booted"; +const TESTREF: &'static str = "testcontent"; +const TDATAPATH: &'static str = "/var/tmp/ostree-test-transaction-data.json"; +const SRVREPO: &'static str = "/var/tmp/ostree-test-srv"; +// Percentage of ELF files to change per update +const TREEGEN_PERCENTAGE: u32 = 15; +/// Total number of reboots +const ITERATIONS: u32 = 10; +/// Try at most this number of times per iteration to interrupt +const ITERATION_RETRIES: u32 = 15; +// We mostly want to test forced interrupts since those are +// most likely to break. +const FORCE_INTERRUPT_PERCENTAGE: u32 = 85; +/// Multiply the average cycle time by this to ensure we sometimes +/// fail to interrupt too. +const FORCE_REBOOT_AFTER_MUL: f64 = 1.1f64; +/// Amount of time in seconds we will delay each web request. +/// FIXME: this should be a function of total number of objects or so +const WEBSERVER_DELAY_SECS: f64 = 0.005; + +/// We choose between these at random +#[derive(EnumIter, Debug, PartialEq, Eq, PartialOrd, Ord, Clone, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +enum PoliteInterruptStrategy { + None, + Stop, + Reboot, +} + +/// We choose between these at random +#[derive(EnumIter, Debug, PartialEq, Eq, Clone, PartialOrd, Ord, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +enum ForceInterruptStrategy { + Kill9, + Reboot, +} + +#[derive(Debug, PartialEq, Eq, Clone, PartialOrd, Ord, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +enum InterruptStrategy { + Polite(PoliteInterruptStrategy), + Force(ForceInterruptStrategy), +} + +#[derive(Debug, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] +#[serde(rename_all = "kebab-case")] +enum UpdateResult { + NotCompleted, + Staged, + Completed, +} + +/// The data passed across reboots by serializing +/// into the AUTOPKGTEST_REBOOT_MARK +#[derive(Serialize, Deserialize, Debug, Default)] +#[serde(rename_all = "kebab-case")] +struct RebootMark { + /// Reboot strategy that was used for this last reboot + reboot_strategy: Option, + /// Counts attempts to interrupt an upgrade + iter: u32, + /// Counts times upgrade completed before we tried to interrupt + before: u32, + /// Results for "polite" interrupt attempts + polite: BTreeMap>, + /// Results for "forced" interrupt attempts + force: BTreeMap>, +} + +impl RebootMark { + fn get_results_map( + &mut self, + strategy: &InterruptStrategy, + ) -> &mut BTreeMap { + match strategy { + InterruptStrategy::Polite(t) => self + .polite + .entry(t.clone()) + .or_insert_with(|| BTreeMap::new()), + InterruptStrategy::Force(t) => self + .force + .entry(t.clone()) + .or_insert_with(|| BTreeMap::new()), + } + } +} + +impl InterruptStrategy { + pub(crate) fn is_noop(&self) -> bool { + match self { + InterruptStrategy::Polite(PoliteInterruptStrategy::None) => true, + _ => false, + } + } +} + +/// TODO add readonly sysroot handling into base ostree +fn testinit() -> Result<()> { + assert!(std::path::Path::new("/run/ostree-booted").exists()); + sh_execute!( + r"if ! test -w /sysroot; then + mount -o remount,rw /sysroot +fi" + )?; + Ok(()) +} + +/// Given a booted ostree, generate a modified version and write it +/// into our srvrepo. This is fairly hacky; it'd be better if we +/// reworked the tree mutation to operate on an ostree repo +/// rather than a filesystem. +fn generate_update(commit: &str) -> Result<()> { + println!("Generating update from {}", commit); + crate::treegen::update_os_tree(SRVREPO, TESTREF, TREEGEN_PERCENTAGE) + .context("Failed to generate new content")?; + // Amortize the prune across multiple runs; we don't want to leak space, + // but traversing all the objects is expensive. So here we only prune 1/5 of the time. + if rand::thread_rng().gen_ratio(1, 5) { + sh_execute!( + "ostree --repo={srvrepo} prune --refs-only --depth=1", + srvrepo = SRVREPO + )?; + } + Ok(()) +} + +/// Create an archive repository of current OS content. This is a bit expensive; +/// in the future we should try a trick using the `parent` property on this repo, +/// and then teach our webserver to redirect to the system for objects it doesn't +/// have. +fn generate_srv_repo(commit: &str) -> Result<()> { + sh_execute!( + r#" + ostree --repo={srvrepo} init --mode=archive + ostree --repo={srvrepo} config set archive.zlib-level 1 + ostree --repo={srvrepo} pull-local /sysroot/ostree/repo {commit} + ostree --repo={srvrepo} refs --create={testref} {commit} + "#, + srvrepo = SRVREPO, + commit = commit, + testref = TESTREF + ) + .context("Failed to generate srv repo")?; + generate_update(commit)?; + Ok(()) +} + +#[derive(Serialize, Deserialize, Debug)] +struct TransactionalTestInfo { + cycle_time: time::Duration, +} + +#[derive(Serialize, Deserialize, Debug, Default)] +struct Kill9Stats { + interrupted: u32, + staged: u32, + success: u32, +} + +#[derive(Serialize, Deserialize, Debug, Default)] +struct RebootStats { + interrupted: u32, + success: u32, +} + +fn upgrade_and_finalize() -> Result<()> { + sh_execute!( + "rpm-ostree upgrade + systemctl start ostree-finalize-staged + systemctl stop ostree-finalize-staged" + ) + .context("Upgrade and finalize failed")?; + Ok(()) +} + +async fn run_upgrade_or_timeout(timeout: time::Duration) -> Result { + let upgrade = tokio::task::spawn_blocking(upgrade_and_finalize); + Ok(tokio::select! { + res = upgrade => { + let _res = res?; + true + }, + _ = tokio::time::delay_for(timeout) => { + false + } + }) +} + +/// The set of commits that we should see +#[derive(Debug)] +struct CommitStates { + booted: String, + orig: String, + prev: String, + target: String, +} + +impl CommitStates { + pub(crate) fn describe(&self, commit: &str) -> Option<&'static str> { + if commit == self.booted { + Some("booted") + } else if commit == self.orig { + Some("orig") + } else if commit == self.prev { + Some("prev") + } else if commit == self.target { + Some("target") + } else { + None + } + } +} + +/// In the case where we've entered via a reboot, this function +/// checks the state of things, and also generates a new update +/// if everything was successful. +fn parse_and_validate_reboot_mark>( + commitstates: &mut CommitStates, + mark: M, +) -> Result { + let markstr = mark.as_ref(); + let mut mark: RebootMark = serde_json::from_str(markstr) + .with_context(|| format!("Failed to parse reboot mark {:?}", markstr))?; + // The first failed reboot may be into the original booted commit + let status = rpmostree::query_status()?; + let firstdeploy = &status.deployments[0]; + // The first deployment should not be staged + assert!(!firstdeploy.staged.unwrap_or(false)); + assert!(firstdeploy.booted); + assert_eq!(firstdeploy.checksum, commitstates.booted); + let reboot_type = if let Some(t) = mark.reboot_strategy.as_ref() { + t.clone() + } else { + anyhow::bail!("No reboot strategy in mark"); + }; + if commitstates.booted == commitstates.target { + mark.get_results_map(&reboot_type) + .entry(UpdateResult::Completed) + .and_modify(|result_e| { + *result_e += 1; + }) + .or_insert(1); + println!("Successfully updated to {}", commitstates.target); + // Since we successfully updated, generate a new commit to target + generate_update(&firstdeploy.checksum)?; + // Update the target state + let srvrepo_obj = ostree::Repo::new(&gio::File::new_for_path(SRVREPO)); + srvrepo_obj.open(gio::NONE_CANCELLABLE)?; + commitstates.target = srvrepo_obj.resolve_rev(TESTREF, false)?.into(); + } else if commitstates.booted == commitstates.orig || commitstates.booted == commitstates.prev { + println!( + "Failed update to {} (booted={})", + commitstates.target, commitstates.booted + ); + mark.get_results_map(&reboot_type) + .entry(UpdateResult::NotCompleted) + .and_modify(|result_e| { + *result_e += 1; + }) + .or_insert(1); + } else { + anyhow::bail!("Unexpected target commit: {}", firstdeploy.checksum); + }; + // Empty this out + mark.reboot_strategy = None; + Ok(mark) +} + +fn validate_pending_commit(pending_commit: &str, commitstates: &CommitStates) -> Result<()> { + if pending_commit != commitstates.target { + sh_execute!("rpm-ostree status -v")?; + sh_execute!( + "ostree show {pending_commit}", + pending_commit = pending_commit + )?; + anyhow::bail!( + "Expected target commit={} but pending={} ({:?})", + commitstates.target, + pending_commit, + commitstates.describe(pending_commit) + ); + } + Ok(()) +} + +/// In the case where we did a kill -9 of rpm-ostree, check the state +fn validate_live_interrupted_upgrade(commitstates: &CommitStates) -> Result { + let status = rpmostree::query_status()?; + let firstdeploy = &status.deployments[0]; + let pending_commit = firstdeploy.checksum.as_str(); + let res = if firstdeploy.staged.unwrap_or(false) { + assert!(!firstdeploy.booted); + validate_pending_commit(pending_commit, &commitstates)?; + UpdateResult::Staged + } else { + if pending_commit == commitstates.booted { + UpdateResult::NotCompleted + } else if pending_commit == commitstates.target { + UpdateResult::Completed + } else { + anyhow::bail!( + "Unexpected pending commit: {} ({:?})", + pending_commit, + commitstates.describe(pending_commit) + ); + } + }; + Ok(res) +} + +fn impl_transaction_test>( + booted_commit: &str, + tdata: &TransactionalTestInfo, + mark: Option, +) -> Result<()> { + let polite_strategies = PoliteInterruptStrategy::iter().collect::>(); + let force_strategies = ForceInterruptStrategy::iter().collect::>(); + + // Gather the expected possible commits + let mut commitstates = { + let srvrepo_obj = ostree::Repo::new(&gio::File::new_for_path(SRVREPO)); + srvrepo_obj.open(gio::NONE_CANCELLABLE)?; + let sysrepo_obj = ostree::Repo::new(&gio::File::new_for_path("/sysroot/ostree/repo")); + sysrepo_obj.open(gio::NONE_CANCELLABLE)?; + + CommitStates { + booted: booted_commit.to_string(), + orig: sysrepo_obj.resolve_rev(ORIGREF, false)?.into(), + prev: srvrepo_obj + .resolve_rev(&format!("{}^", TESTREF), false)? + .into(), + target: srvrepo_obj.resolve_rev(TESTREF, false)?.into(), + } + }; + + let mut mark = if let Some(mark) = mark { + let markstr = mark.as_ref(); + // In the successful case, this generates a new target commit, + // so we pass via &mut. + parse_and_validate_reboot_mark(&mut commitstates, markstr) + .context("Failed to parse reboot mark")? + } else { + RebootMark { + ..Default::default() + } + }; + // Drop the &mut + let commitstates = commitstates; + + assert_ne!(commitstates.booted.as_str(), commitstates.target.as_str()); + + let mut rt = tokio::runtime::Runtime::new()?; + let cycle_time_ms = (tdata.cycle_time.as_secs_f64() * 1000f64 * FORCE_REBOOT_AFTER_MUL) as u64; + // Set when we're trying an interrupt strategy that isn't a reboot, so we will + // re-enter the loop below. + let mut live_strategy: Option = None; + let mut retries = 0; + // This loop is for the non-rebooting strategies - we might use kill -9 + // or not interrupt at all. But if we choose a reboot strategy + // then we'll exit implicitly via the reboot, and reenter the function + // above. + loop { + // Save the previous strategy as a string so we can use it in error + // messages below + let prev_strategy_str = format!("{:?}", live_strategy); + // Process the results of the previous run if any, and reset + // live_strategy to None + if let Some(last_strategy) = live_strategy.take() { + mark.iter += 1; + retries = 0; + let res = validate_live_interrupted_upgrade(&commitstates)?; + if last_strategy.is_noop() { + assert_eq!(res, UpdateResult::Completed) + } + mark.get_results_map(&last_strategy) + .entry(res) + .and_modify(|result_e| { + *result_e += 1; + }) + .or_insert(1); + } + // If we've reached our target iterations, exit the test successfully + if mark.iter == ITERATIONS { + // TODO also add ostree admin fsck to check the deployment directories + sh_execute!( + "echo Performing final validation... + ostree fsck" + )?; + return Ok(()); + } + let mut rng = rand::thread_rng(); + // Pick a strategy for this attempt + let strategy: InterruptStrategy = if rand::thread_rng() + .gen_ratio(FORCE_INTERRUPT_PERCENTAGE, 100) + { + InterruptStrategy::Force(force_strategies.choose(&mut rng).expect("strategy").clone()) + } else { + InterruptStrategy::Polite( + polite_strategies + .choose(&mut rng) + .expect("strategy") + .clone(), + ) + }; + println!("Using interrupt strategy: {:?}", strategy); + // Interrupt usually before the upgrade would + // complete, but also a percentage of the time after. + // The no-op case is special in that we want to wait for it to complete + let sleeptime = if strategy.is_noop() { + // In the no-op case, sleep for minimum of 20x the cycle time, or one day + let ms = std::cmp::min(cycle_time_ms.saturating_mul(20), 24 * 60 * 60 * 1000); + time::Duration::from_millis(ms) + } else { + time::Duration::from_millis(rng.gen_range(0, cycle_time_ms)) + }; + println!( + "force-reboot-time={:?} cycle={:?} status:{:?}", + sleeptime, tdata.cycle_time, &mark + ); + // Reset the target ref to booted, and perform a cleanup + // to ensure we're re-downloading objects each time + sh_execute!( + " + systemctl stop rpm-ostreed + systemctl stop ostree-finalize-staged + ostree reset testrepo:{testref} {booted_commit} + rpm-ostree cleanup -pbrm + ", + testref = TESTREF, + booted_commit = booted_commit + ) + .with_context(|| { + format!( + "Failed pre-upgrade cleanup (prev strategy: {})", + prev_strategy_str.as_str() + ) + })?; + + // The heart of the test - start an upgrade and wait a random amount + // of time to interrupt. If the result is true, then the upgrade completed + // successfully before the timeout. + let res: Result = rt.block_on(async move { run_upgrade_or_timeout(sleeptime).await }); + let res = res.context("Failed during upgrade")?; + if res { + if !strategy.is_noop() { + println!( + "Failed to interrupt upgrade, attempt {}/{}", + retries, ITERATION_RETRIES + ); + retries += 1; + mark.before += 1; + } else { + live_strategy = Some(strategy); + } + let status = rpmostree::query_status()?; + let firstdeploy = &status.deployments[0]; + let pending_commit = firstdeploy.checksum.as_str(); + validate_pending_commit(pending_commit, &commitstates) + .context("Failed to validate pending commit")?; + } else { + // Our timeout fired before the upgrade completed; execute + // the interrupt strategy. + match strategy { + InterruptStrategy::Force(ForceInterruptStrategy::Kill9) => { + sh_execute!( + "systemctl kill -s KILL rpm-ostreed || true + systemctl kill -s KILL ostree-finalize-staged || true" + )?; + live_strategy = Some(strategy); + } + InterruptStrategy::Force(ForceInterruptStrategy::Reboot) => { + mark.reboot_strategy = Some(strategy.clone()); + prepare_reboot(serde_json::to_string(&mark)?)?; + // This is a forced reboot - no syncing of the filesystem. + sh_execute!("reboot -ff")?; + std::thread::sleep(time::Duration::from_secs(60)); + // Shouldn't happen + anyhow::bail!("failed to reboot"); + } + InterruptStrategy::Polite(PoliteInterruptStrategy::None) => { + anyhow::bail!("Failed to wait for uninterrupted upgrade"); + } + InterruptStrategy::Polite(PoliteInterruptStrategy::Reboot) => { + mark.reboot_strategy = Some(strategy.clone()); + Err(reboot(serde_json::to_string(&mark)?))?; + // We either rebooted, or failed to reboot + } + InterruptStrategy::Polite(PoliteInterruptStrategy::Stop) => { + sh_execute!( + "systemctl stop rpm-ostreed || true + systemctl stop ostree-finalize-staged || true" + )?; + live_strategy = Some(strategy); + } + } + } + } +} + +#[itest(destructive = true)] +fn transactionality() -> Result<()> { + testinit()?; + let mark = get_reboot_mark()?; + let cancellable = Some(gio::Cancellable::new()); + let sysroot = ostree::Sysroot::new_default(); + sysroot.load(cancellable.as_ref())?; + assert!(sysroot.is_booted()); + let booted = sysroot.get_booted_deployment().expect("booted deployment"); + let commit: String = booted.get_csum().expect("booted csum").into(); + // We need this static across reboots + let srvrepo = Path::new(SRVREPO); + let firstrun = !srvrepo.exists(); + if let Some(_) = mark.as_ref() { + if firstrun { + anyhow::bail!("Missing {:?}", srvrepo); + } + } else { + if !firstrun { + anyhow::bail!("Unexpected {:?}", srvrepo); + } + generate_srv_repo(&commit)?; + } + + // Let's assume we're changing about 200 objects each time; + // that leads to probably 300 network requests, so we want + // a low average delay. + let webserver_opts = TestHttpServerOpts { + random_delay: Some(time::Duration::from_secs_f64(WEBSERVER_DELAY_SECS)), + ..Default::default() + }; + with_webserver_in(&srvrepo, &webserver_opts, move |addr| { + let url = format!("http://{}", addr); + sh_execute!( + "ostree remote delete --if-exists testrepo + ostree remote add --set=gpg-verify=false testrepo {url}", + url = url + )?; + + if firstrun { + // Also disable some services (like zincati) because we don't want automatic updates + // in our reboots, and it currently fails to start. The less + // we have in each reboot, the faster reboots are. + sh_execute!("systemctl disable --now zincati fedora-coreos-pinger")?; + // And prepare for updates + sh_execute!("rpm-ostree cleanup -pr")?; + generate_update(&commit)?; + // Directly set the origin, so that we're not dependent on the pending deployment. + // FIXME: make this saner + sh_execute!( + " + ostree admin set-origin testrepo {url} {testref} + ostree refs --create testrepo:{testref} {commit} + ostree refs --create={origref} {commit} + ", + url = url, + origref = ORIGREF, + testref = TESTREF, + commit = commit + )?; + // We gather a single "cycle time" at start as a way of gauging how + // long an upgrade should take, so we know when to interrupt. This + // obviously has some pitfalls, mainly when there are e.g. other competing + // VMs when we start but not after (or vice versa) we can either + // interrupt almost always too early, or too late. + let start = time::Instant::now(); + upgrade_and_finalize().context("Firstrun upgrade failed")?; + let end = time::Instant::now(); + let cycle_time = end.duration_since(start); + let tdata = TransactionalTestInfo { + cycle_time: cycle_time, + }; + let mut f = std::io::BufWriter::new(std::fs::File::create(&TDATAPATH)?); + serde_json::to_writer(&mut f, &tdata)?; + f.flush()?; + sh_execute!("rpm-ostree status")?; + } + + let tdata = { + let mut f = std::io::BufReader::new(std::fs::File::open(&TDATAPATH)?); + serde_json::from_reader(&mut f).context("Failed to parse test info JSON")? + }; + + impl_transaction_test(commit.as_str(), &tdata, mark.as_ref())?; + + Ok(()) + })?; + Ok(()) +} diff --git a/tests/inst/src/insttest.rs b/tests/inst/src/insttest.rs deleted file mode 100644 index 1c1fa379..00000000 --- a/tests/inst/src/insttest.rs +++ /dev/null @@ -1,46 +0,0 @@ -use anyhow::Result; -// use structopt::StructOpt; -// // https://github.com/clap-rs/clap/pull/1397 -// #[macro_use] -// extern crate clap; - -mod repobin; -mod sysroot; -mod test; - -fn gather_tests() -> Vec { - test::TESTS - .iter() - .map(|t| libtest_mimic::Test { - name: t.name.into(), - kind: "".into(), - is_ignored: false, - is_bench: false, - data: t, - }) - .collect() -} - -fn run_test(test: &test::TestImpl) -> libtest_mimic::Outcome { - if let Err(e) = (test.data.f)() { - libtest_mimic::Outcome::Failed { - msg: Some(e.to_string()), - } - } else { - libtest_mimic::Outcome::Passed - } -} - -fn main() -> Result<()> { - procspawn::init(); - - // Ensure we're always in tempdir so we can rely on it globally - let tmp_dir = tempfile::Builder::new() - .prefix("ostree-insttest-top") - .tempdir()?; - std::env::set_current_dir(tmp_dir.path())?; - - let args = libtest_mimic::Arguments::from_args(); - let tests = gather_tests(); - libtest_mimic::run_tests(&args, tests, run_test).exit(); -} diff --git a/tests/inst/src/insttestmain.rs b/tests/inst/src/insttestmain.rs new file mode 100644 index 00000000..3fdc1be1 --- /dev/null +++ b/tests/inst/src/insttestmain.rs @@ -0,0 +1,113 @@ +use anyhow::{bail, Result}; +use structopt::StructOpt; + +mod destructive; +mod repobin; +mod rpmostree; +mod sysroot; +mod test; +mod treegen; + +// Written by Ignition +const DESTRUCTIVE_TEST_STAMP: &'static str = "/etc/ostree-destructive-test-ok"; + +#[derive(Debug, StructOpt)] +#[structopt(rename_all = "kebab-case")] +/// Main options struct +enum Opt { + /// List the destructive tests + ListDestructive, + /// Run a destructive test (requires ostree-based host, may break it!) + RunDestructive { name: String }, + /// Run the non-destructive tests + NonDestructive(NonDestructiveOpts), +} + +#[derive(Debug, StructOpt)] +#[structopt(rename_all = "kebab-case")] +enum NonDestructiveOpts { + #[structopt(external_subcommand)] + Args(Vec), +} + +fn libtest_from_test(t: &'static test::Test) -> test::TestImpl { + libtest_mimic::Test { + name: t.name.into(), + kind: "".into(), + is_ignored: false, + is_bench: false, + data: t, + } +} + +fn run_test(test: &test::TestImpl) -> libtest_mimic::Outcome { + if let Err(e) = (test.data.f)() { + libtest_mimic::Outcome::Failed { + msg: Some(e.to_string()), + } + } else { + libtest_mimic::Outcome::Passed + } +} + +fn main() -> Result<()> { + // Ensure we're always in tempdir so we can rely on it globally. + // We use /var/tmp to ensure we have storage space in the destructive + // case. + let tmp_dir = tempfile::Builder::new() + .prefix("ostree-insttest-top") + .tempdir_in("/var/tmp")?; + std::env::set_current_dir(tmp_dir.path())?; + + procspawn::init(); + let args: Vec = std::env::args().collect(); + let opt = { + if args.len() == 1 { + println!("No arguments provided, running non-destructive tests"); + Opt::NonDestructive(NonDestructiveOpts::Args(Vec::new())) + } else { + Opt::from_iter(args.iter()) + } + }; + + match opt { + Opt::ListDestructive => { + for t in test::DESTRUCTIVE_TESTS.iter() { + println!("{}", t.name); + } + return Ok(()); + } + Opt::NonDestructive(subopt) => { + // FIXME add method to parse subargs + let iter = match subopt { + NonDestructiveOpts::Args(subargs) => subargs, + }; + let libtestargs = libtest_mimic::Arguments::from_iter(iter); + let tests: Vec<_> = test::NONDESTRUCTIVE_TESTS + .iter() + .map(libtest_from_test) + .collect(); + libtest_mimic::run_tests(&libtestargs, tests, run_test).exit(); + } + Opt::RunDestructive { name } => { + if !std::path::Path::new(DESTRUCTIVE_TEST_STAMP).exists() { + bail!( + "This is a destructive test; signal acceptance by creating {}", + DESTRUCTIVE_TEST_STAMP + ) + } + if !std::path::Path::new("/run/ostree-booted").exists() { + bail!("An ostree-based host is required") + } + + for t in test::DESTRUCTIVE_TESTS.iter() { + if t.name == name { + (t.f)()?; + println!("ok destructive test: {}", t.name); + return Ok(()); + } + } + bail!("Unknown destructive test: {}", name); + } + } +} diff --git a/tests/inst/src/repobin.rs b/tests/inst/src/repobin.rs index 41fd1390..208eae40 100644 --- a/tests/inst/src/repobin.rs +++ b/tests/inst/src/repobin.rs @@ -77,7 +77,7 @@ fn test_pull_basicauth() -> Result<()> { format!("http://{}@{}/", TEST_HTTP_BASIC_AUTH, addr).into_bytes(), )?; let osroot = Path::new("osroot"); - mkroot(&osroot)?; + crate::treegen::mkroot(&osroot)?; sh_execute!( r#"ostree --repo={serverrepo} init --mode=archive ostree --repo={serverrepo} commit -b os --tree=dir={osroot} >/dev/null diff --git a/tests/inst/src/rpmostree.rs b/tests/inst/src/rpmostree.rs new file mode 100644 index 00000000..fee97355 --- /dev/null +++ b/tests/inst/src/rpmostree.rs @@ -0,0 +1,33 @@ +use anyhow::Result; +use serde_derive::Deserialize; +use serde_json; +use std::process::{Command, Stdio}; + +#[derive(Deserialize)] +#[serde(rename_all = "kebab-case")] +#[allow(unused)] +pub(crate) struct Status { + pub(crate) deployments: Vec, +} + +#[derive(Deserialize)] +#[serde(rename_all = "kebab-case")] +#[allow(unused)] +pub(crate) struct Deployment { + pub(crate) unlocked: Option, + pub(crate) osname: String, + pub(crate) pinned: bool, + pub(crate) checksum: String, + pub(crate) staged: Option, + pub(crate) booted: bool, + pub(crate) serial: u32, + pub(crate) origin: String, +} + +pub(crate) fn query_status() -> Result { + let cmd = Command::new("rpm-ostree") + .args(&["status", "--json"]) + .stdout(Stdio::piped()) + .spawn()?; + Ok(serde_json::from_reader(cmd.stdout.unwrap())?) +} diff --git a/tests/inst/src/test.rs b/tests/inst/src/test.rs index 7178d7bb..24dc8194 100644 --- a/tests/inst/src/test.rs +++ b/tests/inst/src/test.rs @@ -3,9 +3,11 @@ use std::fs::File; use std::io::prelude::*; use std::path::Path; use std::process::Command; +use std::time; use anyhow::{bail, Context, Result}; use linkme::distributed_slice; +use rand::Rng; pub use itest_macro::itest; pub use with_procspawn_tempdir::with_procspawn_tempdir; @@ -28,7 +30,9 @@ pub(crate) struct Test { pub(crate) type TestImpl = libtest_mimic::Test<&'static Test>; #[distributed_slice] -pub(crate) static TESTS: [Test] = [..]; +pub(crate) static NONDESTRUCTIVE_TESTS: [Test] = [..]; +#[distributed_slice] +pub(crate) static DESTRUCTIVE_TESTS: [Test] = [..]; /// Run command and assert that its stderr contains pat pub(crate) fn cmd_fails_with>(mut c: C, pat: &str) -> Result<()> { @@ -53,30 +57,10 @@ pub(crate) fn write_file>(p: P, buf: &str) -> Result<()> { Ok(()) } -pub(crate) fn mkroot>(p: P) -> Result<()> { - let p = p.as_ref(); - for v in &["usr/bin", "etc"] { - std::fs::create_dir_all(p.join(v))?; - } - let verpath = p.join("etc/version"); - let v: u32 = if verpath.exists() { - let s = std::fs::read_to_string(&verpath)?; - let v: u32 = s.trim_end().parse()?; - v + 1 - } else { - 0 - }; - write_file(&verpath, &format!("{}", v))?; - write_file(p.join("usr/bin/somebinary"), &format!("somebinary v{}", v))?; - write_file(p.join("etc/someconf"), &format!("someconf v{}", v))?; - write_file(p.join("usr/bin/vmod2"), &format!("somebinary v{}", v % 2))?; - write_file(p.join("usr/bin/vmod3"), &format!("somebinary v{}", v % 3))?; - Ok(()) -} - #[derive(Default, Debug, Copy, Clone)] pub(crate) struct TestHttpServerOpts { pub(crate) basicauth: bool, + pub(crate) random_delay: Option, } pub(crate) const TEST_HTTP_BASIC_AUTH: &'static str = "foouser:barpw"; @@ -105,6 +89,11 @@ pub(crate) async fn http_server>( sv: Static, opts: TestHttpServerOpts, ) -> Result> { + if let Some(random_delay) = opts.random_delay { + let slices = 100u32; + let n: u32 = rand::thread_rng().gen_range(0, slices); + std::thread::sleep((random_delay / slices) * n); + } if opts.basicauth { if let Some(ref authz) = req.headers().get(http::header::AUTHORIZATION) { match validate_authz(authz.as_ref()) { @@ -149,7 +138,8 @@ pub(crate) async fn http_server>( pub(crate) fn with_webserver_in, F>( path: P, opts: &TestHttpServerOpts, - f: F) -> Result<()> + f: F, +) -> Result<()> where F: FnOnce(&std::net::SocketAddr) -> Result<()>, F: Send + 'static, @@ -163,6 +153,48 @@ where Ok(()) } +/// Parse an environment variable as UTF-8 +pub(crate) fn getenv_utf8(n: &str) -> Result> { + if let Some(v) = std::env::var_os(n) { + Ok(Some( + v.to_str() + .ok_or_else(|| anyhow::anyhow!("{} is invalid UTF-8", n))? + .to_string(), + )) + } else { + Ok(None) + } +} + +/// Defined by the autopkgtest specification +pub(crate) fn get_reboot_mark() -> Result> { + getenv_utf8("AUTOPKGTEST_REBOOT_MARK") +} + +/// Initiate a clean reboot; on next boot get_reboot_mark() will return `mark`. +#[allow(dead_code)] +pub(crate) fn reboot>(mark: M) -> std::io::Error { + let mark = mark.as_ref(); + use std::os::unix::process::CommandExt; + std::process::Command::new("/tmp/autopkgtest-reboot") + .arg(mark) + .exec() +} + +/// Prepare a reboot - you should then initiate a reboot however you like. +/// On next boot get_reboot_mark() will return `mark`. +#[allow(dead_code)] +pub(crate) fn prepare_reboot>(mark: M) -> Result<()> { + let mark = mark.as_ref(); + let s = std::process::Command::new("/tmp/autopkgtest-reboot-prepare") + .arg(mark) + .status()?; + if !s.success() { + anyhow::bail!("{:?}", s); + } + Ok(()) +} + // I put tests in your tests so you can test while you test #[cfg(test)] mod tests { diff --git a/tests/inst/src/treegen.rs b/tests/inst/src/treegen.rs new file mode 100644 index 00000000..7c28fb70 --- /dev/null +++ b/tests/inst/src/treegen.rs @@ -0,0 +1,148 @@ +use anyhow::{Context, Result}; +use commandspec::sh_execute; +use openat_ext::{FileExt, OpenatDirExt}; +use rand::Rng; +use std::fs::File; +use std::io::prelude::*; +use std::os::unix::fs::FileExt as UnixFileExt; +use std::path::Path; + +use crate::test::*; + +/// Each time this is invoked it changes file contents +/// in the target root, in a predictable way. +pub(crate) fn mkroot>(p: P) -> Result<()> { + let p = p.as_ref(); + let verpath = p.join("etc/.mkrootversion"); + let v: u32 = if verpath.exists() { + let s = std::fs::read_to_string(&verpath)?; + let v: u32 = s.trim_end().parse()?; + v + 1 + } else { + 0 + }; + mkvroot(p, v) +} + +// Like mkroot but supports an explicit version +pub(crate) fn mkvroot>(p: P, v: u32) -> Result<()> { + let p = p.as_ref(); + for v in &["usr/bin", "etc"] { + std::fs::create_dir_all(p.join(v))?; + } + let verpath = p.join("etc/.mkrootversion"); + write_file(&verpath, &format!("{}", v))?; + write_file(p.join("usr/bin/somebinary"), &format!("somebinary v{}", v))?; + write_file(p.join("etc/someconf"), &format!("someconf v{}", v))?; + write_file(p.join("usr/bin/vmod2"), &format!("somebinary v{}", v % 2))?; + write_file(p.join("usr/bin/vmod3"), &format!("somebinary v{}", v % 3))?; + Ok(()) +} + +/// Returns `true` if a file is ELF; see https://en.wikipedia.org/wiki/Executable_and_Linkable_Format +pub(crate) fn is_elf(f: &mut File) -> Result { + let mut buf = [0; 5]; + let n = f.read_at(&mut buf, 0)?; + if n < buf.len() { + anyhow::bail!("Failed to read expected {} bytes", buf.len()); + } + Ok(buf[0] == 0x7F && &buf[1..4] == b"ELF") +} + +pub(crate) fn mutate_one_executable_to( + f: &mut File, + name: &std::ffi::OsStr, + dest: &openat::Dir, +) -> Result<()> { + let mut destf = dest + .write_file(name, 0o755) + .context("Failed to open for write")?; + f.copy_to(&destf).context("Failed to copy")?; + // ELF is OK with us just appending some junk + let extra = rand::thread_rng() + .sample_iter(&rand::distributions::Alphanumeric) + .take(10) + .collect::(); + destf + .write_all(extra.as_bytes()) + .context("Failed to append extra data")?; + Ok(()) +} + +/// Find ELF files in the srcdir, write new copies to dest (only percentage) +pub(crate) fn mutate_executables_to( + src: &openat::Dir, + dest: &openat::Dir, + percentage: u32, +) -> Result { + use nix::sys::stat::Mode as NixMode; + assert!(percentage > 0 && percentage <= 100); + let mut mutated = 0; + for entry in src.list_dir(".")? { + let entry = entry?; + if src.get_file_type(&entry)? != openat::SimpleType::File { + continue; + } + let meta = src.metadata(entry.file_name())?; + let st = meta.stat(); + let mode = NixMode::from_bits_truncate(st.st_mode); + // Must be executable + if !mode.intersects(NixMode::S_IXUSR | NixMode::S_IXGRP | NixMode::S_IXOTH) { + continue; + } + // Not suid + if mode.intersects(NixMode::S_ISUID | NixMode::S_ISGID) { + continue; + } + // Greater than 1k in size + if st.st_size < 1024 { + continue; + } + let mut f = src.open_file(entry.file_name())?; + if !is_elf(&mut f)? { + continue; + } + if !rand::thread_rng().gen_ratio(percentage, 100) { + continue; + } + mutate_one_executable_to(&mut f, entry.file_name(), dest) + .with_context(|| format!("Failed updating {:?}", entry.file_name()))?; + mutated += 1; + } + Ok(mutated) +} + +// Given an ostree ref, use the running root filesystem as a source, update +// `percentage` percent of binary (ELF) files +pub(crate) fn update_os_tree>( + repo_path: P, + ostref: &str, + percentage: u32, +) -> Result<()> { + assert!(percentage > 0 && percentage <= 100); + let repo_path = repo_path.as_ref(); + let tempdir = tempfile::tempdir_in(repo_path.join("tmp"))?; + let mut mutated = 0; + { + let tempdir = openat::Dir::open(tempdir.path())?; + let binary_dirs = &["usr/bin", "usr/sbin", "usr/lib", "usr/lib64"]; + let rootfs = openat::Dir::open("/")?; + for v in binary_dirs { + let v = *v; + if let Some(src) = rootfs.sub_dir_optional(v)? { + tempdir.ensure_dir("usr", 0o755)?; + tempdir.ensure_dir(v, 0o755)?; + let dest = tempdir.sub_dir(v)?; + mutated += mutate_executables_to(&src, &dest, percentage) + .with_context(|| format!("Replacing binaries in {}", v))?; + } + } + } + assert!(mutated > 0); + println!("Mutated ELF files: {}", mutated); + sh_execute!("ostree --repo={repo} commit --consume -b {ostref} --base={ostref} --tree=dir={tempdir} --owner-uid 0 --owner-gid 0 --selinux-policy-from-base --link-checkout-speedup --no-bindings --no-xattrs", + repo = repo_path.to_str().unwrap(), + ostref = ostref, + tempdir = tempdir.path().to_str().unwrap()).context("Failed to commit updated content")?; + Ok(()) +} diff --git a/tests/kolainst/.gitignore b/tests/kolainst/.gitignore new file mode 100644 index 00000000..97b5dac6 --- /dev/null +++ b/tests/kolainst/.gitignore @@ -0,0 +1 @@ +destructive-list.txt diff --git a/tests/kolainst/Makefile b/tests/kolainst/Makefile index 6416217e..acfdc3b7 100644 --- a/tests/kolainst/Makefile +++ b/tests/kolainst/Makefile @@ -7,9 +7,11 @@ KOLA_TESTDIR = $(DESTDIR)/usr/lib/coreos-assembler/tests/kola/ostree/ all: for x in $(LIBSCRIPTS); do bash -n "$${x}"; done - (cd ../inst && cargo build --release) + (cd ../inst && cargo run --release -- list-destructive) > destructive-list.txt -install: all +install: install -D -m 0644 -t $(KOLA_TESTDIR) $(LIBSCRIPTS) for x in $(TESTDIRS); do rsync -rlv ./$${x} $(KOLA_TESTDIR)/; done install -D -m 0755 -t $(KOLA_TESTDIR)/nondestructive-rs ../inst/target/release/ostree-test + install -D -m 0644 destructive-stamp.ign $(KOLA_TESTDIR)/destructive-rs/config.ign + ./install-wrappers.sh destructive-list.txt $(KOLA_TESTDIR)/destructive-rs diff --git a/tests/kolainst/destructive-stamp.ign b/tests/kolainst/destructive-stamp.ign new file mode 100644 index 00000000..7a4552fe --- /dev/null +++ b/tests/kolainst/destructive-stamp.ign @@ -0,0 +1,17 @@ +{ + "ignition": { + "version": "3.0.0" + }, + "storage": { + "files": [ + { + "path": "/etc/ostree-destructive-test-ok", + "filesystem": "root", + "mode": 420, + "contents": { + "source": "data:text/plain;base64," + } + } + ] + } +} diff --git a/tests/kolainst/install-wrappers.sh b/tests/kolainst/install-wrappers.sh new file mode 100755 index 00000000..4fbb05c6 --- /dev/null +++ b/tests/kolainst/install-wrappers.sh @@ -0,0 +1,17 @@ +#!/bin/bash +set -xeuo pipefail +# Generates a kola test for each destructive test in the binary +list=$1 +shift +testdir=$1 +shift +ln -Tsf ../nondestructive-rs ${testdir}/data +while read line; do + cat >${testdir}/${line} << EOF +#!/bin/bash +set -xeuo pipefail +dn=\$(dirname $0) +exec \${KOLA_EXT_DATA}/ostree-test run-destructive ${line} +EOF + chmod a+x "${testdir}/${line}" +done < "${list}" From 8715989df34e9f5253ade25ae502fa5560aa1516 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Tue, 18 Aug 2020 15:55:21 +0000 Subject: [PATCH 23/23] Release 2020.5 Mainly to get https://github.com/ostreedev/ostree/pull/2160 out. --- configure.ac | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/configure.ac b/configure.ac index 50646f1e..2eec7464 100644 --- a/configure.ac +++ b/configure.ac @@ -10,7 +10,7 @@ m4_define([year_version], [2020]) m4_define([release_version], [5]) m4_define([package_version], [year_version.release_version]) AC_INIT([libostree], [package_version], [walters@verbum.org]) -is_release_build=no +is_release_build=yes AC_CONFIG_HEADER([config.h]) AC_CONFIG_MACRO_DIR([buildutil]) AC_CONFIG_AUX_DIR([build-aux])