commit: use race-free RemoveNames instead of SetNames

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]>

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)

tests/bud.bats

@@ -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 60 \
+              ${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.

tests/bud/check-race/Containerfile

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