Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add M1 pipeline on Buildkite #672

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Add M1 pipeline on Buildkite #672

wants to merge 2 commits into from

Conversation

igor-makarov
Copy link
Contributor

@igor-makarov igor-makarov commented Jan 24, 2021

Following discussions in #669, @dcvz has volunteered a Buildkite agent running on an M1 Mac.

This PR adds the pipeline that runs tests on it (currently failing because of Typhoeus).

Things to figure out before merge:

  • @dcvz - as the owner of the machine, make sure the pipeline is configured in a clean way that doesn't interfere with the other uses for the machine. The gems are installed into vendor/bundle to minimize interference.
  • @dnkoutso are we fine with merging a PR that has failing builds, assuming there's another PR that's going to address it?
  • I've emailed Buildkite support to enable the open source plan that their pricing page touts. Should we wait for that before merging?

@igor-makarov igor-makarov force-pushed the add-buildkite-m1 branch 2 times, most recently from af26a60 to c3d1cba Compare January 24, 2021 10:39
@dcvz
Copy link
Contributor

dcvz commented Jan 24, 2021

Looks good from my end @igor-makarov! Since this adds support for testing on the M1, is there any reason to not introduce these changes in the branch that aims to make this repo support M1?

It might make more sense than merging this into master and expecting that most branches will fail for now.. if its introduced along with #669 then this would get merged and from there on you would expect PR's to be green for M1.

@igor-makarov
Copy link
Contributor Author

igor-makarov commented Jan 24, 2021

@dcvz The idea behind it is having small, granular PRs and separating concerns between them.

#669 is motivated by Apple Silicon - but isn't directly scoped to it, as the HTTP2 client replacement happens for all architectures.

Additionally, having the pipeline fail on M1 right now provides us with more certainty that #669 actually resolves the problem.

@endocrimes
Copy link
Member

I'm not sure of the default settings, but we might want to make sure that these builds won't run on forked PRs. Buildkite's security model (mixed with us running these directly on the host), means we can't safely allow 3rd party jobs to run without at least some manual verification.

The issues mostly being:

  • Buildkite exposing long lived access tokens to the job fairly trivially (mostly ok because we don't put any secrets in BK)
  • Unconstrained execution on the host means that someone being malicious can run any arbitrary software, including taking over access to the mini

@dcvz
Copy link
Contributor

dcvz commented Jan 24, 2021

@endocrimes thanks for the input! the BK settings are already not building 3rd party forks (recommended already by BK). I also found this issue on BuildKite for a feature to support this need, but no activity there 😢

@igor-makarov does this repo make use of a bot?

Apart from manually triggering builds for 3rd party PR's, one way would be to ask a bot via a comment from an org member to trigger a CI run on a particular fork. This would allow a maintainer to only trigger a build after review, where no malicious code has been detected.

although perhaps that's overkill for just getting an M1 testing environment... :(

EDIT:
I've also disabled local hooks & plugins on the agent. This should mean that the only scripts executed are those described in the .buildkite/pipeline.yml

@igor-makarov
Copy link
Contributor Author

@dcvz There's no bot running on the M1 - I think the bot credentials are a secret kept in GH Actions only.
@endocrimes I've also checked that the M1 pipeline doesn't run on forked PRs. Frankly, I'd rather get our M1 testing from GH but they aren't providing a timeline for when they're gonna make it available.
Judging by their disappointing rollout of macOS 11, I'm not holding my breath.

@igor-makarov
Copy link
Contributor Author

@dcvz @endocrimes I've tried running an M1 pipeline manually on a PR commit and it ran. I think in most PRs we won't need it and if we do, we'll be able to invoke it (albeit in a complicated way).

@igor-makarov
Copy link
Contributor Author

@endocrimes @dcvz @jasl I ended up disabling M1 runs for now as they keep running on all PRs (and failing). We should enable it once #669 is merged as @dcvz suggested.

@igor-makarov igor-makarov marked this pull request as draft January 24, 2021 20:08
@igor-makarov
Copy link
Contributor Author

Update: Buildkite have approved our free account.

@dnkoutso dnkoutso added this to the 1.11.0 milestone Jan 27, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants