Skip to content

Commit 8ce7e9a

Browse files
zeeshanlakhanizeeshanlakhani
authored
[opte, deps] better support for unpublished xde via tools/opte_version_override (#10384)
OPTE has both a userland (oxide-vpc, opte-ioctl) pinned in Cargo.toml and a kernel module (xde) installed via the helios pkg repo. When the two need to be tested in lockstep before xde lands in a helios build, the existing override logic did not capture all requirements and swapping in an unpublished build was not seamless, particularly on CI. This builds out the override mechanism around the existing `tools/opte_version_override` stub. Includes: - install_opte.sh, deploy.sh, and releng image builds source `tools/opte_version_override` and, when OPTE_COMMIT is set, install from the buildomat-published p5p (or curl + rem_drv/add_drv on the deploy ramdisk, which has no pkg(5)) instead of the helios pkg repo. - package.sh tarballs `tools/opte_version_override` so that downstream jobs read the same value. - check-opte-ver.yml adds a check-opte-override job that fails any PR targeting main with a non-empty OPTE_COMMIT, so the override can't accidentally leak on main. - ci_check_opte_ver.sh now enforces OPTE_COMMIT == Cargo dependency rev so kernel/userland ABI drift surfaces at PR time. The default is OPTE_COMMIT="", which doesn't apply this override logic. We also fix a stale "expected lab-opte-0.$API_VER" error message in ci_check_opte_ver.sh. The check itself uses "lab-2.0-opte-0.$API_VER". This work was extracted from multicast work that (at times) has needed an unpublished xde for kernel-side multicast integration(s).
1 parent c533acc commit 8ce7e9a

8 files changed

Lines changed: 243 additions & 50 deletions

File tree

‎.github/buildomat/jobs/a4x2-deploy.sh‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
#:
33
#: name = "a4x2-deploy"
44
#: variety = "basic"
5-
#: target = "lab-2.0-opte-0.31"
5+
#: target = "lab-3.0-opte-0.40"
66
#: output_rules = [
77
#: "/out/falcon/*.log",
88
#: "/out/falcon/*.err",
@@ -129,7 +129,7 @@ chmod +x a4x2
129129
# Install falcon base images
130130
#
131131
export FALCON_DATASET=cpool/falcon
132-
images="debian-11.0_0 helios-2.0_0"
132+
images="debian-11.0_0 helios-3.0_0"
133133
for img in $images; do
134134
file=$img.raw.xz
135135
curl -OL http://catacomb.eng.oxide.computer:12346/falcon/$file

‎.github/buildomat/jobs/deploy.sh‎

Lines changed: 22 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,10 @@ _exit_trap() {
3333
local status=$?
3434
set +o errexit
3535

36+
# Restore the override opteadm from /tmp before debug-evidence
37+
# collection runs, in case anything earlier in this trap or any
38+
# downstream tooling has overwritten the installed binary. The
39+
# /tmp copy is saved by the OPTE_COMMIT install block below.
3640
if [[ "x$OPTE_COMMIT" != "x" ]]; then
3741
pfexec cp /tmp/opteadm /opt/oxide/opte/bin/opteadm
3842
fi
@@ -134,19 +138,6 @@ z_swadm () {
134138
pfexec zlogin oxz_switch /opt/oxide/dendrite/bin/swadm $@
135139
}
136140

137-
# only set this if you want to override the version of opte/xde installed by the
138-
# install_opte.sh script
139-
OPTE_COMMIT=""
140-
if [[ "x$OPTE_COMMIT" != "x" ]]; then
141-
curl -sSfOL https://buildomat.eng.oxide.computer/public/file/oxidecomputer/opte/module/$OPTE_COMMIT/xde
142-
pfexec rem_drv xde || true
143-
pfexec mv xde /kernel/drv/amd64/xde
144-
pfexec add_drv xde || true
145-
curl -sSfOL https://buildomat.eng.oxide.computer/public/file/oxidecomputer/opte/release/$OPTE_COMMIT/opteadm
146-
chmod +x opteadm
147-
cp opteadm /tmp/opteadm
148-
pfexec mv opteadm /opt/oxide/opte/bin/opteadm
149-
fi
150141

151142
#
152143
# XXX work around 14537 (UFS should not allow directories to be unlinked) which
@@ -197,6 +188,24 @@ ptime -m tar xvzf /input/package/work/package.tar.gz
197188
# shellcheck source=/dev/null
198189
source .github/buildomat/ci-env.sh
199190

191+
# Source the OPTE override (if any) from the canonical location and apply it.
192+
#
193+
# When set, download the xde driver and opteadm directly from buildomat and
194+
# swap them in. The deploy target is a ramdisk image without pkg(5), so we
195+
# use rem_drv/add_drv instead of the p5p approach used by install_opte.sh
196+
# and releng.
197+
source tools/opte_version_override
198+
if [[ "x$OPTE_COMMIT" != "x" ]]; then
199+
curl -sSfOL "https://buildomat.eng.oxide.computer/public/file/oxidecomputer/opte/module/$OPTE_COMMIT/xde"
200+
pfexec rem_drv xde || true
201+
pfexec mv xde /kernel/drv/amd64/xde
202+
pfexec add_drv xde || true
203+
curl -sSfOL "https://buildomat.eng.oxide.computer/public/file/oxidecomputer/opte/release/$OPTE_COMMIT/opteadm"
204+
chmod +x opteadm
205+
cp opteadm /tmp/opteadm
206+
pfexec mv opteadm /opt/oxide/opte/bin/opteadm
207+
fi
208+
200209
# Ask buildomat for the range of extra addresses that we're allowed to use, and
201210
# break them up into the ranges we need.
202211

‎.github/buildomat/jobs/package.sh‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,5 +60,7 @@ files=(
6060
target/release/xtask
6161
target/debug/bootstrap
6262
tests/*
63+
tools/opte_version
64+
tools/opte_version_override
6365
)
6466
ptime -m tar cvzf $WORK/package.tar.gz "${files[@]}" "${packages[@]}"

‎.github/workflows/check-opte-ver.yml‎

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
name: check-opte-ver
22
on:
3+
# Run on every PR targeting main, regardless of which paths changed, as the
4+
# override file may have been set in an earlier commit on the branch. So,
5+
# we must check on every PR rather than only when tracked paths change.
36
pull_request:
4-
paths:
5-
- '.github/workflows/check-opte-ver.yml'
6-
- 'Cargo.toml'
7-
- 'tools/opte_version'
7+
branches: [main]
88
jobs:
99
check-opte-ver:
1010
runs-on: ubuntu-22.04
@@ -18,3 +18,18 @@ jobs:
1818
run: cargo install toml-cli@0.2.3
1919
- name: Check OPTE version and rev match
2020
run: ./tools/ci_check_opte_ver.sh
21+
22+
check-opte-override:
23+
runs-on: ubuntu-22.04
24+
steps:
25+
- uses: actions/checkout@11bd71901bbe5b1630ceea73d27597364c9af683 # v4.2.2
26+
with:
27+
ref: ${{ github.event.pull_request.head.sha }} # see omicron#4461
28+
- name: Reject OPTE override on main
29+
run: |
30+
source tools/opte_version_override
31+
if [[ "x$OPTE_COMMIT" != "x" ]]; then
32+
echo "::error::OPTE_COMMIT is set in tools/opte_version_override."
33+
echo "::error::The OPTE override must be cleared before merging to main."
34+
exit 1
35+
fi

‎dev-tools/releng/src/main.rs‎

Lines changed: 120 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,18 @@ async fn main() -> Result<()> {
270270
let opte_version =
271271
fs::read_to_string(WORKSPACE_DIR.join("tools/opte_version")).await?;
272272

273+
// Parse tools/opte_version_override for OPTE_COMMIT. When set, we
274+
// download the override p5p from buildomat and use it as a package
275+
// source during image build instead of the helios pkg repo version.
276+
let opte_override = parse_opte_version_override(
277+
&WORKSPACE_DIR.join("tools/opte_version_override"),
278+
)
279+
.await?;
280+
if let Some(ov) = &opte_override {
281+
info!(logger, "OPTE override active: commit={}", ov.commit);
282+
}
283+
let opte_version = opte_version.trim();
284+
273285
let client = reqwest::ClientBuilder::new()
274286
.connect_timeout(Duration::from_secs(15))
275287
.timeout(Duration::from_secs(120))
@@ -617,7 +629,7 @@ async fn main() -> Result<()> {
617629
.arg("-o") // output directory for image
618630
.arg(args.output_dir.join(format!("os-{}", target)))
619631
.arg("-F") // pass extra image builder features
620-
.arg(format!("optever={}", opte_version.trim()))
632+
.arg(format!("optever={opte_version}"))
621633
.arg("-P") // include all files from extra proto area
622634
.arg(proto_dir.join("root"))
623635
.arg("-N") // image name
@@ -675,11 +687,29 @@ async fn main() -> Result<()> {
675687
.arg(format!("helios={HELIOS_PKGREPO}"))
676688
}
677689

678-
// helios-build experiment-image
679-
jobs.push_command(format!("{}-image", target), image_cmd)
690+
// When OPTE_COMMIT is set, download the override p5p from buildomat
691+
// and add it as a package source for the image build.
692+
if let Some(ov) = &opte_override {
693+
let p5p_path = tempdir.path().join(format!("opte-{target}.p5p"));
694+
jobs.push(
695+
format!("{target}-opte-p5p"),
696+
download_opte_p5p(&logger, &client, &ov.commit, &p5p_path),
697+
);
698+
699+
image_cmd = image_cmd
700+
.arg("-p")
701+
.arg(format!("helios-dev=file://{p5p_path}"));
702+
}
703+
704+
let image_job = jobs
705+
.push_command(format!("{target}-image"), image_cmd)
680706
.after("helios-setup")
681707
.after("helios-incorp")
682-
.after(format!("{}-proto", target));
708+
.after(format!("{target}-proto"));
709+
710+
if opte_override.is_some() {
711+
image_job.after(format!("{target}-opte-p5p"));
712+
}
683713
}
684714
// Build the recovery target after we build the host target. Only one
685715
// of these will build at a time since Cargo locks its target directory;
@@ -887,6 +917,92 @@ async fn build_proto_area(
887917
Ok(())
888918
}
889919

920+
/// Parsed contents of `tools/opte_version_override` when an override is active.
921+
struct OpteOverride {
922+
commit: String,
923+
}
924+
925+
/// Parse `tools/opte_version_override` for `OPTE_COMMIT`. Returns `None` if
926+
/// `OPTE_COMMIT` is unset or empty. Errors if more than one non-comment
927+
/// `OPTE_COMMIT=` assignment is present, since a stale line above the active
928+
/// one would otherwise win silently.
929+
async fn parse_opte_version_override(
930+
path: &Utf8PathBuf,
931+
) -> Result<Option<OpteOverride>> {
932+
let contents = fs::read_to_string(path)
933+
.await
934+
.context("failed to read tools/opte_version_override")?;
935+
936+
let assignments: Vec<&str> = contents
937+
.lines()
938+
.filter_map(|line| line.trim().strip_prefix("OPTE_COMMIT="))
939+
.map(|val| val.trim_matches(|c| c == '"' || c == '\''))
940+
.collect();
941+
942+
if assignments.len() > 1 {
943+
bail!(
944+
"tools/opte_version_override contains {} OPTE_COMMIT \
945+
assignments (expected at most 1)",
946+
assignments.len()
947+
);
948+
}
949+
950+
Ok(assignments
951+
.into_iter()
952+
.find(|val| !val.is_empty())
953+
.map(|commit| OpteOverride { commit: commit.to_string() }))
954+
}
955+
956+
const OPTE_BUILDOMAT_BASE: &str =
957+
"https://buildomat.eng.oxide.computer/public/file/oxidecomputer/opte";
958+
959+
/// Download the OPTE override p5p archive from buildomat.
960+
fn download_opte_p5p(
961+
logger: &Logger,
962+
client: &reqwest::Client,
963+
commit: &str,
964+
dest: &Utf8PathBuf,
965+
) -> impl Future<Output = Result<()>> + Send + 'static {
966+
let logger = logger.clone();
967+
let client = client.clone();
968+
let commit = commit.to_string();
969+
let dest = dest.clone();
970+
async move {
971+
let url = format!("{OPTE_BUILDOMAT_BASE}/repo/{commit}/opte.p5p");
972+
info!(logger, "downloading OPTE override p5p from {url}");
973+
for attempt in 1..=RETRY_ATTEMPTS {
974+
let result = async {
975+
let response =
976+
client.get(&url).send().await?.error_for_status()?;
977+
let bytes = response.bytes().await?;
978+
fs::write(&dest, &bytes).await?;
979+
Ok::<_, anyhow::Error>(())
980+
}
981+
.await;
982+
983+
match result {
984+
Ok(()) => {
985+
info!(logger, "downloaded OPTE p5p to {dest}");
986+
return Ok(());
987+
}
988+
Err(err) => {
989+
if attempt == RETRY_ATTEMPTS {
990+
return Err(err).with_context(|| {
991+
format!("failed to download OPTE p5p from {url}")
992+
});
993+
}
994+
info!(
995+
logger,
996+
"retrying OPTE p5p download (attempt {attempt})"
997+
);
998+
}
999+
}
1000+
}
1001+
1002+
bail!("failed to download OPTE p5p after {RETRY_ATTEMPTS} attempts")
1003+
}
1004+
}
1005+
8901006
async fn host_add_root_profile(host_proto_root: Utf8PathBuf) -> Result<()> {
8911007
fs::create_dir_all(&host_proto_root).await?;
8921008
fs::write(

‎tools/ci_check_opte_ver.sh‎

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,6 @@
22
set -euo pipefail
33

44
source tools/opte_version_override
5-
if [[ "x$OPTE_COMMIT" != "x" ]]; then
6-
exit 0
7-
fi
85

96
# Grab all the oxidecomputer/opte dependencies' revisions
107
readarray -t opte_deps_revs < <(toml get Cargo.toml workspace.dependencies | jq -r 'to_entries | .[] | select(.value.git? | contains("oxidecomputer/opte")?) | .value.rev')
@@ -19,6 +16,19 @@ for rev in "${opte_deps_revs[@]}"; do
1916
fi
2017
done
2118

19+
# When an OPTE override is active, the kernel binary is built from
20+
# $OPTE_COMMIT while the userland (opte-ioctl, oxide-vpc) is built from
21+
# the Cargo dep revision. They must match, otherwise kernel/userland
22+
# ABI drift surfaces as opaque ioctl failures at runtime.
23+
if [[ "x$OPTE_COMMIT" != "x" ]]; then
24+
if [ "$OPTE_REV" != "$OPTE_COMMIT" ]; then
25+
echo "OPTE override mismatch:"
26+
echo " Cargo.toml deps: $OPTE_REV"
27+
echo " tools/opte_version_override: $OPTE_COMMIT"
28+
exit 1
29+
fi
30+
fi
31+
2232
# Grab the API version for this revision
2333
API_VER=$(curl -s https://raw.githubusercontent.com/oxidecomputer/opte/"$OPTE_REV"/crates/opte-api/src/lib.rs | sed -n 's/pub const API_VERSION: u64 = \([0-9]*\);/\1/p')
2434

@@ -71,6 +81,6 @@ BUILDOMAT_DEPLOY_TARGET=$(cat .github/buildomat/jobs/deploy.sh | sed -n 's/#:[ ]
7181
if [ "lab-3.0-opte-0.$API_VER" != "$BUILDOMAT_DEPLOY_TARGET" ]; then
7282
echo "OPTE version mismatch:"
7383
echo "Cargo.toml: $OPTE_REV ($OPTE_VER)"
74-
echo "buildomat deploy job: $BUILDOMAT_DEPLOY_TARGET (expected lab-opte-0.$API_VER)"
84+
echo "buildomat deploy job: $BUILDOMAT_DEPLOY_TARGET (expected lab-3.0-opte-0.$API_VER)"
7585
exit 1
7686
fi

‎tools/install_opte.sh‎

Lines changed: 43 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,12 @@ fi
5151
# Grab the version of the opte package to install
5252
OPTE_VERSION="$(cat "$OMICRON_TOP/tools/opte_version")"
5353

54+
# Source the OPTE override (if any). When OPTE_COMMIT is set, the desired
55+
# OPTE version isn't published to the helios pkg repo yet, so we install
56+
# directly from the override p5p built by OPTE CI on buildomat (see the
57+
# branch on $OPTE_COMMIT below).
58+
source "$OMICRON_TOP/tools/opte_version_override"
59+
5460
OMICRON_FROZEN_PKG_COMMENT="OMICRON-PINNED-PACKAGE"
5561

5662
# Once we install, we mark the package as frozen at that particular version.
@@ -71,22 +77,48 @@ if PKG_FROZEN=$(pkg freeze | grep driver/network/opte); then
7177
pfexec pkg unfreeze driver/network/opte
7278
fi
7379

74-
# Actually install the xde kernel module and opteadm tool
75-
RC=0
76-
pfexec pkg install -v pkg://helios/driver/network/opte@"$OPTE_VERSION" || RC=$?
77-
if [[ "$RC" -eq 0 ]]; then
78-
echo "xde driver installed successfully"
79-
elif [[ "$RC" -eq 4 ]]; then
80-
echo "Correct xde driver already installed"
80+
if [[ "x$OPTE_COMMIT" != "x" ]]; then
81+
# Install from the override p5p archive built by OPTE CI. The p5p
82+
# contains exactly one version (built from $OPTE_COMMIT), which
83+
# generally won't match the canonical $OPTE_VERSION, so we let pkg
84+
# pick the version from the p5p rather than pinning here.
85+
P5P_URL="https://buildomat.eng.oxide.computer/public/file/oxidecomputer/opte/repo/$OPTE_COMMIT/opte.p5p"
86+
P5P_PATH="/tmp/opte-override.p5p"
87+
echo "Downloading override p5p from $P5P_URL"
88+
curl -fL -o "$P5P_PATH" "$P5P_URL"
89+
90+
RC=0
91+
pfexec pkg install -g "$P5P_PATH" driver/network/opte || RC=$?
92+
if [[ "$RC" -eq 0 ]]; then
93+
echo "xde driver installed from override p5p"
94+
elif [[ "$RC" -eq 4 ]]; then
95+
echo "Correct xde driver already installed"
96+
else
97+
echo "Installing xde driver from override p5p failed"
98+
exit "$RC"
99+
fi
100+
rm -f "$P5P_PATH"
81101
else
82-
echo "Installing xde driver failed"
83-
exit "$RC"
102+
# Install the published version from the helios pkg repo.
103+
RC=0
104+
pfexec pkg install -v pkg://helios/driver/network/opte@"$OPTE_VERSION" || RC=$?
105+
if [[ "$RC" -eq 0 ]]; then
106+
echo "xde driver installed successfully"
107+
elif [[ "$RC" -eq 4 ]]; then
108+
echo "Correct xde driver already installed"
109+
else
110+
echo "Installing xde driver failed"
111+
exit "$RC"
112+
fi
84113
fi
85114

115+
# Discover the actually-installed version (may differ from $OPTE_VERSION
116+
# when an override is active) and freeze at that.
117+
INSTALLED_VERSION=$(pkg info -l driver/network/opte | awk '/^[[:space:]]*Version:/ {print $2}')
86118
RC=0
87-
pfexec pkg freeze -c "$OMICRON_FROZEN_PKG_COMMENT" driver/network/opte@"$OPTE_VERSION" || RC=$?
119+
pfexec pkg freeze -c "$OMICRON_FROZEN_PKG_COMMENT" "driver/network/opte@$INSTALLED_VERSION" || RC=$?
88120
if [[ "$RC" -ne 0 ]]; then
89-
echo "Failed to pin opte package to $OPTE_VERSION"
121+
echo "Failed to pin opte package to $INSTALLED_VERSION"
90122
exit $RC
91123
fi
92124

@@ -97,13 +129,3 @@ if [[ "$RC" -ne 0 ]]; then
97129
echo "The \`opteadm\` administration tool is not on your path."
98130
echo "You may add \"/opt/oxide/opte/bin\" to your path to access it."
99131
fi
100-
101-
source $OMICRON_TOP/tools/opte_version_override
102-
103-
if [[ "x$OPTE_COMMIT" != "x" ]]; then
104-
set +x
105-
curl -fOL https://buildomat.eng.oxide.computer/public/file/oxidecomputer/opte/module/$OPTE_COMMIT/xde
106-
pfexec rem_drv xde || true
107-
pfexec mv xde /kernel/drv/amd64/xde
108-
pfexec add_drv xde || true
109-
fi

0 commit comments

Comments
 (0)