Unify the Trigger build loop for GHA and non-GHA builds #92
Conversation
yosifkit
force-pushed
the
combine-loop
branch
from
88d54f9 to
3511be3
Compare
Jenkinsfile.trigger
Outdated
| propagate: false, | ||
| quietPeriod: 5, // seconds | ||
| ) | ||
| // TODO do something useful with "res.result" (especially "res.result != 'SUCCESS'") |
There was a problem hiding this comment.
This is TODONE (we can just delete the line instead of moving it) 😅
| // TODO do something useful with "res.result" (especially "res.result != 'SUCCESS'") |
There was a problem hiding this comment.
Oops, I deleted the wrong TODO comment. 🙈
Jenkinsfile.trigger
Outdated
| set -Eeuo pipefail | ||
|
|
There was a problem hiding this comment.
This defaults to "the system default shell with set -ex" when a shell isn't specified (and we're not using any Bashisms anymore like we were previously):
| set -Eeuo pipefail | |
| set -u +x | |
Jenkinsfile.trigger
Outdated
| url: res.absoluteUrl, | ||
| endTime: (res.startTimeInMillis + res.duration) / 1000.0, // convert to seconds | ||
| ] | ||
| error() |
There was a problem hiding this comment.
Since we got rid of the echo, we could stuff res.result in here (because otherwise it's an exception without a message, which is kind of silly but we don't really have any more information here than "the job failed" 😂)
| error() | |
| error(res.result) |
Jenkinsfile.trigger
Outdated
| @@ -70,6 +71,7 @@ node { | |||
| | index($arch) | |||
| ) | |||
| ) | |||
| | .payload = (gha_payload | @json) | |||
There was a problem hiding this comment.
I think I'd rather call this gha_payload explicitly, and only generate/set/return it when it's actually necessary (which we can then use further down as the determiner so that this becomes the only block that knows/cares about BASHBREW_ARCH=gha):
| | .payload = (gha_payload | @json) | |
| | if env.BASHBREW_ARCH == "gha" then | |
| .gha_payload = (gha_payload | @json) | |
| else . end |
Jenkinsfile.trigger
Outdated
|
|
||
| for (buildObj in queue) { | ||
| def identifier = buildObj.source.arches[buildObj.build.arch].tags[0] | ||
| if (env.BASHBREW_ARCH == 'gha') { |
There was a problem hiding this comment.
As a more generic way to phrase this (so that BASHBREW_ARCH=gha is embedded only in the jq above):
| if (env.BASHBREW_ARCH == 'gha') { | |
| if (buildObj.build.arch != env.BASHBREW_ARCH) { |
(essentially, if we don't already have an obvious single architecture that matches the one we're building, include the architecture in our "identifier" -- we could also add a | .identifier = ... to the jq above to move this up there too, but maybe that's a change too much for now 😄)
Jenkinsfile.trigger
Outdated
|
|
||
| // "catchError" to set "stageResult" :( | ||
| catchError(message: 'Build of "' + identifier + '" failed', buildResult: 'UNSTABLE', stageResult: 'FAILURE') { | ||
| if (env.BASHBREW_ARCH == 'gha') { |
There was a problem hiding this comment.
| if (env.BASHBREW_ARCH == 'gha') { | |
| if (buildObj.gha_payload) { |
Jenkinsfile.trigger
Outdated
| withEnv([ | ||
| 'json=' + json, | ||
| 'payload=' + buildObj.payload |
There was a problem hiding this comment.
| 'payload=' + buildObj.payload | |
| 'payload=' + buildObj.gha_payload, |
yosifkit
force-pushed
the
combine-loop
branch
from
3511be3 to
740729e
Compare
Jenkinsfile.trigger
Outdated
| withEnv([ | ||
| 'json=' + json, | ||
| 'payload=' + buildObj.gha_payload |
There was a problem hiding this comment.
| 'payload=' + buildObj.gha_payload | |
| 'payload=' + buildObj.gha_payload, |
🙈
Jenkinsfile.trigger
Outdated
| propagate: false, | ||
| quietPeriod: 5, // seconds | ||
| ) | ||
| // TODO do something useful with "res.result" (especially "res.result != 'SUCCESS'") |
yosifkit
force-pushed
the
combine-loop
branch
from
740729e to
95ffa53
Compare
This is an attempt to unify the queue loop. I wanted to do it without the node allocation and
curl(i.e., entirely in java/groovy), but the javaURLclass methods are disallowed in the sandbox (security concern of reading local files of the Jenkins node).So, without installing the HTTP Request plugin, we can't post to a URL without
curl(and thus a node allocation) and it is unfortunately mostly unmaintained: "This plugin is up for adoption! We are looking for new maintainers"#91 will be rebased on this if/when this is merged.