store: add independent AddNames and RemoveNames for images,layers,con…

…tainers

Adds AddNames and RemoveNames so operations which are invoked 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 <[email protected]>

containers.go

@@ -86,6 +86,14 @@ type ContainerStore interface {
 	// with the specified ID.
 	SetNames(id string, names []string) error
 
+	// AddNames adds the list of names associated with a container to storage with the
+	// supplied values.
+	AddNames(id string, names []string) error
+
+	// RemoveNames removes the list of names from associated with a container with the
+	// supplied values.
+	RemoveNames(id string, names []string) error
+
 	// Get retrieves information about a container given an ID or name.
 	Get(id string) (*Container, error)
 
@@ -395,6 +403,50 @@ func (r *containerStore) SetNames(id string, names []string) error {
 	return ErrContainerUnknown
 }
 
+func (r *containerStore) AddNames(id string, names []string) error {
+	if container, ok := r.lookup(id); ok {
+		for _, name := range container.Names {
+			delete(r.byname, name)
+			names = append(names, name)
+		}
+		names = dedupeNames(names)
+		for _, name := range names {
+			if otherContainer, ok := r.byname[name]; ok {
+				r.removeName(otherContainer, name)
+			}
+			r.byname[name] = container
+		}
+		container.Names = names
+		return r.Save()
+	}
+	return ErrContainerUnknown
+}
+
+func (r *containerStore) RemoveNames(id string, names []string) error {
+	namesFiltered := make([]string, 1)
+	if container, ok := r.lookup(id); ok {
+		for _, name := range container.Names {
+			delete(r.byname, name)
+			for _, n := range names {
+				if n == name {
+					continue
+				}
+			}
+			namesFiltered = append(namesFiltered, name)
+		}
+		names = dedupeNames(namesFiltered)
+		for _, name := range names {
+			if otherContainer, ok := r.byname[name]; ok {
+				r.removeName(otherContainer, name)
+			}
+			r.byname[name] = container
+		}
+		container.Names = names
+		return r.Save()
+	}
+	return ErrContainerUnknown
+}
+
 func (r *containerStore) Delete(id string) error {
 	container, ok := r.lookup(id)
 	if !ok {

images.go

@@ -138,6 +138,16 @@ type ImageStore interface {
 	// named image references.
 	SetNames(id string, names []string) error
 
+	// AddNames adds the list of names associated with an image to storage with the
+	// supplied values.  The values are expected to be valid normalized
+	// named image references.
+	AddNames(id string, names []string) error
+
+	// RemoveNames removes the list of names from associated with an image with the
+	// supplied values.  The values are expected to be valid normalized
+	// named image references.
+	RemoveNames(id string, names []string) error
+
 	// Delete removes the record of the image.
 	Delete(id string) error
 
@@ -527,6 +537,59 @@ func (r *imageStore) SetNames(id string, names []string) error {
 	return errors.Wrapf(ErrImageUnknown, "error locating image with ID %q", id)
 }
 
+func (r *imageStore) AddNames(id string, names []string) error {
+	if !r.IsReadWrite() {
+		return errors.Wrapf(ErrStoreIsReadOnly, "not allowed to change image name assignments at %q", r.imagespath())
+	}
+	if image, ok := r.lookup(id); ok {
+		for _, name := range image.Names {
+			delete(r.byname, name)
+			names = append(names, name)
+		}
+		names = dedupeNames(names)
+		for _, name := range names {
+			if otherImage, ok := r.byname[name]; ok {
+				r.removeName(otherImage, name)
+			}
+			r.byname[name] = image
+			image.addNameToHistory(name)
+		}
+		image.Names = names
+		return r.Save()
+	}
+	return errors.Wrapf(ErrImageUnknown, "error locating image with ID %q", id)
+}
+
+func (r *imageStore) RemoveNames(id string, names []string) error {
+	if !r.IsReadWrite() {
+		return errors.Wrapf(ErrStoreIsReadOnly, "not allowed to change image name assignments at %q", r.imagespath())
+	}
+
+	namesFiltered := make([]string, 1)
+	if image, ok := r.lookup(id); ok {
+		for _, name := range image.Names {
+			delete(r.byname, name)
+			for _, n := range names {
+				if n == name {
+					continue
+				}
+			}
+			namesFiltered = append(namesFiltered, name)
+		}
+		names = dedupeNames(namesFiltered)
+		for _, name := range names {
+			if otherImage, ok := r.byname[name]; ok {
+				r.removeName(otherImage, name)
+			}
+			r.byname[name] = image
+			image.addNameToHistory(name)
+		}
+		image.Names = names
+		return r.Save()
+	}
+	return errors.Wrapf(ErrImageUnknown, "error locating image with ID %q", id)
+}
+
 func (r *imageStore) Delete(id string) error {
 	if !r.IsReadWrite() {
 		return errors.Wrapf(ErrStoreIsReadOnly, "not allowed to delete images at %q", r.imagespath())

images_test.go

@@ -91,4 +91,24 @@ 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")
+
+	// test independent add and remove operations
+	require.Nil(t, store.AddNames(firstImageID, []string{"5"}))
+	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")
+
+	require.Nil(t, store.RemoveNames(firstImageID, []string{"5"}))
+	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")
 }

layers.go

@@ -223,6 +223,14 @@ type LayerStore interface {
 	// supplied values.
 	SetNames(id string, names []string) error
 
+	// AddNames adds the list of names associated with a layer to storage with the
+	// supplied values.
+	AddNames(id string, names []string) error
+
+	// RemoveNames removes the list of names from associated with a layer with the
+	// supplied values.
+	RemoveNames(id string, names []string) error
+
 	// Delete deletes a layer with the specified name or ID.
 	Delete(id string) error
 
@@ -1060,6 +1068,57 @@ func (r *layerStore) SetNames(id string, names []string) error {
 	return ErrLayerUnknown
 }
 
+func (r *layerStore) AddNames(id string, names []string) error {
+	if !r.IsReadWrite() {
+		return errors.Wrapf(ErrStoreIsReadOnly, "not allowed to change layer name assignments at %q", r.layerspath())
+	}
+	if layer, ok := r.lookup(id); ok {
+		for _, name := range layer.Names {
+			delete(r.byname, name)
+			names = append(names, name)
+		}
+		names = dedupeNames(names)
+		for _, name := range names {
+			if otherLayer, ok := r.byname[name]; ok {
+				r.removeName(otherLayer, name)
+			}
+			r.byname[name] = layer
+		}
+		layer.Names = names
+		return r.Save()
+	}
+	return ErrLayerUnknown
+}
+
+func (r *layerStore) RemoveNames(id string, names []string) error {
+	if !r.IsReadWrite() {
+		return errors.Wrapf(ErrStoreIsReadOnly, "not allowed to change layer name assignments at %q", r.layerspath())
+	}
+	names = dedupeNames(names)
+	namesFiltered := make([]string, 1)
+	if layer, ok := r.lookup(id); ok {
+		for _, name := range layer.Names {
+			delete(r.byname, name)
+			for _, n := range names {
+				if n == name {
+					continue
+				}
+			}
+			namesFiltered = append(namesFiltered, name)
+		}
+		names = dedupeNames(namesFiltered)
+		for _, name := range names {
+			if otherLayer, ok := r.byname[name]; ok {
+				r.removeName(otherLayer, name)
+			}
+			r.byname[name] = layer
+		}
+		layer.Names = names
+		return r.Save()
+	}
+	return ErrLayerUnknown
+}
+
 func (r *layerStore) datadir(id string) string {
 	return filepath.Join(r.layerdir, id)
 }

store.go

@@ -31,6 +31,14 @@ import (
 	"github.com/pkg/errors"
 )
 
+type UpdateNameOperation int
+
+const (
+	SetNames UpdateNameOperation = iota
+	AddNames
+	RemoveNames
+)
+
 var (
 	stores     []*store
 	storesLock sync.Mutex
@@ -370,6 +378,14 @@ type Store interface {
 	// Duplicate names are removed from the list automatically.
 	SetNames(id string, names []string) error
 
+	// SetNames adds the list of names for a layer, image, or container.
+	// Duplicate names are removed from the list automatically.
+	AddNames(id string, names []string) error
+
+	// RemoveNames removes the list of names for a layer, image, or container.
+	// Duplicate names are removed from the list automatically.
+	RemoveNames(id string, names []string) error
+
 	// ListImageBigData retrieves a list of the (possibly large) chunks of
 	// named data associated with an image.
 	ListImageBigData(id string) ([]string, error)
@@ -2051,6 +2067,18 @@ func dedupeNames(names []string) []string {
 }
 
 func (s *store) SetNames(id string, names []string) error {
+	return s.updateNames(id, names, SetNames)
+}
+
+func (s *store) AddNames(id string, names []string) error {
+	return s.updateNames(id, names, AddNames)
+}
+
+func (s *store) RemoveNames(id string, names []string) error {
+	return s.updateNames(id, names, RemoveNames)
+}
+
+func (s *store) updateNames(id string, names []string, op UpdateNameOperation) error {
 	deduped := dedupeNames(names)
 
 	rlstore, err := s.LayerStore()
@@ -2063,7 +2091,14 @@ func (s *store) SetNames(id string, names []string) error {
 		return err
 	}
 	if rlstore.Exists(id) {
-		return rlstore.SetNames(id, deduped)
+		switch op {
+		case SetNames:
+			return rlstore.SetNames(id, deduped)
+		case RemoveNames:
+			return rlstore.RemoveNames(id, deduped)
+		case AddNames:
+			return rlstore.RemoveNames(id, deduped)
+		}
 	}
 
 	ristore, err := s.ImageStore()
@@ -2076,7 +2111,14 @@ func (s *store) SetNames(id string, names []string) error {
 		return err
 	}
 	if ristore.Exists(id) {
-		return ristore.SetNames(id, deduped)
+		switch op {
+		case SetNames:
+			return ristore.SetNames(id, deduped)
+		case RemoveNames:
+			return ristore.RemoveNames(id, deduped)
+		case AddNames:
+			return ristore.RemoveNames(id, deduped)
+		}
 	}
 
 	// Check is id refers to a RO Store
@@ -2114,7 +2156,14 @@ func (s *store) SetNames(id string, names []string) error {
 		return err
 	}
 	if rcstore.Exists(id) {
-		return rcstore.SetNames(id, deduped)
+		switch op {
+		case SetNames:
+			return rcstore.SetNames(id, deduped)
+		case RemoveNames:
+			return rcstore.RemoveNames(id, deduped)
+		case AddNames:
+			return rcstore.RemoveNames(id, deduped)
+		}
 	}
 	return ErrLayerUnknown
 }