WordPress-iOS icon indicating copy to clipboard operation
WordPress-iOS copied to clipboard

Explicitly set mac queue in Buildkite pipeline

Open mokagio opened this issue 1 year ago • 3 comments

We are DRYing the pipeline upload step in our internal Infrastructure as Code (IaC) setup, see https://github.com/Automattic/buildkite-ci/pull/452/files#r1652113066, which in turns will allow us to do some useful things with metadata, see https://github.com/Automattic/buildkite-ci/pull/459.

However, some pipeline, such as the one here, were configured to set agents: queue: mac at upload time, blocking them from adopting the DRY upload step.

This is PR sets the queue at the pipeline level, making it safe to adopt the standard upload step.

Notice that the the only pipeline file to change is pipeline.yml. The other one already had to explicitly set their agents value because they are all started via API, thus bypassing the default upload step and not inheriting the agents value set in it.

Testing

If CI is green, then we're good.

Regression Notes

  1. Potential unintended areas of impact – N.A.
  2. What I did to test those areas of impact (or what existing automated tests I relied on) – N.A.
  3. What automated tests I added (or what prevented me from doing so) – N.A.

PR submission checklist:

  • [x] I have completed the Regression Notes.
  • [x] I have considered adding unit tests for my changes. N.A.
  • [x] I have considered adding accessibility improvements for my changes. N.A.
  • [x] I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary. N.A.

UI changes testing checklist: Not a UI PR.

mokagio avatar Jul 01 '24 01:07 mokagio

WordPress Alpha📲 You can test the changes from this Pull Request in WordPress Alpha by scanning the QR code below to install the corresponding build.
App NameWordPress Alpha WordPress Alpha
ConfigurationRelease-Alpha
Build Numberpr23402-f05f525
Version25.1
Bundle IDorg.wordpress.alpha
Commitf05f525120cc1548019878977a11b332fe850cee
App Center BuildWPiOS - One-Offs #10252
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

wpmobilebot avatar Jul 01 '24 01:07 wpmobilebot

Jetpack Alpha📲 You can test the changes from this Pull Request in Jetpack Alpha by scanning the QR code below to install the corresponding build.
App NameJetpack Alpha Jetpack Alpha
ConfigurationRelease-Alpha
Build Numberpr23402-f05f525
Version25.1
Bundle IDcom.jetpack.alpha
Commitf05f525120cc1548019878977a11b332fe850cee
App Center Buildjetpack-installable-builds #9301
Automatticians: You can use our internal self-serve MC tool to give yourself access to App Center if needed.

wpmobilebot avatar Jul 01 '24 01:07 wpmobilebot

The other one already had to explicitly set their agents value because they are all started via API, thus bypassing the default upload step and not inheriting the agents value set in it.

This is not technically true. When we start a build via API, it still runs the root steps of the pipeline, i.e. the steps set in the .tf file. It's just that the API call we do also sets the PIPELINE env var to the name of the yml file we want to upload, and since the steps set in the .tf file look like buildkite-agent pipeline upload ${PIPELINE:-pipeline.yml}, then that pipeline upload command will upload the provided YML file set by PIPELINE instead of uploading the default pipeline.yml file when that env var is not set (e.g. like it does on a regular build triggered by commit push).

So TL;DR the default upload steps in the .tf are still run (and not bypassed as you suggest) even during builds triggered by API calls. So if e.g. we were to keep agents: in the steps of the .tf and remove it from all the .yml pipelines (even the ones triggered by API), this would still work (not that it's what we want to do, on the contrary, but just saying to illustrate the point)

AliSoftware avatar Jul 01 '24 07:07 AliSoftware

This is not technically true

Right! Thank you for the clarification. I should have consulted the API docs.

mokagio avatar Jul 03 '24 02:07 mokagio

Side note that you'll have to make sure this change is cherry picked to all existing open branches — in particular release/* too — before you can remove it from the .tf!

AliSoftware avatar Jul 03 '24 02:07 AliSoftware