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

Upgrade gradle to 7.6.2 and fix sonarqube pipeline #1393

Open
wants to merge 9 commits into
base: master
Choose a base branch
from

Conversation

DiegoTavares
Copy link
Collaborator

@DiegoTavares DiegoTavares commented Jun 20, 2024

the sonarqube pipeline requires a newer version of the jvm and that's a good excuse for upgrading gradle on the project as the version we have currently is very old.

@bcipriano
Copy link
Collaborator

This looks like the error?

> org/sonar/batch/bootstrapper/EnvironmentInformation has been compiled by a more recent version of the Java Runtime (class file version 61.0), this version of the Java Runtime only recognizes class file versions up to 55.0

For a new JRE, that probably means using a new image?

Yeah, that image we're using is pretty old now:

container: aswf/ci-opencue:2020

@ramonfigueiredo
Copy link
Collaborator

@DiegoTavares

Change the status to Draft!

@DiegoTavares DiegoTavares marked this pull request as draft September 18, 2024 15:36
@DiegoTavares DiegoTavares force-pushed the fix-sonar-pipeline branch 4 times, most recently from 427cdfa to 6a258c1 Compare September 18, 2024 20:21
@DiegoTavares DiegoTavares force-pushed the fix-sonar-pipeline branch 8 times, most recently from b25116d to 67cd97b Compare September 18, 2024 21:48
@DiegoTavares
Copy link
Collaborator Author

I've tried updating the base image to aswf/ci-opencue:2024 but both versions use java11 (class file version 55), which is the correct version for cuebot.

I've also updated target and source compatibility versions to 11 and tested multiple versions around 2.7.1 for org.sonarsource.scanner.gradle:sonarqube-gradle-plugin.

My take is that some other dependency is bringing a newer version of org.sonarqube compiled with java17 (class file version 61), but I haven't been able to figure out what, or how to fix it.

@bcipriano Can you give this a go? It is a blocker for our next release.

@bcipriano
Copy link
Collaborator

I see that the ci-common image we're based on does include a sonar package: https://github.com/AcademySoftwareFoundation/aswf-docker/tree/main/ci-common

I don't know which version of ci-common we're built from. We could ask the ASWF infra folks for more info there.

I'm guessing that system-wide sonar is what's causing the problem here since changing the plugin version appears to have no effect.

This doc looks relevant, it talks about JRE auto-provisioning and how to switch between versions of the JVM: https://docs.sonarsource.com/sonarcloud/appendices/scanner-environment/#maven-gradle

I tried a few things to get this working:

  • Use the openjdk:17 image instead, but our project fails to build.
  • Switch Java versions by: install Java 17, build with Java 11, then set JAVA_HOME and run Sonar with Java 17, but I get a Java error for that too.

My work is in https://github.com/AcademySoftwareFoundation/OpenCue/pull/1515/files

Some other things to explore here:

  1. Try to downgrade that systemwide Sonar to a version that works for us.
  2. Upgrade the project (maybe just upgrade Gradle?) so it doesn't always fail when running with Java 17.
  3. Try to report to Sonar a different way -- the jacocoTestReport step seems to run fine, can we use that output and call Sonar externally, i.e. not via Gradle?

@DiegoTavares
Copy link
Collaborator Author

I don't think this issue is being caused by the external ci-common image as I'm able to replicate it locally on my mac using gradle outside of the docker environment.

My next attempt is to upgrade gradle.

Upgrading gradle solved the dependency issue on the sonarqube step on the cicd pipeline.
@DiegoTavares DiegoTavares force-pushed the fix-sonar-pipeline branch 2 times, most recently from c8d0f5b to d64ba2a Compare September 26, 2024 01:15
@DiegoTavares DiegoTavares changed the title WIP: Attempt to fix sonarcuebe pipeline Upgrade gradle to 7.6.2 and fix sonarqube pipeline Sep 26, 2024
Ideally the `ci-opencue` image could be built with the correct version of the java jdk, but for now we're updating the version before running the pipeline to confirm it really works.

The only way to confirm this works as expected is merging to master as secret-keys are not shared with pipelines triggered by PRs.
@DiegoTavares
Copy link
Collaborator Author

Alright, I think I finally got it to work. In summary:

  • Upgrade gradle to version 7
  • Upgrade the image jdk to version 17

The pipeline will only be triggered when merged to master, running it on this PR got me to a failure in trying to sign into sonar-cloud, which is expected as the SONAR_TOKEN secret is not shared with PRs.

FYI @bcipriano

chown -R aswfuser:aswfgroup .
su -c "cd cuebot && ./gradlew jacocoTestReport sonarqube -Dsonar.login=$(SONAR_TOKEN)" aswfuser
su -c "cd cuebot && ./gradlew jacocoTestReport sonar aswfuser
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think there's a missing quote here. Also, we don't need to define the sonar token?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oops. Fixed the missing quote.

According to the documentation, having SONAR_TOKEN defined as an environment variable is supposed to be enough. Let's see when it makes it to master where we can actually test.

sourceCompatibility = 1.8
targetCompatibility = 1.8
sourceCompatibility = 11
targetCompatibility = 11

ext {
activemqVersion = '5.12.0'
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: Can this use def like grpc / protobuf use now?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess so. I've updated on the last commit to give it a go.

@DiegoTavares DiegoTavares marked this pull request as ready for review September 27, 2024 20:43
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

Successfully merging this pull request may close these issues.

3 participants