Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Configurable strategy to control GraalVM build image pulling #33691

Closed
yrodiere opened this issue May 30, 2023 · 7 comments · Fixed by #33749
Closed

Configurable strategy to control GraalVM build image pulling #33691

yrodiere opened this issue May 30, 2023 · 7 comments · Fixed by #33749
Assignees
Milestone

Comments

@yrodiere
Copy link
Member

Description

When you don't have GraalVM in the path, but have podman/docker installed on your system, Quarkus will default to running the native GraalVM build in a container on podman/docker. Which is great.

One point of pain, though, is that the container images used for the build are floating images: the tags get updated every week to point to a "fresh" image. That's good for security, but for development environments it's a bit overkill and implies frequent re-downloads that may not be acceptable on non-fiber connections.

It could be a good idea to allow configuring the GraalVM build image pulling strategy, e.g. -Dquarkus.native.container-build.pull=always|missing? That way one would be able to set the environment variable QUARKUS_NATIVE_CONTAINER_BUILD_PULL=missing on one's development environment and they would only download a new image when Quarkus switches to a new tag of the image.

See also https://groups.google.com/d/msgid/quarkus-dev/CAKW6fifUP93da%3DZs%2BfqpxtQ6C3yrA5DcdK8OF69AcDirUFh-fA%40mail.gmail.com?utm_medium=email&utm_source=footer

Implementation ideas

I may be missing something, but we could just forward the strategy to docker run/podman run through --pull always|missing|never?

There's been talks of using docker-client in the thread on the mailing list, but I must admit I don't see how that's related to this issue.

@yrodiere
Copy link
Member Author

cc @cescoffier

@cescoffier
Copy link
Member

@yrodiere Wow... I didn't know that docker run had a pull option. Podman has different values (newer). But we can at least use the one they have in common:

"always"|"missing"|"never"

@cescoffier
Copy link
Member

(PS: that would avoid having to re-implement the logic using a docker/podman client).

@yrodiere
Copy link
Member Author

I just noticed docker run defaults to --pull missing and wondered what was going on... It turns out we don't force docker run --pull always, but we do an explicit pull before the run for whatever reason:

log.info("Checking image status " + effectiveBuilderImage);
Process pullProcess = null;
try {
final ProcessBuilder pb = new ProcessBuilder(
Arrays.asList(containerRuntime.getExecutableName(), "pull", effectiveBuilderImage));
pullProcess = ProcessUtil.launchProcess(pb, processInheritIODisabled);
pullProcess.waitFor();
} catch (IOException | InterruptedException e) {
throw new RuntimeException("Failed to pull builder image " + effectiveBuilderImage, e);
} finally {
if (pullProcess != null) {
pullProcess.destroy();
}
}

Unfortunately docker pull doesn't have an equivalent to docker run --pull always|missing, so we would only be able to keep the docker pull when quarkus.native.container-build.pull is set to always, and skip it when it's missing/never...
Which makes sense to me, but I suppose there's a reason we have a separate docker pull in the first place?

@cescoffier
Copy link
Member

The pull was there to pull the latest image (the one with less CVEs).

@yrodiere
Copy link
Member Author

The pull was there to pull the latest image (the one with less CVEs).

Ah, in that case simply using docker run --pull always would work just as well. Good!

@yrodiere yrodiere self-assigned this May 31, 2023
@yrodiere
Copy link
Member Author

yrodiere commented May 31, 2023

Apparently there's another reason to do an explicit docker pull related to feedback:

// we pull the docker image in order to give users an indication of which step the process is at
// it's not strictly necessary we do this, however if we don't the subsequent version command
// will appear to block and no output will be shown
String effectiveBuilderImage = nativeConfig.getEffectiveBuilderImage();

I'll try to find an alternative to docker run --pull.

@quarkus-bot quarkus-bot bot added this to the 3.2 - main milestone Jun 1, 2023
yrodiere added a commit to yrodiere/quarkus that referenced this issue Jun 1, 2023
Follows up on quarkusio#33749.

I forgot to remove this code that was just a remainder of an earlier
approach which didn't go anywhere
(quarkusio#33691 (comment)).

Keeping this code could in theory lead to extra pulls when using
quarkus.native.build-image.pull='always' (the default) and the image
gets updated right while we're building, but more importantly it could
lead to extra, unecessary queries to quay.io, so let's avoid that.
sberyozkin pushed a commit to sberyozkin/quarkus that referenced this issue Jun 21, 2023
Follows up on quarkusio#33749.

I forgot to remove this code that was just a remainder of an earlier
approach which didn't go anywhere
(quarkusio#33691 (comment)).

Keeping this code could in theory lead to extra pulls when using
quarkus.native.build-image.pull='always' (the default) and the image
gets updated right while we're building, but more importantly it could
lead to extra, unecessary queries to quay.io, so let's avoid that.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants