daemon: Add an 'id' param to RegisterClient, log it

This makes the logs a bit more useful, but the ultimate goal
here is to write the originating client `id` to the cached update
data, so users know that e.g. `gnome-software` triggered it.

Closes: coreos#1368
Approved by: jlebon

src/app/rpmostree-dbus-helpers.c

@@ -34,6 +34,8 @@
 #include "rpmostree-util.h"
 #include "rpmostree-rpm-util.h"
 
+#define RPMOSTREE_CLI_ID "cli"
+
 void
 rpmostree_cleanup_peer (GPid *peer_pid)
 {
@@ -211,6 +213,8 @@ rpmostree_load_sysroot (gchar *sysroot,
       g_autoptr(GError) local_error = NULL;
       g_autoptr(GVariantBuilder) options_builder =
         g_variant_builder_new (G_VARIANT_TYPE ("a{sv}"));
+      g_variant_builder_add (options_builder, "{sv}", "id",
+                             g_variant_new_string (RPMOSTREE_CLI_ID));
       g_autoptr(GVariant) res =
         g_dbus_connection_call_sync (connection, bus_name, sysroot_objpath,
                                      "org.projectatomic.rpmostree1.Sysroot",

src/daemon/org.projectatomic.rpmostree1.xml

@@ -32,7 +32,7 @@
          to either invoke methods on the daemon, or monitor status.
          If no clients are registered, the daemon may exit.
 
-         No options are currently defined.
+         'id (type 's') - Package/component name (e.g. `cockpit`, `gnome-software`)
     -->
     <method name="RegisterClient">
       <arg type="a{sv}" name="options" direction="in"/>

src/daemon/rpmostreed-daemon.c

@@ -37,7 +37,7 @@
 #define EXPERIMENTAL_CONFIG_GROUP "Experimental"
 
 struct RpmOstreeClient;
-static struct RpmOstreeClient *client_new (RpmostreedDaemon *self, const char *address);
+static struct RpmOstreeClient *client_new (RpmostreedDaemon *self, const char *address, const char *id);
 
 /**
  * SECTION: daemon
@@ -98,6 +98,7 @@ G_DEFINE_TYPE_WITH_CODE (RpmostreedDaemon, rpmostreed_daemon, G_TYPE_OBJECT,
                                                 rpmostreed_daemon_initable_iface_init))
 
 struct RpmOstreeClient {
+  char *id;
   char *address;
   guint name_watch_id;
   /* In Rust this'd be Option<uid_t> etc. */
@@ -113,6 +114,7 @@ rpmostree_client_free (struct RpmOstreeClient *client)
 {
   if (!client)
     return;
+  g_free (client->id);
   g_free (client->address);
   g_free (client->sd_unit);
   g_free (client);
@@ -121,10 +123,13 @@ rpmostree_client_free (struct RpmOstreeClient *client)
 static char *
 rpmostree_client_to_string (struct RpmOstreeClient *client)
 {
+  g_autoptr(GString) buf = g_string_new ("client(");
+  if (client->id)
+    g_string_append_printf (buf, "id:%s ", client->id);
   /* Since DBus addresses have a leading ':', let's avoid another. Yeah it's not
    * symmetric, but it does read better.
    */
-  g_autoptr(GString) buf = g_string_new ("client(dbus");
+  g_string_append (buf, "dbus");
   g_string_append (buf, client->address);
   if (client->sd_unit)
     g_string_append_printf (buf, " unit:%s", client->sd_unit);
@@ -246,7 +251,7 @@ on_active_txn_changed (GObject *object,
            * then let's gather the relevant client info now.
            */
           if (!clientdata)
-            clientdata = clientdata_owned = client_new (self, sender);
+            clientdata = clientdata_owned = client_new (self, sender, NULL);
           g_autofree char *client_str = rpmostree_client_to_string (clientdata);
           g_autofree char *client_data_msg = NULL;
           if (clientdata->uid_valid)
@@ -665,10 +670,12 @@ get_client_pid (RpmostreedDaemon *self,
 
 /* Given a DBus address, load metadata for it */
 static struct RpmOstreeClient *
-client_new (RpmostreedDaemon *self, const char *address)
+client_new (RpmostreedDaemon *self, const char *address,
+            const char *client_id)
 {
   struct RpmOstreeClient *client = g_new0 (struct RpmOstreeClient, 1);
   client->address = g_strdup (address);
+  client->id = g_strdup (client_id);
   if (rpmostreed_get_client_uid (self, address, &client->uid))
     client->uid_valid = TRUE;
   if (get_client_pid (self, address, &client->pid))
@@ -685,12 +692,13 @@ client_new (RpmostreedDaemon *self, const char *address)
 
 void
 rpmostreed_daemon_add_client (RpmostreedDaemon *self,
-                              const char       *client)
+                              const char       *client,
+                              const char       *client_id)
 {
   if (g_hash_table_lookup (self->bus_clients, client))
     return;
 
-  struct RpmOstreeClient *clientdata = client_new (self, client);
+  struct RpmOstreeClient *clientdata = client_new (self, client, client_id);
   clientdata->name_watch_id =
     g_dbus_connection_signal_subscribe (self->connection,
                                         "org.freedesktop.DBus",

src/daemon/rpmostreed-daemon.h

@@ -34,7 +34,8 @@ gboolean           rpmostreed_get_client_uid        (RpmostreedDaemon *self,
                                                      const char       *client,
                                                      uid_t            *out_uid);
 void               rpmostreed_daemon_add_client     (RpmostreedDaemon *self,
-                                                      const char *client);
+                                                     const char *client,
+                                                     const char *client_id);
 void               rpmostreed_daemon_remove_client  (RpmostreedDaemon *self,
                                                      const char *client);
 char *             rpmostreed_daemon_client_get_string (RpmostreedDaemon *self,

src/daemon/rpmostreed-sysroot.c

@@ -391,11 +391,14 @@ handle_register_client (RPMOSTreeSysroot *object,
                         GDBusMethodInvocation *invocation,
                         GVariant *arg_options)
 {
-  const char *sender;
-  sender = g_dbus_method_invocation_get_sender (invocation);
+  const char *sender = g_dbus_method_invocation_get_sender (invocation);
   g_assert (sender);
 
-  rpmostreed_daemon_add_client (rpmostreed_daemon_get (), sender);
+  g_autoptr(GVariantDict) optdict = g_variant_dict_new (arg_options);
+  const char *client_id = NULL;
+  g_variant_dict_lookup (optdict, "id", "&s", &client_id);
+
+  rpmostreed_daemon_add_client (rpmostreed_daemon_get (), sender, client_id);
   rpmostree_sysroot_complete_register_client (object, invocation);
 
   return TRUE;

tests/vmcheck/test-autoupdate-check.sh

@@ -82,9 +82,15 @@ vm_rpmostree status | grep 'auto updates enabled (check'
 
 # build an *older version* and check that we don't report an update
 vm_build_rpm layered-cake version 2.1 release 2
-vm_rpmostree upgrade --trigger-automatic-update-policy
+cursor=$(vm_get_journal_cursor)
+vm_cmd systemctl start rpm-ostree-automatic.service
+vm_wait_content_after_cursor $cursor 'Txn AutomaticUpdateTrigger.*successful'
 vm_rpmostree status -v > out.txt
 assert_not_file_has_content out.txt "Available update"
+# And check the unit name https://github.com/projectatomic/rpm-ostree/pull/1368
+vm_cmd journalctl -u rpm-ostreed --after-cursor ${cursor} > journal.txt
+assert_file_has_content journal.txt 'client(id:cli.*unit:rpm-ostreed-automatic.service'
+rm -f journal.txt
 
 # build a *newer version* and check that we report an update
 vm_build_rpm layered-cake version 2.1 release 4