WIP: tests: use onebranch amd64 runner for go build tasks#587
Conversation
|
/AzurePipelines run [GITHUB]-trident-pr-e2e |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR updates CI and local build behavior to stop relying on a custom hosted amd64 pool for Go build tasks and instead use the default OneBranch Linux runner, while also adjusting installer tool builds for Azure Linux environments.
Changes:
- Switch amd64 pipeline jobs to rely on the default OneBranch Linux pool by removing explicit
name/hostArchitecturesettings. - Update Makefile installer tool builds to use
GOEXPERIMENT=ms_nocgo_opensslcryptowhen running on Azure Linux, otherwise continue usingCGO_ENABLED=0.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| Makefile | Adds Azure Linux detection to toggle Go build environment variables for installer tool binaries. |
| .pipelines/templates/stages/building_tools/building-tools.yml | Removes explicit amd64 pool selection to use the default Linux runner for building Go tools. |
| .pipelines/templates/stages/azl_installer/build-installer-tools.yml | Removes explicit amd64 pool selection to use the default Linux runner for installer tool builds. |
| if cat /etc/os-release | grep -i azurelinux >/dev/null 2>&1; then \ | ||
| GOEXPERIMENT=ms_nocgo_opensslcrypto go build -o $(CURDIR)/$(INSTALLER_OUT_DIR)/liveinstaller; \ | ||
| else \ | ||
| CGO_ENABLED=0 go build -o $(CURDIR)/$(INSTALLER_OUT_DIR)/liveinstaller; \ | ||
| fi |
There was a problem hiding this comment.
The OS detection pipeline uses cat /etc/os-release | grep ... but only redirects grep’s output; if /etc/os-release is missing (e.g., non-Linux dev env), cat will still emit an error to stderr, adding noisy/confusing output to this target.
Suggestion: avoid cat and run grep directly against the file (and/or redirect file-read errors) so the check is quiet when the file doesn’t exist.
| if cat /etc/os-release | grep -i azurelinux >/dev/null 2>&1; then \ | ||
| GOEXPERIMENT=ms_nocgo_opensslcrypto go build -o $(CURDIR)/$(INSTALLER_OUT_DIR)/attendedinstaller-simulator attendedinstaller_simulator.go; \ | ||
| else \ | ||
| CGO_ENABLED=0 go build -o $(CURDIR)/$(INSTALLER_OUT_DIR)/attendedinstaller-simulator attendedinstaller_simulator.go; \ | ||
| fi |
There was a problem hiding this comment.
Same issue as above: the /etc/os-release check will still print cat errors to stderr if the file is absent because only grep output is redirected.
Suggestion: use grep -qi ... /etc/os-release 2>/dev/null (or similar) to keep this check quiet outside Azure Linux.
|
|
||
|
|
There was a problem hiding this comment.
There’s trailing whitespace / an extra blank indented line here, which can create noisy diffs and fails some whitespace linters.
Suggestion: remove the trailing tab/whitespace on this blank line.
bfjelds
changed the title
🔍 Description
We are currently using our own hosted pool for go builds. This leads to issues when onebranch decides to change things. It is better to use the default test runner if possible.