commit: use race-free RemoveNames instead of SetNames for pruning names
#4190
Conversation
flouthoc
changed the title
RemoveNames instead of SetNamesRemoveNames instead of SetNames for pruning names
|
The test passes on my local I wonder what did i do wrong here that it is failing on CI. @edsantiago Could help me by providing any hints ? My local output ++ time bats --tap . --filter "performing parallel commits"
1..1
ok 1 build: test race in updating image name while performing parallel commits
real 0m34.193s
user 2m13.625s
sys 1m22.299s |
|
Assuming its the wildcard causing issues in CI, so applying this diff --git a/tests/bud.bats b/tests/bud.bats
index 562414e4..70fc9146 100644
--- a/tests/bud.bats
+++ b/tests/bud.bats
@@ -402,7 +402,9 @@ _EOF
# even if some of the individual builds fail! Our actual test is below.
wait
# Number of output bytes must be always same, which confirms that there is no race.
- test $(cat ${TEST_SCRATCH_DIR}/id*|wc -c) = 1775
+ for i in $(seq --format '%02g' 1 $count); do
+ test $(cat ${TEST_SCRATCH_DIR}/id.$i|wc -c) = 71
+ done
# clean all images built for this test
run_buildah rmi --all -f
} |
flouthoc
force-pushed
the
race-free-commit
branch
from
2ba3334 to
9caf0f9
Compare
flouthoc
force-pushed
the
race-free-commit
branch
2 times, most recently
from
640360e to
d895517
Compare
tests/bud.bats
Outdated
| wait | ||
| # Number of output bytes must be always same, which confirms that there is no race. | ||
| for i in $(seq --format '%02g' 1 $count); do | ||
| test $(cat ${TEST_SCRATCH_DIR}/id.$i|wc -c) = 71 |
There was a problem hiding this comment.
Please do not use test. Do not ever use test. That gives no information whatsoever to the poor person trying to debug a test failure. Please use assert.
There was a problem hiding this comment.
So, going back to the first question, you can rewrite it the first way as
assert "$(cat ${TEST_SCRATCH_DIR}/id.* | wc -c)" = 1775 "Total chars in all id.* files"There was a problem hiding this comment.
Thanks @edsantiago , Amended, I'll wait and hope CI goes green now ( passes on my local )
flouthoc
force-pushed
the
race-free-commit
branch
from
d895517 to
ca22c84
Compare
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: edsantiago, flouthoc, giuseppe The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
|
/lgtm |
flouthoc
force-pushed
the
race-free-commit
branch
from
ca22c84 to
c1a8801
Compare
flouthoc
force-pushed
the
race-free-commit
branch
from
c1a8801 to
5f7ed11
Compare
|
I see no fixes for all the other failures...? |
@edsantiago ackd, I'll check other failing tests now. |
flouthoc
force-pushed
the
race-free-commit
branch
from
5f7ed11 to
a7bc044
Compare
I think this should be good now. :) |
|
LGTM and tests are green |
flouthoc
force-pushed
the
race-free-commit
branch
from
a7bc044 to
1228dcd
Compare
|
LGTM |
|
/lgtm |
PR containers/storage#1153 added a dedicated API to remove names assigned image so use `RemoveNames` instead of racy `SetNames`. How to verify ```console printf 'from quay.io/jitesoft/alpine:latest\nrun for i in $(seq 0 10000); do touch /$i; done\n' >Containerfile && for i in `seq 1 25`; do ./buildah build --squash --iidfile id.$i --timestamp 0 . & done; wait; ls -al ``` * Refer to newly added integration test. Closes: containers/podman#15162 Signed-off-by: Aditya R <[email protected]>
flouthoc
force-pushed
the
race-free-commit
branch
from
1228dcd to
354f96f
Compare
Rebased cause merge bot was stuck. |
|
/lgtm |
PR containers/storage#1153 added a dedicated API
to remove names assigned to image so use
RemoveNamesinstead of racy
SetNames.How to verify
Reproduce on local using
printf 'from quay.io/jitesoft/alpine:latest\nrun for i in $(seq 0 10000); do touch /$i; done\n' >Containerfile && for i in `seq 1 25`; do ./buildah build --squash --iidfile id.$i --timestamp 0 . & done; wait; ls -alCloses: containers/podman#15162