From 084751e0b8b253b8bc5946259658f3a1f79384e0 Mon Sep 17 00:00:00 2001 From: Valentin Rothberg Date: Wed, 18 Jan 2023 15:08:04 +0100 Subject: [PATCH] Revert "libimage: pull: do not enforce pull if local image matches" This reverts commit ba3a9c0f770b7f77c7e95760d50f70216d505b8a as it caused a nasty flake on Buildah [1] [1] https://github.com/containers/buildah/pull/4527 Signed-off-by: Valentin Rothberg --- libimage/pull.go | 24 +++++++----------------- libimage/pull_test.go | 23 ----------------------- 2 files changed, 7 insertions(+), 40 deletions(-) diff --git a/libimage/pull.go b/libimage/pull.go index 34a373da3..955132868 100644 --- a/libimage/pull.go +++ b/libimage/pull.go @@ -497,26 +497,16 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str customPlatform := len(options.Architecture)+len(options.OS)+len(options.Variant) > 0 if customPlatform && pullPolicy != config.PullPolicyAlways && pullPolicy != config.PullPolicyNever { - // Unless the specified platform matches the local image, we - // must pessimistically assume that the local image has an - // invalid architecture (see containers/podman/issues/10682). - // Hence, whenever the user requests a custom platform, set the - // pull policy to "newer" to make sure we're pulling down the + // Unless the pull policy is always/never, we must + // pessimistically assume that the local image has an invalid + // architecture (see containers/podman/issues/10682). Hence, + // whenever the user requests a custom platform, set the pull + // policy to "newer" to make sure we're pulling down the // correct image. // // NOTE that this is will even override --pull={false,never}. - localImageMatches := false - if localImage != nil { - _, matches, err := localImage.matchesPlatform(ctx, options.OS, options.Architecture, options.Variant) - if err != nil { - return nil, err - } - localImageMatches = matches - } - if !localImageMatches { - pullPolicy = config.PullPolicyNewer - logrus.Debugf("Enforcing pull policy to %q to pull custom platform (arch: %q, os: %q, variant: %q) - local image may mistakenly specify wrong platform", pullPolicy, options.Architecture, options.OS, options.Variant) - } + pullPolicy = config.PullPolicyNewer + logrus.Debugf("Enforcing pull policy to %q to pull custom platform (arch: %q, os: %q, variant: %q) - local image may mistakenly specify wrong platform", pullPolicy, options.Architecture, options.OS, options.Variant) } if pullPolicy == config.PullPolicyNever { diff --git a/libimage/pull_test.go b/libimage/pull_test.go index 50c57ba8b..28c4a86b8 100644 --- a/libimage/pull_test.go +++ b/libimage/pull_test.go @@ -5,11 +5,9 @@ import ( "fmt" "os" goruntime "runtime" - "strings" "testing" "github.com/containers/common/pkg/config" - "github.com/sirupsen/logrus" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -96,27 +94,6 @@ func TestPullPlatforms(t *testing.T) { require.NoError(t, err, "pull busybox") require.Len(t, pulledImages, 1) - // Now re-pull with the platform explicitly set in the pull options. It - // should not repull an image (or perform a "newer" check) though but - // resolve to the local image. - // - // See containers/podman/issues/17063. - func() { // Anonymous function to make sure logrus is reset even on failure. - builder := strings.Builder{} - logrus.SetOutput(&builder) - logrus.SetLevel(logrus.DebugLevel) - defer builder.Reset() - defer logrus.SetOutput(os.Stderr) - defer logrus.SetLevel(logrus.InfoLevel) - - pullOptions.Architecture = localArch - pullOptions.OS = localOS - pulledImages, err := runtime.Pull(ctx, withTag, config.PullPolicyMissing, pullOptions) - require.NoError(t, err, "pull busybox with same platform as before") - require.Len(t, pulledImages, 1) - require.NotContains(t, builder.String(), "local image may mistakenly specify wrong platform") - }() - // Repulling with a bogus architecture should yield an error and not // choose the local image. pullOptions.Architecture = "bogus"