Kubernetes: Provide properties to remote debugging

[Sgitario]

This PR allows to easily configure the remote debugging and describes how to do this in the documentation.

I've also tried to expose another route to map the java agent port, but as intellij couldn't handle this port from localhost (it threw a handshake error), I decided to not over-complicate this guide and simple do the port-forward bit (see my changes in documentation).

I only did these changes for OpenShift and Kubernetes, not Knative, but let me know if you think it could fit in Knative as well.

Fix #17581
Fix #23765

[Sgitario]

/cc @geoand @iocanel @edeandrea @cmoulliard

[geoand]

Nice idea!

[edeandrea]

Nice idea but I don't see how it fixes #23765 ? The DeploymentConfig that the openshift extension generates overrides the command and it shouldn't. I don't see how this PR addresses that?

[Sgitario]

Nice idea but I don't see how it fixes #23765 ? The DeploymentConfig that the openshift extension generates overrides the command and it shouldn't. I don't see how this PR addresses that?

If I understood correctly the issue you reported, what you wanted to do is to set the java agent configuration. This PR does this using the system property JAVA_TOOL_OPTIONS. See https://github.com/quarkusio/quarkus/pull/24104/files#diff-1788e793d1e03211ebdc536d004c3863b144c52fd594b8c62376b92961146dcfR1227

[edeandrea]

Will setting the JAVA_TOOL_OPTIONS environment variable have any effect if the command on the DeploymentConfig doesn't reference it? I suppose it will according to https://docs.oracle.com/en/java/javase/11/troubleshoot/environment-variables-and-system-properties.html#GUID-BE6E7B7F-A4BE-45C0-9078-AA8A66754B97

I still think the DeploymentConfig in my comment is wrong though and needs to be fixed. The command is

command:
  - java
  - -Dquarkus.http.host=0.0.0.0
  - -Djava.util.logging.manager=org.jboss.logmanager.LogManager
  - -jar
  - /deployments/quarkus-run.jar

Shouldn't that be fixed as well (maybe not in this PR)?

[Sgitario]

Will setting the JAVA_TOOL_OPTIONS environment variable have any effect if the command on the DeploymentConfig doesn't reference it? I suppose it will according to https://docs.oracle.com/en/java/javase/11/troubleshoot/environment-variables-and-system-properties.html#GUID-BE6E7B7F-A4BE-45C0-9078-AA8A66754B97

Yes, using the changes in this PR, users won't need to deal with how to provide the JVM arguments any longer. Internally, it will use the JAVA_TOOL_OPTIONS env variable which works fine to enable remote debugging.

About your comment, you mentioned several things:

• the imagePullPolicy doesn't seem to be respected. Indeed. You reported it in a separated issue and it will be fixed as part of this PR: OpenShift: Fix image pull policy property #24068
• wrong generated manifests when providing the JAVA_DEBUG env var:

Extracted from the comment:

./mvnw clean package -DskipTests \
  -Dquarkus.kubernetes.deployment-target=openshift \
  -Dquarkus.container-image.builder=openshift \
  -Dquarkus.openshift.resources.limits.memory=768Mi \
  -Dquarkus.openshift.resources.requests.memory=256Mi \
  -Dquarkus.openshift.env.vars.JAVA_DEBUG=true

and then inspect the resulting target/kubernetes/openshift.yml I see that the app's DeploymentConfig looks like the following and the command is overridden.

In this scenario, it is almost impossible to turn on remote debug unless I then go in and manually tweak the resulting DeploymentConfig. The DeploymentConfig already references the image (which is already set up for remote debug), so why the need to also specify a command?

The command is auto-generated by the OpenShift extension and you would only specify one at your own risk.
In order to provide JVM arguments, as you did in previous comments, you should use quarkus.openshift.jvm-arguments, what seems wrong to me is that the Quarkus extension is splitting the arguments by comma. I've opened a separate ticket for fixing this: #24136

[edeandrea]

The command is auto-generated by the OpenShift extension and you would only specify one at your own risk.

But why does the DeploymentConfig generate a command section in the first place? To me that seems wrong. Why does the DeploymentConfig contain a command? It doesn't need to since the image itself contains a CMD/ENTRYPOINT.

If I were to docker run the image I wouldn't need to specify a command to run. It would just run the image.

[Sgitario]

The command is auto-generated by the OpenShift extension and you would only specify one at your own risk.

But why does the DeploymentConfig generate a command section in the first place? To me that seems wrong. Why does the DeploymentConfig contain a command? It doesn't need to since the image itself contains a CMD/ENTRYPOINT.

If I were to docker run the image I wouldn't need to specify a command to run. It would just run the image.

I guess this decision was made to be able to use as many images as possible since some images might have a not Quarkus-compatible command, but I will add @iocanel to the thread, so he can clarify this point.

[iocanel]

The command is auto-generated by the OpenShift extension and you would only specify one at your own risk.

But why does the DeploymentConfig generate a command section in the first place? To me that seems wrong. Why does the DeploymentConfig contain a command? It doesn't need to since the image itself contains a CMD/ENTRYPOINT.
If I were to docker run the image I wouldn't need to specify a command to run. It would just run the image.

I guess this decision was made to be able to use as many images as possible since some images might have a not Quarkus-compatible command, but I will add @iocanel to the thread, so he can clarify this point.

@Sgitario @edeandrea: I don't recall what was the initial idea. I do recall though that in other places we assume that it's the images responsibility to set the command unless the user explicitly specifies one. Eitherway, I think that at some point we can change that.

[iocanel]

I am pretty sure this will need to be rebased.

[iocanel]

The PR looks, good to me. But I think that it needs rebase.

[edeandrea]

I do recall though that in other places we assume that it's the images responsibility to set the command unless the user explicitly specifies one.

I would completely agree with that @iocanel. In most cases, if I'm running some image I would assume that the command to run it is already baked into the image.