Skip to content

Commit

Permalink
store: add independent AddNames and RemoveNames for images,layers,con…
Browse files Browse the repository at this point in the history
…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 <arajan@redhat.com>
  • Loading branch information
flouthoc committed Feb 23, 2022
1 parent dc58854 commit 1441d33
Show file tree
Hide file tree
Showing 5 changed files with 246 additions and 3 deletions.
52 changes: 52 additions & 0 deletions containers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down Expand Up @@ -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 {
Expand Down
63 changes: 63 additions & 0 deletions images.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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())
Expand Down
20 changes: 20 additions & 0 deletions images_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}
59 changes: 59 additions & 0 deletions layers.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)
}
Expand Down
55 changes: 52 additions & 3 deletions store.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,14 @@ import (
"github.com/pkg/errors"
)

type UpdateNameOperation int

const (
SetNames UpdateNameOperation = iota
AddNames
RemoveNames
)

var (
stores []*store
storesLock sync.Mutex
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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()
Expand All @@ -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()
Expand All @@ -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
Expand Down Expand Up @@ -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
}
Expand Down

0 comments on commit 1441d33

Please sign in to comment.