jenkins: post-build-status-update

[maclover7]

This creates a new post-build-status-update Jenkins Pipeline. It's job
is to propogate build information from Jenkins CI to the GitHub Bot to
GitHub. The pipeline created here will be called from node-test-commit-*
sub build jobs through the Parameterized Trigger Plugin.

The way this would be used is by creating pre and post build triggers
through the plugin, which would call this pipeline. My suggestion is
that we woll this out to one or two jobs (maybe the linter?), try it
out, and then after that roll it out to all sub builds.

Hopefully, this PR should finally fix the yellow CI statuses on GitHub!

Refs: #790

[maclover7]

I tried this out on ci.nodejs.org, and it works -- check out nodejs/node#14998 and nodejs/node#16703.

[rvagg]

Where will this end up being run? I guess we need to add the worker IP addresses to the bot whitelist? We do all of the work on test-packetnet-ubuntu1604-x64-1 and test-packetnet-ubuntu1604-x64-2, if we're able to restrict this work to those then that'd be easy I suppose.

[maclover7]

We do all of the work on test-packetnet-ubuntu1604-x64-1 and test-packetnet-ubuntu1604-x64-2

Yep -- all (26) of the builds so far have taken place on either of these two machines. These are the only two jenkins-workspace tagged machines, so we would only need to adjust the bot whitelist if more are added with the label. It seems like the bot whitelist doesn't need to be edited as of right now since the requests all went through ok.

[rvagg]

great @maclover7, what are the next steps, do we merge this, add a github-based pipeline for it, add that plugin and wire it up?

[maclover7]

@rvagg yep, here are the next steps:

• merge this pr
• create jenkins pipeline job backed by post-build-status-update.jenkinsfile (there is already a post-build-status-update job, imho we should just delete that and start from scratch, don't want anything edited in web interface to carry over)
• add the plugin
• go to linter job, and add new build step to call the pipeline before/after linting occurs, wrapped in a POST_STATUS_TO_PR parameter conditional step
• let it run for a day or two, and if all goes well, add it to the remaining jobs

[rvagg]

ok, unless anyone @nodejs/build objects I'd be happy to push forward with @maclover7 on this plan today(ish)

[refack]

Do you know if there's a better way to report ⚠️ ?

these three sub-jobs are actually yellow in Jenkins.

If there's no direct mapping IMHO they should be marked as green.

[maclover7]

updated @gibfahn

@refack Do you know what's making it be unstable? I think the issue is parallel/test-async-wrap-uncaughtexception or similar flaky tests failing? Do we normally consider those builds to be "passing"?

[refack]

Do we normally consider those builds to be "passing"?

Yes, we map Jenkin's "unstable" status to runs that fail on only known flaky tests. Which means "failures are unrelated to current PR", so AFAICT they should be reported as passing for the PR (could have a "some tests were flaky" message)

[maclover7]

@refack Ok -- I've added a commit where if the status is unstable then a different message will be shown, but it will be shown as passing in the GitHub UI.

[maclover7]

Did a run of the jenkinsfile, and nodejs/node#14998 test/aix has a green light but a message saying "flaky tests failed".

[apapirovski]

Just found this and wanted to give props for the work! Thanks @maclover7 🙏 💯

[refack]

Suggestion: I'd put the file in a new /jenkins/pipelines/ directory

[maclover7]

Going to merge this in -- it looks like the plugin is already installed on ci.nodejs.org (yay!), so I just need to have write access to node-test-linter to start rolling this out :)

[maclover7]

Landed in 58019a5, going to keep this PR open until node-test-linter is setup :)

[refack]

Landed in 58019a5, going to keep this PR open until node-test-linter is setup :)

FYI since we don't do auditing or backporting, you can just use the Green button to "rebase merge" if the commits are good as is, or "squash merge" is you want to edit the commit message.

[phillipj]

Great job finding a way for us to get up-n-running again!

[maclover7]

Looks like everything is going okay right now... assuming no failures, would someone be able to set me up with access to configure the remaining node-test-* jobs? I can rollout the rest tomorrow or Monday, would be great to wrap this up.

[gibfahn]

would someone be able to set me up with access to configure the remaining node-test-* jobs?

Hopefully done, let me know if I missed any.

[maclover7]

@gibfahn thanks! Here's the current upgrade status:

• node-test-linter
• node-test-commit-arm -- unused?
• node-test-commit-freebsd
• node-test-commit-linux
• node-test-commit-osx -- no permissions
• node-test-commit-plinux -- no permissions
• node-test-commit-smartos -- no permissions
• node-test-commit-windows-fanned -- no permissions
• node-test-commit-linux-fips
• node-test-commit-linuxone
• node-test-commit-arm-fanned
• node-test-commit-aix

Could use permissions on a few more jobs, and then not sure what should be done with node-test-commit-arm

[refack]

I added nodejs*build access to

• node-test-commit-osx
• node-test-commit-plinux
• node-test-commit-smartos
• node-test-commit-windows-fanned

[maclover7]

thanks @refack, updating now

[maclover7]

Settings updated -- going to close and move discussion back to #790.

[MylesBorins]

Great work!!!

[phillipj]

@maclover7 just noticed this in the bot logs:

07:39:51.639Z ERROR bot: Got error when retrieving GitHub commits for PR (req_id=ec2629b5-a83f-4c5b-baa3-9408674e316c, pr=canary, job=test/linux, status=success)
    err: {
      "code": "400",
      "status": "Bad Request",
      "message": "Invalid value for parameter 'number': canary"
    }

..

07:43:44.384Z ERROR bot: Got error when retrieving GitHub commits for PR (req_id=5b885da5-728a-4757-9d78-38bd0918c41e, pr=canary, job=test/linux-fips, status=success)
    err: {
      "code": "400",
      "status": "Bad Request",
      "message": "Invalid value for parameter 'number': canary"
    }

Got any thoughts why some jobs seems to be posting canary as the pr-parameter, rather than a numeric value?

[rvagg]

That'd be from the scheduled daily jobs. They don't have an associated PR and aren't started from node-test-pull-request. In fact some collaborators (myself included) often just use node-test-commit or one of its children directly without giving a PR. So maybe just check if the PR value is numeric or not before posting to the bot?

[refack]

There's also POST_STATUS_TO_PR which the job now ignores (which IMHO is fine), so I'd say either check it or we should remove it.

[maclover7]

Yeah, I was worried this would happen. I tried locally to add a conditional step as a wrapper around triggering post-build-status-update jobs, but it seemed to execute after every shell block, so I didn't end up using that on ci.nodejs.org.

[maclover7]

Hmm, actually, what do you think about passing the POST_STATUS_TO_PR variable to post-build-status-update, and that will figure out if github-bot should be contacted or not?

[gibfahn]

+1 on checking POST_STATUS_TO_PR == "true", and +1 on checking PR.isInteger()

@maclover7 correct me if I'm wrong, but I think #981 will work.