[FOLIO-1763] Sonarcloud analysis not updating GitHub PR and Checks Created: 31/Jan/19  Updated: 03/Jun/20  Resolved: 28/Feb/19

Status: Closed
Project: FOLIO
Components: None
Affects versions: None
Fix versions: None

Type: Bug Priority: P2
Reporter: John Malconian Assignee: David Crossley
Resolution: Done Votes: 0
Labels: ci, platform-backlog
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original estimate: Not Specified

Issue links:
Blocks
is blocked by FOLIO-1828 Configure each GitHub repository "Req... Closed
Relates
relates to FOLIO-1823 Spike: investigate running our own So... Closed
Sprint: Core: Platform - Sprint 56, Core: Platform - Sprint 58, Core: Platform - Sprint 57
Story Points: 5
Development Team: Core: Platform

 Description   

I've received and confirmed multiple cases in which results of Sonarqube analysis are not updated on Github PRs even though the analysis is successfully run and processed by Sonarcloud. Symptoms of this include a Sonarcloud status of "queued" on Github. The issue appears to be intermittent and may indicate a bug on the Sonarcloud end.

The Maven-based scans do update the Checks tab. However their scans seem erratic.



 Comments   
Comment by John Malconian [ 04/Feb/19 ]

I notice that this ui-requests PR doesn't invoke the Sonarqube scan - https://jenkins-aws.indexdata.com/job/folio-org/job/ui-requests/job/PR-234/2/ - So that's an obvious problem to investigate first, I think.

Comment by John Malconian [ 04/Feb/19 ]

I see the following warning in Sonarcloud for branches of UI projects. Not sure what is going here.

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.

https://sonarcloud.io/project/issues?branch=stcom-365-mcl-aria-roles&id=org.folio%3Astripes-smart-components&resolved=false

Comment by John Malconian [ 04/Feb/19 ]

This seems to resemble some of the issue we are seeing:

https://community.sonarsource.com/t/pr-decoration-for-github-no-longer-working/5875/7

Comment by David Crossley [ 05/Feb/19 ]

Regarding the part above about "ui-requests PR doesn't invoke the Sonarqube scan".
Would that be because for ui-* modules the "config.runSonarqube" is false by default, and ui-requests/Jenkinsfile does not switch it on. Whereas for example ui-agreements does have it true, and its scan did run on a recent PR.

Comment by John Malconian [ 05/Feb/19 ]

That sounds about right. Unlike the Maven modules, Sonarqube analysis is an optional config parameter for Stripes/UI modules that is false by default (although it probably should be true by default).

Aside from the ui-requests issue, however, I think that I am seeing that no Sonarqube analysis is actually processed on Sonarcloud for any of the UI/Stripes modules on non-master branches and pull requests. For example:

https://sonarcloud.io/project/issues?branch=UICHKIN-75&id=org.folio%3Aui-checkin&resolved=false

Sonarqube reports "no issues", but I don't think any thing is actually processed.

Also, for Maven modules, Sonarcloud processing seems hit or miss.

The problems started to occur two-three weeks ago after Sonarcloud made some significant changes to how results are analyzed and how Github PRs are decorated. There may be some hints at the end of this thread - https://community.sonarsource.com/t/pr-decoration-for-github-no-longer-working/5875/7 - but we may need to post an inquiry to the community board.

Comment by David Crossley [ 05/Feb/19 ]

Here is another useful report, having similar symptoms, and active today:
https://community.sonarsource.com/t/sonarcloud-doesnt-update-github-status/6325

Comment by David Crossley [ 05/Feb/19 ]

So how are the Sonar scans run for those other ui-* modules that are not configured in their Jenkinsfile.

Comment by David Crossley [ 05/Feb/19 ]

I confirm that processing seems to be not happening for non-master branches and PRs. Looking at ui-circulation today, there are 17 issues reported for a master branch build, but none for the recent PR and its branch.

Comment by David Crossley [ 05/Feb/19 ]

The linked community posts mention ensuring doing 'git fetch' before running sonar-scanner. We use the Jenkins "checkout" function, so i presume that it is doing that.

Investigated our Jenkins "Checkout" stage to try to find any difference between the Maven-based builds and the NPM-based builds. The vars/buildNPM.groovy has an extra section "RelativeTargetDirectory" added a couple of months ago, but only merged 9 days ago with jenkins-pipeline-libs/pull/57 ( FOLIO-1596 Closed ).

I am not clear what that does, but wonder if that could that be the cause?

Comment by David Crossley [ 07/Feb/19 ]

Here is our post today to the SonarSource Community forum:
https://community.sonarsource.com/t/sonarcloud-now-not-updating-github-pr-and-now-only-running-on-master-branch/6595

Comment by David Crossley [ 08/Feb/19 ]

We indicated above that sonar is not running on non-master branches and PRs. That seems to be not true. See this one today where i added deliberate new "code smell":
https://github.com/folio-org/ui-licenses/pull/62
... it is detecting the new issues.

So i adusted this issue Summary, and the post title:
https://community.sonarsource.com/t/sonarcloud-now-not-updating-github-pr-and-checks/6595

He is still puzzled about why the Sonar result is not reported back to GitHub Checks for the NPM-based builds.

He asked if it is happening consistently. I said yes. If anyone sees any differently then please show an example branch/PR.

Comment by John Malconian [ 15/Feb/19 ]

David Crossley I believe I've made some significant progress on this issue.

Github PR decoration for NPM-based builds

There is a command ('npm version') in the NPM build pipeline that is executed prior to Sonarqube analysis that makes a git commit on the Jenkins checkout during the build. This commit updates the SHA1 of HEAD so that it no longer matches the original PR SHA1 on Github and, therefore, confuses Sonarqube. I've added the option '--no-git-tag-version' to the command in the shared pipeline libraries so that no git commit is made during the build. The result is that SonarCloud GitHub PR decoration now works on NPM-based PRs.

The Sonarcloud/Sonarqube Scanner 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" that occurs during branch builds on all projects.

This warning totally makes sense, because, unlike PR builds in Jenkins, ref 'master' is indeed not fetched on branch builds - only the ref of the branch is fetched. But Sonarqube now requires ref 'master' when scanning branch builds. The solution was to add the command

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

prior to invoking the Sonarqube scan during branch builds (for both Maven and NPM-based builds). After this modification, I've verified that the warning is no longer generated.

I've merged these changes into jenkins-pipeline-libs in PR-58. https://github.com/folio-org/jenkins-pipeline-libs/pull/58. See the PR for exact code changes.

I'm going to keep this issue open so that you can monitor the status of these changes over the next few days. It's still not entirely clear to me why some Maven-based PRs are intermittently not decorated and if that issue is related to either I've the changes I've made or something else entirely. When/If you feel the issues have been adequately resolved, then go ahead and close this issue.

Comment by David Crossley [ 18/Feb/19 ]

That is great news. I have done various PRs today, and the Sonar checks are behaving. Will monitor further.

If developers see different behaviour then please tell me. On an open PR, the Sonar check result should reported to the "Show all checks" panel of the Conversation tab, and to the actual "Checks" tab (need to refresh page to see the latter).

Comment by David Crossley [ 18/Feb/19 ]

Updated our SonarSource post:
https://community.sonarsource.com/t/sonarcloud-now-not-updating-github-pr-and-checks/6595

Comment by David Crossley [ 19/Feb/19 ]

Drat.

Assessed 38 pull requests since the jenkins-pipeline-libs/pull/58 was merged to master.
(start:20190215 15:13 UTC .. end:20190219 00:48 UTC)

30 had the decoration of the Sonar result. 8 did not.

Of those 8:
ui-marccat had 4/4 did not decorate
mod-marccat had 2/3 did not decorate

Apart from those 6, the other two were:

https://github.com/folio-org/ui-checkin/pull/164 (email notification for opened PR: Mon, 18 Feb 2019 08:11:53 -0800)
https://github.com/folio-org/mod-inventory-storage/pull/246 (email notification for opened PR: Mon, 18 Feb 2019 08:30:14 -0800)
Sequential. Others 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.

Following through to Sonarcloud as Admin for those two repos shows no problems for "Background Tasks".

Comment by Aliaksei Chumakou [ 20/Feb/19 ]

David Crossley it looks like SonarCloud test coverage on the new code had stopped working for ui-orders a while ago (always 0.0%).
https://github.com/folio-org/ui-orders/pull/266
https://sonarcloud.io/component_measures?id=org.folio%3Aui-orders&pullRequest=266

Comment by David Crossley [ 21/Feb/19 ]

Aliaksei Chumakou Not sure yet if that "coverage on the new code" is related to this FOLIO-1763 Closed . I added a new issue at FOLIO-1818 Closed .

Comment by David Crossley [ 21/Feb/19 ]

Assessed the next 46 PRs for relevant repos (up to 20190220 23:43 UTC).
Apart from 2/2 mod-marccat, these 4 others failed to decorate the PR:

https://github.com/folio-org/mod-circulation/pull/190
https://github.com/folio-org/ui-circulation/pull/255
https://github.com/folio-org/ui-erm-usage/pull/47
https://github.com/folio-org/ui-licenses/pull/96

For each of those 4 their analysis was run, just not reported back to PR.

Comment by David Crossley [ 22/Feb/19 ]

Updated our SonarSource post:
https://community.sonarsource.com/t/sonarcloud-now-not-updating-github-pr-and-checks/6595

The helpful Janos at SonarSource replied to my recent summaries. They are investigating at their end.

It is a great benefit of truly open source that we can actually demonstrate our findings to the post.

Comment by David Crossley [ 25/Feb/19 ]

Found some documentation about this feature change:
https://community.sonarsource.com/t/pr-analysis-results-are-now-displayed-in-the-checks-tab-of-pull-requests-in-github/5870

As explained there, now the results are summarised and displayed on the Checks tab of the PR. Sonar no longer displays each detected code issue on the front Conversation tab of the PR.

Comment by David Crossley [ 26/Feb/19 ]

In our SonarSource post linked above, Janos provided a 'curl' query to verify the decoration. Adding the pattern here for future reference:

curl https://api.github.com/repos/folio-org/<repo-name>/commits/<sha1-of-last-commit-in-the-set>/check-runs \
  -H 'Accept: application/vnd.github.antiope-preview+json' | jq .
Comment by David Crossley [ 26/Feb/19 ]

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".

Comment by David Crossley [ 26/Feb/19 ]

To summarise, there were three parts:

1) 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'.

2) 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

3) Add the branch protection setting "Require branches to be up to date before merging".

Comment by David Crossley [ 26/Feb/19 ]

Now "In review".

Will monitor the PRs for the next day.

Comment by David Crossley [ 28/Feb/19 ]

At our SonarSource post:
https://community.sonarsource.com/t/sonarcloud-now-not-updating-github-pr-and-checks/6595
Janos said:

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

Even if we sign up, cannot see that ticket. So watch the SonarSource Announcements forum, e.g. there is a beta of automatic analysis.

Comment by David Crossley [ 28/Feb/19 ]

Assessed our next 23 relevant PRs from yesterday, since FOLIO-1828 Closed to require branch be up-to-date.
All did decorate the Checks tab with the Sonar result.

Some others were still open PRs which did not have the Checks tab with Sonar report. This was because they had not yet done the up-to-date branch requirement.

All is now well, so closed this ticket.

Generated at Thu Feb 08 23:15:44 UTC 2024 using Jira 1001.0.0-SNAPSHOT#100246-sha1:7a5c50119eb0633d306e14180817ddef5e80c75d.