From 6ec6c714132948dc93d1583787c15afdd5c3253d Mon Sep 17 00:00:00 2001 From: John Starks Date: Fri, 23 May 2025 16:36:13 -0700 Subject: [PATCH 1/7] petri: don't write identical screenshots (#1409) This should reduce artifact size for most test runs. --- petri/src/vm/openvmm/start.rs | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/petri/src/vm/openvmm/start.rs b/petri/src/vm/openvmm/start.rs index d9f2a5a98ff..9d6847178fc 100644 --- a/petri/src/vm/openvmm/start.rs +++ b/petri/src/vm/openvmm/start.rs @@ -277,11 +277,11 @@ impl PetriVmConfigOpenVmm { let mut timer = PolledTimer::new(driver); let log_source = log_source.clone(); tasks.push(driver.spawn("petri-watchdog-screenshot", async move { - let mut count = 0; + let mut image = Vec::new(); + let mut last_image = Vec::new(); loop { timer.sleep(Duration::from_secs(2)).await; - count += 1; - tracing::info!(count, "Taking screenshot."); + tracing::info!("Taking screenshot."); // Our framebuffer uses 4 bytes per pixel, approximating an // BGRA image, however it only actually contains BGR data. @@ -293,7 +293,7 @@ impl PetriVmConfigOpenVmm { let (widthsize, heightsize) = (width as usize, height as usize); let len = widthsize * heightsize * BYTES_PER_PIXEL; - let mut image = vec![0; len]; + image.resize(len, 0); for (i, line) in (0..height).zip(image.chunks_exact_mut(widthsize * BYTES_PER_PIXEL)) { @@ -304,6 +304,11 @@ impl PetriVmConfigOpenVmm { } } + if image == last_image { + tracing::info!("No change in framebuffer, skipping screenshot."); + continue; + } + let r = log_source .create_attachment("screenshot.png") .and_then(|mut f| { @@ -321,8 +326,9 @@ impl PetriVmConfigOpenVmm { if let Err(e) = r { tracing::error!(?e, "Failed to save screenshot"); } else { - tracing::info!(count, "Screenshot saved."); + tracing::info!("Screenshot saved."); } + std::mem::swap(&mut image, &mut last_image); } })); } From 54e9dec0f7506e298fb399ee814f5f824100bcb1 Mon Sep 17 00:00:00 2001 From: John Starks Date: Tue, 27 May 2025 09:59:10 -0700 Subject: [PATCH 2/7] ci: add workflow to publish petri results to blob storage (#1412) Automatically publish petri results to blob storage after a test workflow runs. This will later make it easier to view petri logs, without having to manually download artifacts or sift through the actions log output. --- .github/workflows/upload-petri-results.yml | 83 ++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 .github/workflows/upload-petri-results.yml diff --git a/.github/workflows/upload-petri-results.yml b/.github/workflows/upload-petri-results.yml new file mode 100644 index 00000000000..47b1b74c50a --- /dev/null +++ b/.github/workflows/upload-petri-results.yml @@ -0,0 +1,83 @@ +name: Upload Petri Test Results + +on: + workflow_run: + types: completed + workflows: + - "[flowey] OpenVMM CI" + - "[flowey] OpenVMM PR" + +permissions: + id-token: write + contents: read + actions: read # Needed to download artifacts from other runs + pull-requests: write # Needed to create PR comments + +jobs: + upload-logs: + runs-on: ubuntu-latest + env: + BASE_URL: https://openvmmghtestresults.blob.core.windows.net/results + + steps: + - name: Authenticate to Azure + uses: azure/login@v1 + with: + client-id: ${{ secrets.OPENVMM_TEST_RESULT_CLIENT_ID }} + tenant-id: ${{ secrets.OPENVMM_TENANT_ID }} + subscription-id: ${{ secrets.OPENVMM_SUBSCRIPTION_ID }} + + - name: Download matching artifacts + run: | + if [ "$event" = "pull_request" ]; then + echo "UPLOAD_PATH=pr/$run_id" >> "$GITHUB_ENV" + else + echo "UPLOAD_PATH=ci/$head_branch/$run_id" >> "$GITHUB_ENV" + fi + mkdir -p results + gh run \ + -R "$repo" \ + download "$run_id" \ + -p "*-vmm-tests-logs" \ + -D results + FAIL_COUNT=$(find results -name petri.failed | wc -l) + echo "FAIL_COUNT=$FAIL_COUNT" >> $GITHUB_ENV + env: + event: ${{ github.event.workflow_run.event }} + repo: ${{ github.event.workflow_run.repository.full_name }} + run_id: ${{ github.event.workflow_run.id }} + head_branch: ${{ github.event.workflow_run.head_branch }} + GH_TOKEN: ${{ secrets.GITHUB_TOKEN }} + + - name: Upload to Azure Blob Storage + run: | + az storage blob upload-batch \ + --destination "$BASE_URL" \ + --destination-path "$UPLOAD_PATH" \ + --source results \ + --auth-mode login + + # Store a metadata blob in a separate portion of the hierarchy so that + # it can be cheaply queried. + az storage blob upload \ + --blob-url "$BASE_URL/runs/$UPLOAD_PATH" \ + --file /dev/null \ + --metadata petrifailed="$FAIL_COUNT" \ + --auth-mode login + env: + run_id: ${{ github.event.workflow_run.id }} + + - name: Report failing tests + if: ${{ github.event.workflow_run.event == 'pull_request' && env.FAIL_COUNT > 0 }} + run: | + gh pr comment \ + -R "$repo" \ + "$pr" \ + -F - < Date: Tue, 27 May 2025 13:05:49 -0400 Subject: [PATCH 3/7] deps: update jiff to fix zoneinfo wsl searching (#1407) If you cross compiled and ran inside wsl, you would be hit by https://github.com/BurntSushi/jiff/issues/376. Pickup the fix for this, so launching openhcl with openvmm doesn't take 10+ seconds to just traverse a bunch of files over the wsl share. --- Cargo.lock | 8 ++++---- Cargo.toml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 95fe6eb39eb..b450c3e16b2 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3551,9 +3551,9 @@ checksum = "4a5f13b858c8d314ee3e8f639011f7ccefe71f97f96e50151fb991f267928e2c" [[package]] name = "jiff" -version = "0.2.10" +version = "0.2.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "5a064218214dc6a10fbae5ec5fa888d80c45d611aba169222fc272072bf7aef6" +checksum = "a194df1107f33c79f4f93d02c80798520551949d59dfad22b6157048a88cca93" dependencies = [ "jiff-static", "jiff-tzdb-platform", @@ -3566,9 +3566,9 @@ dependencies = [ [[package]] name = "jiff-static" -version = "0.2.10" +version = "0.2.14" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "199b7932d97e325aff3a7030e141eafe7f2c6268e1d1b24859b753a627f45254" +checksum = "6c6e1db7ed32c6c71b759497fae34bf7933636f75a251b9e736555da426f6442" dependencies = [ "proc-macro2", "quote", diff --git a/Cargo.toml b/Cargo.toml index 1abba1656d0..18c5a3ffb9e 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -455,7 +455,7 @@ igvm = {git = "https://github.com/microsoft/igvm", rev = "365065d7e31da0a0116e79 igvm_defs = {git = "https://github.com/microsoft/igvm", rev = "365065d7e31da0a0116e7934de3ecd85f00bab70", default-features = false, features = [ "unstable" ]} image = { version = "0.25.1", default-features = false } io-uring = "0.7.4" -jiff = "0.2" +jiff = "0.2.14" kvm-bindings = "0.11.1" # Use of these specific REPO will go away when changes are taken upstream. landlock = "0.4.1" From a5fa461ca6388abdeefd678e23aa61f59e0884c6 Mon Sep 17 00:00:00 2001 From: John Starks Date: Tue, 27 May 2025 10:25:38 -0700 Subject: [PATCH 4/7] petri: add and publish log viewer web tool (#1413) Create a simple client-side HTML+JS solution for parsing and viewing petri logs. Point it to the OpenVMM test results storage account, which will soon be populated automatically. Publish it at . --- .github/workflows/openvmm-docs-ci.yaml | 26 +- .../_jobs/consolidate_and_publish_gh_pages.rs | 11 + petri/logview/index.html | 175 +++++++ petri/logview/test.html | 479 ++++++++++++++++++ 4 files changed, 689 insertions(+), 2 deletions(-) create mode 100644 petri/logview/index.html create mode 100644 petri/logview/test.html diff --git a/.github/workflows/openvmm-docs-ci.yaml b/.github/workflows/openvmm-docs-ci.yaml index 1a1e9bfd17c..db955d18925 100644 --- a/.github/workflows/openvmm-docs-ci.yaml +++ b/.github/workflows/openvmm-docs-ci.yaml @@ -556,14 +556,36 @@ jobs: echo "$AgentTempDirNormal/used_artifacts/x64-linux-rustdoc" | flowey v 3 'artifact_use_from_x64-linux-rustdoc' --is-raw-string update echo "$AgentTempDirNormal/used_artifacts/x64-windows-rustdoc" | flowey v 3 'artifact_use_from_x64-windows-rustdoc' --is-raw-string update shell: bash - - name: generate consolidated gh pages html + - name: check if openvmm needs to be cloned run: |- flowey e 3 flowey_core::pipeline::artifact::resolve 1 flowey e 3 flowey_core::pipeline::artifact::resolve 2 flowey e 3 flowey_core::pipeline::artifact::resolve 0 + flowey e 3 flowey_lib_common::git_checkout 0 + flowey v 3 'flowey_lib_common::git_checkout:0:flowey_lib_common/src/git_checkout.rs:469:80' --is-raw-string --condvar flowey_lib_common::git_checkout:1:flowey_lib_common/src/git_checkout.rs:470:46 write-to-env github floweyvar2 + flowey v 3 'flowey_lib_common::git_checkout:1:flowey_lib_common/src/git_checkout.rs:470:46' write-to-env github FLOWEY_CONDITION + shell: bash + - id: flowey_lib_common__git_checkout__1 + uses: actions/checkout@v4 + with: + fetch-depth: '1' + path: repo0 + persist-credentials: ${{ env.floweyvar2 }} + name: checkout repo openvmm + if: ${{ fromJSON(env.FLOWEY_CONDITION) }} + - name: report cloned repo directories + run: |- + flowey v 3 'flowey_lib_common::git_checkout:4:flowey_core/src/node/github_context.rs:55:41' --is-raw-string update --env-source github.workspace <) { ctx.import::(); + ctx.import::(); } fn process_request(request: Self::Request, ctx: &mut NodeCtx<'_>) -> anyhow::Result<()> { @@ -42,14 +43,18 @@ impl SimpleFlowNode for Node { output, } = request; + let repo = ctx.reqv(crate::git_checkout_openvmm_repo::req::GetRepoDir); + let consolidated_html = ctx.emit_rust_stepv("generate consolidated gh pages html", |ctx| { let rendered_guide = rendered_guide.claim(ctx); let rustdoc_windows = rustdoc_windows.claim(ctx); let rustdoc_linux = rustdoc_linux.claim(ctx); + let repo = repo.claim(ctx); |rt| { let rendered_guide = rt.read(rendered_guide); let rustdoc_windows = rt.read(rustdoc_windows); let rustdoc_linux = rt.read(rustdoc_linux); + let repo = rt.read(repo); let consolidated_html = std::env::current_dir()?.join("out").absolute()?; fs_err::create_dir(&consolidated_html)?; @@ -78,6 +83,12 @@ impl SimpleFlowNode for Node { consolidated_html.join("rustdoc/linux"), )?; + // Make petri logview available under `openvmm.dev/test-results/` + flowey_lib_common::_util::copy_dir_all( + repo.join("petri/logview"), + consolidated_html.join("test-results"), + )?; + // as we do not currently have any form of "landing page", // redirect `openvmm.dev` to `openvmm.dev/guide` fs_err::write(consolidated_html.join("index.html"), REDIRECT)?; diff --git a/petri/logview/index.html b/petri/logview/index.html new file mode 100644 index 00000000000..c6dd79352c2 --- /dev/null +++ b/petri/logview/index.html @@ -0,0 +1,175 @@ + + + + + + Petri test results + + + + + +

Loading

+
+
+ + + \ No newline at end of file diff --git a/petri/logview/test.html b/petri/logview/test.html new file mode 100644 index 00000000000..4b7688e1eb4 --- /dev/null +++ b/petri/logview/test.html @@ -0,0 +1,479 @@ + + + + + + Petri test results + + + + + + +

Loading

+

+
+
+
+ + +
+
+
+ + + \ No newline at end of file From ef03ec42a65c2a705cd4e9916bffbe362bc370ab Mon Sep 17 00:00:00 2001 From: Chris Oo Date: Tue, 27 May 2025 15:20:44 -0400 Subject: [PATCH 5/7] ci: fix trigger for upload petri test results (#1416) This didn't seem to trigger, so try changing up the inputs to see if it'll trigger now. --- .github/workflows/upload-petri-results.yml | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/.github/workflows/upload-petri-results.yml b/.github/workflows/upload-petri-results.yml index 47b1b74c50a..dfd2fcae55c 100644 --- a/.github/workflows/upload-petri-results.yml +++ b/.github/workflows/upload-petri-results.yml @@ -2,10 +2,11 @@ name: Upload Petri Test Results on: workflow_run: - types: completed + types: + - completed workflows: - - "[flowey] OpenVMM CI" - - "[flowey] OpenVMM PR" + - '[flowey] OpenVMM CI' + - '[flowey] OpenVMM PR' permissions: id-token: write From ced1562135db6a106e7a9fa83cb53f7ef3557580 Mon Sep 17 00:00:00 2001 From: John Starks Date: Tue, 27 May 2025 12:43:41 -0700 Subject: [PATCH 6/7] flowey: fix saving secret vars (#1417) We are trying to write a non-JSON-formatted value to track that an env-var-sourced variable is secret. Fix this by writing `null`. Also add in some diagnostics and improve the in-memory variable representation to avoid so many allocations. --- flowey/flowey_cli/Cargo.toml | 2 +- flowey/flowey_cli/src/cli/var_db.rs | 2 +- .../flowey_cli/src/var_db/single_json_file.rs | 26 +++++++++---------- 3 files changed, 14 insertions(+), 16 deletions(-) diff --git a/flowey/flowey_cli/Cargo.toml b/flowey/flowey_cli/Cargo.toml index 7909b7cc731..525cb6feb2c 100644 --- a/flowey/flowey_cli/Cargo.toml +++ b/flowey/flowey_cli/Cargo.toml @@ -19,7 +19,7 @@ log.workspace = true parking_lot.workspace = true petgraph.workspace = true serde = { workspace = true, features = ["derive"] } -serde_json.workspace = true +serde_json = { workspace = true, features = ["raw_value"] } serde_yaml.workspace = true toml_edit = { workspace = true, features = ["serde"] } xshell.workspace = true diff --git a/flowey/flowey_cli/src/cli/var_db.rs b/flowey/flowey_cli/src/cli/var_db.rs index 1163f7a7831..0d2fb1f46db 100644 --- a/flowey/flowey_cli/src/cli/var_db.rs +++ b/flowey/flowey_cli/src/cli/var_db.rs @@ -273,7 +273,7 @@ impl VarDb { if is_secret { // Remember that this environment variable is secret so that // it cannot be easily laundered into a non-secret variable. - runtime_var_db.set_var(&env_source_name(&env), false, Vec::new()); + runtime_var_db.set_var(&env_source_name(&env), false, "null".into()); } match backend { diff --git a/flowey/flowey_cli/src/var_db/single_json_file.rs b/flowey/flowey_cli/src/var_db/single_json_file.rs index 84a443fb45c..a80a5816e94 100644 --- a/flowey/flowey_cli/src/var_db/single_json_file.rs +++ b/flowey/flowey_cli/src/var_db/single_json_file.rs @@ -7,6 +7,8 @@ use anyhow::Context; use fs_err::File; use serde::Deserialize; use serde::Serialize; +use serde_json::value::RawValue; +use std::borrow::Cow; use std::collections::BTreeMap; use std::io::Seek; use std::io::Write; @@ -15,8 +17,8 @@ use std::path::Path; /// On-disk format for the var db #[derive(Serialize, Deserialize)] #[serde(transparent)] -struct VarDb { - vars: BTreeMap, +struct VarDb<'a> { + vars: BTreeMap)>, } /// Implements [`flowey_core::node::RuntimeVarDb`] backed by a JSON file. @@ -47,7 +49,7 @@ impl SingleJsonFileVarDb { Ok(Self { file }) } - fn load_db(&mut self) -> VarDb { + fn load_db(&mut self) -> VarDb<'static> { self.file.rewind().unwrap(); serde_json::from_reader(&self.file).expect("corrupt runtime variable db") } @@ -57,30 +59,26 @@ impl flowey_core::node::RuntimeVarDb for SingleJsonFileVarDb { fn try_get_var(&mut self, var_name: &str) -> Option<(Vec, bool)> { let db = self.load_db(); let (is_secret, ref val) = *db.vars.get(var_name)?; - let val = val.to_string(); if is_secret { log::debug!("[db] read var: {} = ", var_name); } else { log::debug!("[db] read var: {} = {}", var_name, val); } - Some((val.into(), is_secret)) + Some((val.get().into(), is_secret)) } fn set_var(&mut self, var_name: &str, is_secret: bool, value: Vec) { + let value: &RawValue = serde_json::from_slice(&value) + .unwrap_or_else(|err| panic!("invalid JSON for var {}: {}", var_name, err)); if is_secret { log::debug!("[db] set var: {} = ", var_name) } else { - log::debug!( - "[db] set var: {} = {}", - var_name, - String::from_utf8_lossy(&value) - ) + log::debug!("[db] set var: {} = {}", var_name, value) }; let mut db = self.load_db(); - let existing = db.vars.insert( - var_name.into(), - (is_secret, serde_json::from_slice(&value).unwrap()), - ); + let existing = db + .vars + .insert(var_name.into(), (is_secret, Cow::Borrowed(value))); assert!(existing.is_none()); // all vars are one-time-write self.file.set_len(0).unwrap(); self.file.rewind().unwrap(); From 4e13b6d60b14f6a74cd4dc2d06daf3acc23c7f07 Mon Sep 17 00:00:00 2001 From: John Starks Date: Tue, 27 May 2025 13:31:08 -0700 Subject: [PATCH 7/7] ci: escape special characters in workflow names (#1418) This should do the trick. --- .github/workflows/upload-petri-results.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/upload-petri-results.yml b/.github/workflows/upload-petri-results.yml index dfd2fcae55c..6cb03d3884b 100644 --- a/.github/workflows/upload-petri-results.yml +++ b/.github/workflows/upload-petri-results.yml @@ -5,8 +5,8 @@ on: types: - completed workflows: - - '[flowey] OpenVMM CI' - - '[flowey] OpenVMM PR' + - '\[flowey\] OpenVMM CI' + - '\[flowey\] OpenVMM PR' permissions: id-token: write