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

Make the use of Mandrel images easier #18129

Closed
zakkak opened this issue Jun 24, 2021 · 5 comments · Fixed by #23096
Closed

Make the use of Mandrel images easier #18129

zakkak opened this issue Jun 24, 2021 · 5 comments · Fixed by #23096
Assignees
Labels
area/mandrel kind/enhancement New feature or request
Milestone

Comments

@zakkak
Copy link
Contributor

zakkak commented Jun 24, 2021

Description

Currently Quarkus defaults to using native-image from $GRAALVM_HOME/bin, $JAVA_HOME/bin, or the system's $PATH. If native-image is not available then it falls back to using a container runtime with the ubi-quarkus-native-image container image.

The main benefit of this is that users can simply run:

./mvnw -Pnative package

and create a native executable of their Quarkus application without any local setup. This also makes it easy to ensure that the users are using the correct container image (and the correct version) since it's defined in build-parent/pom.xml

In order for users to use Mandrel they need to do one of the following:

  • Download Mandrel, set it up, and make sure native-image can be found either in $GRAALVM_HOME/bin, $JAVA_HOME/bin, or the system's $PATH.
  • Use -Dquarkus.native.builder-image=quay.io/quarkus/ubi-quarkus-mandrel:21.0-java11

Now the issue I see with both options is that the user is responsible for figuring out the correct Mandrel version to use.
I believe that Quarkus should offer an option to allow users to state that they would like to build their native executable using Mandrel without the need to worry about the correct version.

e.g.

./mvnw -Pnative `-Dquarkus.native.mandrel=true` package

Implementation ideas

We could possibly introduce yet another property like quarkus.native.mandrel or we could handle something like -Dquarkus.native.builder-image=mandrel and replace mandrel with the ubi-quarkus-mandrel image that is compatible with the Quarkus version being used.

WDYT?

@zakkak zakkak added the kind/enhancement New feature or request label Jun 24, 2021
@quarkus-bot
Copy link

quarkus-bot bot commented Jun 24, 2021

/cc @galderz

@galderz
Copy link
Member

galderz commented Jun 30, 2021

Hmmm, I think I'd leave quarkus.native.builder-image as is, a property that points to a builder image, but I'd give it a default based on the Quarkus version. Instead, I would add something like quarkus.native.provider that takes an enum of graalvm, mandrel or whichever option we might have in the future (e.g. leyden?). Then, to build with mandrel, you'd do:

./mvnw -Pnative package -Dquarkus.native.provider=mandrel

quarkus.native.builder-image would still allow you to tweak the builder image in use, say if you're testing a locally built one, or a newer or older version.

@zakkak
Copy link
Contributor Author

zakkak commented Jan 11, 2022

Although I like how this separates the actual image from the distribution/provider I am afraid it might make things unnecessarily complicated. E.g. , users might think they need to pass -Dquarkus.native.provider=mandrel with -Dquarkus.native.builder-image=quay.io/quarkus/ubi-quarkus-mandrel:21.0-java11 while they shouldn't have to.

I think I like more the option of making quarkus.native.builder-image accept either a value from an enum of graalvm, mandrel or a container image. In that case the default value of quarkus.native.builder-image should become graalvm or mandrel. The only downside I see with this option is that we "reserve" graalvm and mandrel (and whatever might come in the future) making the use of images named like this a bit harder, but I don't expect this to be an issue.

@gsmet @geoand WDYT?

@geoand
Copy link
Contributor

geoand commented Jan 11, 2022

That sounds like an interesting idea, I like it!

@galderz
Copy link
Member

galderz commented Jan 11, 2022

I think that's a fair suggestion. I would suggest validating the contents of the property early on so that any mistakes are clearly reported. So, it's either one of the 2 (currently) reserved words, or a URL.

@zakkak zakkak self-assigned this Jan 11, 2022
zakkak added a commit to zakkak/quarkus that referenced this issue Jan 21, 2022
…age`

This change makes it easier for developers to choose the variant of the
builder image they want to use without having to worry about which
version is the currently supported one. Quarkus will figure that out on
their behalf.

Closes quarkusio#18129
zakkak added a commit to zakkak/quarkus that referenced this issue Jan 21, 2022
…age`

This change makes it easier for developers to choose the variant of the
builder image they want to use without having to worry about which
version is the currently supported one. Quarkus will figure that out on
their behalf.

Closes quarkusio#18129
zakkak added a commit to zakkak/quarkus that referenced this issue Jan 24, 2022
…age`

This change makes it easier for developers to choose the variant of the
builder image they want to use without having to worry about which
version is the currently supported one. Quarkus will figure that out on
their behalf.

Closes quarkusio#18129
@quarkus-bot quarkus-bot bot added this to the 2.8 - main milestone Jan 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/mandrel kind/enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants