Skip to content

Add support for launching EC2 runners - #786

Merged
Kobzol merged 9 commits into
mainfrom
ec2-runners
Jul 29, 2026
Merged

Add support for launching EC2 runners#786
Kobzol merged 9 commits into
mainfrom
ec2-runners

Conversation

@Kobzol

@Kobzol Kobzol commented Jul 23, 2026

Copy link
Copy Markdown
Member

To use optimized EC2 instances for certain CI jobs.

Comment thread src/ec2/ec2-runner-script.sh Outdated
Comment thread src/ec2/ec2-runner-script.sh Outdated
usermod -a -G docker ubuntu
systemctl start docker
sudo --login -u ubuntu bash -c 'mkdir actions-runner'
sudo --login -u ubuntu bash -c 'cd actions-runner && curl -o runner.tar.gz -L https://github.com/actions/runner/releases/download/v2.335.1/actions-runner-linux-x64-2.335.1.tar.gz'

@Mark-Simulacrum Mark-Simulacrum Jul 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the runner will self-update, but we'll probably want to figure out a way to bump the tag here automatically (e.g., https://github.com/actions/runner/releases/tag/v2.336.0 has been released since this was written).

I'm not sure if we can just always use latest or not, this sort of implies no:

Note: Actions Runner follows a progressive release policy, so the latest release might not be available to your enterprise, organization, or repository yet.

IIRC with the old setup we periodically broke and had to bump this constant to unbreak ourselves (30 days after the new release), but there we had disabled the self-update...

View changes since the review

Comment thread src/ec2/mod.rs
.arg("--instance-type")
.arg(instance_type)
.arg("--launch-template")
.arg("LaunchTemplateName=gha-runner,Version=14")

@Mark-Simulacrum Mark-Simulacrum Jul 25, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a FIXME or some other tweak to use Latest here before we ship to production.

View changes since the review

Comment thread Cargo.toml Outdated
@Kobzol
Kobzol marked this pull request as ready for review July 27, 2026 15:28
Comment thread src/ec2/mod.rs
# Note: the runner should self-update
sudo --login -u ubuntu bash -c 'cd actions-runner && curl -o runner.tar.gz -L https://github.com/actions/runner/releases/download/v2.335.1/actions-runner-linux-x64-2.335.1.tar.gz'
sudo --login -u ubuntu bash -c 'cd actions-runner && tar xzf runner.tar.gz'
sudo --login -u ubuntu bash -c 'cd actions-runner && ./run.sh --jitconfig "$JITCONFIG"'

@Mark-Simulacrum Mark-Simulacrum Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess this script doesn't have set -euxo pipefail right now, so even if this all fails we should still reach the shutdown... I think we should leave this as-is for now but I'll continue poking at the AMI stuff so we can remove some of the fallibility in the early parts of this.

View changes since the review

sudo --login -u ubuntu bash -c 'mkdir actions-runner'

# Note: the runner should self-update
sudo --login -u ubuntu bash -c 'cd actions-runner && curl -o runner.tar.gz -L https://github.com/actions/runner/releases/download/v2.335.1/actions-runner-linux-x64-2.335.1.tar.gz'

@Mark-Simulacrum Mark-Simulacrum Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's leave this as-is, it should exercise self-updates, but I suspect we'll want to find a way to continuously bump this. Probably easier to do once we have a dedicated workflow for building the AMI in a separate repo, maybe we can get renovate to do something...

View changes since the review

Comment thread src/ec2/mod.rs

let output = run_command(&mut ec2_cli)
.await
.context("Cannot start ec2 instance")?;

@Mark-Simulacrum Mark-Simulacrum Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this "leak" the raw error from EC2 to github comments or similar? I think there shouldn't be anything too interesting there but maybe worth logging it just to bors's logs rather than externally.

View changes since the review

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, background job errors are never posted as GitHub comments. Even errors during command handling do not expose the inner error details, those are only logged.

Comment thread src/ec2/mod.rs
Comment thread src/ec2/mod.rs
.as_str()
.unwrap_or("unknown instance id")
);

@Mark-Simulacrum Mark-Simulacrum Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would be worth capping the number of instances we're willing to run somehow... not sure exactly how we'd do that though. I think we'll hit account quota limits at 640 vCPUs (~13 12xlarge instances) so maybe that's good enough? I think we can circle back to this if we run into issues in practice.

View changes since the review

Comment thread src/config.rs
Comment thread src/config.rs
/// Allowed instance types that can be launched.
/// If empty, bors will launch any instance specified in the `runs-on` field.
#[serde(default)]
pub allowed_instances: Vec<String>,

@Mark-Simulacrum Mark-Simulacrum Jul 28, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, this is read from the main branch, right? It's a bit of a pain that we'd need to land a PR to allow experimentation with a new instance type (or use infra bypass to get the token like I did originally). It's probably OK though, we can either remove the restriction or figure out some way to relax it if we need to later.

View changes since the review

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can experiment in bors-kindergarten, there a change of the config is a push to main, which is ~instantly propagated to bors (well, takes up to 15 minutes without restarting bors, but that's enough for experimentation, I think).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Kindergarten doesn't let us evaluate runner duration on jobs from r-l/r though, right?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's true. We can hardcode a set of default options later. Getting a PR merged here and redeploying production bors should be much faster than landing a rust-lang/rust PR 😆

@Kobzol

Kobzol commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

In addition to this PR, we need to:

  • Configure Workflow job webhook events for staging/production bors GitHub App.
  • Configure organization permission Self-hosted runners (write) for staging/production bors GitHub App.

Can you configure it, please?

@Mark-Simulacrum

Copy link
Copy Markdown
Member

Configured both staging and prod with the relevant permissions, I think.

@Kobzol
Kobzol added this pull request to the merge queue Jul 29, 2026
Merged via the queue into main with commit 504a2c8 Jul 29, 2026
5 checks passed
@Kobzol
Kobzol deleted the ec2-runners branch July 29, 2026 06:52
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.

2 participants