From 38d072a17567f762d70025abb61a80716e04c8bd Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Thu, 28 Mar 2024 15:15:06 +0100 Subject: [PATCH 1/4] test: bump the container boot timeout when running w/o acceleration With plain QEMU on a saturated AWS region we might just barely miss the timeout window, causing unexpected test fails: [ 688.681324] systemd-nspawn[1332]: [ OK ] Finished systemd-user-sessions.service. [ 689.451267] systemd-nspawn[1332]: [ OK ] Started console-getty.service. [ 689.572874] systemd-nspawn[1332]: [ OK ] Reached target getty.target. [ 693.634609] testsuite-74.sh[1223]: + at_exit [ 693.634609] testsuite-74.sh[1223]: + rm -fv -- /tmp/test-dump /tmp/test-usr-dump /tmp/make-dump [ 693.838395] testsuite-74.sh[1502]: removed '/tmp/test-dump' [ 693.838395] testsuite-74.sh[1502]: removed '/tmp/test-usr-dump' [ 693.838395] testsuite-74.sh[1502]: removed '/tmp/make-dump' [ 693.951114] testsuite-74.sh[670]: + echo 'Subtest /usr/lib/systemd/tests/testdata/units/testsuite-74.coredump.sh failed' [ 693.951114] testsuite-74.sh[670]: Subtest /usr/lib/systemd/tests/testdata/units/testsuite-74.coredump.sh failed [ 693.951114] testsuite-74.sh[670]: + return 1 [ 694.659094] systemd[1]: testsuite-74.service: Main process exited, code=exited, status=1/FAILURE [ 694.719563] systemd[1]: testsuite-74.service: Failed with result 'exit-code'. [ 694.882069] systemd[1]: Failed to start testsuite-74.service. [ 695.574445] systemd[1]: Reached target testsuite.target. [ 696.174844] systemd[1]: Starting end.service... [ 699.509408] systemd-nspawn[1332]: [ 699.509408] systemd-nspawn[1332]: CentOS Stream 9 [ 699.509408] systemd-nspawn[1332]: Kernel 5.14.0-432.el9.x86_64 on an x86_64 (pts/0) [ 699.509408] systemd-nspawn[1332]: Also, move the rest of container the setup for the user xattrs test into the condition, since doing it without the actual test is pretty pointless. --- test/units/testsuite-74.coredump.sh | 24 ++++++++++++++---------- 1 file changed, 14 insertions(+), 10 deletions(-) diff --git a/test/units/testsuite-74.coredump.sh b/test/units/testsuite-74.coredump.sh index 6552643ee9..f48607c0db 100755 --- a/test/units/testsuite-74.coredump.sh +++ b/test/units/testsuite-74.coredump.sh @@ -77,24 +77,26 @@ rm -fv /run/systemd/coredump.conf.d/99-external.conf # Wait a bit for the coredumps to get processed timeout 30 bash -c "while [[ \$(coredumpctl list -q --no-legend $CORE_TEST_BIN | wc -l) -lt 4 ]]; do sleep 1; done" -# Make sure we can forward crashes back to containers -CONTAINER="testsuite-74-container" +if cgroupfs_supports_user_xattrs; then + # Make sure we can forward crashes back to containers + CONTAINER="testsuite-74-container" -mkdir -p "/var/lib/machines/$CONTAINER" -mkdir -p "/run/systemd/system/systemd-nspawn@$CONTAINER.service.d" -# Bind-mounting /etc into the container kinda defeats the purpose of --volatile=, -# but we need the ASan-related overrides scattered across /etc -cat > "/run/systemd/system/systemd-nspawn@$CONTAINER.service.d/override.conf" << EOF + mkdir -p "/var/lib/machines/$CONTAINER" + mkdir -p "/run/systemd/system/systemd-nspawn@$CONTAINER.service.d" + # Bind-mounting /etc into the container kinda defeats the purpose of --volatile=, + # but we need the ASan-related overrides scattered across /etc + cat > "/run/systemd/system/systemd-nspawn@$CONTAINER.service.d/override.conf" < Date: Thu, 28 Mar 2024 15:20:22 +0100 Subject: [PATCH 2/4] test: check if sd-executor doesn't complain during (de)serializing This was usually caught by "accident" by other tests that parse output of commands executed by sd-executor, so let's have an explicit check for this. --- test/units/end.sh | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/test/units/end.sh b/test/units/end.sh index 4c6c0afa0e..5e2943758a 100755 --- a/test/units/end.sh +++ b/test/units/end.sh @@ -9,5 +9,9 @@ set -o pipefail # Here, the redundant '[ ]' in the pattern is required in order not to match the logged command itself. (! journalctl -q -o short-monotonic --grep 'Warning: cannot close sd-bus connection[ ].*after fork' >>/failed) +# Check if sd-executor doesn't complain about not being able to (de)serialize stuff +(! journalctl -q -o short-monotonic --grep "[F]ailed to parse serialized line" >>/failed) +(! journalctl -q -o short-monotonic --grep "[F]ailed to (de)?serialize \w+" >>/failed) + systemctl poweroff --no-block exit 0 From 347756ed810e28bac5e5b0c1f12978f91dfa65c8 Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Tue, 2 Apr 2024 18:31:03 +0200 Subject: [PATCH 3/4] core: correctly deserialize credentials with empty payload For example with SetCredential=mycred: the data payload is empty, but it is still a valid credential. This reorders the arguments when serializing credentials, so the possibly empty argument is not at the end of the serialized string. This way we can still easily use the extract_many_words() machinery, and with the use of EXTRACT_DONT_COALESCE_SEPARATORS properly deserialize even an empty credential. This changes LoadCredentials= as well just to keep the code for (de)serializing both directives in sync. --- src/core/execute-serialize.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/core/execute-serialize.c b/src/core/execute-serialize.c index 1ae77f39fb..9e402e5e69 100644 --- a/src/core/execute-serialize.c +++ b/src/core/execute-serialize.c @@ -2544,14 +2544,14 @@ static int exec_context_serialize(const ExecContext *c, FILE *f) { if (base64mem(sc->data, sc->size, &data) < 0) return log_oom_debug(); - r = serialize_item_format(f, "exec-context-set-credentials", "%s %s %s", sc->id, yes_no(sc->encrypted), data); + r = serialize_item_format(f, "exec-context-set-credentials", "%s %s %s", sc->id, data, yes_no(sc->encrypted)); if (r < 0) return r; } ExecLoadCredential *lc; HASHMAP_FOREACH(lc, c->load_credentials) { - r = serialize_item_format(f, "exec-context-load-credentials", "%s %s %s", lc->id, yes_no(lc->encrypted), lc->path); + r = serialize_item_format(f, "exec-context-load-credentials", "%s %s %s", lc->id, lc->path, yes_no(lc->encrypted)); if (r < 0) return r; } @@ -3668,7 +3668,7 @@ static int exec_context_deserialize(ExecContext *c, FILE *f) { _cleanup_(exec_set_credential_freep) ExecSetCredential *sc = NULL; _cleanup_free_ char *id = NULL, *encrypted = NULL, *data = NULL; - r = extract_many_words(&val, " ", 0, &id, &encrypted, &data); + r = extract_many_words(&val, " ", EXTRACT_DONT_COALESCE_SEPARATORS, &id, &data, &encrypted); if (r < 0) return r; if (r != 3) @@ -3700,7 +3700,7 @@ static int exec_context_deserialize(ExecContext *c, FILE *f) { _cleanup_(exec_load_credential_freep) ExecLoadCredential *lc = NULL; _cleanup_free_ char *id = NULL, *encrypted = NULL, *path = NULL; - r = extract_many_words(&val, " ", 0, &id, &encrypted, &path); + r = extract_many_words(&val, " ", EXTRACT_DONT_COALESCE_SEPARATORS, &id, &path, &encrypted); if (r < 0) return r; if (r != 3) From 5d7b7bf1d488c6aa6dc2e0068e46fb61a8d75d1e Mon Sep 17 00:00:00 2001 From: Frantisek Sumsal Date: Tue, 2 Apr 2024 18:32:22 +0200 Subject: [PATCH 4/4] test: don't hide exit code of the systemd-run process --- test/units/testsuite-54.sh | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/test/units/testsuite-54.sh b/test/units/testsuite-54.sh index 7618e92c9f..3ed81f4359 100755 --- a/test/units/testsuite-54.sh +++ b/test/units/testsuite-54.sh @@ -8,13 +8,20 @@ set -eux systemd-analyze log-level debug -run_with_cred_compare() { +run_with_cred_compare() ( local cred="${1:?}" local exp="${2?}" + local log_file shift 2 - diff <(systemd-run -p SetCredential="$cred" --wait --pipe -- systemd-creds "$@") <(echo -ne "$exp") -} + log_file="$(mktemp)" + # shellcheck disable=SC2064 + trap "rm -f '$log_file'" RETURN + + set -o pipefail + systemd-run -p SetCredential="$cred" --wait --pipe -- systemd-creds "$@" | tee "$log_file" + diff "$log_file" <(echo -ne "$exp") +) # Sanity checks #