Skip to content

add "cargo xtask local" to run buildomat locally#91

Open
emilyalbini wants to merge 1 commit intomainfrom
ea-kwvtkkzqoztv
Open

add "cargo xtask local" to run buildomat locally#91
emilyalbini wants to merge 1 commit intomainfrom
ea-kwvtkkzqoztv

Conversation

@emilyalbini
Copy link
Copy Markdown
Member

This PR adds the cargo xtask local family of commands to run a local instance of buildomat with minimal configuration.

The centerpiece if the cargo xtask local setup command, backed by the xtask-setup crate (it's split from xtask as it requires a lot of dependencies). It currently supports configuring buildomat-server, buildomat-github-server, and buildomat-factory-aws, and at startup asks which components the user wants to be configured.

There are then the cargo xtask local BINARY commands, which invokes the binary with the flags required to use the local setup. cargo xtask local buildomat also gives access to the CLI, pointed to the local server.

Other changes included in the PR:

  • Added a BUILDOMAT_CONFIG environment variable to the buildomat CLI to point to a separate configuration file, so that cargo xtask local buildomat can transparently use the correct credentials.
  • Made specifying the SSH key optional in the AWS factory.
  • Added the ability to allocate public IPs to instances in the AWS factory.

@emilyalbini emilyalbini requested a review from jclulow March 24, 2026 15:02
@emilyalbini
Copy link
Copy Markdown
Member Author

Rebased to fix merge conflicts.

pub subnet: ConfigFileAwsSubnets,
pub tag: String,
pub key: String,
#[serde(default)]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Do you need the serde(default) attribute for Option members? Presumably it's just None if not present?

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.

Good call, I don't know how I left it there 😅

pub key: Option<String>,
pub security_group: String,
pub limit_total: usize,
#[serde(default = "default_false")]
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think false is already the Default::default() value for bool isn't it?

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.

So, while that is correct, I personally prefer to add an explicit default value rather than deferring to Default::default. That way it's obvious what the default value is by glancing at the source code.

If you prefer to replace it with #[serde(default)] I can do the switch.

Comment on lines +41 to +43
eprintln!();
eprintln!("buildomat-github-server setup");
eprintln!("=============================");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think all the regular text in here ought to just use println!() so that it comes out on stdout?

root.join("etc").join("app.toml"),
toml::to_string_pretty(&Config {
id: app.id,
secret: String::new(), /* XXX this seems unused? */
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this probably is unused at this point. I think it used to hold an oauth client secret, but that doesn't end up getting used by the GitHub client we're using at this point.

Maybe just remove secret altogether?

Comment on lines +84 to +86
let db = SQLite::open(root.join("var/data.sqlite3"))
.context("failed to create the database")?;
db.query_row("PRAGMA journal_mode=WAL;", (), |_| Ok(()))?;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

From what I recall, I think it's sufficient to just create an empty file here (as with, e.g., touch)? Then we can leave the specifics (like WAL mode) to the database wrapper.

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.

I do remember somewhere that it wasn't enough (maybe on facade?), but I checked and it indeed works. I'll switch it to just touching the file.

Comment on lines +65 to +67
let db = SQLite::open(root.join("data/data.sqlite3"))
.context("failed to create the database")?;
db.query_row("PRAGMA journal_mode=WAL;", (), |_| Ok(()))?;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think this could also just be analogous to touch to create an empty file (if missing)?

Comment on lines +35 to +37
eprintln!();
eprintln!("buildomat-server setup");
eprintln!("======================");
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Probably println!() throughout this file too?

Comment on lines +666 to +667
The first thing to do is setting it up. You will need [a configured AWS
profile] pointing to the AWS account that will host storage and compute, and
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

What does square brackets mean in markdown like this?

Comment on lines +383 to +401
* We are intentionally building the x86_64-unknown-linux-musl variant of
* the agent. Linux is what the AWS factory configures to run, and the musl
* variant prevents issues with older glibcs or building on NixOS.
*/
eprintln!("building the agent...");
let status = cargo()
.args(["build", "--bin", "buildomat-agent"])
.arg("--release")
.arg("--target=x86_64-unknown-linux-musl")
.status()?;
if !status.success() {
std::process::exit(status.code().unwrap_or(1));
}
std::fs::copy(
local
.join("../..")
.join("target/x86_64-unknown-linux-musl/release/buildomat-agent"),
local.join("buildomat-agent-linux"),
)?;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I think it would be better if we built the illumos buildomat-agent binary here rather than the Linux one, as that's generally the primary focus here.

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.

I can change it to also build the illumos one if the current host is illumos, but last time I tried creating a cross-compilation environment from NixOS to illumos I didn't have much success. A Linux MUSL build AFAIK can be done from either Linux or illumos.

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