SonarCloud now not updating GitHub PR and Checks

dcrossley · February 7, 2019, 6:13am

We have been using SonarCloud successfully for a while, but in recent weeks some parts have broken.

There are a number of threads here on this forum with very similar symptoms, e.g.
https://community.sonarsource.com/t/sonarcloud-doesnt-update-github-status/6325

We have many Maven-based and JavaScript-based public repositories at GitHub. The JavaScript-based ones are failing more drastically, but some of the Maven-based ones show problems too.

We use Jenkins Pipeline. An early stage uses the “checkout” step with the “GitSCM class”.

For the stage that runs Sonar, we use the “sonar-maven-plugin” for the Maven ones, and the “bin/sonar-scanner” for the JavaScript ones.

I selected one of each type (Java-based and JavaScript-based) where know that these still have some Sonar-detected issues in their master branch. Other past closed PRs are available to see the similar problems, but of course their Jenkins outputs might be removed now.

So to demonstrate some of the problems, today i made two fresh pull-requests in each repository, with one of them unmerged, to enable investigation of the checks.

A Maven-based repository:
https://github.com/folio-org/mod-audit-filter/pull/20 (merged)
https://github.com/folio-org/mod-audit-filter/pull/21 (open)

The “Checks” tab does show the sonar result.
The “Conversation” tab link to “Show all checks” does show the “SonarCloud” check result.
Follow the link via Jenkins for the “branch” build, then via its “Sonar” Jenkins stage. Follow link to sonarcloud where the page says “No issues”. See warning note in top-right “Could not find ref ‘master’ in refs/heads or refs/remotes/origin.”
Follow the link via Jenkins for the “pr-merge” build, then via its “Sonar” Jenkins stage. Follow link to sonarcloud where the page says “No issues”. There is no warning note.

A JavaScript-based repository:
https://github.com/folio-org/ui-licenses/pull/59 (merged)
https://github.com/folio-org/ui-licenses/pull/60 (open)

The “Checks” tab simply says “Queued xx ago”.
The “Conversation” tab link to “Show all checks” does not show the “SonarCloud” check result.
Follow the link via Jenkins for the “branch” build, then via its “Sonar” Jenkins stage. Follow link to sonarcloud where the page says “No issues”. See warning note in top-right “Could not find ref ‘master’ in refs/heads or refs/remotes/origin.”
Follow the link via Jenkins for the “pr-merge” build, then via its “Sonar” Jenkins stage. Follow link to sonarcloud where the page says “No issues”. There is no warning note.

Here are the parameters for running Sonar in each type:

Maven-based:

  if (env.CHANGE_ID) {
    echo "PR request: $env.CHANGE_ID"
    withCredentials(...snip...) {
      withSonarQubeEnv('SonarCloud') {
        sh "mvn -B org.sonarsource.scanner.maven:sonar-maven-plugin:${sonarMvnPluginVer}:sonar " +
                "-Dsonar.organization=folio-org -Dsonar.verbose=true " +
                "-Dsonar.pullrequest.base=master " +
                "-Dsonar.pullrequest.branch=${env.BRANCH_NAME} " +
                "-Dsonar.pullrequest.key=${env.CHANGE_ID} " +
                "-Dsonar.pullrequest.provider=github " + 
                "-Dsonar.pullrequest.github.repository=folio-org/${env.projectName}"
                // "-Dsonar.pullrequest.github.endpoint=https://api.github.com"
      }
    }   
  }       
  else {    
    withSonarQubeEnv('SonarCloud') {
      if (env.BRANCH_NAME != 'master') {
        sh "mvn -B org.sonarsource.scanner.maven:sonar-maven-plugin:${sonarMvnPluginVer}:sonar " +
             "-Dsonar.organization=folio-org -Dsonar.verbose=true " +
             "-Dsonar.branch.name=${env.BRANCH_NAME} " +
             "-Dsonar.branch.target=master"
      }     
      else {
        sh "mvn -B org.sonarsource.scanner.maven:sonar-maven-plugin:${sonarMvnPluginVer}:sonar " + 
             "-Dsonar.organization=folio-org -Dsonar.verbose=true"
      }     
    }       
  }

JavaScript-based:

  withCredentials(...snip...) {
    withSonarQubeEnv('SonarCloud') {
      def scannerHome = tool 'SonarQube Scanner'
      def excludeFiles = '...snip...'
      if (env.CHANGE_ID) {
        sh "${scannerHome}/bin/sonar-scanner " +
          "-Dsonar.projectKey=org.folio:${env.projectName} " +
          "-Dsonar.projectName=${env.projectName} " +
          "-Dsonar.organization=folio-org " +
          "-Dsonar.sources=. " +
          "-Dsonar.language=js " +
          "-Dsonar.exclusions=${excludeFiles} " +
          "-Dsonar.pullrequest.base=master " +
          "-Dsonar.pullrequest.key=${env.CHANGE_ID} " +
          "-Dsonar.pullrequest.branch=${env.BRANCH_NAME} " +
          "-Dsonar.pullrequest.provider=github " +
          "-Dsonar.pullrequest.github.repository=folio-org/${env.projectName} "
          // "-Dsonar.pullrequest.github.endpoint=https://api.github.com"
      }
      else {
        if (env.BRANCH_NAME != 'master' ) { 
          sh "${scannerHome}/bin/sonar-scanner " +
            "-Dsonar.organization=folio-org " +
            "-Dsonar.projectKey=org.folio:${env.projectName} " +
            "-Dsonar.projectName=${env.projectName} " +
            "-Dsonar.branch.name=${env.BRANCH_NAME} " +
            "-Dsonar.branch.target=master " +
            "-Dsonar.sources=. " +
            "-Dsonar.language=js " +
            "-Dsonar.exclusions=${excludeFiles} " +
            "-Dsonar.javascript.lcov.reportPaths=${lcovPath}/lcov.info"
        }
        else {
          sh "${scannerHome}/bin/sonar-scanner " +
            "-Dsonar.organization=folio-org " +
            "-Dsonar.projectKey=org.folio:${env.projectName} " +
            "-Dsonar.projectName=${env.projectName} " +
            "-Dsonar.sources=. " +
            "-Dsonar.language=js " +
            "-Dsonar.exclusions=${excludeFiles} " +
            "-Dsonar.javascript.lcov.reportPaths=${lcovPath}/lcov.info"
        }
      }
    }
  }

We hope that that detail helps to determine what is amiss.

janos · February 7, 2019, 7:04am

Hi,

Thank you for the detailed report, this is very helpful. I can answer about some of these issues, and I would like more info about others.

This happens for example when the branch specified by -Dsonar.pullrequest.base is not found as a local branch or fetched on origin. The consequence is that the files and lines changed in the branch since it was forked from its base may not be computed correctly. Which in turn leads to incorrect listing of issues on SonarCloud, often no issues at all. The solution is to fetch the target branch before running the analysis.

I’m puzzled by the missing Check in case of the JavaScript example, because the scanner parameters look equivalent to the Maven example, and correct.

Does this happen consistently or intermittently?
Do you see anything interesting in the output of the scanner run?
Do you see anything interesting in the Background Tasks page of the project for such analyses?

dcrossley · February 7, 2019, 8:06am

Glad to make the effort. And thanks for answering.

I reckon consistently.

I was hoping that you could. That is the beauty of a full opensource project – we can show our Jenkins output. Follow a Details link to the Jenkins Pipeline, then select the “Sonar” stage, then select the “Shell” output. Other than the “refs” warning, no i cannot (but i am no expert either).

Other repository PRs might be useful to inspect. Any repo name “ui-" or "stripes-”.

Sorry, what is that?

dcrossley · February 8, 2019, 12:21am

Ah, i am now an admin so can see that.

There are no pending or failed tasks.

Is there something in particular that i should look for in the “Scanner Context” of each?

dcrossley · February 8, 2019, 12:26am

Regarding the “refs” warning, we will investigate adding the explicit ‘git fetch’ and report back.

My original post indicated that Sonar was only running on master branch. Not true. I needed to make a deliberate new error. So adjusted this post’s title.

So we still need to determine why the Checks are not being reported back to GitHub PR.

janos · February 14, 2019, 8:34am

I looked into the Details links, trying to find out the sha1 that the analysis was executed on. This is important, because roughly speaking SonarCloud will decorate the sha1 of the working directory. (To be more precise, if we detect that the first parent of the analyzed sha1 is the head of the PR, then we use the head of the PR.) If the analysis sees a different sha1, and ends up not decorating the head of the PR but something else, then it will not have a visible effect.

If you can locate the sha1 that the analysis ran on, and it’s different from the head, you can verify that it got decorated with:

curl https://api.github.com/repos/folio-org/ui-licenses/commits/THESHA1/check-runs \
  -H 'Accept: application/vnd.github.antiope-preview+json'  | jq .

Another thing, I was looking for the scanner logs, but I couldn’t find it. Can you please help me find it?

About the background tasks page, yes I meant if there are any errors. If there are none, then I don’t expect clues from there.

dcrossley · February 14, 2019, 2:07pm

Regarding the “scanner logs”, the route is described above. Here is the
direct link
Then open the “Shell script” toggle to see the output.

Or the full console output from Jenkins might be easier. Here is its direct link

dcrossley · February 14, 2019, 4:06pm

Thanks Janos, we are investigating the Checkout stage and its various SHA1s and will return with our findings.

There is only one actual commit in that PR: d6d29b925699b4ea5c0bee7c876b7d42214b4e18 and we are repeating locally the steps that Jenkins did.

dcrossley · February 18, 2019, 3:09am

My colleague has made some changes which have improved our situation. Summary:

A) Our JavaScript-based PRs were not being decorated with the Sonar check result. This behaviour was happening consistently.

Solution: Following our Jenkins Checkout stage and before the Sonar stage we have a SetEnvironment stage. One of its actions does ‘npm version’ and this by default was making an extra commit, which explains why the SHA1 was different to the analysed SHA1, and why the ‘curl’ command provided above was returning empty. Instead we now do ‘npm --no-git-tag-version version’ so no extra commit is made during the build. Now for the PRs, the Checks tab and the “Show all checks” panel of the Conversation tab are both decorated.

B) For both our JavaScript-based and Maven-based branch builds, we were getting this warning:

Could not find ref 'master' in refs/heads or refs/remotes/origin.
You may see unexpected issues and changes.
Please make sure to fetch this ref before pull request analysis.

Solution: Unlike PR builds in our Jenkins, ref ‘master’ is indeed not fetched on branch builds – only the ref of the branch is fetched. But Sonar now requires ref ‘master’ when scanning branch builds. So doing the following before running Sonar, and now no Warning:

git fetch --no-tags ${env.projUrl} +refs/heads/master:refs/remotes/origin/master

dcrossley · February 18, 2019, 3:11am

C) Our Maven-based PRs were mostly being decorated with the Sonar check result. However this was not happening consistently – occasionally there was no decoration (an example PR).

Note that that was prior to doing item B above (if that makes a difference).

I will be monitoring both JavaScript-based and Maven-based PRs over the next few days.

dcrossley · February 18, 2019, 3:14am

A related question:

While a PR is still open, the Sonar check result is listed in the “Show all checks” panel of the Conversation tab. However after the PR is merged, the “View details” panel of the Conversation tab does not show the Sonar check (an example PR). Is that normal behaviour?

dcrossley · February 19, 2019, 9:16am

Drat. We are still seeing some undecorated PRs since the fixes described above. Out of 38 PRs assessed since then (start:20190215 15:13 UTC … end:20190219 00:48 UTC) there were 8 not decorated. So better, but it is erratic.

An example JavaScript-based (email notification for opened PR: Mon, 18 Feb 2019 08:11:53 -0800)
An example Maven-based (email notification for opened PR: Mon, 18 Feb 2019 08:30:14 -0800)
Sequential. Other repos close to that time, both before and after were okay. And mod-inventory-storage PRs had other successfully decorated PRs during the time range mentioned at the start of this Comment.

The Admin page for those two shows no problems with “Background Tasks”.

dcrossley · February 22, 2019, 7:37am

Assessed the next batch of 44 PRs, and 4 of them failed to decorate the PR. Their analysis was run, just not reported back to PR. Other PRs for those repositories in the same period were properly decorated.

So configuration and everything else must be correct.

It is definitely intermittent.

Grasping at straws now, but could it be that the call from SonarCloud back to GitHub has some network failure and Sonar does not detect an error for that.

janos · February 22, 2019, 8:01am

Thanks @dcrossley, your investigation and messages are very clear and helpful. We are investigating at our side to identify the causes, and will get back to you soon.

dcrossley · February 22, 2019, 9:31am

Glad that we can help each other. If there is any more information that can be gathered, then please say.

dcrossley · February 26, 2019, 6:33am

Hey @Janos We reckon that we have solved the apparent “intermittent” decoration of the PR Checks tab.

After creating a branch for a PR, then the master might be subsequently updated. When our Jenkins is preparing for the sonar scan, the master is merged into the PR head. That of course can make a different SHA1 which does not correlate with the SHA1 that is being decorated with the Sonar result.

All repositories will now have the branch protection setting “Require branches to be up to date before merging”.

dcrossley · February 26, 2019, 6:35am

To summarise, there were three parts:

An extra commit was being made by our Jenkins when preparing for the scan by doing ‘npm version’. Instead do ‘npm --no-git-tag-version version’.
Remove the Warning note Could not find ref 'master' in refs/heads or refs/remotes/origin by doing the following before doing the Sonar scan:

git fetch --no-tags ${env.projUrl} +refs/heads/master:refs/remotes/origin/master

Add the branch protection setting “Require branches to be up to date before merging”.

janos · February 26, 2019, 8:53am

Thanks @dcrossley for the clear summary and conclusion.

At our side, we have the following in progress:

Improve our monitoring, to make it easier in the future to match strangely behaving analyses with our logs, and making it easier to track down the root cause.
Improve the way we decorate, to avoid such confusion as you experienced in this thread. Ticket to track: SONARCLOUD-467