Skip to content

Commit

Permalink
phd: automatically fetch crucible-downstairs from Buildomat (#604)
Browse files Browse the repository at this point in the history
Currently, in order to run the `phd` tests that involve Crucible disks,
the path to a local `crucible-downstairs` binary must be provided via a
command-line argument. When no `crucible-downstairs` binary path is
provided, the tests that use Crucible are skipped. This means that the
tests which use Crucible disks are not currently able to run on CI, as
no `crucible-downstairs` binary is present.

This branch extends the `phd-framework` crate's artifact store to
support automatically fetch a `crucible-downstairs` binary from
Buildomat when configured via the command line. This PR adds a new
`--crucible-downstairs-commit` CLI argument to `phd-runner`. If present,
this argument configures PHD to fetch a `crucible-downstairs` binary
from Buildomat.

The `--crucible-downstairs-commit` argument can either be the string
`auto`, or a 40-character Git hash. When `auto` is passed, Git revision
to fetch from Buildomat is determined automatically based on the
`propolis` workspace's Git dependency on the `crucible` crate, to ensure
that the downstairs binary matches the version of the upstairs library
that Propolis was built against. Otherwise, if a Git hash is provided,
the artifact for that commit hash is fetched from Buildomat, instead.

The behavior of the existing `--crucible-downstairs-cmd` is unchanged,
so a local `crucible-downstairs` binary can still be used rather than
downloading it from Buildomat. This way, users can still run `phd` tests
against a local Crucible build, if necessary. The new
`--crucible-downstairs-commit` argument conflicts with
`--crucible-downstairs-cmd`, so if both are provided, `phd-runner` will
print an error. If _neither_ argument is provided, tests that require
Crucible are skipped, maintaining the default behavior of not
downloading anything unless explicitly configured to.

The Buildomat job for Crucible publishes the entire Illumos Crucible
zone as a tarball, rather than separate files for each artifact.
Therefore, it was necessary to extend the `phd-framework` artifact store
to support a configuration for extracting an individual file from within
a tarball. The SHA256 used for the artifact is the SHA of the entire
tarball, since Crucible's Buildomat job publishes the tarball's SHA, but
once the tarball is downloaded, the `crucible-downstairs` binary is
extracted from the tarball into the object store.
  • Loading branch information
hawkw authored Jan 17, 2024
1 parent 28e7565 commit ed21e45
Show file tree
Hide file tree
Showing 13 changed files with 566 additions and 104 deletions.
1 change: 1 addition & 0 deletions .github/buildomat/jobs/phd-run.sh
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ set +e
(RUST_BACKTRACE=1 ptime -m pfexec $runner \
--emit-bunyan \
run \
--crucible-downstairs-commit auto \
--propolis-server-cmd $propolis \
--artifact-toml-path $artifacts \
--tmp-directory $tmpdir \
Expand Down
30 changes: 28 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ bitvec = "1.0"
byteorder = "1"
bytes = "1.1"
camino = "1.1.6"
cargo_metadata = "0.18.1"
cc = "1.0.73"
cfg-if = "1.0.0"
chrono = "0.4.19"
Expand All @@ -108,6 +109,7 @@ errno = "0.2.8"
expectorate = "1.0.5"
fatfs = "0.3.6"
futures = "0.3"
flate2 = "1.0.28"
hex = "0.4.3"
http = "0.2.9"
hyper = "0.14"
Expand Down Expand Up @@ -139,6 +141,7 @@ slog-dtrace = "0.2.3"
slog-term = "2.8"
strum = "0.25"
syn = "1.0"
tar = "0.4"
tempfile = "3.2"
thiserror = "1.0"
tokio = "1"
Expand Down
2 changes: 2 additions & 0 deletions phd-tests/framework/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ camino = { workspace = true, features = ["serde1"] }
cfg-if.workspace = true
errno.workspace = true
futures.workspace = true
flate2.workspace = true
hex.workspace = true
libc.workspace = true
propolis-client.workspace = true
Expand All @@ -27,6 +28,7 @@ serde_json.workspace = true
slog.workspace = true
slog-async.workspace = true
slog-term.workspace = true
tar.workspace = true
thiserror.workspace = true
tokio = { workspace = true, features = ["full"] }
tokio-tungstenite.workspace = true
Expand Down
58 changes: 56 additions & 2 deletions phd-tests/framework/src/artifacts/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
//! Support for working with files consumed by PHD test runs.
use serde::{Deserialize, Serialize};
use std::str::FromStr;

mod manifest;
mod store;
Expand All @@ -13,20 +14,27 @@ pub use store::Store as ArtifactStore;

pub const DEFAULT_PROPOLIS_ARTIFACT: &str = "__DEFAULT_PROPOLIS";

#[derive(Clone, Debug, Serialize, Deserialize)]
pub const CRUCIBLE_DOWNSTAIRS_ARTIFACT: &str = "__DEFAULT_CRUCIBLE_DOWNSTAIRS";

#[derive(Clone, Debug, Serialize, Eq, PartialEq)]
#[serde(transparent)]
pub struct Commit(String);

#[derive(Clone, Debug, Serialize, Deserialize, Eq, PartialEq)]
#[serde(rename_all = "snake_case")]
enum ArtifactKind {
GuestOs(crate::guest_os::GuestOsKind),
Bootrom,
PropolisServer,
CrucibleDownstairs,
}

#[derive(Clone, Debug, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
enum ArtifactSource {
/// Get the artifact from Buildomat. This downloads from
/// https://buildomat.eng.oxide.computer/public/file/REPO/SERIES/COMMIT.
Buildomat { repo: String, series: String, commit: String, sha256: String },
Buildomat { repo: String, series: String, commit: Commit, sha256: String },

/// Get the artifact from the manifest's list of remote artifact servers.
RemoteServer { sha256: String },
Expand All @@ -48,4 +56,50 @@ struct Artifact {
/// The source to use to obtain this artifact if it's not present on the
/// host system.
source: ArtifactSource,

/// If present, this artifact is a tarball, and the provided file should be
/// extracted.
untar: Option<camino::Utf8PathBuf>,
}

impl FromStr for Commit {
type Err = anyhow::Error;

fn from_str(s: &str) -> Result<Self, Self::Err> {
let s = s.trim();

// Ensure this looks like a valid Git commit.
anyhow::ensure!(
s.len() == 40,
"Buildomat requires full (40-character) Git commit hashes"
);

for c in s.chars() {
if !c.is_ascii_hexdigit() {
anyhow::bail!(
"'{c}' is not a valid hexadecimal digit; Git \
commit hashes should consist of the characters \
[0-9, a-f, A-F]"
);
}
}

Ok(Self(s.to_string()))
}
}

impl std::fmt::Display for Commit {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
f.write_str(&self.0)
}
}

impl<'de> Deserialize<'de> for Commit {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
let s = String::deserialize(deserializer)?;
FromStr::from_str(&s).map_err(serde::de::Error::custom)
}
}
Loading

0 comments on commit ed21e45

Please sign in to comment.