kestra-io / plugin-scripts

https://kestra.io/plugins/
Apache License 2.0
9 stars 11 forks source link

Bad Docker API call if task contains `containerImage` that contains tag #175

Open vlk-charles opened 2 months ago

vlk-charles commented 2 months ago

Describe the issue

When the containerImage property of a task contains a value that contains a tag (specified after :), the generated API call to the Docker (or in our case Podman) daemon contains something like the following:

/images/create?fromImage=ghcr.io%2Fkestra-io%2Fkestrapy%3Alatest&tag=latest

Notice that the tag is supplied in two places, once in the image name itself (fromImage) and once in the tag parameter. I assume Docker proper handles this more gracefully, otherwise this would have never made it into a stable Kestra release. The Podman daemon however normalizes this to ghcr.io/kestra-io/kestrapy:latest:latest and fails with "invalid reference format". This is logged by the Podman daemon:

time="2024-09-06T15:54:19Z" level=debug msg="Looking up image \"ghcr.io/kestra-io/kestrapy:latest:latest\" in local containers storage"
time="2024-09-06T15:54:19Z" level=info msg="Request Failed(Internal Server Error): normalizing image: normalizing name for compat API: invalid reference format"
@ - - [06/Sep/2024:15:54:19 +0000] "POST /images/create?fromImage=ghcr.io%2Fkestra-io%2Fkestrapy%3Alatest&tag=latest HTTP/1.1" 500 174 "" "Apache-HttpClient/5.3.1 (Java/21.0.4)"

No further communication between Kestra and the daemon occurs and Kestra fails the task execution. The log in Kestra is not very helpful and only mentions a "broken pipe" in the Docker client:

java.lang.RuntimeException: java.io.IOException: Broken pipe
 at com.github.dockerjava.httpclient5.ApacheDockerHttpClientImpl.execute(ApacheDockerHttpClientImpl.java:210)
 ...

This can be worked around by explicitly including containerImage without a tag such as ghcr.io/kestra-io/kestrapy. A non-latest tag cannot be used however.

This was discovered because the default value changed from a simple python to ghcr.io/kestra-io/kestrapy:latest (commit 88b7352).

Note that if a tag other than latest is provided, it gets reflected in both places in the call:

time="2024-09-06T21:18:53Z" level=debug msg="Looking up image \"python:3.12.5:3.12.5\" in local containers storage"
time="2024-09-06T21:18:53Z" level=info msg="Request Failed(Internal Server Error): normalizing image: normalizing name for compat API: invalid reference format"
@ - - [06/Sep/2024:21:18:53 +0000] "POST /images/create?fromImage=python%3A3.12.5&tag=3.12.5 HTTP/1.1" 500 174 "" "Apache-HttpClient/5.3.1 (Java/21.0.4)"

Environment

vlk-charles commented 2 months ago

Calling docker pull python:3.12.5 with the official (albeit fairly old) Docker client produces this HTTP request:

POST /v1.40/images/create?fromImage=python&tag=3.12.5 HTTP/1.1
vlk-charles commented 2 months ago

Trivial flow to reproduce issue:

id: TRIVIAL_DOCKER
namespace: test
description: Trivial flow to check Docker connectivity
tasks:
- id: trivial_python
  type: io.kestra.plugin.scripts.python.Script
  #containerImage: ghcr.io/kestra-io/kestrapy
  script: print('Hello Kestra')

Uncommenting the line results in successful execution.

vlk-charles commented 2 months ago

Behavior unchanged in the newly released 0.18.5.

Ben8t commented 2 months ago

I'm not able to reproduce with Docker based configuration - so probably related to Podman indeed.

vlk-charles commented 2 months ago

From the API documentation (emphasis mine):

So a call like /images/create?fromImage=ghcr.io%2Fkestra-io%2Fkestrapy%3Alatest&tag=latest might not violate the specification, but I guess the behavior is undefined. What happens if the two tags differ? What if fromImage contains a tag but no tag parameter is specified? This should cause the daemon to pull "all tags for the given image". What is then really the point of putting the tag in fromImage?

It seems like the safest thing a client can do is never put a tag in fromImage and always put one in tag (unless it really wants all tags pulled).

smunteankestra commented 2 months ago

tried on 0.18.6 and on 0.19 (develop) the flow provided by @vlk-charles and it works flawlessly can't reproduce it, @vlk-charles maybe you try on 0.18.06 it was released yesterday

id: TRIVIAL_DOCKER namespace: test description: Trivial flow to check Docker connectivity tasks:

vlk-charles commented 2 months ago

@smunteankestra Did you try this with Podman? Docker accepts the API call and just drops the tag in fromImage if another one is supplied in tag. Podman chokes on this and tries to pull image:tag1:tag2.

I have not tried 0.18.6 yet.

accarin commented 2 months ago

First, my team and I love Kestra, and we are grateful for all the work you've put into this project!

Unfortunately, we've run into the exact same issues when using podman.

I think the culprit might be the general use of docker-java/exemplarily in: https://github.com/kestra-io/kestra/blob/develop/script/src/main/java/io/kestra/plugin/scripts/runner/docker/Docker.java#L737

Internally, the PullImageCmd parameter from docker-java is called "repository" and its name already suggests to not use the fully qualified image name (including tags). Otherwise, the builder method withTag would as well be redundant. Wouldn't it?

See: https://github.com/docker-java/docker-java/blob/main/docker-java-core/src/main/java/com/github/dockerjava/core/command/PullImageCmdImpl.java#L20

The Docker API calls are translated accordingly as already stated in https://github.com/kestra-io/plugin-scripts/issues/175#issuecomment-2348948903 by @vlk-charles.

The following sample code works for me with Docker and Podman:

PullImageCmd pull = dockerClient.pullImageCmd("hello-world");
pull
  .withTag("linux")
  .exec(new PullImageResultCallback())
  .awaitCompletion();

where

PullImageCmd pull = dockerClient.pullImageCmd("hello-world:linux");
pull
  .withTag("linux")
  .exec(new PullImageResultCallback())
  .awaitCompletion();

fails for the latter.

As I'm new to and not entirely familiar with the Kestra source code, your thoughts on this matter would be highly appreciated!

anna-geller commented 5 days ago

@smunteankestra seems important to the community so let's validate thoroughly and coordinate next steps with @Ben8t 👍