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

Add Quarkus devfile #144

Merged
merged 3 commits into from
Jan 8, 2020
Merged

Add Quarkus devfile #144

merged 3 commits into from
Jan 8, 2020

Conversation

svor
Copy link
Contributor

@svor svor commented Nov 11, 2019

Signed-off-by: svor vsvydenk@redhat.com

What does this PR do?

Adds a devfile to support Qurakus tools.

This devfile includes

  • 2 components:

    • dev component is based on quay.io/quarkus/centos-quarkus-maven:19.2.1 which contains Java 8, Maven 3.6.0 and GraalVM 19.2.1
    • qurkus plugin (for more information take a look at Add Quarkus plugin che-plugin-registry#291)
  • 2 commands:

    • mvn package
    • mvn compile quarkus:dev
  • The application could be debugged by predefined debug configuration in case mvn compile quarkus:dev was executed before.

This devfile doesn't include commands to run native compilation because it requires more than 3GB RAM for workspace in total and it's not available on Hosted Che.

What issues does this PR fix or reference?

eclipse-che/che#14506

@svor svor self-assigned this Nov 11, 2019
-
type: dockerimage
alias: maven
image: quay.io/eclipse/che-java8-maven:nightly
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should include the ability to run the native compilation to this devfile

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you please clarify why do we need that?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like GraalVM Native Image generation requires at least 2Gi RAM to perform a native image build. So, if we give this value + 1,5Gi for Quarkus VS Code plugin (Java LS, Quarkus LS, Java Debugger) container, it'd be more than 3Gi - it is too much for che.openshift.io

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To me we have to produce a devfile for che. Not che.openshift.io.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

upstream devfile registry is used on Hosted Che, so all the devfiles in the registry must be compatible with Hosted Che. Otherwise, we need to come up with Hosted Che specific devfile registry (smth. that should be avoided)

Copy link
Contributor

@sunix sunix Nov 27, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like GraalVM Native Image generation requires at least 2Gi RAM to perform a native image build. So, if we give this value + 1,5Gi for Quarkus VS Code plugin (Java LS, Quarkus LS, Java Debugger) container, it'd be more than 3Gi - it is too much for che.openshift.io

Could you give a try in a single container with vscode ext for java/quarkus and mvn/graalvm/jdk8 ?

Copy link
Member

@ibuziuk ibuziuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will it be compatible with Hosted Che (3GB ram)?

@tsmaeder
Copy link
Contributor

will it be compatible with Hosted Che (3GB ram)?
modulo native compilation, it should

@svor
Copy link
Contributor Author

svor commented Nov 27, 2019

@ibuziuk except building a native executable, it should

@sunix
Copy link
Contributor

sunix commented Nov 27, 2019

Not including native executable compilation would show only 1/3 of the value of Che for quarkus developers. IMHO

@svor
Copy link
Contributor Author

svor commented Nov 27, 2019

It is possible to execute native compilation but it'll be failed with OOM on hosted Che, that's why I didn't add commands into devfile. If the user will have more OOM for Che it could add such commands into the workspace or just execute them in the dev terminal

@sunix
Copy link
Contributor

sunix commented Nov 27, 2019

@ibuziuk

upstream devfile registry is used on Hosted Che, so all the devfiles in the registry must be compatible with Hosted Che. Otherwise, we need to come up with Hosted Che specific devfile registry (smth. that should be avoided)

If the goal of hosted Che is to show the value of Che to potential/possible users, for a Quarkus devfile, we need to have Quarkus + native compilation. If we don't have that, there is no value compared to a VSCode or another IDE. We need to reconsider the 3Go limitation and give 4 or 5Gigs at least if we cannot optimize it.

@ibuziuk
Copy link
Member

ibuziuk commented Nov 27, 2019

We need to reconsider the 3Go limitation and give 4 or 5Gigs at least if we cannot optimize it.

isn't it smth you have already raised on the planning? This is a PM decision, so, for now, all the devfiles should be compatible with the 3Gb RAM limits available on Hosted Che - https://www.eclipse.org/che/docs/che-7/hosted-che/#terms-of-service_hosted-che

Signed-off-by: svor <vsvydenk@redhat.com>
@amisevsk
Copy link
Contributor

IMO native compilation is a deploy step most of the time (with the caveat there are some minor incompatibilities). If I was developing a quarkus app, I my flow would be something like

  • Develop using quarkus:dev for hot code reload and testing/debugging
  • Once ready to deploy, run a builder separately to create and test the native binary.

Native compilation in the workspace is a "nice to have" but not essential, since generally native compilation is something you would do once, at the end of preparing a PR.

Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just a few suggestions.

devfiles/quarkus/devfile.yaml Outdated Show resolved Hide resolved
devfiles/quarkus/devfile.yaml Show resolved Hide resolved
devfiles/quarkus/devfile.yaml Outdated Show resolved Hide resolved
Signed-off-by: svor <vsvydenk@redhat.com>
@sunix
Copy link
Contributor

sunix commented Dec 24, 2019

IMO native compilation is a deploy step most of the time (with the caveat there are some minor incompatibilities). If I was developing a quarkus app, I my flow would be something like

  • Develop using quarkus:dev for hot code reload and testing/debugging
  • Once ready to deploy, run a builder separately to create and test the native binary.

Native compilation in the workspace is a "nice to have" but not essential, since generally native compilation is something you would do once, at the end of preparing a PR.

Yes this is nice to have but essential to show the value of Che: this is what is hard to have if you are working on a Quarkus app on a traditional Desktop environment. The current devfile is easy to setup with VSCode + quarkus ext... so what is the point to use Che ?

Signed-off-by: svor <vsvydenk@redhat.com>
@nickboldt
Copy link
Contributor

nickboldt commented Jan 6, 2020

Looks like all of Angel's concerns have been addressed; which only leaves @ibuziuk 's question of 3G vs. 4-5G as the required memory footprint.

Meanwhile upstream the quarkus jdk8 plugin PR is approved: eclipse-che/che-plugin-registry#291

Copy link
Contributor

@amisevsk amisevsk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

Personally, I'm against adding native compilation as a default, since adding 2Gi to the runtime memory requirements for this workspace is too high a cost for something that generally won't be executed often. I would like to see native compilation added when Che supports something like a short-lived builder container, so that just running the workspace doesn't take 2Gi of quota that is unused 90% of the time.

@tsmaeder
Copy link
Contributor

tsmaeder commented Jan 6, 2020

Folks, we should not make merging this PR dependent on the native compilation commands. In the short run, we cannot increase memory on hosted Che. In the short run, we cannot run a separate devfile registry for hosted che.
IMO, we have 3 courses of action open:

  1. Merge the PR "as is"
  2. Wrap native compilation with a script that graciously fails when memory is lacking (aka on hosted che) and run that as the native compile command
  3. Don't add Quarkus support.

@slemeur make a decision, please.

@nickboldt
Copy link
Contributor

nickboldt commented Jan 6, 2020

My $0.02 based on existing CRW 2.1 requirements (eg., https://issues.redhat.com/browse/CRW-586 and https://issues.redhat.com/browse/CRW-329 ) ...

#3 isn't an option - Quarkus support is required for CRW 2.1, and being able to dogfood it in some way in Che 7.7 and in Hosted Che is IMHO a valuable way to ensure it's working downstream too.

#2 seems like the best approach - failing gracefully is an important part of UX when running on disparate clusters. Error message on failure should include detailed call to action so workspace user knows they need to ask for more RAM from their cluster administrator. I would further tell the user that if they don't have access to their administrator, this functionality won't be available (eg., if they need 5G in Hosted Che, they're going to have a bad time.)

@tsmaeder
Copy link
Contributor

tsmaeder commented Jan 8, 2020

I would go ahead and merge this PR as is. We can add native compile commands in the next sprint.

@slemeur
Copy link

slemeur commented Jan 8, 2020

+1 on the approach

@amisevsk amisevsk merged commit 8f4a62a into master Jan 8, 2020
@svor svor deleted the sv/quarkus branch January 8, 2020 15:04
nickboldt pushed a commit that referenced this pull request Jan 8, 2020
* Add quarkus devfile

Signed-off-by: svor <vsvydenk@redhat.com>
sparkoo pushed a commit to sparkoo/che-devfile-registry that referenced this pull request Dec 4, 2020
* Added definitions for gwt ide versions since 6.19.0

6.19.0
7.0.0-beta-1.0
7.0.0-beta-2.0
7.0.0-beta-3.0
7.0.0-beta-4.0
next
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants