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

AppCDS docs on usage with JIB are slightly misleading #43733

Closed
DeMol-EE opened this issue Oct 6, 2024 · 12 comments · Fixed by #43946
Closed

AppCDS docs on usage with JIB are slightly misleading #43733

DeMol-EE opened this issue Oct 6, 2024 · 12 comments · Fixed by #43946

Comments

@DeMol-EE
Copy link
Contributor

DeMol-EE commented Oct 6, 2024

In the documentation on AppCDS, it is stated that "simply setting quarkus.package.jar.appcds.enabled to true" is enough when using the quarkus-container-image-jib extension, but it is important to also set quarkus.package.jar.appcds.use-container to false (assuming that the choice for jib implies that docker is not available). Does anyone agree that it might be better to explicitly mention this?

Copy link

quarkus-bot bot commented Oct 6, 2024

/cc @geoand (jib)

@geoand
Copy link
Contributor

geoand commented Oct 13, 2024

Can you elaborate a little more on this?

Thanks

@geoand geoand added the triage/needs-feedback We are waiting for feedback. label Oct 13, 2024
@DeMol-EE
Copy link
Contributor Author

DeMol-EE commented Oct 14, 2024

Sure. I was reading this blog post on project Leyden, which lead me to Quarkus’ AppCDS guide. There, I was reminded about jib. I remembered reading about it before on the container image guide but I had never given it much attention because I skipped straight to docker (which was familiar).
After reading the blog post, I was excited about both technologies, especially because the AppCDS guide mentions that you practically get it for free in combination with jib. So, I set out on an undertaking to migrate all our services from docker to jib, and, in the process, also opt-in to AppCDS. As the AppCDS guide suggests, I added the flag -Dquarkus.package.jar.appcds.enabled=true to our maven build command, but was a little surprised by errors coming from AppCDS about the absence of a docker environment. I re-read both guides to see if I missed anything but didn’t find any clues until I started digging further online to discover the quarkus.package.jar.appcds.use-container flag.
I had falsely assumed that, given that we were now also using jib, the default for this flag would be "false" (I figured this because I understood that one of the main points of using jib was to not need to have docker), but I was mostly surprised I hadn’t read about this flag in either guide.
Don’t get me wrong, Quarkus still makes it child’s play to use jib and AppCDS; I merely wanted to suggest perhaps adding the "use-container" flag to either (or both) guides. What do you think?

@geoand geoand added area/documentation and removed triage/needs-feedback We are waiting for feedback. labels Oct 14, 2024
@geoand
Copy link
Contributor

geoand commented Oct 14, 2024

Thanks a lot!

Your description is spot on (took me a while to remember how this works).

Would you like to enhance the documention along the lines of your proposal?

@DeMol-EE
Copy link
Contributor Author

DeMol-EE commented Oct 14, 2024

I could give it a go, sure, but now I’m wondering if it wouldn’t make more sense to change the default of the flag when it detects jib is used. Is that actually possible?

@geoand
Copy link
Contributor

geoand commented Oct 14, 2024

I'll have to look at it when I have some time

@geoand
Copy link
Contributor

geoand commented Oct 14, 2024

So IIUC, the machine on which you are building the container image does not have access to Docker?

In that case, setting quarkus.package.jar.appcds.use-container=false is indeed needed, however you need to make sure that the Java version used on that machine is exactly the same as the one used in the container image.

@DeMol-EE
Copy link
Contributor Author

Indeed. So I suppose it isn't that trivial: jib can not just figure out which kind of image to build, this needs to be aligned with the Java version running the build, which could be anything. If this is working for me at this point in time, I guess I got lucky!
However, does having a docker environment change much? Perhaps then the AppCDS plugin is smart enough to use a container image that matches the one from jib?
Even so, doesn't it sort of defeat the purpose of jib, then? Though I suppose it is still nice not to have dockerfiles...

@geoand
Copy link
Contributor

geoand commented Oct 14, 2024

However, does having a docker environment change much?

What it allows us to do is pull the base image and use the Java version from it to run the AppCDS creation process locally.

Perhaps then the AppCDS plugin is smart enough to use a container image that matches the one from jib?

That is exactly what is happening.

Even so, doesn't it sort of defeat the purpose of jib, then? Though I suppose it is still nice not to have dockerfiles...

Jib does docker-less push, not build and that's only one of its selling points. But I do see your point.

@DeMol-EE
Copy link
Contributor Author

Thanks for clearing that out. I need to take another look at our pipeline to ensure our AppCDS setup actually works. Perhaps I'll just add docker back to the environment (it's available anyway) and still stick to jib for the simplified setup.
I'll see if I can free some time to prepare a PR with an extra sentence for the AppCDS guide mentioning the use-container flag, if that's still OK?

@geoand
Copy link
Contributor

geoand commented Oct 14, 2024

Definitely!

@DeMol-EE
Copy link
Contributor Author

I’ve opened #43946

@geoand geoand closed this as completed Oct 18, 2024
@geoand geoand added this to the 3.17 - main milestone Oct 18, 2024
@gsmet gsmet modified the milestones: 3.17 - main, 3.16.0 Oct 21, 2024
@rsvoboda rsvoboda modified the milestones: 3.16.0, 3.15.2 Oct 30, 2024
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.

4 participants