containers / podman

Podman: A tool for managing OCI containers and pods.
https://podman.io
Apache License 2.0
23.74k stars 2.41k forks source link

Regression: Quadlet: conflict names is nondeterministically failing #24047

Closed edsantiago closed 1 month ago

edsantiago commented 1 month ago

Has something changed in the processing of $QUADLET_UNIT_DIRS?

not ok 215 |252| quadlet conflict names in 2083ms
# #/vvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvvv
# #|     FAIL: quadlet should show Notify=yes
# #| expected: =~ Notify=yes
# #|   actual: .... Notify=no

First seen today.

Reproducer:

$ while :;do hack/bats --rootless 252:conflict || break;done
...
fails on my laptop within 3-4 iterations. [EDIT: Usually. Right now, of course, on my final test, it took about 20 iterations]

[sys] |252| quadlet conflict names

x x x x x x
sys(2) podman(2) debian-13(1) rootless(2) host(2) sqlite(2)
fedora-40(1)

For extra credit, a patch similar to this one might be appreciated by future maintainers looking at the test failure.

diff --git a/test/system/252-quadlet.bats b/test/system/252-quadlet.bats
index 3ccf97d269..ada1198106 100644
--- a/test/system/252-quadlet.bats
+++ b/test/system/252-quadlet.bats
@@ -230,13 +230,13 @@ EOF

     cat > $dir1/$quadlet_file <<EOF
 [Container]
-Image=$IMAGE
+Image=quay.io/libpod/this-is-the-one:wewant
 Notify=yes
 EOF

     cat > $dir2/$quadlet_file <<EOF
 [Container]
-Image=$IMAGE
+Image=quay.io/libpod/bad-bad-bad:nonono
 Notify=no
 EOF
     QUADLET_UNIT_DIRS="$dir1:$dir2" run \
edsantiago commented 1 month ago

OBTW it's not an alphanumeric-sorting issue: I tried dir1=aaa, dir2=bbb and vice-versa, and it still flakes unpredictably.

vrothberg commented 1 month ago

@ygalblum, ideas? I didn't find the time to follow all recent Quadlet improvements.

giuseppe commented 1 month ago

commit 133ea31ffb9e6cd9ccbbc01053463f04c11f7f18 introduced the regression. Moving from []string to map[string]struct{} broke the assumption that the entries are ordered

ygalblum commented 1 month ago

@giuseppe is correct, thanks. I think we need to keep the order assumption because it provides predictability as to which file is processed and which is skipped. So, I'll need to rework on that change.

giuseppe commented 1 month ago

the previous code was not deterministic for sub dirs. I think we need to sort the dir entries too

ygalblum commented 1 month ago

Are you sure? Because, the previous code used WalkDir which states that The files are walked in lexical order, which makes the output deterministic and each new directory was appended to the end of the list.

giuseppe commented 1 month ago

sorry you are right. WalkDir is fine