Skip to content

Commit

Permalink
libimage, ManifestList: fs lock and reload to provide race-free Add API
Browse files Browse the repository at this point in the history
`podman manifest add` uses `ManifestList.Add(` but of now `Add(` does
not locks while adding instances to the list thus causing race scenarios
where storage is not reloaded and overrided by another invocation of the
command.

Following problem is solved in two steps

* Add -> LockByInstance: Acquire a fs lock by instance ID so other
  invocation waits until this invocation completes its write.
* Add -> LockByInstance -> reload: Reload instance digests from storage
  just after acquiring lock to make sure we are not overriding any just
written instance.

Reproducer: containers/podman#14667 (comment)

Closes: containers/podman#14667

[NO NEW TESTS NEEDED]
[NO TESTS NEEDED]
This needes integration tests so its hard to verify race in CI.

Signed-off-by: Aditya R <[email protected]>
  • Loading branch information
flouthoc committed Jul 14, 2022
1 parent 415ead5 commit 8a786cc
Showing 1 changed file with 26 additions and 1 deletion.
27 changes: 26 additions & 1 deletion libimage/manifest_list.go
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,21 @@ func (m *ManifestList) saveAndReload() error {
return nil
}

// Reload the image and list instances from storage
func (m *ManifestList) reload() error {
listID := m.ID()
if err := m.image.reload(); err != nil {
return err
}
image, list, err := m.image.runtime.lookupManifestList(listID)
if err != nil {
return err
}
m.image = image
m.list = list
return nil
}

// getManifestList is a helper to obtain a manifest list
func (i *Image) getManifestList() (manifests.List, error) {
_, list, err := manifests.LoadFromImage(i.runtime.store, i.ID())
Expand Down Expand Up @@ -253,7 +268,17 @@ func (m *ManifestList) Add(ctx context.Context, name string, options *ManifestLi
Password: options.Password,
}
}

locker, err := manifests.LockerForImage(m.image.runtime.store, m.ID())
if err != nil {
return "", err
}
locker.Lock()
defer locker.Unlock()
// Make sure to reload the image from the containers storage to fetch
// the latest data (e.g., new or delete digests).
if err := m.reload(); err != nil {
return "", err
}
newDigest, err := m.list.Add(ctx, systemContext, ref, options.All)
if err != nil {
return "", err
Expand Down

0 comments on commit 8a786cc

Please sign in to comment.