diff --git a/containers.go b/containers.go index a1e3e3029..6e2f72381 100644 --- a/containers.go +++ b/containers.go @@ -81,18 +81,8 @@ type rwContainerStore interface { // convenience of the caller, nothing more. Create(id string, names []string, image, layer, metadata string, options *ContainerOptions) (*Container, error) - // SetNames updates the list of names associated with the container - // with the specified ID. - // Deprecated: Prone to race conditions, suggested alternatives are `AddNames` and `RemoveNames`. - SetNames(id string, names []string) error - - // AddNames adds the supplied values to the list of names associated with the container with - // the specified id. - AddNames(id string, names []string) error - - // RemoveNames removes the supplied values from the list of names associated with the container with - // the specified id. - RemoveNames(id string, names []string) error + // updateNames modifies names associated with a container based on (op, names). + updateNames(id string, names []string, op updateNameOperation) error // Get retrieves information about a container given an ID or name. Get(id string) (*Container, error) @@ -388,19 +378,6 @@ func (r *containerStore) removeName(container *Container, name string) { container.Names = stringSliceWithoutValue(container.Names, name) } -// Deprecated: Prone to race conditions, suggested alternatives are `AddNames` and `RemoveNames`. -func (r *containerStore) SetNames(id string, names []string) error { - return r.updateNames(id, names, setNames) -} - -func (r *containerStore) AddNames(id string, names []string) error { - return r.updateNames(id, names, addNames) -} - -func (r *containerStore) RemoveNames(id string, names []string) error { - return r.updateNames(id, names, removeNames) -} - func (r *containerStore) updateNames(id string, names []string, op updateNameOperation) error { container, ok := r.lookup(id) if !ok { diff --git a/images.go b/images.go index 50559f0dc..ce45942b6 100644 --- a/images.go +++ b/images.go @@ -129,21 +129,10 @@ type rwImageStore interface { // read-only) layer. That layer can be referenced by multiple images. Create(id string, names []string, layer, metadata string, created time.Time, searchableDigest digest.Digest) (*Image, error) - // SetNames replaces the list of names associated with an image with the - // supplied values. The values are expected to be valid normalized + // updateNames modifies names associated with an image based on (op, names). + // The values are expected to be valid normalized // named image references. - // Deprecated: Prone to race conditions, suggested alternatives are `AddNames` and `RemoveNames`. - SetNames(id string, names []string) error - - // AddNames adds the supplied values to the list of names associated with the image with - // the specified id. The values are expected to be valid normalized - // named image references. - AddNames(id string, names []string) error - - // RemoveNames removes the supplied values from the list of names associated with the image with - // the specified id. The values are expected to be valid normalized - // named image references. - RemoveNames(id string, names []string) error + updateNames(id string, names []string, op updateNameOperation) error // Delete removes the record of the image. Delete(id string) error @@ -516,19 +505,6 @@ func (i *Image) addNameToHistory(name string) { i.NamesHistory = dedupeNames(append([]string{name}, i.NamesHistory...)) } -// Deprecated: Prone to race conditions, suggested alternatives are `AddNames` and `RemoveNames`. -func (r *imageStore) SetNames(id string, names []string) error { - return r.updateNames(id, names, setNames) -} - -func (r *imageStore) AddNames(id string, names []string) error { - return r.updateNames(id, names, addNames) -} - -func (r *imageStore) RemoveNames(id string, names []string) error { - return r.updateNames(id, names, removeNames) -} - func (r *imageStore) updateNames(id string, names []string, op updateNameOperation) error { if !r.IsReadWrite() { return fmt.Errorf("not allowed to change image name assignments at %q: %w", r.imagespath(), ErrStoreIsReadOnly) diff --git a/images_test.go b/images_test.go index 01594c11f..03b3746af 100644 --- a/images_test.go +++ b/images_test.go @@ -24,7 +24,7 @@ func addTestImage(t *testing.T, store rwImageStore, id string, names []string) { ) require.Nil(t, err) - require.Nil(t, store.SetNames(id, names)) + require.Nil(t, store.updateNames(id, names, setNames)) } func TestAddNameToHistorySuccess(t *testing.T) { @@ -72,7 +72,7 @@ func TestHistoryNames(t *testing.T) { // And When store.Lock() defer store.Unlock() - require.Nil(t, store.SetNames(firstImageID, []string{"1", "2", "3", "4"})) + require.Nil(t, store.updateNames(firstImageID, []string{"1", "2", "3", "4"}, setNames)) // Then firstImage, err = store.Get(firstImageID) @@ -91,13 +91,13 @@ func TestHistoryNames(t *testing.T) { require.Equal(t, secondImage.NamesHistory[1], "2") // test independent add and remove operations - require.Nil(t, store.AddNames(firstImageID, []string{"5"})) + require.Nil(t, store.updateNames(firstImageID, []string{"5"}, addNames)) firstImage, err = store.Get(firstImageID) require.Nil(t, err) require.Equal(t, firstImage.NamesHistory, []string{"4", "3", "2", "1", "5"}) // history should still contain old values - require.Nil(t, store.RemoveNames(firstImageID, []string{"5"})) + require.Nil(t, store.updateNames(firstImageID, []string{"5"}, removeNames)) firstImage, err = store.Get(firstImageID) require.Nil(t, err) require.Equal(t, firstImage.NamesHistory, []string{"4", "3", "2", "1", "5"}) diff --git a/layers.go b/layers.go index 406892d54..199fb6e4a 100644 --- a/layers.go +++ b/layers.go @@ -214,18 +214,8 @@ type rwLayerStore interface { // Put combines the functions of CreateWithFlags and ApplyDiff. Put(id string, parent *Layer, names []string, mountLabel string, options map[string]string, moreOptions *LayerOptions, writeable bool, flags map[string]interface{}, diff io.Reader) (*Layer, int64, error) - // SetNames replaces the list of names associated with a layer with the - // supplied values. - // Deprecated: Prone to race conditions, suggested alternatives are `AddNames` and `RemoveNames`. - SetNames(id string, names []string) error - - // AddNames adds the supplied values to the list of names associated with the layer with the - // specified id. - AddNames(id string, names []string) error - - // RemoveNames remove the supplied values from the list of names associated with the layer with the - // specified id. - RemoveNames(id string, names []string) error + // updateNames modifies names associated with a layer based on (op, names). + updateNames(id string, names []string, op updateNameOperation) error // Delete deletes a layer with the specified name or ID. Delete(id string) error @@ -1091,19 +1081,6 @@ func (r *layerStore) removeName(layer *Layer, name string) { layer.Names = stringSliceWithoutValue(layer.Names, name) } -// Deprecated: Prone to race conditions, suggested alternatives are `AddNames` and `RemoveNames`. -func (r *layerStore) SetNames(id string, names []string) error { - return r.updateNames(id, names, setNames) -} - -func (r *layerStore) AddNames(id string, names []string) error { - return r.updateNames(id, names, addNames) -} - -func (r *layerStore) RemoveNames(id string, names []string) error { - return r.updateNames(id, names, removeNames) -} - func (r *layerStore) updateNames(id string, names []string, op updateNameOperation) error { if !r.IsReadWrite() { return fmt.Errorf("not allowed to change layer name assignments at %q: %w", r.layerspath(), ErrStoreIsReadOnly) diff --git a/store.go b/store.go index 3836fcd87..1c646f325 100644 --- a/store.go +++ b/store.go @@ -1286,22 +1286,14 @@ func (s *store) CreateLayer(id, parent string, names []string, mountLabel string func (s *store) CreateImage(id string, names []string, layer, metadata string, options *ImageOptions) (*Image, error) { if layer != "" { - lstore, err := s.getLayerStore() - if err != nil { - return nil, err - } - lstores, err := s.getROLayerStores() + layerStores, err := s.allLayerStores() if err != nil { return nil, err } var ilayer *Layer - for _, s := range append([]roLayerStore{lstore}, lstores...) { + for _, s := range layerStores { store := s - if store == lstore { - store.Lock() - } else { - store.RLock() - } + store.RLock() defer store.Unlock() err := store.ReloadIfChanged() if err != nil { @@ -2137,16 +2129,7 @@ func (s *store) updateNames(id string, names []string, op updateNameOperation) e return nil } layerFound = true - switch op { - case setNames: - return rlstore.SetNames(id, deduped) - case removeNames: - return rlstore.RemoveNames(id, deduped) - case addNames: - return rlstore.AddNames(id, deduped) - default: - return errInvalidUpdateNameOperation - } + return rlstore.updateNames(id, deduped, op) }); err != nil || layerFound { return err } @@ -2161,16 +2144,7 @@ func (s *store) updateNames(id string, names []string, op updateNameOperation) e return err } if ristore.Exists(id) { - switch op { - case setNames: - return ristore.SetNames(id, deduped) - case removeNames: - return ristore.RemoveNames(id, deduped) - case addNames: - return ristore.AddNames(id, deduped) - default: - return errInvalidUpdateNameOperation - } + return ristore.updateNames(id, deduped, op) } // Check is id refers to a RO Store @@ -2204,16 +2178,7 @@ func (s *store) updateNames(id string, names []string, op updateNameOperation) e return nil } containerFound = true - switch op { - case setNames: - return rcstore.SetNames(id, deduped) - case removeNames: - return rcstore.RemoveNames(id, deduped) - case addNames: - return rcstore.AddNames(id, deduped) - default: - return errInvalidUpdateNameOperation - } + return rcstore.updateNames(id, deduped, op) }); err != nil || containerFound { return err } @@ -2946,40 +2911,16 @@ func (s *store) ContainerParentOwners(id string) ([]int, []int, error) { } func (s *store) Layers() ([]Layer, error) { - lstore, err := s.getLayerStore() - if err != nil { - return nil, err - } - - layers, err := func() ([]Layer, error) { - lstore.Lock() - defer lstore.Unlock() - if err := lstore.Load(); err != nil { - return nil, err - } - return lstore.Layers() - }() - if err != nil { - return nil, err - } - - lstores, err := s.getROLayerStores() - if err != nil { - return nil, err - } - - for _, s := range lstores { - store := s - store.RLock() - defer store.Unlock() - if err := store.ReloadIfChanged(); err != nil { - return nil, err - } + var layers []Layer + if done, err := s.readAllLayerStores(func(store roLayerStore) (bool, error) { storeLayers, err := store.Layers() if err != nil { - return nil, err + return true, err } layers = append(layers, storeLayers...) + return false, nil + }); done { + return nil, err } return layers, nil } @@ -3314,7 +3255,6 @@ func (s *store) FromContainerRunDirectory(id, file string) ([]byte, error) { func (s *store) Shutdown(force bool) ([]string, error) { mounted := []string{} - modified := false rlstore, err := s.getLayerStore() if err != nil { @@ -3348,7 +3288,6 @@ func (s *store) Shutdown(force bool) ([]string, error) { } break } - modified = true } } } @@ -3364,16 +3303,6 @@ func (s *store) Shutdown(force bool) ([]string, error) { err = fmt.Errorf("(graphLock.Touch failed: %v) %w", err2, err) } } - modified = true - } - if modified { - if err2 := rlstore.Touch(); err2 != nil { - if err == nil { - err = err2 - } else { - err = fmt.Errorf("rlstore.Touch failed: %v) %w", err2, err) - } - } } return mounted, err }