add support for Spring Framework 6 AOT engine #2402

joshlong · 2022-10-01T02:15:38Z

Hi

Please find a proof-of-concept introduction of AOT support in Spring Boot 3, per #2398 .

I've tested the proposed changes and they work with both Spring Boot 3 and Spring Boot 2.

Spring Boot 3 is not yet GA, however. So, i've had to add snapshot and milestone repositories. Obviously, this won't do in a library like the Kubernetes Java client which needs to be releasable all the time. I'm not sure how you want to handle this. Maybe we create a separate module that doesn't get released and that has the appropriate repositories? Maybe we just stash this PR until November 24th, when Spring Boot 3 is due to go GA? (maybe a little later than that, who knows...)

Also, for my contribution to work, I need to add a dependency on an external library called Reflections. This is used to capture all the code-generated types for CRDs, for example, in the Kubernetes API and in the user's auto-configured packages in a Spring Boot application. This library is only used at compile time, and only when the user is using Spring Boot 3 AOT compilation. that's not the default. I don't know how to handle that, either. I suppose you could make the package optional and then make sure that the interested developer who wnats to use AOT processing gets a warning or something when they try it without that class on the classpath? I don't like that approach very much. It'd be nice if the library was just there and it worked for anybody who wants to build an AOT/ GraalVM native image, out-of-the-box.

Is there a Maven scope i don't know about that limits the lifetime of the binary explicitly to the time the AOT engine runs?

I tried this new support out on a sample Kubernetes CRD+Controller demo and it works just fine, better than what I was able to do before with Spring Native, actually. From ac consumer's perspective, I didn't have to do anything to make code from the Kubernetes Java client do its magic. I was able to delete code, even.

thanks for your time and consideration

…lVM native images.

linux-foundation-easycla · 2022-10-01T02:15:41Z

The committers listed above are authorized under a signed CLA.

✅ login: joshlong / name: Josh Long (6f54d63)

k8s-ci-robot · 2022-10-01T02:15:46Z

Welcome @joshlong!

It looks like this is your first PR to kubernetes-client/java 🎉. Please refer to our pull request process documentation to help your PR have a smooth ride to approval.

You will be prompted by a bot to use commands during the review process. Do not be afraid to follow the prompts! It is okay to experiment. Here is the bot commands documentation.

You can also check if kubernetes-client/java has its own contribution guidelines.

You may want to refer to our testing guide if you run into trouble with your tests not passing.

If you are having difficulty getting your pull request seen, please follow the recommended escalation practices. Also, for tips and tricks in the contribution process you may want to read the Kubernetes contributor cheat sheet. We want to make sure your contribution gets all the attention it needs!

Thank you, and welcome to Kubernetes. 😃

k8s-ci-robot · 2022-10-01T02:15:49Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: joshlong
Once this PR has been reviewed and has the lgtm label, please assign brendandburns for approval by writing /assign @brendandburns in a comment. For more information see:The Kubernetes Code Review Process.

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

Needs approval from an approver in each of these files:

OWNERS

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

brendandburns · 2022-10-02T00:31:56Z

pom.xml

@@ -482,49 +487,49 @@
              <include>src/test/groovy/**/*.groovy</include>
            </includes>

-            <importOrder>  <!-- or a custom ordering -->


Please remove these whitespace changes.

i believe this has now been addressed.

brendandburns · 2022-10-02T00:31:56Z

pom.xml

@@ -482,49 +487,49 @@
              <include>src/test/groovy/**/*.groovy</include>
            </includes>

-            <importOrder>  <!-- or a custom ordering -->


Please remove these whitespace changes.

i believe this has now been addressed.

brendandburns · 2022-10-02T00:32:29Z

...t/spring/extended/controller/config/aot/KubernetesBeanFactoryInitializationAotProcessor.java

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


Is there a reason why there are empty comments after each of these lines?

brendandburns · 2022-10-02T00:36:10Z

I took a look at the code changes and other than the whitespace changes which need to be reverted, this looks fine to me.

I will let @yue9944882 take a look at the Spring side and comment on how we want to sequence this into the released client.

joshlong · 2022-10-02T20:40:03Z

Hi, thanks for the quick replies. somehow, the biggest change of my original PR - the fact that it needs a snapshot or milestone (non-Maven central) repository was not in the original pom.xml. Don't ask me how. So I took the code on main, applied it over mine, re-applied the changes without accidentally invoking my IDE's code reformatter, and here we are.

I've updated the PR

brendandburns · 2022-10-03T18:14:40Z

We already have a spring component (spring-boot-actuator) marked as optional https://github.com/kubernetes-client/java/blob/master/pom.xml#L177 so I think that for the reflections dependency, it makes sense mark it optional also, although I don't feel super strongly about this, but we've definitely gotten feedback about not wanting this library to pull in lots and lots of dependencies.

Regarding the milestone dependency, you're definitely right that we don't want to pull this in until it is released. So it feels like we should leave this PR open until it is released. There's not too much overlap w/ this PR so it shouldn't be too hard to keep it rebased.

joshlong · 2022-10-03T22:37:31Z

Awesome! Both of those points - delaying until Boot 3 GA and marking Reflections as optional - are reasonable and I appreciate them. I'll follow up sometime after Spring Boot 3 is GA.

Maybe we could do something where, if you try to run the code and do an AOT compilation, it can check that reflections is on the classpath and if it isn't then warn the user to add it to their classpath? That shouldn't be too hard. Spring has a boolean ClassUtils.isPresent (String className, ClassLoader) we can put the warning behind. We'd have to update that one class in my PR.

I'm pretty pleased how far this has come in just a week. Thanks for a great project, and I appreciate the fast turnarounds. Hopefully, people interested in doing so will soon (before 2023, maybe?) be able to build Kubernetes clients and controllers that take 50mb of RAM and startup In no time at all soon enough.

brendandburns · 2022-10-04T22:33:04Z

Adding that warning seems totally fine to me.

Thanks

yue9944882 · 2022-10-10T08:11:41Z

Error: COMPILATION ERROR :
Error: /home/runner/work/java/java/spring/src/main/java/io/kubernetes/client/spring/extended/controller/config/KubernetesInformerProperties.java:[17,51] cannot access org.springframework.boot.context.properties.ConfigurationProperties
bad class file: /home/runner/.m2/repository/org/springframework/boot/spring-boot/3.0.0-M5/spring-boot-3.0.0-M5.jar(org/springframework/boot/context/properties/ConfigurationProperties.class)
class file has wrong version 61.0, should be 52.0

@joshlong the idea SGTM, but i noticed the error messages above in the build log. is spring-boot 3 still compatible with java 8? there's still a distinct portion of users actively using java 8, so the compatibility should be preserved as well.

k8s-ci-robot · 2022-10-22T06:50:44Z

@joshlong: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

joshlong · 2022-10-30T12:56:23Z

Hi - no, spring boot 3 requires a Java 17 baseline. If we're not willing to insist on java 17 here, could we extract it into a standalone .jar so that people using the Spring Boot starter in boot 2 and 3 get the same jar but those wishing to leverage the new GraalVM AOT support get the new spring boot 3-specific code? That's less than ideal, tho :) we're all gonna have to cut over to 17 eventually

brendandburns · 2022-11-17T16:52:53Z

I filed an issue so we can discuss how we want to handle Spring 6 and it's dependency on Java 17

#2452

yue9944882 · 2022-12-06T04:27:16Z

/close

closing it b/c #2457 was merged

k8s-ci-robot · 2022-12-06T04:27:19Z

@yue9944882: Closed this PR.

In response to this:

/close

closing it b/c #2457 was merged

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

OlgaMaciaszek · 2022-12-06T22:16:45Z

Hi @brendandburns, when is a release containing these changes scheduled for?

add support for Spring Framework 6 AOT engine, which can produce Graa…

6f54d63

…lVM native images.

k8s-ci-robot added the cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. label Oct 1, 2022

k8s-ci-robot requested review from brendandburns and yue9944882 October 1, 2022 02:15

k8s-ci-robot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. and removed cncf-cla: no Indicates the PR's author has not signed the CNCF CLA. labels Oct 1, 2022

brendandburns reviewed Oct 2, 2022

View reviewed changes

joshlong added 3 commits October 2, 2022 13:19

polishing the AOT support

b8eb8f0

polishing the AOT support

41aa204

polish

7284a24

k8s-ci-robot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Oct 22, 2022

k8s-ci-robot closed this Dec 6, 2022

ryanjbaxter mentioned this pull request Dec 6, 2022

Bump version to 16.0.2 spring-cloud/spring-cloud-kubernetes#1163

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

add support for Spring Framework 6 AOT engine #2402

add support for Spring Framework 6 AOT engine #2402

joshlong commented Oct 1, 2022

linux-foundation-easycla bot commented Oct 1, 2022 •

edited

Loading

k8s-ci-robot commented Oct 1, 2022

k8s-ci-robot commented Oct 1, 2022

brendandburns Oct 2, 2022

joshlong Oct 2, 2022

brendandburns Oct 2, 2022

joshlong Oct 2, 2022

brendandburns Oct 2, 2022

brendandburns commented Oct 2, 2022

joshlong commented Oct 2, 2022

brendandburns commented Oct 3, 2022

joshlong commented Oct 3, 2022

brendandburns commented Oct 4, 2022

yue9944882 commented Oct 10, 2022

k8s-ci-robot commented Oct 22, 2022

joshlong commented Oct 30, 2022 •

edited

Loading

brendandburns commented Nov 17, 2022

yue9944882 commented Dec 6, 2022

k8s-ci-robot commented Dec 6, 2022

OlgaMaciaszek commented Dec 6, 2022

add support for Spring Framework 6 AOT engine #2402

add support for Spring Framework 6 AOT engine #2402

Conversation

joshlong commented Oct 1, 2022

linux-foundation-easycla bot commented Oct 1, 2022 • edited Loading

k8s-ci-robot commented Oct 1, 2022

k8s-ci-robot commented Oct 1, 2022

brendandburns Oct 2, 2022

Choose a reason for hiding this comment

joshlong Oct 2, 2022

Choose a reason for hiding this comment

brendandburns Oct 2, 2022

Choose a reason for hiding this comment

joshlong Oct 2, 2022

Choose a reason for hiding this comment

brendandburns Oct 2, 2022

Choose a reason for hiding this comment

brendandburns commented Oct 2, 2022

joshlong commented Oct 2, 2022

brendandburns commented Oct 3, 2022

joshlong commented Oct 3, 2022

brendandburns commented Oct 4, 2022

yue9944882 commented Oct 10, 2022

k8s-ci-robot commented Oct 22, 2022

joshlong commented Oct 30, 2022 • edited Loading

brendandburns commented Nov 17, 2022

yue9944882 commented Dec 6, 2022

k8s-ci-robot commented Dec 6, 2022

OlgaMaciaszek commented Dec 6, 2022

linux-foundation-easycla bot commented Oct 1, 2022 •

edited

Loading

joshlong commented Oct 30, 2022 •

edited

Loading