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

feat: Quarkus native source #4764

Merged
merged 9 commits into from
Oct 4, 2023
Merged

feat: Quarkus native source #4764

merged 9 commits into from
Oct 4, 2023

Conversation

squakez
Copy link
Contributor

@squakez squakez commented Sep 21, 2023

With this PR we remove the need to create a dynamic container image builder. The tool image is still required to build a Quarkus native image, but we now leverage the refactored pipeline process which gives us the possibility to run the native compilation as a custom task (more design details in the doc of this PR):

camel_k_pipeline

Pending tasks to complete:

  • Simplify quarkus traits native parameters
  • extend resources and limits configuration to all pipeline tasks
  • make the builder trait independent from the quarkus trait --> not immediatly feasible as builder and quarkus trait are referencing each other Follow up issue: Make Builder trait independent from the Quarkus trait #4771
  • move configuration resources from the build to the native user task

Closes #4648

Release Note

feat: Quarkus native source

@squakez squakez added the trigger native test Use this label in PR when you want to trigger Quarkus Native tests label Sep 21, 2023
@squakez squakez force-pushed the feat/4648 branch 3 times, most recently from e2db40c to 5ffc483 Compare September 25, 2023 14:57
@squakez squakez changed the title POC to collect feedback from a full e2e execution - do not merge feat: Quarkus native source Sep 25, 2023
@squakez squakez added the kind/feature New feature or request label Sep 25, 2023
@squakez squakez marked this pull request as ready for review September 25, 2023 15:03
@squakez squakez force-pushed the feat/4648 branch 4 times, most recently from b1c856c to f6697d4 Compare September 27, 2023 10:37
@github-actions
Copy link
Contributor

🐫 Thank you for contributing!

Code Coverage Report ✔️ - Coverage changed: 33.1% --> 33.3% (Coverage difference: +.2%)

@github-actions
Copy link
Contributor

🐫 Thank you for contributing!

Code Coverage Report ✔️ - Coverage changed: 33.1% --> 33.3% (Coverage difference: +.2%)

@github-actions
Copy link
Contributor

🐫 Thank you for contributing!

Code Coverage Report ✔️ - Coverage changed: 33.1% --> 33.3% (Coverage difference: +.2%)

@squakez
Copy link
Contributor Author

squakez commented Sep 27, 2023

@essobedo I'd need your help with this. For some reason I am not able to understand, the DSL with sources support is not working with the changes applied. I tried to debug and it seems that the sources are really embedded. However, the execution does not take them in account. Would you mind having a look? hopefully you can spot some problem I am not able to see at this stage. Thanks in advance!

@squakez
Copy link
Contributor Author

squakez commented Sep 28, 2023

@essobedo I'd need your help with this. For some reason I am not able to understand, the DSL with sources support is not working with the changes applied. I tried to debug and it seems that the sources are really embedded. However, the execution does not take them in account. Would you mind having a look? hopefully you can spot some problem I am not able to see at this stage. Thanks in advance!

Nevermind, I think I managed to understand the root cause. Basically it's the support of native-sources which seems to be missing on Camel Quarkus side.

@essobedo
Copy link
Contributor

@squakez Oh sorry, I missed your message, do you still need some help on this?

@squakez
Copy link
Contributor Author

squakez commented Sep 29, 2023

@squakez Oh sorry, I missed your message, do you still need some help on this?

No probl, maybe I'll ask you a review on Camel Quarkus later in the day. Thanks!

@squakez squakez linked an issue Sep 29, 2023 that may be closed by this pull request
@squakez
Copy link
Contributor Author

squakez commented Sep 29, 2023

Depends on apache/camel-quarkus#5380

@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2023

🐫 Thank you for contributing!

Code Coverage Report ✔️ - Coverage changed: 33.1% --> 33.3% (Coverage difference: +.2%)

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2023

🐫 Thank you for contributing!

Code Coverage Report ✔️ - Coverage changed: 33.1% --> 33.3% (Coverage difference: +.2%)

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2023

🐫 Thank you for contributing!

Code Coverage Report ✔️ - Coverage changed: 33.1% --> 33.3% (Coverage difference: +.2%)

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2023

🐫 Thank you for contributing!

Code Coverage Report ✔️ - Coverage changed: 33.1% --> 33.3% (Coverage difference: +.2%)

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2023

🐫 Thank you for contributing!

Code Coverage Report ✔️ - Coverage changed: 33.1% --> 33.3% (Coverage difference: +.2%)

@github-actions
Copy link
Contributor

github-actions bot commented Oct 3, 2023

🐫 Thank you for contributing!

Code Coverage Report ✔️ - Coverage changed: 33.1% --> 33.3% (Coverage difference: +.2%)

1 similar comment
@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2023

🐫 Thank you for contributing!

Code Coverage Report ✔️ - Coverage changed: 33.1% --> 33.3% (Coverage difference: +.2%)

* Move the previous logic to native-source package type
* A new task takes care to trigger a pipeline task to run Native Image
* Split the builder task into 2 tasks
   - builder takes care of the building part (mvn package)
   - package task does the packaging (after custom tasks)

Ref apache#4648
It was introduced to decouple the operator from the runtime. No longer needed as the native phase is executed using catalog tool image directly.
This parameter exposed implementations details not user friendly. Enabling mode parameter which accept either `jvm` or `native` seems a better UX.
It seems that usage of  `mode` create some problem when unmarshalling the CRD
@squakez squakez merged commit aee0c5f into apache:main Oct 4, 2023
10 of 11 checks passed
@squakez squakez deleted the feat/4648 branch October 4, 2023 10:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/feature New feature or request trigger native test Use this label in PR when you want to trigger Quarkus Native tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

E2E failure on common TestRunExtraRepository test Separating Java and native image compilation
2 participants