diff --git a/libimage/pull.go b/libimage/pull.go index e9ba5293c..fa7bd28c5 100644 --- a/libimage/pull.go +++ b/libimage/pull.go @@ -458,37 +458,18 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str } } - customPlatform := false - if len(options.Architecture)+len(options.OS)+len(options.Variant) > 0 { - customPlatform = true - // Unless the pull policy is "always", 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 "always" to make sure - // we're pulling down the image. + customPlatform := len(options.Architecture)+len(options.OS)+len(options.Variant) > 0 + if customPlatform && pullPolicy != config.PullPolicyAlways && pullPolicy != config.PullPolicyNever { + // 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}. This is - // very likely a bug but a consistent one in Podman/Buildah and should - // be addressed at a later point. - if pullPolicy != config.PullPolicyAlways && pullPolicy != config.PullPolicyNever { - switch { - // User input clearly refer to a local image. - case strings.HasPrefix(imageName, "localhost/"): - logrus.Debugf("Enforcing pull policy to %q to support custom platform (arch: %q, os: %q, variant: %q)", "never", options.Architecture, options.OS, options.Variant) - pullPolicy = config.PullPolicyNever - - // Image resolved to a local one, so let's still have a - // look at the registries or aliases but use it - // otherwise. - case strings.HasPrefix(resolvedImageName, "localhost/"): - logrus.Debugf("Enforcing pull policy to %q to support custom platform (arch: %q, os: %q, variant: %q)", "newer", options.Architecture, options.OS, options.Variant) - pullPolicy = config.PullPolicyNewer - - default: - logrus.Debugf("Enforcing pull policy to %q to support custom platform (arch: %q, os: %q, variant: %q)", "always", options.Architecture, options.OS, options.Variant) - pullPolicy = config.PullPolicyAlways - } - } + // NOTE that this is will even override --pull={false,never}. + 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 { @@ -534,6 +515,12 @@ func (r *Runtime) copySingleImageFromRegistry(ctx context.Context, imageName str sys := r.systemContextCopy() resolved, err := shortnames.Resolve(sys, imageName) if err != nil { + // TODO: that is a too big of a hammer since we should only + // ignore errors that indicate that there's no alias and no + // USRs. Must be addressed in c/image first. + if localImage != nil && pullPolicy == config.PullPolicyNewer { + return []string{resolvedImageName}, nil + } return nil, err } diff --git a/libimage/pull_test.go b/libimage/pull_test.go index 1aebd35f4..1a48e0201 100644 --- a/libimage/pull_test.go +++ b/libimage/pull_test.go @@ -128,6 +128,36 @@ func TestPullPlatforms(t *testing.T) { require.Len(t, pulledImages, 1) } +func TestPullPlatformsWithEmptyRegistriesConf(t *testing.T) { + runtime, cleanup := testNewRuntime(t, testNewRuntimeOptions{registriesConfPath: "/dev/null"}) + defer cleanup() + ctx := context.Background() + pullOptions := &PullOptions{} + pullOptions.Writer = os.Stdout + + localArch := goruntime.GOARCH + localOS := goruntime.GOOS + + imageName := "quay.io/libpod/busybox" + newTag := "crazy:train" + + pulledImages, err := runtime.Pull(ctx, imageName, config.PullPolicyAlways, pullOptions) + require.NoError(t, err, "pull "+imageName) + require.Len(t, pulledImages, 1) + + err = pulledImages[0].Tag(newTag) + require.NoError(t, err, "tag") + + // See containers/podman/issues/12707: a custom platform will enforce + // pulling via newer. Older versions enforced always which can lead to + // errors. + pullOptions.OS = localOS + pullOptions.Architecture = localArch + pulledImages, err = runtime.Pull(ctx, newTag, config.PullPolicyMissing, pullOptions) + require.NoError(t, err, "pull "+newTag) + require.Len(t, pulledImages, 1) +} + func TestPullPolicy(t *testing.T) { runtime, cleanup := testNewRuntime(t) defer cleanup() diff --git a/libimage/runtime_test.go b/libimage/runtime_test.go index f8e679cfc..e24552cad 100644 --- a/libimage/runtime_test.go +++ b/libimage/runtime_test.go @@ -18,10 +18,14 @@ func TestMain(m *testing.M) { os.Exit(m.Run()) } +type testNewRuntimeOptions struct { + registriesConfPath string +} + // Create a new Runtime that can be used for testing. The second return value // is a clean-up function that should be called by users to make sure all // temporary test data gets removed. -func testNewRuntime(t *testing.T) (runtime *Runtime, cleanup func()) { +func testNewRuntime(t *testing.T, options ...testNewRuntimeOptions) (runtime *Runtime, cleanup func()) { workdir, err := ioutil.TempDir("", "testStorageRuntime") require.NoError(t, err) storeOptions := &storage.StoreOptions{ @@ -36,6 +40,10 @@ func testNewRuntime(t *testing.T) (runtime *Runtime, cleanup func()) { SystemRegistriesConfDirPath: "/dev/null", } + if len(options) == 1 && options[0].registriesConfPath != "" { + systemContext.SystemRegistriesConfPath = options[0].registriesConfPath + } + runtime, err = RuntimeFromStoreOptions(&RuntimeOptions{SystemContext: systemContext}, storeOptions) require.NoError(t, err) require.Equal(t, runtime.systemContext.BigFilesTemporaryDir, tmpdir())