storage: Add SetNameWithOptions to make destructive nature of SetName…

… optional

Adds `SetNameWithOptions` so operations which are invoke in parallel
manner can use it without destroying names from storage.

For instance

We are deleting names which were already written in store.
This creates faulty behavior when builds are invoked in parallel manner, as
this removes names for other builds.

To fix this behavior we must append to already written names and
override if needed. But this should be optional and not break public API

Following patch will be used by parallel operations at podman or buildah end, directly or indirectly.

Signed-off-by: Aditya R <arajan@redhat.com>

containers.go

@@ -86,6 +86,10 @@ type ContainerStore interface {
 	// with the specified ID.
 	SetNames(id string, names []string) error
 
+	// SetNamesWithOptions updates the list of names associated with the container with optional destructive nature
+	// with the specified ID.
+	SetNamesWithOptions(id string, names []string, opts SetNameOptions) error
+
 	// Get retrieves information about a container given an ID or name.
 	Get(id string) (*Container, error)
 
@@ -378,11 +382,21 @@ func (r *containerStore) removeName(container *Container, name string) {
 }
 
 func (r *containerStore) SetNames(id string, names []string) error {
-	names = dedupeNames(names)
+	opts := SetNameOptions{
+		AppendOnly: false,
+	}
+	return r.SetNamesWithOptions(id, names, opts)
+}
+
+func (r *containerStore) SetNamesWithOptions(id string, names []string, opts SetNameOptions) error {
 	if container, ok := r.lookup(id); ok {
 		for _, name := range container.Names {
 			delete(r.byname, name)
+			if opts.AppendOnly {
+				names = append(names, name)
+			}
 		}
+		names = dedupeNames(names)
 		for _, name := range names {
 			if otherContainer, ok := r.byname[name]; ok {
 				r.removeName(otherContainer, name)

images.go

@@ -136,7 +136,7 @@ type ImageStore interface {
 	// SetNames replaces the list of names associated with an image with the
 	// supplied values.  The values are expected to be valid normalized
 	// named image references.
-	SetNames(id string, names []string) error
+	SetNames(id string, names []string, opts SetNameOptions) error
 
 	// Delete removes the record of the image.
 	Delete(id string) error
@@ -505,15 +505,18 @@ func (i *Image) addNameToHistory(name string) {
 	i.NamesHistory = dedupeNames(append([]string{name}, i.NamesHistory...))
 }
 
-func (r *imageStore) SetNames(id string, names []string) error {
+func (r *imageStore) SetNames(id string, names []string, opts SetNameOptions) error {
 	if !r.IsReadWrite() {
 		return errors.Wrapf(ErrStoreIsReadOnly, "not allowed to change image name assignments at %q", r.imagespath())
 	}
-	names = dedupeNames(names)
 	if image, ok := r.lookup(id); ok {
 		for _, name := range image.Names {
 			delete(r.byname, name)
+			if opts.AppendOnly {
+				names = append(names, name)
+			}
 		}
+		names = dedupeNames(names)
 		for _, name := range names {
 			if otherImage, ok := r.byname[name]; ok {
 				r.removeName(otherImage, name)

images_test.go

@@ -26,7 +26,8 @@ func addTestImage(t *testing.T, store ImageStore, id string, names []string) {
 	)
 
 	require.Nil(t, err)
-	require.Nil(t, store.SetNames(id, names))
+	opts := SetNameOptions{AppendOnly: false}
+	require.Nil(t, store.SetNames(id, names, opts))
 }
 
 func TestAddNameToHistorySuccess(t *testing.T) {
@@ -74,7 +75,8 @@ func TestHistoryNames(t *testing.T) {
 	// And When
 	store.Lock()
 	defer store.Unlock()
-	require.Nil(t, store.SetNames(firstImageID, []string{"1", "2", "3", "4"}))
+	opts := SetNameOptions{AppendOnly: false}
+	require.Nil(t, store.SetNames(firstImageID, []string{"1", "2", "3", "4"}, opts))
 
 	// Then
 	firstImage, err = store.Get(firstImageID)
@@ -91,4 +93,25 @@ func TestHistoryNames(t *testing.T) {
 	require.Len(t, secondImage.NamesHistory, 2)
 	require.Equal(t, secondImage.NamesHistory[0], "3")
 	require.Equal(t, secondImage.NamesHistory[1], "2")
+
+	// set name with option AppenOnly true must not remove old names
+	opts = SetNameOptions{AppendOnly: true}
+	require.Nil(t, store.SetNames(firstImageID, []string{"1", "2", "3", "4"}, opts))
+	firstImage, err = store.Get(firstImageID)
+	require.Nil(t, err)
+	require.Len(t, firstImage.NamesHistory, 4)
+	require.Equal(t, firstImage.NamesHistory[0], "4")
+	require.Equal(t, firstImage.NamesHistory[1], "3")
+	require.Equal(t, firstImage.NamesHistory[2], "2")
+	require.Equal(t, firstImage.NamesHistory[3], "1")
+
+	require.Nil(t, store.SetNames(firstImageID, []string{"5"}, opts))
+	firstImage, err = store.Get(firstImageID)
+	require.Nil(t, err)
+	require.Len(t, firstImage.NamesHistory, 5)
+	require.Equal(t, firstImage.NamesHistory[0], "4")
+	require.Equal(t, firstImage.NamesHistory[1], "3")
+	require.Equal(t, firstImage.NamesHistory[2], "2")
+	require.Equal(t, firstImage.NamesHistory[3], "1")
+	require.Equal(t, firstImage.NamesHistory[4], "5")
 }

layers.go

@@ -221,7 +221,7 @@ type LayerStore interface {
 
 	// SetNames replaces the list of names associated with a layer with the
 	// supplied values.
-	SetNames(id string, names []string) error
+	SetNames(id string, names []string, opts SetNameOptions) error
 
 	// Delete deletes a layer with the specified name or ID.
 	Delete(id string) error
@@ -1039,15 +1039,18 @@ func (r *layerStore) removeName(layer *Layer, name string) {
 	layer.Names = stringSliceWithoutValue(layer.Names, name)
 }
 
-func (r *layerStore) SetNames(id string, names []string) error {
+func (r *layerStore) SetNames(id string, names []string, opts SetNameOptions) error {
 	if !r.IsReadWrite() {
 		return errors.Wrapf(ErrStoreIsReadOnly, "not allowed to change layer name assignments at %q", r.layerspath())
 	}
-	names = dedupeNames(names)
 	if layer, ok := r.lookup(id); ok {
 		for _, name := range layer.Names {
 			delete(r.byname, name)
+			if opts.AppendOnly {
+				names = append(names, name)
+			}
 		}
+		names = dedupeNames(names)
 		for _, name := range names {
 			if otherLayer, ok := r.byname[name]; ok {
 				r.removeName(otherLayer, name)

store.go

@@ -154,6 +154,11 @@ type FlaggableStore interface {
 	SetFlag(id string, flag string, value interface{}) error
 }
 
+type SetNameOptions struct {
+	// Only appends new names and does not removes anything
+	AppendOnly bool
+}
+
 type StoreOptions = types.StoreOptions
 
 // Store wraps up the various types of file-based stores that we use into a
@@ -370,6 +375,10 @@ type Store interface {
 	// Duplicate names are removed from the list automatically.
 	SetNames(id string, names []string) error
 
+	// SetNamesWithOptions accepts options with capablity to either change name or append names
+	// Duplicate names are removed from the list automatically.
+	SetNamesWithOptions(id string, names []string, opts SetNameOptions) error
+
 	// ListImageBigData retrieves a list of the (possibly large) chunks of
 	// named data associated with an image.
 	ListImageBigData(id string) ([]string, error)
@@ -2051,6 +2060,13 @@ func dedupeNames(names []string) []string {
 }
 
 func (s *store) SetNames(id string, names []string) error {
+	opts := SetNameOptions{
+		AppendOnly: false,
+	}
+	return s.SetNamesWithOptions(id, names, opts)
+}
+
+func (s *store) SetNamesWithOptions(id string, names []string, opts SetNameOptions) error {
 	deduped := dedupeNames(names)
 
 	rlstore, err := s.LayerStore()
@@ -2063,7 +2079,7 @@ func (s *store) SetNames(id string, names []string) error {
 		return err
 	}
 	if rlstore.Exists(id) {
-		return rlstore.SetNames(id, deduped)
+		return rlstore.SetNames(id, deduped, opts)
 	}
 
 	ristore, err := s.ImageStore()
@@ -2076,7 +2092,7 @@ func (s *store) SetNames(id string, names []string) error {
 		return err
 	}
 	if ristore.Exists(id) {
-		return ristore.SetNames(id, deduped)
+		return ristore.SetNames(id, deduped, opts)
 	}
 
 	// Check is id refers to a RO Store
@@ -2114,6 +2130,9 @@ func (s *store) SetNames(id string, names []string) error {
 		return err
 	}
 	if rcstore.Exists(id) {
+		if opts.AppendOnly {
+			return rcstore.SetNamesWithOptions(id, deduped, opts)
+		}
 		return rcstore.SetNames(id, deduped)
 	}
 	return ErrLayerUnknown