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

Support OpenJDK S2I #1305

Closed
siamaksade opened this issue Mar 7, 2019 · 24 comments
Closed

Support OpenJDK S2I #1305

siamaksade opened this issue Mar 7, 2019 · 24 comments

Comments

@siamaksade
Copy link

When Using OpenJDK S2I to build and deploy the examples on OpenShift, builds succeeds but the deployment fails.

oc new-app java~https://github.com/quarkusio/quarkus-quickstarts --context-dir=getting-started --name=quarkus

The deployment fails with the following error since it can't determine which JAR file to use:

ERROR: Neither $JAVA_MAIN_CLASS nor $JAVA_APP_JAR is set and 2 JARs found in /deployments (1 expected)
sh-4.2$ ls /deployments/ -l
total 40
drwxrwxr-x. 2 jboss root     6 Oct 17 22:12 data
-rw-r--r--. 1 jboss root 34673 Mar  7 13:49 quarkus-quickstart-1.0-SNAPSHOT-runner.jar
-rw-r--r--. 1 jboss root  3353 Mar  7 13:49 quarkus-quickstart-1.0-SNAPSHOT.jar
@stuartwdouglas
Copy link
Member

Try <uberJar>true</uberJar> in the maven plugin config

@siamaksade
Copy link
Author

Do any of the quickstarts have it enabled to test with OpenShift?

@cescoffier
Copy link
Member

cescoffier commented Mar 7, 2019 via email

@vorburger
Copy link
Contributor

@siamaksade @cescoffier I may be able to help with this one - will try to do so tomorrow or Monday...

FYI, background: fabric8io-images/s2i#123 is kind of related to this...

see also #304

@cescoffier
Copy link
Member

@vorburger That would be awesome!

@geoand
Copy link
Contributor

geoand commented Mar 7, 2019

Using the uberjar config should do the trick since in that case we add the .original sufficient to the original jar, thus leaving a single .jar file.

@siamaksade
Copy link
Author

It would be valuable to add a quickstart for OpenShift similar to getting-started-kubernetes and with uberjar config enabled

@geoand
Copy link
Contributor

geoand commented Mar 8, 2019

@siamaksade Good idea. What else would you like to see in that guide?

@cescoffier
Copy link
Member

cescoffier commented Mar 8, 2019 via email

@siamaksade
Copy link
Author

@geoand a quickstart like getting-started-openshift that possibly has also fabric8 in it and one can just run mvn fabric8:deploy, and that would create a build on OpenShift using the OpenJDK builder image and deploys the Quarkus app.

@cescoffier a guide is also would be great (#1306). One wouldn't even find it currently on quarkus.io since item in the guides page is called "Deploying Applications on Kubernetes" rather than "Deploying Applications on Kubernetes/OpenShift". Furthremore, it needs instructions on how to use S2I for the build, in addition to the docker build strategy.

@geoand
Copy link
Contributor

geoand commented Mar 8, 2019

If we agree that it is needed, I can definitely add it

@vorburger
Copy link
Contributor

The last section of the kubernetes guide covers OpenShift.
One wouldn't even find it currently on quarkus.io since item in the guides page is called "Deploying Applications on Kubernetes" rather than "Deploying Applications on Kubernetes/OpenShift".

@siamaksade @cescoffier OK so let's fix (or further discuss) that point over in quarkusio/quarkusio.github.io#124 ...

It would be valuable to add a quickstart for OpenShift similar to getting-started-kubernetes and with uberjar config enabled

But if we can get oc new-app to work (original issue here; I'll look more into it next), then we may not actually need a separate getting-started-openshift anymore, no?

@geoand a quickstart like getting-started-openshift that possibly has also fabric8 in it and one can just run mvn fabric8:deploy, and that would create a build on OpenShift using the OpenJDK builder image and deploys the Quarkus app.

@siamaksade @geoand I'm personally actually not sure if I am huge fan of fabric8:deploy, and whether we should be further promoting it here... IMHO "normal" S2I on-cluster build from Git repo is what one would want to do to biuld from source? And to push local not yet committed changes, there's no need for a Maven plugin, but simply s2i build --copy . should do? To be tested and documented, of course.

@siamaksade
Copy link
Author

siamaksade commented Mar 8, 2019

@vorburger sure. I understand your reluctance toward fabric8 😃

@vorburger
Copy link
Contributor

Try <uberJar>true</uberJar> in the maven plugin config

@stuartwdouglas @cescoffier see #1344 ...

I can see two solutions forward: a) I could do what's needed in the S2I Builder image to make it pick the (bigger) runner.jar over the (smaller) other normal JAR; or b) we could make the quarkus-maven-plugin produce only a single JAR? (Or do that jar.original rename thing but without bloating the runner JAR.)

Doing b) may be quicker, and would have the advantage that it would work for all S2I images (if I did the (a) solution it would be in https://github.com/fabric8io-images/s2i; the later the same would have to separately be redone over in https://github.com/jboss-container-images/openjdk because of fabric8io-images/s2i#218).

Note that with uberjar you will be in JVM mode and not in native mode.

Understood; this issue (for #1306) is about building a Quarkus app in OpenShift with S2i in JVM mode, not native. We'll tackle that later under #304.

@siamaksade
Copy link
Author

(b) is preferable IMHO for reasons you mentioned. S2I builder image just sees 2 jars files and any heuristic for picking one would end up being the wrong choice for other use-cases.

@geoand
Copy link
Contributor

geoand commented Mar 8, 2019

I opened a PR (#1345) for b) - let's see what people think

geoand added a commit to geoand/quarkus that referenced this issue Mar 8, 2019
@cescoffier
Copy link
Member

cescoffier commented Mar 8, 2019 via email

@cescoffier
Copy link
Member

cescoffier commented Mar 8, 2019 via email

@vorburger
Copy link
Contributor

Didn’t read everything, but if we could about the fat jar in a container that would be great...

can you elaborate a little bit on why? I trust you have good reasons - I'd love to learn about them!

Can’t we have a builder that just take the regular runner and copy the libs?

we could, but that requires work in the S2I base image(s; several!), they probably won't know that they have to copy lib/ ... it's doable, but what's the "justification" ? Using uberJar for producing a FAT JAR may be the best solution after all, for the short term. (And sorry for the back and forth!)

@cescoffier
Copy link
Member

A docker container is already your packaging unit, creating another packaging unit in it is just looking for bad performances at startup time.

In addition, we are creating a huge top layer and every update is going to resync the whole "fat" layer, which we would avoid with using the runner (as it's only your application code and resources).

vorburger added a commit to vorburger/quarkus-quickstarts that referenced this issue Mar 8, 2019
vorburger added a commit to vorburger/quarkus-quickstarts that referenced this issue Mar 8, 2019
@vorburger
Copy link
Contributor

vorburger commented Mar 8, 2019

quarkusio/quarkus-quickstarts#81 now has a working (tested) proposed first version! That is still based on <uberJar>true. I have separately checked out if I can apply the idea from @gastaldi mentioned in #1345 to incrementally improve it, that's in quarkusio/quarkus-quickstarts#82.

vorburger added a commit to vorburger/quarkus that referenced this issue Mar 8, 2019
quarkusio#1306

see also quarkusio#1305

requires quarkusio/quarkus-quickstarts#81

and then to actually publish the link to this new guide in the menu
it needs quarkusio/quarkusio.github.io#126
geoand added a commit to geoand/quarkus that referenced this issue Mar 12, 2019
vorburger added a commit to vorburger/quarkus-quickstarts that referenced this issue Mar 22, 2019
via .s2i/environment with suitable options,
and by fixing the JAR's finalName in POM,

see quarkusio/quarkus#1305

Credit goes to @gastaldi for "inspiration" for this taken from Launcher.
@vorburger
Copy link
Contributor

quarkusio/quarkus-quickstarts#81 has gone through some iterations, and we now have something (hopefully) final (now without <uberJar>true). @siamaksade hope you're OK if we close this issue after quarkusio/quarkus-quickstarts#81 and #1353 have been merged?

@vorburger
Copy link
Contributor

@siamaksade OK for you to close this issue, now that both quarkusio/quarkus-quickstarts#81 as well as #1353 have been merged, and that quarkusio/quarkusio.github.io#126 is hopefully about to be merged?

@gsmet gsmet changed the title Deployment fails when OpenJDK S2I is used Support OpenJDK S2I Apr 2, 2019
@cescoffier cescoffier added this to the 0.13.0 milestone Apr 2, 2019
@cescoffier
Copy link
Member

This can be closed. quarkusio/quarkusio.github.io#126 need a bit of rework (very very small ordering issue).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants