diff --git a/commit.go b/commit.go index d340ca0a2fa..af3164f8173 100644 --- a/commit.go +++ b/commit.go @@ -8,7 +8,6 @@ import ( "io" "io/ioutil" "os" - "strings" "time" "github.com/containers/buildah/pkg/blobcache" @@ -374,17 +373,11 @@ func (b *Builder) Commit(ctx context.Context, dest types.ImageReference, options } if err == nil { imgID = img.ID - prunedNames := make([]string, 0, len(img.Names)) - for _, name := range img.Names { - if !(nameToRemove != "" && strings.Contains(name, nameToRemove)) { - prunedNames = append(prunedNames, name) + if nameToRemove != "" { + if err = b.store.RemoveNames(imgID, []string{nameToRemove}); err != nil { + return imgID, nil, "", fmt.Errorf("failed to remove temporary name from image %q: %w", imgID, err) } - } - 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) - } - 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) diff --git a/tests/bud.bats b/tests/bud.bats index 5fe1a137adb..b864f89ff68 100644 --- a/tests/bud.bats +++ b/tests/bud.bats @@ -390,6 +390,25 @@ _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" { + # 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 60 \ + ${BUILDAH_BINARY} ${BUILDAH_REGISTRY_OPTS} ${ROOTDIR_OPTS} $WITH_POLICY_JSON build --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. + 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 +} + # 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. diff --git a/tests/bud/check-race/Containerfile b/tests/bud/check-race/Containerfile new file mode 100644 index 00000000000..633954251d7 --- /dev/null +++ b/tests/bud/check-race/Containerfile @@ -0,0 +1,2 @@ +from quay.io/jitesoft/alpine:latest +run for i in $(seq 0 10000); do touch /$i; done