add a module for Spring Boot 3 AOT #2457

joshlong · 2022-11-22T01:49:09Z

this supercedes my last PR. in this example I've preserved Spring Framework 5 / Spring Boot 2 / Java 8 support in the client-java-spring-integration module, adding only one text file for Spring Boot 3 support.

I've also created a new module, client-java-spring-aot-integration, that contributes GraalVm hints using Spring Boot 3's AOT engine. This module depends on Spring Framework 6 / Spring Boot 3 / Java 17 / the third party Reflections library.

I've built a trivial controller here https://github.com/kubernetes-native-java/controllers-101 and linked it against the -aot module in this PR, and it works :)

yue9944882 · 2022-11-22T06:15:55Z

...in/java/io/kubernetes/client/spring/aot/KubernetesBeanFactoryInitializationAotProcessor.java

+        return (generationContext, beanFactoryInitializationCode) -> {
+            RuntimeHints hints = generationContext.getRuntimeHints();
+            String[] classNames = new String[]{
+                    "com.google.gson.JsonElement",//


these class names need to be secured by unit-test, in case the linked classes are renamed/moved in the future. we can do this in a follow-up.

yue9944882 · 2022-11-22T06:19:07Z

spring-aot/pom.xml

+    <!--
+     todo :   we don't need this after 24 november, 2022
+    -->
+    <repositories>


nit:how about we lift/centralize these repository declarations to the parent pom?

they won't be required after the 24th. no need to add them at all, afaict

Let's wait until after the 24th and get these removed.

yue9944882 · 2022-11-22T06:20:28Z

spring-aot/pom.xml

+        <dependency>
+            <groupId>org.reflections</groupId>
+            <artifactId>reflections</artifactId>
+            <version>0.10.2</version>


nit: move the versioned dependency to the parent pom and let the children modules inherit.

yue9944882

can you run the formatter so that Verify Source Format job can pass?

yue9944882 · 2022-11-22T07:12:53Z

@joshlong the new module will block the compilation of test jobs running on platforms older than java17, i think we need to explicitly exclude spring-aot from these compatibility tests (e.g. either by setting --pl "!spring-aot" when running maven cmd or declaring a new profile in the parent pom). and then we need to copy/paste a separate test job for coveringspring-aot on java17 platform from the existing ones. i opened #2459 to show the expected edits on the github action configurations, can you incorporate/rebase the pull?

joshlong · 2022-11-22T11:08:09Z

can you run the formatter so that Verify Source Format job can pass?

what formatter?

joshlong · 2022-11-22T11:28:20Z

ive addressed most of the issues, i think. i hope this works :) I think you mean mvn spotless:apply for the formatted, but I'm not sure. i ran that, tho.

brendandburns · 2022-11-22T22:58:56Z

.github/workflows/maven.yml

@@ -22,12 +22,34 @@ jobs:
          java-version: 8.0.x
      - name: Verify Format and License
        run: mvn spotless:check
+  legacy-build:


I'd really rather not maintain two different build targets, it's a recipe for drift and cut/paste errors.

Is there a way that we can combine them together with a conditional depending on the java version, that should be possible.

sure they can be merged into one conditional target, i updated #2459 to show how it works. @joshlong by any chance can you rebase again onto my latest commit?

brendandburns · 2022-11-22T22:59:37Z

pom.xml

@@ -561,5 +562,21 @@ limitations under the License.
        <module>fluent-gen</module>
      </modules>
    </profile>
+    <profile>


I don't know if it is possible in a pom, but is there a way to inherit this? I'd really prefer to avoid duplication if we can avoid it.

i'm not sure about that, now it's already declared in the root-level pom. as far as i know, it's not inheriting from any other project/places.

brendandburns · 2022-11-22T23:01:10Z

spring-aot/pom.xml

+            <dependency>
+                <groupId>org.springframework.boot</groupId>
+                <artifactId>spring-boot-dependencies</artifactId>
+                <version>3.0.0-SNAPSHOT</version>


Can we wait until this is released, I'd rather not depend on a snapshot version.

Absolutely ;) that'll be on Thursday this week. Happy thanksgiving if you celebrate! I am certainly grateful for everything y'all are doing in this great project

let's switch to non-snapshot release after that

brendandburns · 2022-11-22T23:01:34Z

spring-aot/pom.xml

+            <plugin>
+                <groupId>org.jacoco</groupId>
+                <artifactId>jacoco-maven-plugin</artifactId>
+                <version>0.8.8</version>


pull this version up to the parent pom (this may be a problem in all of our poms...)

yue9944882 · 2022-11-23T06:38:30Z

I think you mean mvn spotless:apply for the formatted

@joshlong exactly, looks like all the tests passed now

joshlong · 2022-11-25T19:27:46Z

Hi - i just updated the PR to use Spring Boot 3 GA! Happy day!

The PR removes all mentions of snapshots and snapshot repositories. I also avoid redundantly declaring the Spring Boot BOM in the child module, overriding the spring-boot.version property instead.

Also, I am not sure if this was intentional or if I missed something in my earlier PR, but I noticed that the spring-aot module wasn't among those enumerated in the <modules> section in the top-most pom.xml so I added that.

Signed-off-by: yue9944882 <291271447@qq.com>

yue9944882 · 2022-11-28T05:55:47Z

.github/workflows/maven.yml

-          mvn clean test -q -B --define=org.slf4j.simpleLogger.log.org.apache.maven.cli.transfer.Slf4jMavenTransferListener=warn
+          if [ $(grep -E '^(8|11)\.' <<< '${{ matrix.java }}') ]; then
+            # some module doesn't compile on java platform lower than 17, need to skip them by specifying a profile
+            MODS_OVERRIDES='-pl !spring-aot'


excluding spring-aot for java8 and java11 build ^^^

yue9944882 · 2022-11-28T05:56:26Z

@brendandburns @joshlong i appended another commit to this thread so that the test workflow can pass after we bump java version to 17. the java 8/11 compatibility is meanwhile assured by #2457 (comment)

joshlong · 2022-11-30T21:52:51Z

whats the next step?

yue9944882 · 2022-12-01T05:24:05Z

@joshlong it looks like https://github.com/kubernetes-client/java/pull/2457/files#r1029896501 hasn't been resolved yet, can you move the plugin declaration to the root?

joshlong · 2022-12-01T06:09:27Z

@joshlong it looks like https://github.com/kubernetes-client/java/pull/2457/files#r1029896501 hasn't been resolved yet, can you move the plugin declaration to the root?

Hi - what version? The spring.boot.version? We don't want the global version to be 3.0, right? we need 2.x for everything except this module. (or am I missing something?)

and the rest of the XML in that pom.xml is just a copy and paste of whatever is in your other pom.xml. I'm happy that this PR is contributing to the discussion around healthy refactorings to your build structure, but it's above my paygrade :) please do whatever you need to do to the build so that you're happy, I just want to see this thing merged so I can stop worrying about it. it's been many months, after all, since this discussion started.

yue9944882 · 2022-12-02T03:47:30Z

and the rest of the XML in that pom.xml is just a copy and paste of whatever is in your other pom.xml

i think @brendandburns is talking about the version of the jacoco plugin, but yeah there're some other pom.xml duplicating the plugin declaration. we can lift them to the parent pom through out the project in a follow-up.

yue9944882

/lgtm
/approve
/hold

ready to merge it, before that holding this pull a few days if @brendandburns has additional review comments

brendandburns · 2022-12-02T16:56:42Z

/lgtm
/approve

We can do the pom cleanup in a different PR.

@joshlong it would be great if we could get your example controller merged into the examples directory.

k8s-ci-robot · 2022-12-02T16:56:47Z

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: brendandburns, joshlong, yue9944882

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

~~OWNERS~~ [brendandburns,yue9944882]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

joshlong · 2022-12-05T07:12:02Z

I'll get the example controller cleaned up and send a separate PR.

It was a lot of fun working with y'all so far. Thanks for allowing me the opportunity to contribute, even if ever so slightly, to your much-appreciated efforts.

reset

2e57a31

k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Nov 22, 2022

k8s-ci-robot requested review from brendandburns and yue9944882 November 22, 2022 01:49

k8s-ci-robot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Nov 22, 2022

joshlong mentioned this pull request Nov 22, 2022

How to handle Spring + Java 8? #2452

Closed

yue9944882 reviewed Nov 22, 2022

View reviewed changes

joshlong added 3 commits November 22, 2022 03:15

gh

5ac6132

versions

e1685f4

apply spotless

294038b

brendandburns reviewed Nov 22, 2022

View reviewed changes

moving to Spring Boot 3 GA.

6f04a47

yue9944882 force-pushed the boot3-aot branch 2 times, most recently from b859f45 to 985765a Compare November 28, 2022 05:17

test-infra: merges github action test scripts and bumps jdk

3d710de

Signed-off-by: yue9944882 <291271447@qq.com>

yue9944882 force-pushed the boot3-aot branch from 985765a to 3d710de Compare November 28, 2022 05:37

yue9944882 reviewed Nov 28, 2022

View reviewed changes

yue9944882 mentioned this pull request Nov 28, 2022

[WIP] Github-action: Separate java17 test jobs #2459

Closed

yue9944882 reviewed Dec 2, 2022

View reviewed changes

k8s-ci-robot added the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 2, 2022

k8s-ci-robot assigned yue9944882 Dec 2, 2022

k8s-ci-robot added lgtm "Looks good to me", indicates that a PR is ready to be merged. approved Indicates a PR has been approved by an approver from all required OWNERS files. labels Dec 2, 2022

brendandburns removed the do-not-merge/hold Indicates that a PR should not merge because someone has issued a /hold command. label Dec 2, 2022

k8s-ci-robot assigned brendandburns Dec 2, 2022

k8s-ci-robot merged commit ce32410 into kubernetes-client:master Dec 2, 2022

This was referenced Dec 6, 2022

Update release.yml #2483

Closed

add support for Spring Framework 6 AOT engine #2402

Closed

brendandburns mentioned this pull request Dec 7, 2022

Switch to Java 17 for release action. #2487

Merged

yue9944882 mentioned this pull request Dec 12, 2022

Chore: Merge jacoco maven plugin declaration to parent pom #2493

Merged

This was referenced Apr 2, 2024

NullPointerException when compiled to Spring Native binary #3271

Open

Fix Spring AOT model relfection (#2457) #3276

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add a module for Spring Boot 3 AOT #2457

add a module for Spring Boot 3 AOT #2457

joshlong commented Nov 22, 2022

yue9944882 Nov 22, 2022

yue9944882 Nov 22, 2022

joshlong Nov 22, 2022

brendandburns Nov 22, 2022

yue9944882 Nov 22, 2022

joshlong Nov 22, 2022

yue9944882 left a comment

yue9944882 commented Nov 22, 2022

joshlong commented Nov 22, 2022 •

edited

Loading

joshlong commented Nov 22, 2022

brendandburns Nov 22, 2022

yue9944882 Nov 24, 2022

brendandburns Nov 22, 2022

yue9944882 Nov 24, 2022

brendandburns Nov 22, 2022

joshlong Nov 23, 2022 •

edited

Loading

yue9944882 Nov 24, 2022

brendandburns Nov 22, 2022

yue9944882 commented Nov 23, 2022

joshlong commented Nov 25, 2022

yue9944882 Nov 28, 2022

yue9944882 commented Nov 28, 2022

joshlong commented Nov 30, 2022

yue9944882 commented Dec 1, 2022

joshlong commented Dec 1, 2022

yue9944882 commented Dec 2, 2022

yue9944882 left a comment

brendandburns commented Dec 2, 2022

k8s-ci-robot commented Dec 2, 2022

joshlong commented Dec 5, 2022

add a module for Spring Boot 3 AOT #2457

add a module for Spring Boot 3 AOT #2457

Conversation

joshlong commented Nov 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yue9944882 left a comment

Choose a reason for hiding this comment

yue9944882 commented Nov 22, 2022

joshlong commented Nov 22, 2022 • edited Loading

joshlong commented Nov 22, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

joshlong Nov 23, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

yue9944882 commented Nov 23, 2022

joshlong commented Nov 25, 2022

Choose a reason for hiding this comment

yue9944882 commented Nov 28, 2022

joshlong commented Nov 30, 2022

yue9944882 commented Dec 1, 2022

joshlong commented Dec 1, 2022

yue9944882 commented Dec 2, 2022

yue9944882 left a comment

Choose a reason for hiding this comment

brendandburns commented Dec 2, 2022

k8s-ci-robot commented Dec 2, 2022

joshlong commented Dec 5, 2022

joshlong commented Nov 22, 2022 •

edited

Loading

joshlong Nov 23, 2022 •

edited

Loading