Skip to content

Commit

Permalink
libimage: pull: ignore platform for local image lookup
Browse files Browse the repository at this point in the history
We must ignore the platform of a local image when doing lookups.  Some
images set an incorrect or even invalid platform (see
containers/podman/issues/10682).  Doing the lookup while ignoring the
platform checks prevents redundantly downloading the same image.

Note that this has the consequence that a `--pull-never --arch=hurz` may
chose a local image of another architecture.  However, I estimate the
benefit of continuing to allow potentially invalid images higher than
not running them (and breaking workloads).

The changes required to touch the corrupted checks.  I used the occasion
to make the corrupted checks a bit cheaper.

Signed-off-by: Valentin Rothberg <[email protected]>
  • Loading branch information
vrothberg committed Jun 23, 2021
1 parent 9082857 commit bb19eb2
Show file tree
Hide file tree
Showing 4 changed files with 42 additions and 6 deletions.
5 changes: 4 additions & 1 deletion libimage/corrupted_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,9 @@ func TestCorruptedImage(t *testing.T) {
_, err = image.Inspect(ctx, false)
require.Error(t, err, "inspecting corrupted image should fail")

err = image.isCorrupted(imageName)
require.Error(t, err, "image is corrupted")

exists, err = runtime.Exists(imageName)
require.NoError(t, err, "corrupted image exists should not fail")
require.False(t, exists, "corrupted image should not be marked to exist")
Expand All @@ -75,7 +78,7 @@ func TestCorruptedImage(t *testing.T) {
require.Len(t, pulledImages, 1)
image = pulledImages[0]

// Inpsecting a repaired image should work.
// Inspecting a repaired image should work.
_, err = image.Inspect(ctx, false)
require.NoError(t, err, "inspecting repaired image should work")
}
18 changes: 18 additions & 0 deletions libimage/image.go
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,24 @@ func (i *Image) reload() error {
return nil
}

// isCorrupted returns an error if the image may be corrupted.
func (i *Image) isCorrupted(name string) error {
// If it's a manifest list, we're good for now.
if _, err := i.getManifestList(); err == nil {
return nil
}

ref, err := i.StorageReference()
if err != nil {
return err
}

if _, err := ref.NewImage(context.Background(), nil); err != nil {
return errors.Errorf("Image %s exists in local storage but may be corrupted: %v", name, err)
}
return nil
}

// Names returns associated names with the image which may be a mix of tags and
// digests.
func (i *Image) Names() []string {
Expand Down
20 changes: 18 additions & 2 deletions libimage/pull.go
Original file line number Diff line number Diff line change
Expand Up @@ -323,7 +323,7 @@ func (r *Runtime) copyFromRegistry(ctx context.Context, ref types.ImageReference
// from a registry. On successful pull it returns the used fully-qualified
// name that can later be used to look up the image in the local containers
// storage.
func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName string, pullPolicy config.PullPolicy, options *PullOptions) ([]string, error) {
func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName string, pullPolicy config.PullPolicy, options *PullOptions) ([]string, error) { //nolint:gocyclo
// Sanity check.
if err := pullPolicy.Validate(); err != nil {
return nil, err
Expand All @@ -339,11 +339,27 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str
// resolved name for pulling. Assume we're doing a `pull foo`.
// If there's already a local image "localhost/foo", then we should
// attempt pulling that instead of doing the full short-name dance.
localImage, resolvedImageName, err = r.LookupImage(imageName, nil)
lookupOptions := &LookupImageOptions{
// NOTE: we must ignore the platform of a local image when
// doing lookups. Some images set an incorrect or even invalid
// platform (see containers/podman/issues/10682). Doing the
// lookup while ignoring the platform checks prevents
// redundantly downloading the same image.
IgnorePlatform: true,
}
localImage, resolvedImageName, err = r.LookupImage(imageName, lookupOptions)
if err != nil && errors.Cause(err) != storage.ErrImageUnknown {
logrus.Errorf("Looking up %s in local storage: %v", imageName, err)
}

// If the local image is corrupted, we need to repull it.
if localImage != nil {
if err := localImage.isCorrupted(imageName); err != nil {
logrus.Error(err)
localImage = nil
}
}

if pullPolicy == config.PullPolicyNever {
if localImage != nil {
logrus.Debugf("Pull policy %q but no local image has been found for %s", pullPolicy, imageName)
Expand Down
5 changes: 2 additions & 3 deletions libimage/runtime.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,9 +141,8 @@ func (r *Runtime) Exists(name string) (bool, error) {
if image == nil {
return false, nil
}
// Inspect the image to make sure if it's corrupted or not.
if _, err := image.Inspect(context.Background(), false); err != nil {
logrus.Errorf("Image %s exists in local storage but may be corrupted: %v", name, err)
if err := image.isCorrupted(name); err != nil {
logrus.Error(err)
return false, nil
}
return true, nil
Expand Down

0 comments on commit bb19eb2

Please sign in to comment.