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

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

Closed
4 of 23 tasks
slemeur opened this issue Jul 19, 2019 · 28 comments
Closed
4 of 23 tasks
Assignees
Labels
area/languages Issues related to Language extensions or plugins integration. kind/bug Outline of a bug - must adhere to the bug report template. status/info-needed More information is needed before the issue can move into the “analyzing” state for engineering.

Comments

@slemeur
Copy link
Contributor

slemeur commented Jul 19, 2019

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

  • 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

  • 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

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

Environment

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

Additional context

@slemeur slemeur added the kind/bug Outline of a bug - must adhere to the bug report template. label Jul 19, 2019
@rhopp rhopp added area/languages Issues related to Language extensions or plugins integration. status/analyzing An issue has been proposed and it is currently being analyzed for effort and implementation approach labels Jul 19, 2019
@rhopp
Copy link
Contributor

rhopp commented Jul 19, 2019

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

@svor
Copy link
Contributor

svor commented Jul 22, 2019

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
Copy link
Contributor

l0rd commented Jul 22, 2019

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

@tsmaeder
Copy link
Contributor

@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
Copy link
Contributor

svor commented Jul 22, 2019

@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
Copy link
Contributor

svor commented Jul 22, 2019

Here is the similar problem: takari/maven-wrapper#128

@tsmaeder
Copy link
Contributor

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

@amisevsk
Copy link
Contributor

@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 eclipse-che/che-devfile-registry#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
Copy link
Contributor

@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
Copy link
Contributor

@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
Copy link
Contributor

tsmaeder commented Jul 23, 2019

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
Copy link
Contributor

@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 Update dockerfile patch and make new java devfiles used patched images che-devfile-registry#42 we are only using the community maven image (and no longer using mvnw).

@tsmaeder
Copy link
Contributor

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

@tsmaeder tsmaeder added status/info-needed More information is needed before the issue can move into the “analyzing” state for engineering. and removed status/analyzing An issue has been proposed and it is currently being analyzed for effort and implementation approach labels Sep 3, 2019
@tsmaeder
Copy link
Contributor

tsmaeder commented Sep 3, 2019

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

@svor
Copy link
Contributor

svor commented Nov 8, 2019

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
Copy link
Contributor

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

@slemeur
Copy link
Contributor Author

slemeur commented Nov 11, 2019

@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
Copy link
Contributor Author

slemeur commented Nov 11, 2019

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

@svor
Copy link
Contributor

svor commented Nov 11, 2019

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
Copy link
Contributor

amisevsk commented Nov 12, 2019

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
Copy link
Contributor

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
Copy link
Contributor

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?

@tsmaeder tsmaeder mentioned this issue Nov 25, 2019
26 tasks
@amisevsk
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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

@tsmaeder
Copy link
Contributor

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

@tsmaeder
Copy link
Contributor

carlossg/docker-maven#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
Copy link
Contributor

svor commented Dec 4, 2019

@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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/languages Issues related to Language extensions or plugins integration. kind/bug Outline of a bug - must adhere to the bug report template. status/info-needed More information is needed before the issue can move into the “analyzing” state for engineering.
Projects
None yet
Development

No branches or pull requests

6 participants