Merge pull request #1700 from Luap99/parallel-tag

libimage: support parallel tag/untag

libimage/image.go

@@ -580,17 +580,13 @@ func (i *Image) Tag(name string) error {
 		defer i.runtime.writeEvent(&Event{ID: i.ID(), Name: name, Time: time.Now(), Type: EventTypeImageTag})
 	}
 
-	newNames := append(i.Names(), ref.String())
-	if err := i.runtime.store.SetNames(i.ID(), newNames); err != nil {
+	if err := i.runtime.store.AddNames(i.ID(), []string{ref.String()}); err != nil {
 		return err
 	}
 
 	return i.reload()
 }
 
-// to have some symmetry with the errors from containers/storage.
-var errTagUnknown = errors.New("tag not known")
-
 // TODO (@vrothberg) - `docker rmi sha256:` will remove the digest from the
 // image.  However, that's something containers storage does not support.
 var errUntagDigest = errors.New("untag by digest not supported")
@@ -625,21 +621,7 @@ func (i *Image) Untag(name string) error {
 		defer i.runtime.writeEvent(&Event{ID: i.ID(), Name: name, Time: time.Now(), Type: EventTypeImageUntag})
 	}
 
-	removedName := false
-	newNames := []string{}
-	for _, n := range i.Names() {
-		if n == name {
-			removedName = true
-			continue
-		}
-		newNames = append(newNames, n)
-	}
-
-	if !removedName {
-		return fmt.Errorf("%s: %w", name, errTagUnknown)
-	}
-
-	if err := i.runtime.store.SetNames(i.ID(), newNames); err != nil {
+	if err := i.runtime.store.RemoveNames(i.ID(), []string{name}); err != nil {
 		return err
 	}
 

libimage/image_test.go

@@ -3,7 +3,9 @@ package libimage
 import (
 	"context"
 	"errors"
+	"fmt"
 	"os"
+	"sync"
 	"testing"
 
 	"github.com/containers/common/pkg/config"
@@ -261,21 +263,8 @@ func TestInspectHealthcheck(t *testing.T) {
 }
 
 func TestTag(t *testing.T) {
-	// Note: this will resolve pull from the GCR registry (see
-	// testdata/registries.conf).
-	busyboxLatest := "docker.io/library/busybox:latest"
-
-	runtime, cleanup := testNewRuntime(t)
+	runtime, image, cleanup := getImageAndRuntime(t)
 	defer cleanup()
-	ctx := context.Background()
-
-	pullOptions := &PullOptions{}
-	pullOptions.Writer = os.Stdout
-	pulledImages, err := runtime.Pull(ctx, busyboxLatest, config.PullPolicyMissing, pullOptions)
-	require.NoError(t, err)
-	require.Len(t, pulledImages, 1)
-
-	image := pulledImages[0]
 
 	digest := "sha256:adab3844f497ab9171f070d4cae4114b5aec565ac772e2f2579405b78be67c96"
 
@@ -307,26 +296,64 @@ func TestTag(t *testing.T) {
 	}
 
 	// Check for specific error.
-	err = image.Tag("foo@" + digest)
+	err := image.Tag("foo@" + digest)
 	require.True(t, errors.Is(err, errTagDigest), "check for specific digest error")
 }
 
-func TestUntag(t *testing.T) {
-	// Note: this will resolve pull from the GCR registry (see
-	// testdata/registries.conf).
-	busyboxLatest := "docker.io/library/busybox:latest"
-
-	runtime, cleanup := testNewRuntime(t)
+func TestTagAndUntagParallel(t *testing.T) {
+	runtime, image, cleanup := getImageAndRuntime(t)
 	defer cleanup()
-	ctx := context.Background()
 
-	pullOptions := &PullOptions{}
-	pullOptions.Writer = os.Stdout
-	pulledImages, err := runtime.Pull(ctx, busyboxLatest, config.PullPolicyMissing, pullOptions)
-	require.NoError(t, err)
-	require.Len(t, pulledImages, 1)
+	tagCount := 10
+	wg := sync.WaitGroup{}
 
-	image := pulledImages[0]
+	origNames := image.Names()
+
+	names := make([]string, 0, tagCount)
+	names = append(names, origNames...)
+
+	// Test tag in parallel, the extra go routine is critical for the test do not remove that.
+	wg.Add(tagCount)
+	for i := 0; i < tagCount; i++ {
+		name := fmt.Sprintf("localhost/tag-%d:latest", i)
+		names = append(names, name)
+		go func(name string) {
+			defer wg.Done()
+			err := image.Tag(name)
+			require.NoError(t, err, "parallel tag should have succeeded")
+		}(name)
+	}
+
+	// wait for all routines to finish
+	wg.Wait()
+
+	newImg, _, err := runtime.LookupImage(image.ID(), nil)
+	require.NoError(t, err, "image should have resolved locally")
+	// Note use ElementsMatch because the order is unspecified to the parallel nature
+	require.ElementsMatch(t, names, newImg.Names(), "tag image names should contain same elements")
+
+	// Test untag in parallel
+	wg.Add(tagCount)
+	for i := 0; i < tagCount; i++ {
+		name := fmt.Sprintf("localhost/tag-%d:latest", i)
+		names = append(names, name)
+		go func(name string) {
+			defer wg.Done()
+			err := image.Untag(name)
+			require.NoError(t, err, "parallel untag should have succeeded")
+		}(name)
+	}
+	// wait for all routines to finish
+	wg.Wait()
+
+	newImg, _, err = runtime.LookupImage(image.ID(), nil)
+	require.NoError(t, err, "image should have resolved locally")
+	require.Equal(t, origNames, newImg.Names(), "untag image names should contain same elements")
+}
+
+func TestUntag(t *testing.T) {
+	runtime, image, cleanup := getImageAndRuntime(t)
+	defer cleanup()
 
 	digest := "sha256:adab3844f497ab9171f070d4cae4114b5aec565ac772e2f2579405b78be67c96"
 
@@ -360,6 +387,25 @@ func TestUntag(t *testing.T) {
 	}
 
 	// Check for specific error.
-	err = image.Untag(digest)
+	err := image.Untag(digest)
 	require.True(t, errors.Is(err, errUntagDigest), "check for specific digest error")
 }
+
+func getImageAndRuntime(t *testing.T) (*Runtime, *Image, func()) {
+	// Note: this will resolve pull from the GCR registry (see
+	// testdata/registries.conf).
+	busyboxLatest := "docker.io/library/busybox:latest"
+
+	runtime, cleanup := testNewRuntime(t)
+	ctx := context.Background()
+
+	pullOptions := &PullOptions{}
+	pullOptions.Writer = os.Stdout
+	pulledImages, err := runtime.Pull(ctx, busyboxLatest, config.PullPolicyMissing, pullOptions)
+	require.NoError(t, err)
+	require.Len(t, pulledImages, 1)
+
+	image := pulledImages[0]
+
+	return runtime, image, cleanup
+}