portable: detect drop-in-only attachments

Commit edea370222 (portable: remove drop-in configs even if the main unit file does not exist)
taught detach to handle leftover .service.d directories
after the main unit symlink was removed.

portable_get_state_internal() still had the same blind
spot: it only considered regular unit entries, so portablectl
is-attached could report detached while a portable drop-in directory was still present.

Handle those drop-in-only entries like detach does, preserve
the existing unit-file based enabled checks when the
main unit file is still present, and add TEST-29 coverage.

Signed-off-by: dongshengyuan <dongshengyuan@uniontech.com>

diff --git a/src/portable/portable.c b/src/portable/portable.c
index f0bdb40fd22f3d8a505c041875a26d53f0e50311..c1abe16985d7a92ddb0739548551d1ecdc914d31 100644 (file)
--- a/src/portable/portable.c
+++ b/src/portable/portable.c
@@ -2295,6 +2295,42 @@ static int test_chroot_dropin(
         return r;
 }
 
+static int portable_attached_dirent_name(
+                const struct dirent *de,
+                char **ret_unit_name,
+                bool *ret_dropin) {
+
+        _cleanup_free_ char *unit_name = NULL;
+        const char *dropin_suffix;
+
+        assert(de);
+        assert(ret_unit_name);
+
+        /* When a portable service is enabled with "portablectl --copy=symlink --enable --now attach",
+         * and is disabled with "portablectl --enable --now detach", which calls DisableUnitFilesWithFlags
+         * DBus method, the main unit file is removed, but its drop-ins are not. Hence, we need to list both
+         * main unit files and drop-in directories (without the main unit files). */
+
+        dropin_suffix = endswith(de->d_name, ".d");
+        if (dropin_suffix)
+                unit_name = strndup(de->d_name, dropin_suffix - 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))
+                return 0;
+
+        if (dropin_suffix ? !IN_SET(de->d_type, DT_LNK, DT_DIR) : !IN_SET(de->d_type, DT_LNK, DT_REG))
+                return 0;
+
+        *ret_unit_name = TAKE_PTR(unit_name);
+        if (ret_dropin)
+                *ret_dropin = dropin_suffix != NULL;
+        return 1;
+}
+
 int portable_detach(
                 RuntimeScope scope,
                 sd_bus *bus,
@@ -2331,31 +2367,17 @@ int portable_detach(
 
         FOREACH_DIRENT(de, d, return log_debug_errno(errno, "Failed to enumerate '%s' directory: %m", where)) {
                 _cleanup_free_ char *marker = NULL, *unit_name = NULL;
-                const char *dot;
-
-                /* When a portable service is enabled with "portablectl --copy=symlink --enable --now attach",
-                 * and is disabled with "portablectl --enable --now 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))
+                r = portable_attached_dirent_name(de, &unit_name, /* ret_dropin= */ NULL);
+                if (r < 0)
+                        return r;
+                if (r == 0)
                         continue;
 
                 /* Filter out duplicates */
                 if (set_contains(unit_files, unit_name))
                         continue;
 
-                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, unit_name, name_or_path, extension_image_paths, &marker);
                 if (r < 0)
                         return r;
@@ -2521,39 +2543,54 @@ static int portable_get_state_internal(
         }
 
         FOREACH_DIRENT(de, d, return log_debug_errno(errno, "Failed to enumerate '%s' directory: %m", where)) {
-                UnitFileState state;
+                _cleanup_free_ char *unit_name = NULL;
+                bool dropin;
 
-                if (!unit_name_is_valid(de->d_name, UNIT_NAME_ANY))
+                r = portable_attached_dirent_name(de, &unit_name, &dropin);
+                if (r < 0)
+                        return r;
+                if (r == 0)
                         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))
-                        continue;
+                if (dropin) {
+                        /* If the main unit file still exists, let the regular entry handle it so that
+                         * enabled/running state is determined from the unit file as before. */
+                        r = RET_NERRNO(faccessat(dirfd(d), unit_name, F_OK, AT_SYMLINK_NOFOLLOW));
+                        if (r >= 0)
+                                continue;
+                        if (r != -ENOENT)
+                                return log_debug_errno(r, "Failed to check if '%s/%s' exists: %m", where, unit_name);
+                }
 
-                r = test_chroot_dropin(d, where, de->d_name, name_or_path, extension_image_paths, NULL);
+                r = test_chroot_dropin(d, where, unit_name, name_or_path, extension_image_paths, NULL);
                 if (r < 0)
                         return r;
                 if (r == 0)
                         continue;
 
-                r = unit_file_lookup_state(scope, &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_LINKED_RUNTIME))
-                        found_enabled = true;
+                if (!dropin) {
+                        UnitFileState state;
 
-                r = unit_file_is_active(bus, de->d_name, error);
+                        r = unit_file_lookup_state(scope, &paths, unit_name, &state);
+                        if (r < 0)
+                                return log_debug_errno(r, "Failed to determine unit file state of '%s': %m", unit_name);
+                        if (!IN_SET(state, UNIT_FILE_STATIC, UNIT_FILE_DISABLED, UNIT_FILE_LINKED, UNIT_FILE_LINKED_RUNTIME))
+                                found_enabled = true;
+                }
+
+                r = unit_file_is_active(bus, unit_name, error);
                 if (r < 0)
                         return r;
                 if (r > 0)
                         found_running = true;
 
-                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();
         }
 
         *ret = found_running ? (!set_isempty(unit_files) && (flags & PORTABLE_RUNTIME) ? PORTABLE_RUNNING_RUNTIME : PORTABLE_RUNNING) :

diff --git a/test/units/TEST-29-PORTABLE.directory.sh b/test/units/TEST-29-PORTABLE.directory.sh
index 8f35c9ee0838210e709c1706a9718554e5cbd3fc..42f280b78ad2cc4c22637b772888b6aba3255468 100755 (executable)
--- a/test/units/TEST-29-PORTABLE.directory.sh
+++ b/test/units/TEST-29-PORTABLE.directory.sh
@@ -144,9 +144,17 @@ portablectl detach --now --runtime --extension /tmp/app0 /tmp/rootdir app0
 # 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
+# Attach and detach again to check if drop-in-only leftovers are still recognized and removed.
 portablectl "${ARGS[@]}" attach --copy=symlink --now --runtime /tmp/rootdir minimal-app0
+rm /run/systemd/system.attached/minimal-app0*.service
+status="$(portablectl is-attached --runtime /tmp/rootdir)"
+[[ "${status}" == "running-runtime" ]]
+systemctl stop minimal-app0.service minimal-app0-foo.service
+status="$(portablectl is-attached --runtime /tmp/rootdir)"
+[[ "${status}" == "attached-runtime" ]]
 portablectl detach --now --runtime --enable /tmp/rootdir minimal-app0
+[[ ! -d /run/systemd/system.attached/minimal-app0.service.d ]]
+[[ ! -d /run/systemd/system.attached/minimal-app0-foo.service.d ]]
 
 # The wrong file should be ignored, given the right one has the xattr set
 trap 'rm -rf /var/cache/wrongext' EXIT