Merge pull request #23533 from yuwata/portable-remove-drop-in-configs

portable: remove drop-in configs

src/portable/portable.c

@@ -1559,40 +1559,47 @@ int portable_detach(
         }
 
         FOREACH_DIRENT(de, d, return log_debug_errno(errno, "Failed to enumerate '%s' directory: %m", where)) {
-                _cleanup_free_ char *marker = NULL;
-                UnitFileState state;
+                _cleanup_free_ char *marker = NULL, *unit_name = NULL;
+                const char *dot;
 
-                if (!unit_name_is_valid(de->d_name, UNIT_NAME_ANY))
+                /* When a portable service is enabled with "portablectl --runtime attach", and is disabled
+                 * with "portablectl --enable detach", which calls DisableUnitFilesWithFlags DBus method,
+                 * the main unit file is removed, but its drop-ins are not. Hence, here we need to list both
+                 * main unit files and drop-in directories (without the main unit files). */
+
+                dot = endswith(de->d_name, ".d");
+                if (dot)
+                        unit_name = strndup(de->d_name, dot - de->d_name);
+                else
+                        unit_name = strdup(de->d_name);
+                if (!unit_name)
+                        return -ENOMEM;
+
+                if (!unit_name_is_valid(unit_name, UNIT_NAME_ANY))
                         continue;
 
                 /* Filter out duplicates */
-                if (set_contains(unit_files, de->d_name))
+                if (set_contains(unit_files, unit_name))
                         continue;
 
-                if (!IN_SET(de->d_type, DT_LNK, DT_REG))
+                if (dot ? !IN_SET(de->d_type, DT_LNK, DT_DIR) : !IN_SET(de->d_type, DT_LNK, DT_REG))
                         continue;
 
-                r = test_chroot_dropin(d, where, de->d_name, name_or_path, extension_image_paths, &marker);
+                r = test_chroot_dropin(d, where, unit_name, name_or_path, extension_image_paths, &marker);
                 if (r < 0)
                         return r;
                 if (r == 0)
                         continue;
 
-                r = unit_file_lookup_state(LOOKUP_SCOPE_SYSTEM, &paths, de->d_name, &state);
-                if (r < 0)
-                        return log_debug_errno(r, "Failed to determine unit file state of '%s': %m", de->d_name);
-                if (!IN_SET(state, UNIT_FILE_STATIC, UNIT_FILE_DISABLED, UNIT_FILE_LINKED, UNIT_FILE_RUNTIME, UNIT_FILE_LINKED_RUNTIME))
-                        return sd_bus_error_setf(error, BUS_ERROR_UNIT_EXISTS, "Unit file '%s' is in state '%s', can't detach.", de->d_name, unit_file_state_to_string(state));
-
-                r = unit_file_is_active(bus, de->d_name, error);
+                r = unit_file_is_active(bus, unit_name, error);
                 if (r < 0)
                         return r;
                 if (!FLAGS_SET(flags, PORTABLE_REATTACH) && r > 0)
-                        return sd_bus_error_setf(error, BUS_ERROR_UNIT_EXISTS, "Unit file '%s' is active, can't detach.", de->d_name);
+                        return sd_bus_error_setf(error, BUS_ERROR_UNIT_EXISTS, "Unit file '%s' is active, can't detach.", unit_name);
 
-                r = set_put_strdup(&unit_files, de->d_name);
+                r = set_ensure_consume(&unit_files, &string_hash_ops_free, TAKE_PTR(unit_name));
                 if (r < 0)
-                        return log_debug_errno(r, "Failed to add unit name '%s' to set: %m", de->d_name);
+                        return log_oom_debug();
 
                 for (const char *p = marker;;) {
                         _cleanup_free_ char *image = NULL;

src/test/test-mountpoint-util.c

@@ -54,8 +54,6 @@ TEST(mnt_id) {
         void *k;
         int r;
 
-        log_info("/* %s */", __func__);
-
         assert_se(f = fopen("/proc/self/mountinfo", "re"));
         assert_se(h = hashmap_new(&trivial_hash_ops));
 

test/units/testsuite-29.sh

@@ -152,6 +152,14 @@ portablectl inspect --cat --extension app0 --extension app1 rootdir app0 app1 |
 
 portablectl detach --now --runtime --extension /tmp/app0 --extension /tmp/app1 /tmp/rootdir app0 app1
 
+# Attempt to disable the app unit during detaching. Requires --copy=symlink to reproduce.
+# Provides coverage for https://github.com/systemd/systemd/issues/23481
+portablectl "${ARGS[@]}" attach --copy=symlink --now --runtime /tmp/rootdir minimal-app0
+portablectl detach --now --runtime --enable /tmp/rootdir minimal-app0
+# attach and detach again to check if all drop-in configs are removed even if the main unit files are removed
+portablectl "${ARGS[@]}" attach --copy=symlink --now --runtime /tmp/rootdir minimal-app0
+portablectl detach --now --runtime --enable /tmp/rootdir minimal-app0
+
 umount /tmp/rootdir
 umount /tmp/app0
 umount /tmp/app1