daemon: Also generate CachedUpdate in DownloadUpdateRpmDiff

Moving to caching the GVariant to disk rather than during RPMOSTreeOS
reloads broke the legacy `DownloadUpdateRpmDiff` path because it relied
on the implied recalculations that occur on reloads to update
`CachedUpdate`.

This patch series was initially going to be about just migrating all the
legacy APIs to make use of the new metadata, which would have fixed this
properly. But we first need some real coverage in that aread, which is
very poor right now. I'd like to investigate integrating the Cockpit
tests (at least the ostree-specific parts) into our CI to remedy this.

Anyways, for now at least, let's fix Cockpit.

Closes: #1300

Closes: #1303
Approved by: cgwalters

src/daemon/rpmostreed-os.c

@@ -463,6 +463,20 @@ merge_compatible_txn (RpmostreedOS *self,
   return NULL;
 }
 
+static gboolean
+refresh_cached_update (RpmostreedOS*, GError **error);
+
+static void
+on_auto_update_done (RpmostreedTransaction *transaction, RpmostreedOS *self)
+{
+  g_autoptr(GError) local_error = NULL;
+  if (!refresh_cached_update (self, &local_error))
+    {
+      sd_journal_print (LOG_WARNING, "Failed to refresh CachedUpdate property: %s",
+                        local_error->message);
+    }
+}
+
 static gboolean
 os_handle_download_update_rpm_diff (RPMOSTreeOS *interface,
                                     GDBusMethodInvocation *invocation)
@@ -498,6 +512,11 @@ os_handle_download_update_rpm_diff (RPMOSTreeOS *interface,
   if (transaction == NULL)
     goto out;
 
+  /* Make sure we refresh CachedUpdate after the transaction. This normally happens
+   * automatically if new data was downloaded (through the repo mtime bump --> UPDATED
+   * signal), but we also want to do this even if the data was already present. */
+  g_signal_connect (transaction, "closed", G_CALLBACK (on_auto_update_done), self);
+
   rpmostreed_transaction_monitor_add (self->transaction_monitor, transaction);
 
 out:
@@ -711,20 +730,6 @@ start_deployment_txn (GDBusMethodInvocation  *invocation,
                                             cancellable, error);
 }
 
-static gboolean
-refresh_cached_update (RpmostreedOS*, GError **error);
-
-static void
-on_auto_update_done (RpmostreedTransaction *transaction, RpmostreedOS *self)
-{
-  g_autoptr(GError) local_error = NULL;
-  if (!refresh_cached_update (self, &local_error))
-    {
-      sd_journal_print (LOG_WARNING, "Failed to refresh CachedUpdate property: %s",
-                        local_error->message);
-    }
-}
-
 typedef void (*InvocationCompleter)(RPMOSTreeOS*,
                                     GDBusMethodInvocation*,
                                     GUnixFDList*,

src/daemon/rpmostreed-transaction-types.c

@@ -330,6 +330,17 @@ package_diff_transaction_execute (RpmostreedTransaction *transaction,
         rpmostree_output_message ("No change.");
     }
 
+  if (upgrading)
+    {
+      /* For backwards compatibility, we still need to make sure the CachedUpdate property
+       * is updated here. To do this, we cache to disk in libostree mode (no DnfSack), since
+       * that's all we updated here. This conflicts with auto-updates for now, though we
+       * need better test coverage before uniting those two paths. */
+      OstreeDeployment *booted_deployment = ostree_sysroot_get_booted_deployment (sysroot);
+      if (!generate_update_variant (repo, booted_deployment, NULL, cancellable, error))
+        return FALSE;
+    }
+
   return TRUE;
 }
 

tests/vmcheck/test-cached-rpm-diffs.sh

@@ -58,6 +58,33 @@ call_dbus() {
     -m org.projectatomic.rpmostree1.OS.$method "$@"
 }
 
+if vm_cmd test -x /usr/bin/python3; then
+  py=python3
+else
+  py=python
+fi
+
+run_transaction() {
+  method=$1; shift
+  sig=$1; shift
+  args=$1; shift
+  cur=$(vm_get_journal_cursor)
+  # use ansible for this so we don't have to think about hungry quote-eating ssh
+  vm_shell_inline <<EOF
+$py -c '
+import dbus
+addr = dbus.SystemBus().call_blocking(
+  "org.projectatomic.rpmostree1", "$ospath", "org.projectatomic.rpmostree1.OS",
+  "$method", "$sig", ($args))
+t = dbus.connection.Connection(addr)
+t.call_blocking(
+  "org.projectatomic.rpmostree1", "/",
+  "org.projectatomic.rpmostree1.Transaction", "Start", "", ())
+t.close()'
+EOF
+  vm_wait_content_after_cursor $cur "Txn $method on .* successful"
+}
+
 call_dbus GetCachedDeployRpmDiff "vDeploy" [] > out.txt
 assert_file_has_content out.txt "<'vmcheck'>"
 assert_file_has_content out.txt "$deploy_csum"
@@ -85,3 +112,9 @@ assert_file_has_content out.txt "pkg1"
 assert_file_has_content out.txt "pkg2"
 assert_file_has_content out.txt "pkg3"
 echo "ok GetCachedRebaseRpmDiff"
+
+# This is not a super realistic test since we don't actually download anything.
+# Still it checks that we properly update the cache
+run_transaction DownloadUpdateRpmDiff "" ""
+vm_assert_status_jq '.["cached-update"]["version"] == "vUpdate"'
+echo "ok DownloadUpdateRpmDiff"