commit: use race-free RemoveNames instead of SetNames

PR https://github.com/containers/storage/pull/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: https://github.com/containers/podman/issues/15162

Signed-off-by: Aditya R <arajan@redhat.com>
This commit is contained in:
Aditya R 2022-08-17 13:54:40 +05:30
parent f1c022d84b
commit 354f96f6b6
No known key found for this signature in database
GPG Key ID: 8E5A8A19DF7C8673
3 changed files with 27 additions and 7 deletions

View File

@ -374,17 +374,17 @@ func (b *Builder) Commit(ctx context.Context, dest types.ImageReference, options
}
if err == nil {
imgID = img.ID
prunedNames := make([]string, 0, len(img.Names))
toPruneNames := make([]string, 0, len(img.Names))
for _, name := range img.Names {
if !(nameToRemove != "" && strings.Contains(name, nameToRemove)) {
prunedNames = append(prunedNames, name)
if nameToRemove != "" && strings.Contains(name, nameToRemove) {
toPruneNames = append(toPruneNames, name)
}
}
if len(prunedNames) < len(img.Names) {
if err = b.store.SetNames(imgID, prunedNames); err != nil {
return imgID, nil, "", fmt.Errorf("failed to prune temporary name from image %q: %w", imgID, err)
if len(toPruneNames) > 0 {
if err = b.store.RemoveNames(imgID, toPruneNames); err != nil {
return imgID, nil, "", fmt.Errorf("failed to remove temporary name from image %q: %w", imgID, err)
}
logrus.Debugf("reassigned names %v to image %q", prunedNames, img.ID)
logrus.Debugf("removing %v from assigned names to image %q", nameToRemove, img.ID)
dest2, err := is.Transport.ParseStoreReference(b.store, "@"+imgID)
if err != nil {
return imgID, nil, "", fmt.Errorf("error creating unnamed destination reference for image: %w", err)

View File

@ -390,6 +390,24 @@ _EOF
assert "$output" !~ "unwanted stage"
}
# Note: Please skip this tests in case of podman-remote build
@test "build: test race in updating image name while performing parallel commits" {
_prefetch alpine
# Run 25 parallel builds using the same Containerfile
local count=25
for i in $(seq --format '%02g' 1 $count); do
timeout --foreground -v --kill=10 300 \
${BUILDAH_BINARY} ${BUILDAH_REGISTRY_OPTS} ${ROOTDIR_OPTS} $WITH_POLICY_JSON build --quiet --squash --iidfile ${TEST_SCRATCH_DIR}/id.$i --timestamp 0 -f $BUDFILES/check-race/Containerfile >/dev/null &
done
# Wait for all background builds to complete. Note that this succeeds
# 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.
assert "$(cat ${TEST_SCRATCH_DIR}/id.* | wc -c)" = 1775 "Total chars in all id.* files"
# clean all images built for this test
run_buildah rmi --all -f
}
# Test skipping images with FROM but stage name also conflicts with additional build context
# so selected stage should be still skipped since it is not being actually used by additional build
# context is being used.

View File

@ -0,0 +1,2 @@
FROM alpine
RUN for i in $(seq 0 1000); do touch /$i; done