From 1441d3301794e5678806de04154bbccb753b896c Mon Sep 17 00:00:00 2001 From: Aditya R Date: Wed, 23 Feb 2022 20:43:34 +0530 Subject: [PATCH] store: add independent AddNames and RemoveNames for images,layers,containers 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 --- containers.go | 52 +++++++++++++++++++++++++++++++++++++++++ images.go | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++ images_test.go | 20 ++++++++++++++++ layers.go | 59 ++++++++++++++++++++++++++++++++++++++++++++++ store.go | 55 ++++++++++++++++++++++++++++++++++++++++--- 5 files changed, 246 insertions(+), 3 deletions(-) diff --git a/containers.go b/containers.go index 5425f03394..7a773dfb82 100644 --- a/containers.go +++ b/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 { + for _, n := range names { + if n == name { + continue + } + } + delete(r.byname, name) + 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 { diff --git a/images.go b/images.go index 882ba78943..74d8f5fb76 100644 --- a/images.go +++ b/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 { + for _, n := range names { + if n == name { + continue + } + } + delete(r.byname, name) + 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()) diff --git a/images_test.go b/images_test.go index 24512448c0..747c8b1c08 100644 --- a/images_test.go +++ b/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") } diff --git a/layers.go b/layers.go index 985d5f644b..91fa0182a3 100644 --- a/layers.go +++ b/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 @@ -1061,6 +1069,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 { + for _, n := range names { + if n == name { + continue + } + } + delete(r.byname, name) + 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) } diff --git a/store.go b/store.go index f49266c2c5..19096d48c7 100644 --- a/store.go +++ b/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 }