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

assemble broken for Spring Boot and devtools since 2.3.0 #171

Open
jeffmaury opened this issue Sep 19, 2018 · 13 comments
Open

assemble broken for Spring Boot and devtools since 2.3.0 #171

jeffmaury opened this issue Sep 19, 2018 · 13 comments

Comments

@jeffmaury
Copy link

The s2i assemble script is supposed to explode the Spring Boot fat JAR if Spring Boot devtools dependency is found. It is broken since 2.3.0 as unzip is not found anymore in the image.

@vorburger
Copy link
Collaborator

@jeffmaury I'm interested in helping with this (unless you're about to raise a PR with a fix?).

This, like e.g. #169, may have been an impact of @rhuss's #165, and in the move FROM jboss/base-jdk:8 to centos we lost unzip in the image? I can put together a PR to add that back in, but...

... ideally, I would like to use this as the opportunity to learn more about the feature behind this, where the s2i assemble script explodes the Spring Boot fat JAR if Spring Boot devtools dependency is found ... do we have doc about this? Interested, because just the other day @edewit and I were talking how something like https://github.com/GoogleContainerTools/skaffold-tools-for-java (see https://github.com/GoogleContainerTools/skaffold) for OpenShift would like like... but sounds like we may already have this? If I can learn more about it, we can make sure it doesn't break anymore here in the future.

@vorburger
Copy link
Collaborator

@jeffmaury are you having the problem with the centos or the rhel variant of the base image?

@vorburger
Copy link
Collaborator

vorburger commented Sep 19, 2018

Oh, just found https://github.com/fabric8io-images/s2i/tree/master/java/images/centos#spring-boot-automatic-restarts from #81 by @dhirajsb ... let me see if I can add an examples/spring-devtools (alongside my earlier maven, maven-wrapper, binary, gradle) which showcases, and integration tests this (for future non-regression).

@vorburger
Copy link
Collaborator

@jeffmaury (or @dhirajsb or anyone else familiar with this stuff .. and FYI @edewit) I've toyed with this a bit and come up with #173 BUT (a) I cannot yet reproduce anything broken due to a missing unzip; (b) I don't really fully understand yet how it's supposed to work anyway...

I could debug it further by looking at what #81 did in assemble, but it feels like I'm missing part of the picture here... perhaps yould could comment on my example's README how to get that to actually work (and reproduce your problem) ?

@rhuss
Copy link
Contributor

rhuss commented Sep 20, 2018

Not really remember how it works, just that the Spring Boot Devtools, which are required for hot-reload support, don't work on fat jar but only on the exploded one. 'would be interesting whether for new Spring Boot versions this is still the case.

@jeffmaury
Copy link
Author

So the assemble script works that way:

  1. checks if the fat jar contains spring-devtools
  2. if true unzip the fat jar
  3. then parse the MANIFEST.MF to get the Spring Boot application class name
  4. then launch Java with specific parameters to use either the Spring Boot JAR ou WAR launcher and giving the Spring Boot application class name as argument

so step 2 is broken as it uses unzip which is not anymore in the image. Why don't we use jar for that ?

@rhuss
Copy link
Contributor

rhuss commented Sep 20, 2018

so step 2 is broken as it uses unzip which is not anymore in the image. Why don't we use jar for that ?

good point, should have been catched in the review as the feature has been introduced.

@vorburger
Copy link
Collaborator

So the assemble script works that way:

@jeffmaury but why does my example, where the fat fat JAR does contain spring-devtools, not cause the unzip and reproduce this problem? A reproducer (example) would be a useful first step here IMHO.

@jeffmaury
Copy link
Author

I supposed your sample is correct but the script is wrong as unzip is not there and you need to check the logs to check for the exploded message

@vorburger vorburger mentioned this issue Nov 7, 2018
@vorburger
Copy link
Collaborator

so step 2 is broken as it uses unzip which is not anymore in the image. Why don't we use jar for that ?

That's an idea, and we could do this e.g. in https://github.com/OASIS-learn-study/swissarmyknife-minecraft-server/pull/14/files, but here in this project it's not just search & replace of "unzip" by "jar xvf", because of args (-l & -p) - and I wouldn't want to raise a "blind" PR, without being able to test it (but see above; help with a reproducer test would be welcome!).

Alternatively, therefore, and IMHO safer and better for anyone else this may affect, we should just fix the regression we introduced in the move FROM jboss/base-jdk:8 to centos were we lost unzip ? I'm proposing to do this in #185.

@vorburger
Copy link
Collaborator

@jeffmaury with the just merged #185 unzip is back in this image.

Are you able to test to confirm if this issue is fixed? Using master OK, or do you need a release to Docker Hub? Do you just need a release of the upstream CentOS based community image, or also of downstream RHEL based one?

@tandeday
Copy link

Using Spring Boot 2.1.3.RELEASE the code works, but the test is invalid as my fat jar does not include the dev tools jar. Skipping that test and just look for the main class would in my opinion be sufficient.

@vorburger
Copy link
Collaborator

@m86194 a Pull Request with (more) fixes for this, or a working test, would probably be welcome. (FYI I'm no longer involved with this project, and just un-watched this issue.)

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

No branches or pull requests

4 participants