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

Set JAVA_VERSION and/or JAVA_HOME so that skaffold debug works #33

Closed
dsyer opened this issue Oct 23, 2020 · 14 comments
Closed

Set JAVA_VERSION and/or JAVA_HOME so that skaffold debug works #33

dsyer opened this issue Oct 23, 2020 · 14 comments

Comments

@dsyer
Copy link

dsyer commented Oct 23, 2020

Skaffold has a neat debug feature, but it doesn't work with buildpack images because it tries to detect a Java application and fails. It looks for environment variables and also the entrypoint of the container (but that doesn't show any sign of being Java related in a buildpack image): https://github.com/GoogleContainerTools/skaffold/blob/master/pkg/skaffold/debug/transform_jvm.go#L39-L53.

@dsyer
Copy link
Author

dsyer commented Oct 23, 2020

BTW, I'm pretty sure this used to work in the summer. I was quite surprised it didn't work out of the box today because I've used it before in workshops (e.g. at Spring One).

@nebhale nebhale closed this as completed Oct 23, 2020
@nebhale nebhale reopened this Oct 23, 2020
@nebhale
Copy link
Member

nebhale commented Oct 23, 2020

@dsyer Given our discussion earlier, is this still an open issue for you?

@dsyer
Copy link
Author

dsyer commented Oct 24, 2020

Yes. This is somewhat unrelated. I still have to set JAVA_VERSION manually in my podspec to get the debugger to work.

@mydogspies
Copy link

Just as a heads up to you guys. Built an image last night with current buildpack and Spring version 2.3.5/java 11 and the JAVA_HOME is still not set which is a bit of a pain when one needs to execute some standard tools on container startup.

@dmikusa
Copy link
Contributor

dmikusa commented Sep 28, 2021

@dsyer @mydogspies - Hi, is this something that you're still having issues with?

The Java buildpack should be setting JAVA_HOME. The JAVA_VERSION isn't a variable that we set, but you could set it with the environment-variables buildpack.

Please let me know if it's still presenting issues. Thanks!

@dsyer
Copy link
Author

dsyer commented Sep 28, 2021

I see JAVA_HOME but no JAVA_VERSION or JAVA_TOOL_OPTIONS (which are the two things skaffold is looking for). So it's still an issue.

@dmikusa
Copy link
Contributor

dmikusa commented Sep 28, 2021

JAVA_TOOL_OPTIONS should definitely be set. If nothing else, the memory calculator will add memory settings this way. That would happen dynamically at runtime though, so it wouldn't be set until right before the Java process starts.

How does skaffold detect if an env variable is set? What is it checking & when?

@dsyer
Copy link
Author

dsyer commented Sep 28, 2021

JAVA_TOOL_OPTIONS is not set if I docker run -ti ... /bin/bash and look at the environment. Is that the wrong way to test? There's a link to the source code above.

func (t jdwpTransformer) IsApplicable(config ImageConfiguration) bool {
	if _, found := config.Env["JAVA_TOOL_OPTIONS"]; found {
		return true
	}
	if _, found := config.Env["JAVA_VERSION"]; found {
		return true
	}
	if len(config.Entrypoint) > 0 && !isEntrypointLauncher(config.Entrypoint) {
		return config.Entrypoint[0] == "java" || strings.HasSuffix(config.Entrypoint[0], "/java")
	}
	return len(config.Arguments) > 0 &&
		(config.Arguments[0] == "java" || strings.HasSuffix(config.Arguments[0], "/java"))
}

UPDATE: I was wrong. My container is crashing so I never get to see JAVA_TOOL_OPTIONS.

@dmikusa
Copy link
Contributor

dmikusa commented Sep 28, 2021

JAVA_TOOL_OPTIONS is not set if I docker run -ti ... /bin/bash and look at the environment. Is that the wrong way to test?

You have to invoke the launcher if you want to test like this. The launcher is what sets up the environment at runtime.

https://buildpacks.io/docs/app-developer-guide/run-an-app/#user-provided-shell-process

docker run --rm --entrypoint launcher -it multi-process-app bash

I took a look at that link, from what I see in the comments it's pulling the env from the OCI image. That would make sense why it's not finding it. That buildpack images don't have that set on the image.

...
            "Env": [
                "PATH=/cnb/process:/cnb/lifecycle:/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin",
                "CNB_LAYERS_DIR=/layers",
                "CNB_APP_DIR=/workspace",
                "CNB_PLATFORM_API=0.6",
                "CNB_DEPRECATION_MODE=quiet"
            ],
...

The env variables are set by the launcher when the image is run. I don't think there's anything we can do as buildpack authors to change that behavior. It's just how buildpacks work. In addition, buildpacks doesn't set any arguments so the last check they are doing isn't going to work either.

I think we might be able to make the entry point check pass though. It's looking for an entry point with java or ending with /java. Here's an example from CNB image:

            "Entrypoint": [
                "/cnb/process/web"
            ],

A buildpack is going to have an entry point set to the process type that should run. We can control the process type name, so we could have one that is /cnb/process/java, which it seems like would match.

The easiest way to try this would be to add a Procfile to the root of your JAR, where the Procfile contains java: java org.springframework.boot.loader.JarLauncher. Then pack build .. --default-process=java. As the build runs, you should see the Procfile buildpack run and contribute an extra process type called java. We'll set this as the default process type, so it should appear on the image.

Ex:

> echo 'java: java org.springframework.boot.loader.JarLauncher' > Procfile
> jar uf build/libs/demo-0.0.1-SNAPSHOT.jar Procfile
> pb gradle -p build/libs/demo-0.0.1-SNAPSHOT.jar --default-process java

docker inspect produces...

            "Entrypoint": [
                "/cnb/process/java"
            ],

If that works, we can explore making that easier. The executable-jar CNB adds some process types, just not one called java. We could easily have it add one called java. The trick would be that the default, for spec reasons should be web. That would mean a user wanting to take advantage of this skaffold functionality would need to include --default-process java in their pack build command.

There are possibly other ways we could trigger setting java as the default process type, like we could use an env variable like BP_SKAFFOLD_ENABLED=true but that's actually more typing, so I'm not sure it's better. If there are any env variables that skaffold sets automatically or any files it rights or config changes it makes, something that we could trigger off, we could use that too. I'm just not sure what it's setting when it runs.

@dsyer
Copy link
Author

dsyer commented Sep 29, 2021

It wouldn't work if you use Spring Boot to build the image (spring-projects/spring-boot#6626 (comment)). I guess if you use pack you can do it. Is there an environment variable for --default-process?

@dmikusa
Copy link
Contributor

dmikusa commented Sep 29, 2021

OK, I was hoping we could use Profile to just test this and see if it works when we add that additional entrypoint. Since that doesn't look like it'll be an option, I can hack together a test version of executable-jar that adds in the additional entrypoint. Give me a little time and I'll get you an image you can try. I just want to make sure skaffold does what we expect it to when the entrypoint has the name java.

Is there an environment variable for --default-process?

No, not that I've seen. That is a pack-specific cli argument. It doesn't look like the Boot integration has a similar option. That could be a problem to making this work as well.

@dsyer
Copy link
Author

dsyer commented Sep 29, 2021

If you just need a proof of concept I can try and craft something with pack. Eventually it would be good to make it work with Spring Boot.

@dsyer
Copy link
Author

dsyer commented Sep 29, 2021

It's funny, I can't find a way to make skaffold fail to debug now. So that PoC is redundant. I think maybe the CNB features of skaffold have improved and it recognizes the Java container that way. We should close this.

@dmikusa
Copy link
Contributor

dmikusa commented Sep 29, 2021

Well, that's good news! Thanks for the update. If you see weirdness in the future, just let us know.

@dmikusa dmikusa closed this as completed Sep 29, 2021
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