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.

Closes: #595
Approved by: jlebon

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,8 @@ status_generic (RPMOSTreeSysroot *sysroot_proxy,
       guint64 t = 0;
       int serial;
       gboolean is_booted;
-      const guint max_key_len = strlen ("GPGSignature");
+      const gboolean was_first = first;
+      const guint max_key_len = strlen ("PendingBaseVersion");
       g_autoptr(GVariant) signatures = NULL;
       g_autofree char *timestamp_string = NULL;
 
@@ -198,13 +200,48 @@ 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);
+
+      /* 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;
+
+          if (g_variant_dict_lookup (dict, "pending-base-checksum", "&s", &pending_checksum))
+            {
+              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))

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,37 @@ 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);
     }
   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, base_checksum, &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 +312,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);

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
+}

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

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"

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