Merge pull request #32144 from bluca/portable_clean

portablectl: add --clean parameter for detaching

4 files changed, 73 insertions, 6 deletions

diff --git a/man/portablectl.xml b/man/portablectl.xml
index bcfaf7ced15..5e88eb309dc 100644
--- a/man/portablectl.xml
+++ b/man/portablectl.xml
@@ -392,6 +392,15 @@
       </varlistentry>
 
       <varlistentry>
+        <term><option>--clean</option></term>
+
+        <listitem><para>When detaching ensure the configuration, state, logs, cache, and runtime data
+        directories of the portable service are removed from the host system.</para>
+
+        <xi:include href="version-info.xml" xpointer="v256"/></listitem>
+      </varlistentry>
+
+      <varlistentry>
         <term><option>--extension=<replaceable>PATH</replaceable></option></term>
 
         <listitem><para>Add an additional image <replaceable>PATH</replaceable> as an overlay on
diff --git a/shell-completion/bash/portablectl b/shell-completion/bash/portablectl
index 61a9610aa3a..8d88ab46bc1 100644
--- a/shell-completion/bash/portablectl
+++ b/shell-completion/bash/portablectl
@@ -36,7 +36,8 @@ _portablectl() {
     local cur=${COMP_WORDS[COMP_CWORD]} prev=${COMP_WORDS[COMP_CWORD-1]}
     local -A OPTS=(
         [STANDALONE]='-q --quiet --runtime --no-reload --cat --no-pager --no-legend
-                              --no-ask-password --enable --now -h --help --version'
+                              --no-ask-password --enable --now -h --help --version
+                              --clean --no-block --force'
         [ARG]='-p --profile --copy -H --host -M --machine --extension'
     )
 
diff --git a/src/portable/portablectl.c b/src/portable/portablectl.c
index 0f67e4432f2..57b930d6cba 100644
--- a/src/portable/portablectl.c
+++ b/src/portable/portablectl.c
@@ -50,6 +50,7 @@ static bool arg_now = false;
 static bool arg_no_block = false;
 static char **arg_extension_images = NULL;
 static bool arg_force = false;
+static bool arg_clean = false;
 
 STATIC_DESTRUCTOR_REGISTER(arg_extension_images, strv_freep);
 
@@ -769,13 +770,49 @@ static int maybe_stop_enable_restart(sd_bus *bus, sd_bus_message *reply) {
         return 0;
 }
 
-static int maybe_stop_disable(sd_bus *bus, char *image, char *argv[]) {
+static int maybe_clean_units(sd_bus *bus, char **units) {
+        int r;
+
+        assert(bus);
+
+        if (!arg_clean)
+                return 0;
+
+        STRV_FOREACH(name, units) {
+                _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL;
+                _cleanup_(sd_bus_message_unrefp) sd_bus_message *m = NULL;
+
+                r = bus_message_new_method_call(bus, &m, bus_systemd_mgr, "CleanUnit");
+                if (r < 0)
+                        return bus_log_create_error(r);
+
+                r = sd_bus_message_append(m, "s", *name);
+                if (r < 0)
+                        return bus_log_create_error(r);
+
+                r = sd_bus_message_append_strv(m, STRV_MAKE("all", "fdstore"));
+                if (r < 0)
+                        return bus_log_create_error(r);
+
+                r = sd_bus_call(bus, m, 0, &error, NULL);
+                if (r < 0)
+                        return log_error_errno(
+                                        r,
+                                        "Failed to call CleanUnit on portable service %s: %s",
+                                        *name,
+                                        bus_error_message(&error, r));
+        }
+
+        return 0;
+}
+
+static int maybe_stop_disable_clean(sd_bus *bus, char *image, char *argv[]) {
         _cleanup_(bus_wait_for_jobs_freep) BusWaitForJobs *wait = NULL;
         _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL;
-        _cleanup_strv_free_ char **matches = NULL;
+        _cleanup_strv_free_ char **matches = NULL, **units = NULL;
         int r;
 
-        if (!arg_enable && !arg_now)
+        if (!arg_enable && !arg_now && !arg_clean)
                 return 0;
 
         r = determine_matches(argv[1], argv + 2, true, &matches);
@@ -829,6 +866,10 @@ static int maybe_stop_disable(sd_bus *bus, char *image, char *argv[]) {
 
                 (void) maybe_start_stop_restart(bus, name, "StopUnit", wait);
                 (void) maybe_enable_disable(bus, name, false);
+
+                r = strv_extend(&units, name);
+                if (r < 0)
+                        return log_oom();
         }
 
         r = sd_bus_message_exit_container(reply);
@@ -840,6 +881,9 @@ static int maybe_stop_disable(sd_bus *bus, char *image, char *argv[]) {
         if (r < 0)
                 return r;
 
+        /* Need to ensure all units are stopped before calling CleanUnit, as files might be in use. */
+        (void) maybe_clean_units(bus, units);
+
         return 0;
 }
 
@@ -942,7 +986,7 @@ static int detach_image(int argc, char *argv[], void *userdata) {
 
         (void) polkit_agent_open_if_enabled(arg_transport, arg_ask_password);
 
-        (void) maybe_stop_disable(bus, image, argv);
+        (void) maybe_stop_disable_clean(bus, image, argv);
 
         method = strv_isempty(arg_extension_images) && !arg_force ? "DetachImage" : "DetachImageWithExtensions";
 
@@ -1265,6 +1309,9 @@ static int help(int argc, char *argv[], void *userdata) {
                "     --extension=PATH         Extend the image with an overlay\n"
                "     --force                  Skip 'already active' check when attaching or\n"
                "                              detaching an image (with extensions)\n"
+               "     --clean                  When detaching, also remove configuration, state,\n"
+               "                              cache, logs or runtime data of the portable\n"
+               "                              service(s)\n"
                "\nSee the %s for details.\n",
                program_invocation_short_name,
                ansi_highlight(),
@@ -1291,6 +1338,7 @@ static int parse_argv(int argc, char *argv[]) {
                 ARG_NO_BLOCK,
                 ARG_EXTENSION,
                 ARG_FORCE,
+                ARG_CLEAN,
         };
 
         static const struct option options[] = {
@@ -1312,6 +1360,7 @@ static int parse_argv(int argc, char *argv[]) {
                 { "no-block",        no_argument,       NULL, ARG_NO_BLOCK        },
                 { "extension",       required_argument, NULL, ARG_EXTENSION       },
                 { "force",           no_argument,       NULL, ARG_FORCE           },
+                { "clean",           no_argument,       NULL, ARG_CLEAN           },
                 {}
         };
 
@@ -1421,6 +1470,10 @@ static int parse_argv(int argc, char *argv[]) {
                         arg_force = true;
                         break;
 
+                case ARG_CLEAN:
+                        arg_clean = true;
+                        break;
+
                 case '?':
                         return -EINVAL;
 
diff --git a/test/units/testsuite-29.sh b/test/units/testsuite-29.sh
index 7af0ed45786..977d76e22f0 100755
--- a/test/units/testsuite-29.sh
+++ b/test/units/testsuite-29.sh
@@ -304,7 +304,11 @@ grep -q -F "LogExtraFields=PORTABLE_EXTENSION_NAME_AND_VERSION=app" /run/systemd
 grep -q -F "LogExtraFields=PORTABLE_EXTENSION=app1" /run/systemd/system.attached/app1.service.d/20-portable.conf
 grep -q -F "LogExtraFields=PORTABLE_EXTENSION_NAME_AND_VERSION=app_1" /run/systemd/system.attached/app1.service.d/20-portable.conf
 
-portablectl detach --now --runtime --extension /tmp/app0 --extension /tmp/app1 /tmp/rootdir app0 app1
+portablectl detach --clean --now --runtime --extension /tmp/app0 --extension /tmp/app1 /tmp/rootdir app0 app1
+
+# Ensure --clean remove state and other directories belonging to the portable image being detached
+test ! -d /var/lib/app0
+test ! -d /run/app0
 
 # Ensure that mixed mode copies the images and units (client-owned) but symlinks the profile (OS owned)
 portablectl "${ARGS[@]}" attach --copy=mixed --runtime --extension /tmp/app0 --extension /tmp/app1 /tmp/rootdir app0 app1