From 969f319e80088c7ee0fe88a209b7d4557a7d28b4 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Thu, 26 Jan 2017 23:31:53 -0500 Subject: [PATCH 1/4] Add `pending-base-commit` to status One thing that's very confusing about OSTree is there are two layers - deployments and the refs/commits. If one does an `rpm-ostree upgrade`, but then e.g. `ostree admin undeploy 0`, you still have the new revision in the repo. We don't do a good job of displaying this state, or helping people clean it up. Down the line, I also want to better support something like `rpm-ostree pull` to cache updates explicitly *without* deploying. This commit just adds a bit of information to the status display. We might want to have better formatting, but I think this an OK start. --- src/app/rpmostree-builtin-status.c | 38 +++++++++++++- src/daemon/rpmostreed-deployment-utils.c | 54 +++++++++++++++----- tests/common/libtest.sh | 13 +++++ tests/vmcheck/test-initramfs.sh | 63 +++++++++--------------- tests/vmcheck/test-layering-basic.sh | 4 ++ tests/vmcheck/test-layering-relayer.sh | 8 +++ 6 files changed, 126 insertions(+), 54 deletions(-) diff --git a/src/app/rpmostree-builtin-status.c b/src/app/rpmostree-builtin-status.c index 1c49479049..b14e34f18c 100644 --- a/src/app/rpmostree-builtin-status.c +++ b/src/app/rpmostree-builtin-status.c @@ -111,6 +111,7 @@ status_generic (RPMOSTreeSysroot *sysroot_proxy, { g_autoptr(GVariant) child = g_variant_iter_next_value (&iter); g_autoptr(GVariantDict) dict = NULL; + gboolean is_locally_assembled; const gchar *const*origin_packages = NULL; const gchar *origin_refspec; const gchar *id; @@ -123,7 +124,7 @@ status_generic (RPMOSTreeSysroot *sysroot_proxy, guint64 t = 0; int serial; gboolean is_booted; - const guint max_key_len = strlen ("GPGSignature"); + const guint max_key_len = strlen ("PendingBaseVersion"); g_autoptr(GVariant) signatures = NULL; g_autofree char *timestamp_string = NULL; @@ -198,13 +199,46 @@ status_generic (RPMOSTreeSysroot *sysroot_proxy, { print_kv ("Timestamp", max_key_len, timestamp_string); } - if (origin_packages || regenerate_initramfs) + is_locally_assembled = origin_packages || regenerate_initramfs; + if (is_locally_assembled) { const char *base_checksum; g_assert (g_variant_dict_lookup (dict, "base-checksum", "&s", &base_checksum)); print_kv ("BaseCommit", max_key_len, base_checksum); } print_kv ("Commit", max_key_len, checksum); + + /* Pending state, but only for the booted comit */ + if (is_booted) + { + const gchar *pending_checksum = NULL; + const gchar *pending_version = NULL; + + if (g_variant_dict_lookup (dict, "pending-base-checksum", "&s", &pending_checksum)) + { + g_autofree char *version_time = NULL; + print_kv (is_locally_assembled ? "PendingBaseCommit" : "PendingCommit", + max_key_len, pending_checksum); + g_assert (g_variant_dict_lookup (dict, "pending-base-timestamp", "t", &t)); + g_variant_dict_lookup (dict, "pending-base-version", "&s", &pending_version); + + if (pending_version) + { + g_autoptr(GDateTime) timestamp = g_date_time_new_from_unix_utc (t); + g_autofree char *version_time = NULL; + + if (timestamp != NULL) + timestamp_string = g_date_time_format (timestamp, "%Y-%m-%d %T"); + else + timestamp_string = g_strdup_printf ("(invalid timestamp)"); + + version_time = g_strdup_printf ("%s (%s)", pending_version, timestamp_string); + print_kv (is_locally_assembled ? "PendingBaseVersion" : "PendingVersion", + max_key_len, version_time); + } + } + } + print_kv ("OSName", max_key_len, os_name); if (!g_variant_dict_lookup (dict, "gpg-enabled", "b", &gpg_enabled)) diff --git a/src/daemon/rpmostreed-deployment-utils.c b/src/daemon/rpmostreed-deployment-utils.c index 9b2fb9a54e..688697c5ca 100644 --- a/src/daemon/rpmostreed-deployment-utils.c +++ b/src/daemon/rpmostreed-deployment-utils.c @@ -140,7 +140,8 @@ rpmostreed_deployment_generate_blank_variant (void) static void variant_add_commit_details (GVariantDict *dict, - GVariant *commit) + const char *prefix, + GVariant *commit) { g_autoptr(GVariant) metadata = NULL; g_autofree gchar *version_commit = NULL; @@ -152,9 +153,11 @@ variant_add_commit_details (GVariantDict *dict, g_variant_lookup (metadata, "version", "s", &version_commit); if (version_commit != NULL) - g_variant_dict_insert (dict, "version", "s", version_commit); + g_variant_dict_insert (dict, glnx_strjoina (prefix ?: "", "version"), + "s", version_commit); if (timestamp > 0) - g_variant_dict_insert (dict, "timestamp", "t", timestamp); + g_variant_dict_insert (dict, glnx_strjoina (prefix ?: "", "timestamp"), + "t", timestamp); } GVariant * @@ -166,11 +169,14 @@ rpmostreed_deployment_generate_variant (OstreeDeployment *deployment, g_autoptr(GVariant) commit = NULL; g_autoptr(RpmOstreeOrigin) origin = NULL; g_autofree gchar *id = NULL; + const char *base_checksum; GVariant *sigs = NULL; /* floating variant */ GVariantDict dict; + const char *refspec; + g_autofree char *pending_base_commitrev = NULL; const gchar *osname = ostree_deployment_get_osname (deployment); const gchar *csum = ostree_deployment_get_csum (deployment); gint serial = ostree_deployment_get_deployserial (deployment); @@ -189,6 +195,8 @@ rpmostreed_deployment_generate_variant (OstreeDeployment *deployment, if (!origin) return NULL; + refspec = rpmostree_origin_get_refspec (origin); + g_variant_dict_init (&dict, NULL); g_variant_dict_insert (&dict, "id", "s", id); @@ -198,18 +206,38 @@ rpmostreed_deployment_generate_variant (OstreeDeployment *deployment, g_variant_dict_insert (&dict, "checksum", "s", csum); if (rpmostree_origin_is_locally_assembled (origin)) { - const char *parent = ostree_commit_get_parent (commit); - g_assert (parent); - g_variant_dict_insert (&dict, "base-checksum", "s", parent); - sigs = rpmostreed_deployment_gpg_results (repo, rpmostree_origin_get_refspec (origin), - parent, &gpg_enabled); + base_checksum = ostree_commit_get_parent (commit); + g_assert (base_checksum); + g_variant_dict_insert (&dict, "base-checksum", "s", base_checksum); + sigs = rpmostreed_deployment_gpg_results (repo, refspec, base_checksum, &gpg_enabled); } else - sigs = rpmostreed_deployment_gpg_results (repo, rpmostree_origin_get_refspec (origin), - csum, &gpg_enabled); + { + base_checksum = csum; + sigs = rpmostreed_deployment_gpg_results (repo, refspec, csum, &gpg_enabled); + } + variant_add_commit_details (&dict, NULL, commit); + + if (!ostree_repo_resolve_rev (repo, refspec, TRUE, + &pending_base_commitrev, error)) + return NULL; + + if (pending_base_commitrev && strcmp (pending_base_commitrev, base_checksum) != 0) + { + g_autoptr(GVariant) pending_base_commit = NULL; + + if (!ostree_repo_load_variant (repo, + OSTREE_OBJECT_TYPE_COMMIT, + pending_base_commitrev, + &pending_base_commit, + error)) + return NULL; + + g_variant_dict_insert (&dict, "pending-base-checksum", "s", pending_base_commitrev); + variant_add_commit_details (&dict, "pending-base-", pending_base_commit); + } - variant_add_commit_details (&dict, commit); - g_variant_dict_insert (&dict, "origin", "s", rpmostree_origin_get_refspec (origin)); + g_variant_dict_insert (&dict, "origin", "s", refspec); if (rpmostree_origin_get_packages (origin) != NULL) g_variant_dict_insert (&dict, "packages", "^as", rpmostree_origin_get_packages (origin)); if (sigs != NULL) @@ -285,7 +313,7 @@ rpmostreed_commit_generate_cached_details_variant (OstreeDeployment *deployment, if (osname != NULL) g_variant_dict_insert (&dict, "osname", "s", osname); g_variant_dict_insert (&dict, "checksum", "s", head); - variant_add_commit_details (&dict, commit); + variant_add_commit_details (&dict, NULL, commit); g_variant_dict_insert (&dict, "origin", "s", origin_refspec); if (sigs != NULL) g_variant_dict_insert_value (&dict, "signatures", sigs); diff --git a/tests/common/libtest.sh b/tests/common/libtest.sh index ffacc0e524..14bb233d42 100644 --- a/tests/common/libtest.sh +++ b/tests/common/libtest.sh @@ -382,3 +382,16 @@ ensure_dbus () exec "$topsrcdir/tests/utils/setup-session.sh" "$self" fi } + +# Assert that @expression is true in @jsonfile +assert_status_jq() { + vm_rpmostree status --json > status.json + + for expression in $@; do + if ! jq -e "${expression}" >/dev/null < status.json; then + jq . < status.json | sed -e 's/^/# /' >&2 + echo 1>&2 "${expression} failed to match in status.json" + exit 1 + fi + done +} diff --git a/tests/vmcheck/test-initramfs.sh b/tests/vmcheck/test-initramfs.sh index 834b055577..162813d224 100755 --- a/tests/vmcheck/test-initramfs.sh +++ b/tests/vmcheck/test-initramfs.sh @@ -26,17 +26,6 @@ set -x # SUMMARY: Tests for the `initramfs` functionality -assert_jq() { - expression=$1 - jsonfile=$2 - - if ! jq -e "${expression}" >/dev/null < $jsonfile; then - jq . < $jsonfile | sed -e 's/^/# /' >&2 - echo 1>&2 "${expression} failed to match in $jsonfile" - exit 1 - fi -} - vm_send_test_repo base=$(vm_get_booted_csum) @@ -55,20 +44,19 @@ assert_file_has_content err.txt "reboot.*used with.*enable" echo "ok initramfs state" vm_rpmostree initramfs --enable -vm_rpmostree status --json > status.json -assert_jq '.deployments[1].booted' status.json -assert_jq '.deployments[0]["regenerate-initramfs"]' status.json -assert_jq '.deployments[1]["regenerate-initramfs"]|not' status.json +assert_status_jq \ + '.deployments[1].booted' \ + '.deployments[0]["regenerate-initramfs"]' \ + '.deployments[1]["regenerate-initramfs"]|not' vm_reboot assert_not_streq $base $(vm_get_booted_csum) -vm_rpmostree status --json > status.json -assert_jq '.deployments[0].booted' status.json -assert_jq '.deployments[0]["regenerate-initramfs"]' status.json -assert_jq '.deployments[0]["initramfs-args"]|length == 0' status.json -assert_jq '.deployments[1]["regenerate-initramfs"]|not' status.json -assert_jq '.deployments[1]["initramfs-args"]|not' status.json +assert_status_jq '.deployments[0].booted' \ + '.deployments[0]["regenerate-initramfs"]' \ + '.deployments[0]["initramfs-args"]|length == 0' \ + '.deployments[1]["regenerate-initramfs"]|not' \ + '.deployments[1]["initramfs-args"]|not' if vm_rpmostree initramfs --enable 2>err.txt; then assert_not_reached "Unexpectedly succeeded at enabling" @@ -78,24 +66,21 @@ echo "ok initramfs enabled" vm_rpmostree initramfs --disable vm_reboot -vm_rpmostree status --json > status.json -assert_jq '.deployments[0].booted' status.json -assert_jq '.deployments[0]["regenerate-initramfs"]|not' status.json -assert_jq '.deployments[1]["regenerate-initramfs"]' status.json +assert_status_jq '.deployments[0].booted' \ + '.deployments[0]["regenerate-initramfs"]|not' \ + '.deployments[1]["regenerate-initramfs"]' echo "ok initramfs disabled" vm_reboot_cmd rpm-ostree initramfs --enable --reboot -vm_rpmostree status --json > status.json -assert_jq '.deployments[0].booted' status.json -assert_jq '.deployments[0]["regenerate-initramfs"]' status.json -assert_jq '.deployments[1]["regenerate-initramfs"]|not' status.json +assert_status_jq '.deployments[0].booted' \ + '.deployments[0]["regenerate-initramfs"]' \ + '.deployments[1]["regenerate-initramfs"]|not' vm_reboot_cmd rpm-ostree initramfs --disable --reboot -vm_rpmostree status --json > status.json -assert_jq '.deployments[0].booted' status.json -assert_jq '.deployments[0]["regenerate-initramfs"]|not' status.json -assert_jq '.deployments[1]["regenerate-initramfs"]' status.json +assert_status_jq '.deployments[0].booted' \ + '.deployments[0]["regenerate-initramfs"]|not' \ + '.deployments[1]["regenerate-initramfs"]' echo "ok initramfs enable disable reboot" @@ -104,12 +89,12 @@ for file in first second; do vm_cmd touch /etc/rpmostree-initramfs-testing-$file vm_rpmostree initramfs --enable --arg="-I" --arg="/etc/rpmostree-initramfs-testing-$file" vm_reboot - vm_rpmostree status --json > status.json - assert_jq '.deployments[0].booted' status.json - assert_jq '.deployments[0]["regenerate-initramfs"]' status.json - assert_jq '.deployments[0]["initramfs-args"]|index("-I") == 0' status.json - assert_jq '.deployments[0]["initramfs-args"]|index("/etc/rpmostree-initramfs-testing-'${file}'") == 1' status.json - assert_jq '.deployments[0]["initramfs-args"]|length == 2' status.json + assert_status_jq \ + '.deployments[0].booted' \ + '.deployments[0]["regenerate-initramfs"]' \ + '.deployments[0]["initramfs-args"]|index("-I") == 0' \ + '.deployments[0]["initramfs-args"]|index("/etc/rpmostree-initramfs-testing-'${file}'") == 1' \ + '.deployments[0]["initramfs-args"]|length == 2' initramfs=$(vm_cmd grep ^initrd /boot/loader/entries/ostree-fedora-atomic-0.conf | sed -e 's,initrd ,/boot/,') test -n "${initramfs}" vm_cmd lsinitrd $initramfs > lsinitrd.txt diff --git a/tests/vmcheck/test-layering-basic.sh b/tests/vmcheck/test-layering-basic.sh index 2d49bff3ff..65fc2a377e 100755 --- a/tests/vmcheck/test-layering-basic.sh +++ b/tests/vmcheck/test-layering-basic.sh @@ -33,6 +33,8 @@ vm_send_test_repo # make sure the package is not already layered vm_assert_layered_pkg foo absent +assert_status_jq '.deployments[0]["base-checksum"]|not' \ + '.deployments[0]["pending-base-checksum"]|not' # Be sure an unprivileged user exists vm_cmd getent passwd bin @@ -44,6 +46,8 @@ vm_rpmostree pkg-add foo-1.0 echo "ok pkg-add foo" vm_reboot +assert_status_jq '.deployments[0]["base-checksum"]' \ + '.deployments[0]["pending-base-checksum"]|not' vm_assert_layered_pkg foo-1.0 present echo "ok pkg foo added" diff --git a/tests/vmcheck/test-layering-relayer.sh b/tests/vmcheck/test-layering-relayer.sh index 564fad1079..5946591bc6 100755 --- a/tests/vmcheck/test-layering-relayer.sh +++ b/tests/vmcheck/test-layering-relayer.sh @@ -53,10 +53,18 @@ reboot_and_assert_base() { # UPGRADE +assert_status_jq '.deployments[0]["base-checksum"]' \ + '.deployments[0]["pending-base-checksum"]|not' # let's synthesize an upgrade commit=$(vm_cmd ostree commit -b vmcheck --tree=ref=vmcheck) vm_rpmostree upgrade +assert_status_jq '.deployments[1]["base-checksum"]' \ + '.deployments[1]["pending-base-checksum"]' +vm_rpmostree status --json > status.json reboot_and_assert_base $commit +assert_status_jq '.deployments[0]["base-checksum"]' \ + '.deployments[0]["pending-base-checksum"]|not' \ + '.deployments[1]["pending-base-checksum"]' echo "ok upgrade" vm_assert_layered_pkg foo present From 4a342e65f6dc6e58797edfdcf1c8bbe47569fc48 Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 28 Jan 2017 11:50:13 -0500 Subject: [PATCH 2/4] fixup! Add `pending-base-commit` to status --- src/app/rpmostree-builtin-status.c | 1 - 1 file changed, 1 deletion(-) diff --git a/src/app/rpmostree-builtin-status.c b/src/app/rpmostree-builtin-status.c index b14e34f18c..fd2d0072f9 100644 --- a/src/app/rpmostree-builtin-status.c +++ b/src/app/rpmostree-builtin-status.c @@ -216,7 +216,6 @@ status_generic (RPMOSTreeSysroot *sysroot_proxy, if (g_variant_dict_lookup (dict, "pending-base-checksum", "&s", &pending_checksum)) { - g_autofree char *version_time = NULL; print_kv (is_locally_assembled ? "PendingBaseCommit" : "PendingCommit", max_key_len, pending_checksum); g_assert (g_variant_dict_lookup (dict, "pending-base-timestamp", "t", &t)); From fb643b73e5858a654ce350edead23cc9cda5a07a Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Mon, 30 Jan 2017 09:59:55 +0100 Subject: [PATCH 3/4] fixup! Add `pending-base-commit` to status --- tests/common/libtest.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/common/libtest.sh b/tests/common/libtest.sh index 14bb233d42..ae36a5f54a 100644 --- a/tests/common/libtest.sh +++ b/tests/common/libtest.sh @@ -387,7 +387,7 @@ ensure_dbus () assert_status_jq() { vm_rpmostree status --json > status.json - for expression in $@; do + for expression in "$@"; do if ! jq -e "${expression}" >/dev/null < status.json; then jq . < status.json | sed -e 's/^/# /' >&2 echo 1>&2 "${expression} failed to match in status.json" From 388ba4b49ace5a82723596072104706934361bcb Mon Sep 17 00:00:00 2001 From: Colin Walters Date: Sat, 4 Feb 2017 10:39:52 -0500 Subject: [PATCH 4/4] fixup! Add `pending-base-commit` to status --- src/app/rpmostree-builtin-status.c | 8 ++++++-- src/daemon/rpmostreed-deployment-utils.c | 3 +-- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/src/app/rpmostree-builtin-status.c b/src/app/rpmostree-builtin-status.c index fd2d0072f9..0b85ff5abf 100644 --- a/src/app/rpmostree-builtin-status.c +++ b/src/app/rpmostree-builtin-status.c @@ -124,6 +124,7 @@ status_generic (RPMOSTreeSysroot *sysroot_proxy, guint64 t = 0; int serial; gboolean is_booted; + const gboolean was_first = first; const guint max_key_len = strlen ("PendingBaseVersion"); g_autoptr(GVariant) signatures = NULL; g_autofree char *timestamp_string = NULL; @@ -208,8 +209,11 @@ status_generic (RPMOSTreeSysroot *sysroot_proxy, } print_kv ("Commit", max_key_len, checksum); - /* Pending state, but only for the booted comit */ - if (is_booted) + /* Show any difference between the baseref vs head, but only for the + booted commit, and only if there isn't a pending deployment. Otherwise + it's either unnecessary or too noisy. + */ + if (is_booted && was_first) { const gchar *pending_checksum = NULL; const gchar *pending_version = NULL; diff --git a/src/daemon/rpmostreed-deployment-utils.c b/src/daemon/rpmostreed-deployment-utils.c index 688697c5ca..b6c3f0460e 100644 --- a/src/daemon/rpmostreed-deployment-utils.c +++ b/src/daemon/rpmostreed-deployment-utils.c @@ -209,13 +209,12 @@ rpmostreed_deployment_generate_variant (OstreeDeployment *deployment, base_checksum = ostree_commit_get_parent (commit); g_assert (base_checksum); g_variant_dict_insert (&dict, "base-checksum", "s", base_checksum); - sigs = rpmostreed_deployment_gpg_results (repo, refspec, base_checksum, &gpg_enabled); } else { base_checksum = csum; - sigs = rpmostreed_deployment_gpg_results (repo, refspec, csum, &gpg_enabled); } + sigs = rpmostreed_deployment_gpg_results (repo, refspec, base_checksum, &gpg_enabled); variant_add_commit_details (&dict, NULL, commit); if (!ostree_repo_resolve_rev (repo, refspec, TRUE,