There's a little bit of outdated documentation here https://www.mediawiki.org/wiki/Continuous_integration/SonarQube_Scanner I'll try to update it soon.

@Mstyles ^ sorry forgot to mention you on that last comment.

Alright, having looked through the code in integration/config again here's a summary of what is going on.

Overview

The code health reports are generated by checking out the code for a patch (or if it's postmerge, for the master branch), then running the sonarscanner application.

That application reads from a .sonar-project.properties file (more on that later) which tells Sonar which directories have source code, which directories have test files, which directories/files should be excluded, what type of project (PHP, Java, and so on) it is, etc.

The sonar-scanner application sends all of those files to a remote server. It will also send over any code coverage reports that have been generated by other scripts (sonar-scanner doesn't do that on its own).

The destination could be the self-hosted version of SonarQube. But in our case we currently use the hosted version of SonarQube which is at https://sonarcloud.io.

Then we have a script which asks the SonarQube server "are you done analyzing the code yet?" every few seconds, until it has a response. Based on the result of that response, we output success/failure in a comment in Gerrit.

Implementing for Java

There are a few things you'll need to do. We have a docker image for running sonar-scanner, it's in the integration/config repo under dockerfiles/java8-sonar-scanner. It has a run.sh script which is customized for MediaWiki core and extensions. Most of its complexity is in making it work when a .sonar-project.properties file doesn't exist, which is the case for most extensions and for MediaWiki core. If your team is OK with committing a .sonar-project.properties file to your Java projects, you can definitely simplify things. I would start by making a run-java.sh script and copying the contents of run.sh, and I think you could remove just about everything between set -x and the invocation of /opt/sonar-scanner towards then end. It's a good idea to test out your /run-java.sh script locally.

This is kind of complicated. You need to install docker-pkg and run docker-pkg -c dockerfiles/config.yaml. (Also, if you're on a mac, you need to install an older version of docker-pkg because HEAD doesn't work with macOS. I'm running abe9117. Clone the repo and check out that commit, then follow the build instructions.) Somewhat unintuitively, you need to bump the version for the "changelog" file each time you make changes to the run-java.sh script (so that docker-pkg knows to build the new image). What I do usually is run a command like this

docker run \
                    --volume /Users/kharlan/src/yourproject/log://workspace/log \
                    --volume /Users/kharlan/src/yourproject/cache://cache \
                    --volume /Users/kharlan/src/yourproject://workspace/src \
              --rm -it \
              --entrypoint=bash \
                --env SONAR_API_KEY={YOU_NEED_ONE} \
              --env ZUUL_PROJECT={you can see this in the URL in Gerrit or by looking at params in a Jenkins job} \
                docker-registry.wikimedia.org/releng/java8-sonar-scanner:0.4.12 -X  -Dsonar.host.url=https://sonarcloud.io -Dsonar.organization=wmftest -Dsonar.projectKey={your-java-project} -Dsonar.projectName={your-java-project}

Then I use docker cp to copy my modifications into the running container. Then in the container I can do /run.sh to see how my script works. I would recommend creating your own account/organization on sonarcloud.io and using an API key there as a sandbox for experimenting, rather than using the wmftest organization.

Configuration

In layout.yaml you'll want to make your own version of mw-java-codehealth-patch based on what's in mwext-codehealth-patch.

Then I'd look in mediawiki.yaml and search for the codehealth string to find existing code. You'll want to create a project with a name like java-codehealth (look for mwext-codehealth for existing code example), and then a job-template, again with a name like java-codehealth-{name}. This template is where you define the steps for your build, like checking out the code, running tests and generating code coverage, and then finally running the sonar scanner. I'm pretty sure the project/job-template shouldn't go in mediawiki.yaml (that's supposed to be for MediaWiki core/extensions) but I don't know off hand which file is more appropriate. Maybe @hashar or @thcipriani have ideas?

Finally once everything is set up, you want to add your java projects to the codehealth pipeline you've defined for them. You can see examples of that in layout.yaml where extension-codehealth is defined with a patch and postmerge job, and then various extensions have name: extension-codehealth added to their templates.

Sorry this is all so complicated. That's how our configuration is at the moment :\ I'd be happy to review patches or talk through it in a meeting, just let me know!

Thanks so much for your very detailed write up. I hope that can make it into official documentation somewhere

After chatting with @Gehel he mentioned that all of the maven projects have a parent pom and that sonar-scanner could be run directly from maven. Or I can do the docker image changes that you're suggesting and see how that goes. The properties file option is still on the table as well but as a last approach.

I'm planning to test out both approaches, but if you have a preference either way, please let me know.

@Mstyles I think that could work (see also these docs) and it seems like less work than the Docker image approach. It's unclear to me how you'd take the result and tell Gerrit about it, though. But maybe try going the maven approach and experiment with it locally, then see if it would make sense to also do in CI.

Tangentially, I plan to work on T217008 sometime this month, and when that is implemented you would not need to try to poll SonarQube for the analysis result. Instead, SonarQube would use a webhook to ping an application that would then post comments in Gerrit and vote with a -1 or +1. That would probably simplify things a bit for this task.

@Mstyles I would keep the new run-java.sh script in the same dockerfiles/java8-sonar-scanner directory. Then, when defining your job template for jjb, you'd override the default entrypoint, so it would look something like this:

- docker-run-with-log-and-workspace-cache:
    image: 'docker-registry.wikimedia.org/releng/java8-sonar-scanner:0.4.12'
    options: '--volume "$(pwd)"/src:/workspace/src'
    logdir: '/workspace/log'
    args: '{sonar_args}'
    entrypoint: /run-java.sh

We should probably rename run.sh to run-php.sh and do the same entrypoint override for the other jobs

This is kind of complicated. You need to install docker-pkg and run docker-pkg -c dockerfiles/config.yaml. (Also, if you're on a mac, you need to install an older version of docker-pkg because HEAD doesn't work with macOS. I'm running abe9117. Clone the repo and check out that commit, then follow the build instructions.)

Update, there's a workaround for this which is documented at https://gerrit.wikimedia.org/r/c/operations/docker-images/docker-pkg/+/555751

Change 556513 had a related patch set uploaded (by Mstyles; owner: Mstyles):
[integration/config@master] Java sonarcloud implementation

https://gerrit.wikimedia.org/r/556513

I put a WIP patch out. I haven't included the changes needed for the php renaming or anything that will have to happen in the jib directory. Those could also go in a separate patch if that makes things more readable. I would love to get feedback and I was wondering if there were any other ways to test outside of running the docker container locally which I've been doing. Also, I plan to put the project/job-template in the search.yaml file, that seems like a reasonable home.

I put a WIP patch out. I haven't included the changes needed for the php renaming or anything that will have to happen in the jib directory. Those could also go in a separate patch if that makes things more readable

yeah separate patch for that sounds good.

I would love to get feedback and I was wondering if there were any other ways to test outside of running the docker container locally which I've been doing

Running the container is probably the best way to ensure that what you're doing in development will work in the CI environment. But you can always just run ./run-java.sh locally, so long as you have set all your environment variables properly and the path to sonar-scanner exists on your host system. Let me know if you have questions about getting set up with that.

Also, I plan to put the project/job-template in the search.yaml file, that seems like a reasonable home.

Sounds good to me!

Change 557154 had a related patch set uploaded (by Mstyles; owner: Mstyles):
[integration/config@master] jib: Add Java codehealth project and job-template

https://gerrit.wikimedia.org/r/557154

Change 556513 merged by jenkins-bot:
[integration/config@master] dockerfiles: Add Java sonarcloud implementation

https://gerrit.wikimedia.org/r/556513

Change 557154 merged by jenkins-bot:
[integration/config@master] jib: Add new java-codehealth-patch job and template

https://gerrit.wikimedia.org/r/557154

jjb job is now available to be hooked up in layout.yaml.

Thanks @Jdforrester-WMF
@kostajh I had some questions about the layout. I know you said to create an extension-codehealth-java similar to https://github.com/wikimedia/integration-config/blob/master/zuul/layout.yaml#L884. There doesn't seem to be a standardized pattern for the urls in the same way for non extension projects. Also, some of the java jobs have different post merge jobs that run from each other. I was thinking of adding codehealth separately to each project, in these blocks, https://gerrit.wikimedia.org/r/plugins/gitiles/integration/config/+/master/zuul/layout.yaml#8115. I'm not sure if that will affect future plans that you have with the codehealth stuff going forward

I know you said to create an extension-codehealth-java similar to https://github.com/wikimedia/integration-config/blob/master/zuul/layout.yaml#L884

I wouldn't worry about this for now. You can not add anything for this part of the config and the success/fail link will point to the jenkins build console output, which sucks from a usability standpoint, but the good news is that the SonarQube Bot project is now posting comments in Gerrit with a link to the report in sonarcloud. Also, once T240935 is resolved then the jenkins output shouldn't show up anywhere in gerrit anyway.

I was thinking of adding codehealth separately to each project, in these blocks, https://gerrit.wikimedia.org/r/plugins/gitiles/integration/config/+/master/zuul/layout.yaml#8115.

Yes, that sounds about right. In layout.yaml we have this around line 1540:

- name: extension-codehealth
  postmerge:
    - mwext-codehealth-master-non-voting
  codehealth:
    - mwext-codehealth-patch

So you'd want to create something like - name: java-codehealth and fill in postmerge/codehealth with jobs from the java-codehealth config you made in search.yaml.

Then in https://gerrit.wikimedia.org/r/plugins/gitiles/integration/config/+/master/zuul/layout.yaml#8115 you'd do something like:

- name: search/extra-analysis
    test:
      - search-extra-analysis-maven-java8-docker
    gate-and-submit:
      - search-extra-analysis-maven-java8-docker
    postmerge:
      - search-extra-analysis-maven-java8-docker-site-publish
    codehealth:
      - java-codehealth-patch

Also, some of the java jobs have different post merge jobs that run from each other.

Could you clarify this please? I don't think that matters if one job triggers another. The important thing is that after something's been merged into master, we re-run the analysis of the code and send it to SonarQube.

Change 559230 had a related patch set uploaded (by Mstyles; owner: Mstyles):
[integration/config@master] layout: Run sonar scanner analysis for java projects

https://gerrit.wikimedia.org/r/559230

What I was trying to say is that all of the projects are currently sending their analysis to SonarQube, so I didn't want to change any postmerge jobs. I put what I thought in the patch

Trying to summarize IRC discussion that I had with @kostajh (sorry, this is a bit messy, there is a lot I don't understand with JJB, Zuul, ...):

A diagram to support the discussion.

There are some open questions on if we can reduce duplication of work. The approach in the current open CR is to have different pipelines for standard build and code health build. So pre-merge, we have a standard build (mvn clean install) which creates the artifacts, runs the tests, report on coverage, etc... and a code health build which does sonar analysis. Sonar analysis requires the build artifacts, so we need to create them in that case as well. The post-merge is similar, with an additional step to publish documentation to https://doc.wikimedia.org. Documentation also requires the build artifacts to already exist.

The approach currently implemented (only for the post-merge) is to have a single pipeline, with multiple steps that are sharing build artifacts via an external docker volume. This allows to run mvn clean install only once, and reuse the artifacts for sonar and documentation. But this reduces the visibility on the different steps.

I'm unsure what the best approach is. We'll need some more discussion.

Note that there are also some discussion of doing the sonar analysis as a publisher. I'm not exactly sure what that means and how this could be implemented.

Saving Guillaume diagram here for later reference

Change 559641 had a related patch set uploaded (by Mstyles; owner: Mstyles):
[integration/config@master] dockerfiles: Add zuul branch properties

https://gerrit.wikimedia.org/r/559641

I am all for reducing duplication but in this case, perhaps we can see if we get it working first and then try to reduce the duplication?

In T238004#5755923, @Mstyles wrote:

I am all for reducing duplication but in this case, perhaps we can see if we get it working first and then try to reduce the duplication?

Done is better than perfect! I fully agree on having something working first!

Change 559641 merged by jenkins-bot:
[integration/config@master] dockerfiles: [java8-sonar-scanner] Add zuul branch properties

https://gerrit.wikimedia.org/r/559641

Change 563222 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] jjb: Switch to new version of java8-sonar-scanner

https://gerrit.wikimedia.org/r/563222

Change 563225 had a related patch set uploaded (by Mstyles; owner: Mstyles):
[integration/config@master] jjb: bump java sonar scanner image version

https://gerrit.wikimedia.org/r/563225

Change 563222 abandoned by Jforrester:
jjb: Switch to new version of java8-sonar-scanner

https://gerrit.wikimedia.org/r/563222

Change 563225 merged by jenkins-bot:
[integration/config@master] jjb: bump java sonar scanner image version to 0.5.2

https://gerrit.wikimedia.org/r/563225

Change 559230 merged by jenkins-bot:
[integration/config@master] layout: Run sonar scanner analysis for Java projects

https://gerrit.wikimedia.org/r/559230

Mentioned in SAL (#wikimedia-releng) [2020-01-09T17:26:13Z] <James_F> Zuul: Run sonar scanner analysis in codehealth pipeline for select Java projects T238004

https://integration.wikimedia.org/ci/job/java-codehealth-patch/1/console failed with 00:00:06.760 [FATAL tini (9)] exec /run-java failed: No such file or directory – does this need to be /run-java.sh?

… fixing that gives 00:00:06.178 /run-java.sh: line 11: ./mvnw: No such file or directory

you're right, it's a typo. It should be /run-java.sh. Pushing up a patch now

Change 563229 had a related patch set uploaded (by Mstyles; owner: Mstyles):
[integration/config@master] jjb: fix typo for script

https://gerrit.wikimedia.org/r/563229

Change 563229 merged by jenkins-bot:
[integration/config@master] jjb: fix typo for script

https://gerrit.wikimedia.org/r/563229

Change 563246 had a related patch set uploaded (by Jforrester; owner: Jforrester):
[integration/config@master] jjb: [java-codehealth-patch] Explicitly set workdir

https://gerrit.wikimedia.org/r/563246

Change 563250 had a related patch set uploaded (by Mstyles; owner: Mstyles):
[search/extra@master] Update README.md for patch sonar testing

https://gerrit.wikimedia.org/r/563250

Change 563246 merged by jenkins-bot:
[integration/config@master] jjb: [java-codehealth-patch] Actually run docker-ci-src-setup-simple

https://gerrit.wikimedia.org/r/563246

@kostajh everything has been merged, and the code health job runs with sonar analysis after a patch for java projects. However we're not seeing any results from bots in the test patch in search extra (https://gerrit.wikimedia.org/r/563250) with analysis here: https://sonarcloud.io/project/activity?id=org.wikimedia.search%3Aextra-parent. Does the bot know about java projects?

Change 563250 abandoned by Mstyles:
Update README.md for patch sonar testing

Reason:
need to test on a new branch

https://gerrit.wikimedia.org/r/563250

Change 563267 had a related patch set uploaded (by Mstyles; owner: Mstyles):
[search/extra@master] Changing README.md for testing purposes

https://gerrit.wikimedia.org/r/563267

Change 563279 had a related patch set uploaded (by Jforrester; owner: Mstyles):
[search/extra@test-branch] Changing README.md for testing purposes

https://gerrit.wikimedia.org/r/563279

Change 563319 had a related patch set uploaded (by Mstyles; owner: Mstyles):
[integration/config@master] dockerfiles: add extra sonar arg for correct project name

https://gerrit.wikimedia.org/r/563319

Change 563321 had a related patch set uploaded (by Mstyles; owner: Mstyles):
[integration/config@master] jjb: version bump to 0.5.3

https://gerrit.wikimedia.org/r/563321

Change 563321 had a related patch set uploaded (by Mstyles; owner: Mstyles):
[integration/config@master] jjb: version bump to 0.5.3

https://gerrit.wikimedia.org/r/563321

Change 563319 merged by jenkins-bot:
[integration/config@master] dockerfiles: [java8-sonar-scanner] Add extra arg for correct project name

https://gerrit.wikimedia.org/r/563319

Change 563321 merged by jenkins-bot:
[integration/config@master] jjb: [java8-sonar-scanner] Bump to 0.5.3

https://gerrit.wikimedia.org/r/563321

@Mstyles OK, well having logged in as the SonarQube Bot (username: sonarqubebot), I can see from the gerrit UI that I don't have the ability to mark the change as verified

So we need to fix that at the Gerrit permissions level. @Gehel or @thcipriani do you have an idea of how this can be done?

Change 564585 had a related patch set uploaded (by Kosta Harlan; owner: Kosta Harlan):
[labs/tools/sonarqubebot@master] Use gerritProjectName if provided

https://gerrit.wikimedia.org/r/564585

Looking at permissions in Gerrit, our java projects inherit from "search" or from "wikidata/query". I don't seem to have the rights to modify those. I'm also wondering if we should address access for the sonarqubebot at an even higher level (All-Projects?).

Maybe @hashar can also help...

I think you'd want to add SonarQube bot to https://gerrit.wikimedia.org/r/#/admin/groups/14,members

In T238004#5802271, @Jdforrester-WMF wrote:

I think you'd want to add SonarQube bot to https://gerrit.wikimedia.org/r/#/admin/groups/14,members

I don't think we should add it to the JenkinsBot group, since that has V+2 and submit permissions across a lot of repositories. Per @kostajh on IRC, this bot just needs V+1. I think it's own dedicated group would be best.

@Mstyles I think the last piece to figure out is the branch issue; codehealth check on for example https://gerrit.wikimedia.org/r/c/wikidata/query/rdf/+/556188 doesn't result in a comment being posted because the branch is master. For MediaWiki extension and Core codehealth jobs this is intentional, because when the branch is master that means the analysis is being run post-merge.

But for Java it appears that this condition exists pre-merge. So we need to update the sonarqubebot code to handle this case, maybe by passing another parameter to sonar:scanner as we did for gerritProjectName? Something like:

./mvnw -gs /settings.xml sonar:sonar -Dsonar.analysis.gerritProjectName="$ZUUL_PROJECT" -Dsonar.branch.target="$ZUUL_BRANCH" -Dsonar.branch.name="$ZUUL_CHANGE-$ZUUL_PATCHSET" -Dsonar.analysis.allowCommentOnMaster=1

@kostajh do we still need to separate sonar args for master vs non master branches then? it seems that we should be able to send all of the same sonar args whether or not the branch is master. I'm not sure how to tell the bot if something is pre or post merge.

Change 565164 had a related patch set uploaded (by Mstyles; owner: Mstyles):
[integration/config@master] dockerfiles: [java8-sonar-scanner] add sonar flag

https://gerrit.wikimedia.org/r/565164

Change 565165 had a related patch set uploaded (by Mstyles; owner: Mstyles):
[integration/config@master] jjb: [java8-sonar-scanner] version bump to 0.5.4

https://gerrit.wikimedia.org/r/565165

Change 565164 merged by jenkins-bot:
[integration/config@master] dockerfiles: [java8-sonar-scanner] add sonar flag

https://gerrit.wikimedia.org/r/565164

Mentioned in SAL (#wikimedia-releng) [2020-01-17T19:52:34Z] <James_F> Docker: Publishing java8-sonar-scanner:0.5.4 on contint1001 T238004

Change 565165 merged by jenkins-bot:
[integration/config@master] jjb: [java8-sonar-scanner] version bump to 0.5.4

https://gerrit.wikimedia.org/r/565165

Change 563279 abandoned by Jforrester:
Changing README.md for testing purposes

https://gerrit.wikimedia.org/r/563279

OK, this seems to now be working as planned, right? Only thing left is actually voting V+1 when it passes, I think.

very exciting to see it work here: https://gerrit.wikimedia.org/r/c/search/extra/+/563267. I know @Gehel mentioned trying to refactor where the same job runs for both pre-merge and post-merge but after chatting with @Jdforrester-WMF, it seems that convention is to have separate pre and post merge jobs. I would be happy to call this done.

Mentioned in SAL (#wikimedia-releng) [2020-01-20T10:10:53Z] <addshore> the last zuul reload also pulled in "jjb: [java8-sonar-scanner] version bump to 0.5.4" .. T238004

Change 563267 abandoned by Mstyles:
Changing README.md for testing purposes

Reason:
testing is complete

https://gerrit.wikimedia.org/r/563267