Che: Importing spring/pet-clinic in the default java-maven stack does not behave properly

[slemeur]

Describe the bug

Importing spring/pet-clinic in the default java-maven stack does not behave properly
It is not possible to build the project properly, it is not possible to run the project.

The error reported is:

[ERROR] Unknown lifecycle phase "/home/user/.m2". You must specify a valid lifecycle phase or a goal in the format <plugin-prefix>:<goal> or <plugin-group-id>:<plugin-artifact-id>[:<plugin-version>]:<goal>. Available lifecycle phases are: validate, initialize, generate-sources, process-sources, generate-resources, process-resources, compile, process-classes, generate-test-sources, process-test-sources, generate-test-resources, process-test-resources, test-compile, process-test-classes, test, prepare-package, package, pre-integration-test, integration-test, post-integration-test, verify, install, deploy, pre-clean, clean, post-clean, pre-site, site, post-site, site-deploy. -> [Help 1]

Che version

• [x] latest
• [ ] nightly
• [ ] other: please specify

Steps to reproduce

1. Create a workspace from the java-maven stack
2. Open a terminal and do a git clone of https://github.com/spring-projects/spring-petclinic
3. Stil from the terminal, try to run the commands provided in the readme of the project.

Expected behavior

It builds, it runs

Runtime

• [x] kubernetes (include output of kubectl version)
• [ ] Openshift (include output of oc version)
• [ ] minikube (include output of minikube version and kubectl version)
• [ ] minishift (include output of minishift version and oc version)
• [ ] docker-desktop + K8S (include output of docker version and kubectl version)
• [ ] other: (please specify)

Screenshots

Installation method

• [x] chectl
• [ ] che-operator
• [ ] minishift-addon
• [ ] I don't know

Environment

• [ ] my computer
  
  
  
  • [ ] Windows
  
  
  • [ ] Linux
  
  
  • [x] macOS
  
  
• [ ] Cloud
  
  
  
  • [ ] Amazon
  
  
  • [ ] Azure
  
  
  • [ ] GCE
  
  
  • [ ] other (please specify)
  
  
• [ ] other: please specify

Additional context

[rhopp]

@tsmaeder Could your team take a look on this for initial investigation/estimation/triage?

[svor]

The problem is that maven-wrapper which provides executed mvnw and maven docker image use the same name of environment variable - MAVEN_CONFIG. This variable is set in devfile with value /home/user/.m2 and this value isn't valid for mvnw.

@l0rd @amisevsk @tsmaeder do we really need MAVEN_CONFIG in our devfiles?

[l0rd]

@svor it was necessary when we were not patching the images. It may be useless now but we need to check.

[tsmaeder]

@svor what do you mean "they use the same environment variable"? Are you saying that the mvnw script is picking up the value from the env var or that it is overriding the variable? In what way is this causing the build to fail?

[svor]

@tsmaeder mvnw is picking up the value:

exec "$JAVACMD" \
  $MAVEN_OPTS \
  -classpath "$MAVEN_PROJECTBASEDIR/.mvn/wrapper/maven-wrapper.jar" \
  "-Dmaven.home=${M2_HOME}" "-Dmaven.multiModuleProjectDirectory=${MAVEN_PROJECTBASEDIR}" \
  ${WRAPPER_LAUNCHER} $MAVEN_CONFIG "$@"

And the value which was set in devfile (/home/user/.m2) isn't valid for this executable command

[svor]

Here is the similar problem: https://github.com/takari/maven-wrapper/issues/128

[tsmaeder]

Seems to be well known issue: https://issues.jenkins-ci.org/browse/JENKINS-47890

[amisevsk]

@svor is correct on the underlying issue; the problem is that the Spring sample project we use is incompatible with e.g. the community maven image (which has MAVEN_CONFIG set whether or not we set it outselves ($MAVEN_CONFIG=/root.m2)

Should be fixed in https://github.com/eclipse/che-devfile-registry/pull/42, which moves away from using mvn-wrapper in default commands. This PR also moves away from our che-stacks-based image to use the patched community image for this devfile.

[tsmaeder]

@amisevsk I was under the impression that setting MAVEN_CONFIG is a workaround for not having a proper home directory. But as I understand it, we patch the image to have a home dir. Why not just remove the MAVEN_CONFIG declaration?

[amisevsk]

@tsmaeder Because the way the community maven image is set up is to use MAVEN_CONFIG, with a default to /root/.m2 [1]. I'll try again, as perhaps in our overriding of the entrypoint this is avoided, but from what I've seen it is necessary.

[1] - https://github.com/carlossg/docker-maven#packaging-a-local-repository-with-the-image

[tsmaeder]

I've seen that we moved away from the maven comunity image and use a centos one in some instances. What's the right way to go, @amisevsk ? Because even if we don't use mvnw in our commands, folks are going to use that from the command line. It's what they are used to.

[amisevsk]

@tsmaeder To be honest, I'm not sure there is a good solution; mvn-wrapper is incompatible with the community maven image. Even outside the context of Che, trying to build the spring boot sample we're using should fail in the maven image. From the context of "any image for a workspace"; even without arbitrary UID issues this doesn't work.

As for Che, there are a few things

• I'm testing removal of $MAVEN_CONFIG, and it seems to not cause issues; I'll open a PR to remove the env var (this does nothing for the problem we're discussing, though)
• We can include unset $MAVEN_CONFIG in the patch, but I've thus far avoided image-specific patches
• We can find a new base maven image to use; I've thus far been avoiding swapping underlying images for devfiles, since we've had various subtle issues in the past (case in point, here). As of PR https://github.com/eclipse/che-devfile-registry/pull/42 we are only using the community maven image (and no longer using mvnw).

[tsmaeder]

So unless we want to move away from the mvn community image, this is not fixable, really. @slemeur

[tsmaeder]

@l0rd using community images is a "strategic" goal. Not sure what we want to do with this one.

[svor]

This problem is also actual if we want to use vscode-quarkus extension. To run a default example it generates a command which starts with mvnw like ./mvnw quarkus:dev

[tsmaeder]

@lord @slemeur we need a decision here: either we create our own maven image, or we cannot use quarkus and springboot examples.

[slemeur]

@svor : Even if the maven community stack we are using is not compatible by default, it seems the user should be able to figure out how to reconfigure the workspace so that it changes the $MAVEN_CONFIG. What would be the possible flows?

[slemeur]

@svor : Would using a different image for a Quarkus stack be a solution for that?

[svor]

What would be the possible flows?

@slemeur To do that we can unset $MAVEN_CONFIG in the devfile, like this: https://github.com/eclipse/che-devfile-registry/blob/sv/quarkus/devfiles/quarkus/devfile.yaml#L28
I've tested this flow and looks like it doesn't cause issues

[amisevsk]

It's kind of a strange issue, and I don't know the best approach in fixing it; the reason mvnw is used by quickstarts is (if I understand correctly) to simplify dev dependencies setup when building the code. One of the advantages we tout with Che is that you use a container with the exact dependencies you need, so mvnw shouldn't be necessary -- the image should just be able to do it directly.

From the README for maven-wrapper:

The Maven Wrapper is an easy way to ensure a user of your Maven build has everything necessary to run your Maven build.

This should not be a concern in Che.

On the other hand, mvnw might be useful for a general image like we currently use, where we don't ensure a specific version of maven is installed. In this case, I don't see a better way of fixing this than unsetting the MAVEN_CONFIG env var in the devfile.

[tsmaeder]

MAVEN_CONFIG seems to be used in the entry point of the maven image (https://github.com/carlossg/docker-maven#packaging-a-local-repository-with-the-image). Can we be sure that setting the env var to the empty string is not going to break that support?

[tsmaeder]

Also, doing this from a devfile will not fix the problem when folks are importing their own projects using .mvnw.
No, this needs to be fixed on the image level. Can we override the entrypoint to unset the env var?

[amisevsk]

Can we override the entrypoint to unset the env var?

We sure could, but I don't know where. We don't maintain any custom images except for the arbitrary UID patch (which is image-agnostic) -- if we open that to image-specific patches, we're left with the Che 6 scenario of maintaining Dockerfiles for images we use.

[tsmaeder]

we're left with the Che 6 scenario of maintaining Dockerfiles for images we use

Well, the dockerfile would be specific to the example, not the tooling. It's "bring your own dev image" anyway.

[tsmaeder]

Let's do the "unset env var in devfile" fix.

[tsmaeder]

Also, we should file an issue against the maven image.

[tsmaeder]

https://github.com/carlossg/docker-maven/issues/77 seems to indicate the issue has been fixed a long time ago. @svor can you find out if/why that does not help us?

[svor]

@tsmaeder They do UNSET https://github.com/carlossg/docker-maven/blob/master/jdk-8/mvn-entrypoint.sh#L45, but this code executes as ENTRYPOINT which we override for our dev-container's image