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

Explicitly set java version as a query parameter in the java-quarkus zip urls #366

Merged
merged 1 commit into from
Apr 29, 2024

Conversation

vinokurig
Copy link
Contributor

What does this PR do?:

Explicitly set java version in the java-quarkus zip urls to java 17, in order to fix the error from the init-compile command:

INFO] Recompiling the module because of changed dependency.
[INFO] Compiling 2 source files with javac [debug release 21] to target/classes
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  15.345 s
[INFO] Finished at: 2024-04-12T10:05:13Z
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-compiler-plugin:3.12.1:compile (default-compile) on project code-with-quarkus: Fatal error compiling: error: release version 21 not supported -> [Help 1]

Which issue(s) this PR fixes:

N/A

PR acceptance criteria:

  • Contributing guide
    Have you read the devfile registry contributing guide and followed its instructions?
  • Test automation
    Does this repository's tests pass with your changes?
  • Documentation
    Does any documentation need to be updated with your changes?
  • Check Tools Provider
    Have you tested the changes with existing tools, i.e. Odo, Che, Console? (See devfile registry contributing guide on how to test changes)

How to test changes / Special notes to the reviewer:

Copy link

openshift-ci bot commented Apr 15, 2024

Hi @vinokurig. Thanks for your PR.

I'm waiting for a devfile member to verify that this patch is reasonable to test. If it is, they should reply with /ok-to-test on its own line. Until that is done, I will not automatically test new commits in this PR, but the usual testing commands by org members will still work. Regular contributors should join the org to skip this step.

Once the patch is verified, the new status will be reflected by the ok-to-test label.

I understand the commands that are listed here.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

Copy link
Collaborator

@maxandersen maxandersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 if java 17 is the default for the images.

Is there a reason not using 21 by default here?

@vinokurig
Copy link
Contributor Author

Is there a reason not using 21 by default here?

The init-compile command fails with java 21. The error is in the description.

Copy link
Collaborator

@ibuziuk ibuziuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unrelated to java version
@maxandersen do we need -Djava.util.logging.manager=org.jboss.logmanager.LogManager arg in the commands?

ould not load Logmanager "org.jboss.logmanager.LogManager"
java.lang.ClassNotFoundException: org.jboss.logmanager.LogManager
        at org.codehaus.plexus.classworlds.strategy.SelfFirstStrategy.loadClass(SelfFirstStrategy.java:50)
        at org.codehaus.plexus.classworlds.realm.ClassRealm.unsynchronizedLoadClass(ClassRealm.java:271)
        at org.codehaus.plexus.classworlds.realm.ClassRealm.loadClass(ClassRealm.java:247)
        at org.codehaus.plexus.classworlds.realm.ClassRealm.loadClass(ClassRealm.java:239)
        at java.logging/java.util.logging.LogManager$1.run(LogManager.java:239)
        at java.logging/java.util.logging.LogManager$1.run(LogManager.java:223)
        at java.base/java.security.AccessController.doPrivileged(AccessController.java:318)
        at java.logging/java.util.logging.LogManager.<clinit>(LogManager.java:222)
        at java.logging/java.util.logging.Logger.demandLogger(Logger.java:649)
        at java.logging/java.util.logging.Logger.getLogger(Logger.java:718)
        at java.logging/java.util.logging.Logger.getLogger(Logger.java:702)
        at com.google.inject.internal.util.Stopwatch.<clinit>(Stopwatch.java:27)
        at com.google.inject.internal.InternalInjectorCreator.<init>(InternalInjectorCreator.java:62)
        at com.google.inject.Guice.createInjector(Guice.java:87)
        at com.google.inject.Guice.createInjector(Guice.java:69)
        at com.google.inject.Guice.createInjector(Guice.java:59)
        at org.codehaus.plexus.DefaultPlexusContainer.addPlexusInjector(DefaultPlexusContainer.java:481)
        at org.codehaus.plexus.DefaultPlexusContainer.<init>(DefaultPlexusContainer.java:206)
        at org.apache.maven.cli.MavenCli.container(MavenCli.java:651)
        at org.apache.maven.cli.MavenCli.doMain(MavenCli.java:286)
        at org.apache.maven.cli.MavenCli.main(MavenCli.java:196)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
        at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:77)
        at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
        at java.base/java.lang.reflect.Method.invoke(Method.java:568)
        at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced(Launcher.java:282)
        at org.codehaus.plexus.classworlds.launcher.Launcher.launch(Launcher.java:225)
        at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode(Launcher.java:406)
        at org.codehaus.plexus.classworlds.launcher.Launcher.main(Launcher.java:347)
[INFO] Scanning for projects...
[INFO] 

@maxandersen
Copy link
Collaborator

In the pom file? Yes.
No other way to guarantee the performanct logger gets installed first.

That should not give such error. When does that happen?

Copy link
Collaborator

@maxandersen maxandersen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 good to go with 17 as rhbq don't default to 21 yet.

@openshift-ci openshift-ci bot added the lgtm Looks good to me label Apr 22, 2024
@vinokurig
Copy link
Contributor Author

@michael-valdron Could you please merge if you don't have any objections.

Copy link
Member

@michael-valdron michael-valdron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think to follow versioning I'd suggest the following:

  • Create patch version copies with the PR changes:
    • 1.3.1
    • 1.4.1
  • Deprecate the base versions that are broken by adding the Deprecated tag:

See deprecation example.

@openshift-ci openshift-ci bot removed the lgtm Looks good to me label Apr 22, 2024
@michael-valdron
Copy link
Member

I think to follow versioning I'd suggest the following:

* Create patch version copies with the PR changes:
  
  * `1.3.1`
  * `1.4.1`

* Deprecate the base versions that are broken by adding the `Deprecated` tag:
  
  * [`1.3.0`](https://github.com/devfile/registry/blob/7ef133621b0e0df71811abcff21c970ffc44f93f/stacks/java-quarkus/1.3.0/devfile.yaml#L7)
  * [`1.4.0`](https://github.com/devfile/registry/blob/7ef133621b0e0df71811abcff21c970ffc44f93f/stacks/java-quarkus/1.4.0/devfile.yaml#L7)

See deprecation example.

This also mean 1.4.1 becomes the new default version.

@vinokurig
Copy link
Contributor Author

@michael-valdron

I think to follow versioning I'd suggest the following:

Create patch version copies with the PR changes:
1.3.1
1.4.1
Deprecate the base versions that are broken by adding the Deprecated tag:
1.3.0
1.4.0
See deprecation example.

This also mean 1.4.1 becomes the new default version.

Done

@vinokurig vinokurig force-pushed the java-quarkus branch 3 times, most recently from d199636 to fe623c9 Compare April 23, 2024 15:47
@michael-valdron
Copy link
Member

/ok-to-test

@vinokurig
Copy link
Contributor Author

/retest

@vinokurig vinokurig force-pushed the java-quarkus branch 2 times, most recently from a2284ee to 98aceab Compare April 24, 2024 10:45
…zip urls

Signed-off-by: ivinokur <ivinokur@redhat.com>
@vinokurig
Copy link
Contributor Author

@michael-valdron do you have an idea why the tests are failing?

@michael-valdron
Copy link
Member

@vinokurig

The odo v2 failure is an ongoing issue we are tracking: devfile/api#1514

The odo v3 failure seems to be triggerred by test case failures on the redhat-product starter project:

Summarizing 4 Failures:
  [FAIL] test starter projects from devfile stacks [It] stack: java-quarkus version: 1.3.0 starter: redhat-product
  /home/runner/work/registry/registry/tests/odov3/odo_test.go:263
  [FAIL] test starter projects from devfile stacks [It] stack: java-quarkus version: 1.4.0 starter: redhat-product
  /home/runner/work/registry/registry/tests/odov3/odo_test.go:263
  [FAIL] test starter projects from devfile stacks [It] stack: java-quarkus version: 1.3.1 starter: redhat-product
  /home/runner/work/registry/registry/tests/odov3/odo_test.go:263
  [FAIL] test starter projects from devfile stacks [It] stack: java-quarkus version: 1.4.1 starter: redhat-product
  /home/runner/work/registry/registry/tests/odov3/odo_test.go:263

In fact, almost looks like it could be the same issue causing devfile/api#1514:

odo logs stderr: 
  [FAILED] in [It] - /home/runner/work/registry/registry/tests/odov3/odo_test.go:263 @ 04/24/24 14:24:43.977
  Executing: odo delete namespace -f btpymstq
  << Timeline

  [FAILED] Expected
      <*errors.errorString | 0xc0003280c0>: 
      http://127.0.0.1:44665/ did not return 200
      {
          s: "http://127.0.0.1:44665/ did not return 200",
      }
  to be nil
  In [It] at: /home/runner/work/registry/registry/tests/odov3/odo_test.go:263 @ 04/24/24 14:24:43.977

I'll open an issue to investigate this one as well. #369 will temporarily skip odo testing on java-quarkus until these issues are resolved.

@michael-valdron
Copy link
Member

Opened an issue tied to odo v3 test failures: devfile/api#1533

FYI @vinokurig

@vinokurig
Copy link
Contributor Author

@michael-valdron Thank you for the explanation, can we proceed with current changes?

Copy link
Member

@michael-valdron michael-valdron left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/lgtm

Tested deployments locally, works on both odo v2 and odo v3.

odo v2 with java-quarkus v1.3.1 stack and community starter project:
image

odo v3 with java-quarkus v1.4.1 stack and community starter project:
image

redhat-product starter project is failing but is not affected by these changes.

@openshift-ci openshift-ci bot added lgtm Looks good to me approved labels Apr 26, 2024
@vinokurig
Copy link
Contributor Author

@michael-valdron Could you please merge?

Copy link

openshift-ci bot commented Apr 29, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: ibuziuk, maxandersen, michael-valdron, vinokurig

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@michael-valdron
Copy link
Member

#366 (review)

Child sample works with these changes to parent:

image

@michael-valdron michael-valdron merged commit b22ca8d into devfile:main Apr 29, 2024
8 of 10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants