Download image when starting a container using the compat API

[mscherer]

Docker/moby will automatically pull images when using the API (or the CLI, for
that matter) to create a container, but podman don't. This break the compatibility with
some software.

I tested that using woddpecker CI, and when I use docker on F35 as the backend, it doesn't fail. Using podman-docker fail, with "image uknown".

What this PR does / why we need it:

Align the compatibility API with moby.

How to verify it

There is a attached test case. Otherwise, deploying woodpecker and using podman-docker is the way to go, or do a call the
/v1.33/containers/xxxx/start API with a image not already downloaded (eg, one not in the output of podman images) and see it fail.

Which issue(s) this PR fixes:

None

Special notes for your reviewer:

I am not 100% happy with the code, especially with the fact the call to LookupImage is duplicated. I am however not fluent enough in go to write something more idiomatic.

[openshift-ci]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: mscherer
To complete the pull request process, please assign tomsweeneyredhat after the PR has been reviewed.
You can assign the PR to them by writing /assign @tomsweeneyredhat in a comment when ready.

The full list of commands accepted by this bot can be found here.

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[vrothberg]

LGTM

In order to pass CI, you need to sign your commit via git commit --ammend -s and repush.

@containers/podman-maintainers PTAL

[mheon]

Are we absolutely sure that this is what Docker does? Because Docker definitely documents that they return a 404 from the create endpoint when the image isn't present.

I'd be more inclined to believe our error response is slightly wrong and it's not triggering a conditional that would normally pull the image?

[mheon]

I went digging in the Moby source, and I can confirm that they do not do this - it's definitely a 404 on image not existing when Create is hit.

[vrothberg]

Thank you for checking, @mheon! Are the Python bindings doing the pull? Just guessing.

[mheon]

Possible? Didn't check around there.
I know that some Docker clients do a blind /containers/create call, and checks the HTTP return with a string comparison (despite having a dedicated HTTP return code, 404, for "no such image") on the returned error, and from there attempt a pull if they think it's a missing image. My suspicion is that is what is happening and our error might not match precisely enough for this to work.

[mscherer]

Ok, I will check woodpecker code. I just tested with docker and podman (with and without the patch), and indeed, it worked with docker, but I should have searched further rather than just jump in coding.

[mscherer]

Ok so indeed, here is what is returned by docker (along a 404):

{"message":"No such image: a6543/test_git_plugin:latest"}

and so woodpecker do "POST /v1.33/images/create?fromImage=a6543%2Ftest_git_plugin&tag=latest"

I will compare with the same trace from podman.

[mscherer]

And what podman HEAD answer:

{"cause":"image not known","message":"docker.io/a6543/test_git_plugin:latest: image not known","response":404}

[mscherer]

Ok so indeed, the retry is on woodpecker side on https://github.com/woodpecker-ci/woodpecker/blob/master/pipeline/backend/docker/docker.go#L91-L92

        _, err := e.client.ContainerCreate(ctx, config, hostConfig, nil, nil, proc.Name)
        if client.IsErrNotFound(err) {

Client is from github.com/moby/moby/client, https://github.com/moby/moby/blob/master/client/errors.go#L43

So the issue is not just "some client", but anything that use the official binding. I will close that PR, try to fix, and reopen a new one once I found it.

[rhatdan]

Would this have fixed the problem:

diff --git a/pkg/api/handlers/utils/errors.go b/pkg/api/handlers/utils/errors.go
index 4a8005bfd..22894d414 100644
--- a/pkg/api/handlers/utils/errors.go
+++ b/pkg/api/handlers/utils/errors.go
@@ -26,7 +26,7 @@ func Error(w http.ResponseWriter, apiMessage string, code int, err error) {
        // Log detailed message of what happened to machine running podman service
        log.Infof("Request Failed(%s): %s", http.StatusText(code), err.Error())
        em := errorhandling.ErrorModel{
-               Because:      (errors.Cause(err)).Error(),
+               Because:      strings.ReplaceAll((errors.Cause(err)).Error(), "image not known", "No such image"),
                Message:      err.Error(),
                ResponseCode: code,
        }

[mscherer]

Nope. It still say "image not known"

[mscherer]

Or rather, the json returned is:

{"cause":"No such image","message":"docker.io/a6543/test_git_plugin:latest: image not known","response":404}

So it change "cause", but not "message".

[mscherer]

diff --git a/pkg/api/handlers/utils/errors.go b/pkg/api/handlers/utils/errors.go
index 4a8005bfd..438dce399 100644
--- a/pkg/api/handlers/utils/errors.go
+++ b/pkg/api/handlers/utils/errors.go
@@ -3,7 +3,7 @@ package utils
 import (
        "fmt"
        "net/http"
-
+"strings"
        "github.com/containers/podman/v3/libpod/define"
        "github.com/containers/podman/v3/pkg/errorhandling"
        "github.com/containers/storage"
@@ -26,8 +26,8 @@ func Error(w http.ResponseWriter, apiMessage string, code int, err error) {
        // Log detailed message of what happened to machine running podman service
        log.Infof("Request Failed(%s): %s", http.StatusText(code), err.Error())
        em := errorhandling.ErrorModel{
-               Because:      (errors.Cause(err)).Error(),
-               Message:      err.Error(),
+               Because:      strings.ReplaceAll((errors.Cause(err)).Error(), "image not known", "No such image"),
+               Message:      strings.ReplaceAll(err.Error(), "image not known", "No such image"),
                ResponseCode: code,
        }
        WriteJSON(w, code, em)

seems to work (or rather, seems to complain about "Error response from daemon: failed to resolve image name: short-name resolution enforced but cannot prompt without a TTY" (which is weird, because this is not a short-name)

[mscherer]

ok so the short-name issue is on woodpecker side.

Just changing the message is enough to fix the logic from moby client.