Skip to content

Latest commit

 

History

History
366 lines (318 loc) · 22.6 KB

contribute.md

File metadata and controls

366 lines (318 loc) · 22.6 KB

Contributing

NVX takes contributions as pull requests to the dev branch of microsoft/nvx. This guide covers the workflow, the conventions that reviewers expect, the quality gates, test ownership, the layout and expectations of tests, and how OpenVMM changes reach NVX. Report security vulnerabilities as SECURITY.md describes, never in a public issue.

Workflow

  1. Clone the repository with its OpenVMM submodule and run scripts/nvx.py verify, as Setup describes.
  2. Create a topic branch from dev.
  3. Make the change with its tests and documentation, then run the quality gates and tests that cover it.
  4. Open a pull request against dev as a draft or ready for review.
  5. Address the review until the pull request meets the merge rules below.
  6. Queue the pull request, or ask a maintainer to, with gh pr merge NUMBER or Merge when ready on its page.

Branch rules on dev require a maintainer's approval of the pull request's most recent push, from someone other than its pusher, every review thread resolved, and a passing Required status check, the CI job that confirms that every job the change schedules succeeded. Copilot reviews every push, drafts included. Pull requests merge through GitHub's merge queue, which runs Required status check again on the pull request merged into the latest dev before it merges it (see Merge queue), and with a merge commit, so every commit on the branch lands in dev; give each one a subject that follows the conventions.

CI runs the jobs that need the OpenVMM deploy key or a hypervisor runner, including the NVX CLI tests, only for pushes, merge groups, scheduled and manually dispatched full runs, and same-repository pull requests. A pull request from a fork runs the remaining GitHub-hosted checks, and a maintainer stages it on a microsoft/nvx branch to run the rest; see Job graph and runner capacity. A merge group runs in microsoft/nvx whatever repository its pull requests come from, though, so queueing a fork's pull request runs its code, its workflow included, with the deploy key and on the hypervisor runners before it merges; see Merge queue.

Commits and pull requests

Item Convention
Commit subject area: summary, where area is the component, command, or directory that changes, such as doc, ci, guest, sandbox, or openvmm. Start the summary with a lowercase imperative verb and omit the trailing period: ci: run each OpenVMM suite only when its inputs change.
Commit body What changed and why, when the subject does not say enough.
Pull request title The commit subject of a single-commit pull request; otherwise a summary in the same form.
Pull request body What changed and why, then a Validation section that lists the checks you ran and the gates you leave to CI, such as backends you could not test. The pull request template holds these sections.
Issue references Closes #N or Fixes #N first when the pull request completes the issue; Part of #N or Refs #N otherwise.
Review threads Reply with the commit that addresses the comment, or with the evidence that no change is needed, then resolve the thread once the pull request's head holds that outcome. scripts/pr_threads.py lists the threads, and checks each reply and resolution.

Leave the performance histories in data/*.csv to CI: successful dev pushes record their results, and the nightly full run appends them.

Draft or Ready

CI runs for a pull request, whatever its base, when it is opened, reopened, pushed to, or marked ready for review, and skips its lint, build, and test jobs while the pull request is a draft. A pull request stacked on another's branch gets its own checks and keeps them when GitHub retargets it to dev (see Stacked pull requests). A new push cancels the pull request's run in progress; converting it back to a draft does not. A ready pull request that changes more than documentation runs the hypervisor jobs that its changes select on the shared self-hosted KVM, MSHV, and WHP runner pools, so:

  • Open a Draft when the change touches more than documentation and has not yet passed the quality gates and tests that cover it. Mark it ready once it has.
  • Open it Ready otherwise. A documentation-only change, in which every path is under doc/ or ends in .md, skips the guest artifacts, the OpenVMM builds, and every hypervisor job; see Change classification.

Quality gates

CI runs these checks on GitHub-hosted runners. Run the ones that cover your change before you push; Setup lists the commands for the Python, shell, and link checks, and python3 scripts/local_gates.py --base origin/dev lists the gates that a change selects, with the commands of the actions below, and runs them with --run.

Code Checks Definition
Python in scripts/ and .github/specula/ Ruff lint and format with 88-column lines, Pyright in strict mode over scripts/ against Linux and Windows APIs, and the Specula wiring tests. The code targets Python 3.10. check-quality, pyproject.toml
Guest and setup shell scripts ShellCheck, and shfmt with four-space indentation and indented case branches. Scripts are POSIX sh unless check-quality checks them as Bash. check-quality
scripts/setup/setup-windows-whp.ps1 PowerShell syntax check-quality
Markdown files and doc/ paths Every relative link and image in a tracked Markdown file resolves to a tracked file, and every anchor to a heading slug that GitHub generates, ignoring code. Every doc/ path that a tracked file mentions resolves too, except the test fixtures that DOC_PATH_FIXTURES lists. check-quality, check_markdown_links.py
Tests Every test lives in a test file, and production files hold only the test hooks that the allowlist allows; see Test layout check-quality, check_test_layout.py
nvx-rs/ rustfmt, Clippy and rustdoc with warnings denied, Clippy also for the cli and testing features alone, tests including a bundled-release fixture, the minimum supported Rust version, a macOS check, the public API against its snapshots in nvx-rs/public-api/, and, through nvx.py, the musl lint, tests, build, digest manifest, and static-ELF check of the guest binaries; see Rust crates check-nvx-rs, public_api.py
tests/runtime-adapter/ rustfmt, then Clippy and rustdoc with warnings denied, the build, the tests, and the minimum supported Rust version of nvx, in the protocol-only, host-only, and, on Linux, static musl guest-only configurations; see Rust crates check-nvx-rs
NVX CLI Compiles scripts/, runs every scripts/test_*.py module, and checks command help, on Linux and Windows, and compares the copies of OpenVMM's protocol vectors in nvx-rs/tests/vectors/openvmm/, and of the identities of its built-in CPU profiles in nvx-rs/src/host/cli/cpu_generations.json, with the submodule's files validate-nvx, ci.yml

check-quality names each shell script that it checks, and a CLI test fails unless validate-nvx runs every scripts/test_*.py module on both operating systems, so add a new script or test module to those lists.

A change to the public API of nvx updates its snapshots, which list it on Linux and on Windows, so that the change shows in the pull request's diff:

python3 scripts/public_api.py --update

The script builds rustdoc's JSON with the Rust release that rust-toolchain.toml pins, and adds the musl and MSVC targets to it; see Rust crates.

Test ownership

OpenVMM owns control-plane coverage. Its own CI runs its unit, documentation, and VMM tests on each pull request into the pinned branch and on each push to it, and NVX CI checks that they passed the pinned revision instead of running them (see OpenVMM CI). Run its unit and documentation tests locally with scripts/nvx.py test-openvmm-unit and its VMM tests with scripts/nvx.py test-openvmm --backend BACKEND. The VMM tests boot OpenVMM's own test guests, except the management-RPC (TTRPC) microVM tests, which boot NVX's kernel and Alpine initramfs when test-openvmm runs them.

NVX owns behavior that depends on its patched Linux kernel, its Alpine, Ubuntu, and Azure Linux userspaces, guest binaries and scripts, SMP behavior, or virtio devices. Add those scenarios to scripts/nvx_tools/microvm_tests.py and run them with scripts/nvx.py test-microvm --backend BACKEND. Keep guest workloads in scripts/nvx_tools/microvm_test_scripts and retain complete failure logs. The scenarios that drive the public nvx-rs sandbox commands live in scripts/nvx_tools/managed_exec_tests.py, scripts/nvx_tools/sandbox_lifecycle_tests.py, scripts/nvx_tools/sandbox_proxy_tests.py, and scripts/nvx_tools/sandbox_ram_upper_tests.py, and microvm_tests.py registers them. sandbox and the control protocol belong to nvx-rs (microsoft/nvx#359), so the scenarios drive them as commands, compile egress policies with nvx-rs egress compile, and leave the control session to nvx-rs: the managed-lifecycle scenario builds OpenVMM's command line and runs the ignored nvx-rs test tests/managed_lifecycle.rs, which drives the guest. Add no Python client of the control protocol, lifecycle, or egress logic.

Unit tests for the host tooling live in scripts/test_*.py. The nvx crate keeps the unit tests of each module in the module's tests module, in files of their own beside its source, as Test layout describes, and integration tests in its tests directory, and also runs tests/guest_tools.rs against the exact guest-profile binaries. Tests of the NVX mechanics, such as the conformance vectors of the control protocol, belong in nvx. Tests of what an independent adapter observes through the public API of nvx alone, such as its own framing over the control session between the host and the guest, belong in the runtime adapter, whose consolidated suite the microVM jobs run on each backend with scripts/nvx.py test-runtime-adapter --backend BACKEND. Continuous integration describes how CI checks OpenVMM's own CI and what the NVX microVM suite covers.

Test layout

Tests live apart from the code that they test, so that a change to one never hides in a change to the other, and so that the guest images, which carry the code of nvx-rs but not its tests, never rebuild for a test. A Rust crate's test files are those beneath its tests directory and, in its src/, the tests.rs files and the files beneath a tests directory. Its other files in src/ are production files, which declare their unit tests only as an out-of-line tests module that only tests compile:

#[cfg(test)]
mod tests;

src/a/b.rs keeps its unit tests in src/a/b/tests.rs, or in src/a/b/tests/mod.rs and the files beside it, and src/a/mod.rs keeps them in src/a/tests.rs. A test file belongs to a module that the crate's library, one of its binaries, or one of its integration tests compiles for tests, through the mod declarations from the target's root, since no other test runs. Cargo.toml names the targets, or Cargo discovers them, and a target with test = false runs none; a declaration that a cfg excludes from every build of tests, such as #[cfg(any())] or #[cfg(not(test))], compiles nothing for them. The Rust files of tests/fixtures/ and tests/vectors/ are data, not modules. The tests module may be pub(crate), so that the tests of other modules use its fixtures and helpers, and its cfg may add conditions, as #[cfg(all(test, unix))] does. The code that only tests use belongs in it too: fixtures, helpers, test doubles, and the methods of production types that only tests call, in an impl block of the tests module. A production file holds no other test code: outside its function bodies, no inline module, no module of another name, and no item that a cfg requiring test guards; no test attribute, such as #[test], #[should_panic], or #[ignore]; no proptest! block; and no #![cfg(test)]. What remains are test hooks, the code that cannot move out of the code that it hooks: the attributes and cfg! invocations whose predicates mention test, such as a field, a statement, or a match arm that only tests compile, or a #[cfg_attr(test, derive(...))]. The seams of scripts/test-layout-allowlist.json list how many each production file may hold. A change that adds a hook records it there, and a reviewer checks that the hook cannot move. A change that removes one runs --tighten, which lowers the seams to the hooks that the files hold; it never raises an entry or adds one.

Python unit tests are the test_*.py modules of scripts/, .github/specula/tests/, and .github/skills/*/scripts/, and the end-to-end scenario drivers are the *_tests.py modules of scripts/nvx_tools/, whose guest workloads live in scripts/nvx_tools/microvm_test_scripts/. No other module imports unittest, pytest, doctest, a test_* module, or the PowerShell test support of scripts/powershell_testing.py.

check_test_layout.py checks these rules, which check-quality runs. It reads the package and target tables of each Cargo.toml, whose keys it reads when they hold a string or a boolean on their own lines, and reports a declaration of a target that it cannot read. It reads the files that Git tracks, so stage a new file before you run it:

python3 scripts/check_test_layout.py
python3 scripts/check_test_layout.py --tighten   # after removing a test hook

Test expectations

Tests take their expectations from what NVX specifies, never from output that an implementation recorded. The documents in doc/, the READMEs of the crates and of their tests, and the interfaces that they name, such as the kernel's and OpenVMM's, specify option grammars, file and wire formats, error codes, and the contracts among the host, OpenVMM, and the guest. The nvx crate and its nvx-rs CLI are the reference of the behavior that they ported from nvx.py: tests derived from the specification replaced the golden tests that compared them with what Python did (microsoft/nvx#722), as the crate's Testing section describes.

  • A deliberate change of behavior updates its specification and the tests derived from it, in the same change, never recorded output. Fixtures, such as the boot-token cases and the records of the store and of sandbox configurations in nvx-rs/tests/fixtures/, are written by hand from the specification, and no change adds one that records what an implementation does.
  • Callers rely on exit statuses, error codes, and stable fragments of messages, such as the names of options and the paths of fields, but not on the wording of messages, which a change may revise. Tests of what callers observe, such as the CLI's scenario tests, check those.
  • A conformance vector holds bytes that no codec produced, the only check against peers that share no code with nvx: change one only with a protocol version, which gets vector files of its own.
  • Property tests use proptest.
  • To show that new tests catch what the tests that they replace caught, compare the mutants that each selection catches with nvx-rs mutation testing.

Changes to existing tests

A change to an existing test can hide the regression that the test caught, so the Test changes workflow labels every pull request that modifies or removes an existing test asset tests-modified, and keeps one comment on it that lists each, with links to it before and after the change. It compares the pull request's head with its merge base, item by item: a test, a test helper, the imports and inner attributes of a test module, and the test hooks of a production item; a fixture, a conformance vector, a scenario list, or another data file of the tests; an end-to-end scenario driver, guest workload, or guest probe; the fake OpenVMM; an allowlist entry that is new or allows more, or a change to the layout check, the flagging script, or their workflow; and a removed or changed line of the actions and workflow that run the tests and of the ci.yml jobs that run them, or an added line there that may skip them, and a test job that ci.yml no longer has. New tests flag nothing, and neither does code that moves unchanged under the same cfg predicates, its own and those that its module inherits, such as a tests module that moves to its own file, nor formatting, comments, lint attributes, visibility, or imports that a module only adds. A test whose module no target compiles any more, or compiles under other predicates, as a changed declaration of an ancestor module, or a manifest that sets the test key of its target to false or roots the target elsewhere, may make it, counts as removed or modified. Each target that compiles a module, through each chain of declarations, counts, so a test that one of them no longer compiles counts as modified. A comma counts in the token tree of a macro, unless the macro is one of the standard library's that take expressions and a trailing comma, such as assert_eq!.

The label blocks nothing. It asks the reviewers to check each listed change against the specification that the test derives from: that a modified assertion still checks what it did, or that the behavior changed on purpose, and that a removed test's coverage moved elsewhere or is no longer needed. Explain each such change in the pull request's body. Run the comparison before you push:

python3 scripts/check_test_changes.py --base origin/dev

OpenVMM

Access

openvmm/ is a Git submodule of the public microsoft/openvmm repository. .gitmodules records its URL and tracked feature/microvm/dev branch, and each NVX commit pins one exact revision. Clone with --recurse-submodules, or run scripts/nvx.py init in an existing checkout, as Setup describes. scripts/nvx.py verify fails until the submodule is checked out at the pinned revision. A checkout that predates the move from nanvix/openvmm keeps fetching from there until git submodule sync -- openvmm adopts the recorded URL.

Source changes

Change OpenVMM through a pull request to the feature/microvm/dev branch of microsoft/openvmm that follows its contributor instructions, including their Clippy, rustdoc, test, and formatting checks, then promote the reviewed revision into NVX.

feature/microvm/dev carries the microVM commits on top of the main branch of microsoft/openvmm. The OpenVMM upstream roadmap groups those commits into planned pull requests to main and marks those that stay fork-only.

Pin promotion

A pin promotion moves the openvmm gitlink to a reviewed OpenVMM revision:

  • Make the promotion a two-commit topic branch based directly on the current dev. The first commit should change only the gitlink unless the revision needs coordinated NVX changes, with a subject such as openvmm: pin auto's fallback to host CPU profiles. The second maps the pinned commits in the OpenVMM upstream roadmap and changes nothing else, with a subject such as doc: map the VM time fixes in the OpenVMM roadmap. Never merge OpenVMM history into NVX.
  • After committing, run scripts/nvx.py verify, which checks that the submodule checkout matches the recorded pin.
  • nvx-rs checks copies of OpenVMM's control-session protocol vectors, which the NVX CLI jobs compare with the submodule's files. When a revision changes them, copy them into nvx-rs/tests/vectors/openvmm/ unchanged and record the revision, in the gitlink commit; see OpenVMM's vectors. nvx-rs run also reads a copy of the identities of OpenVMM's built-in CPU profiles, nvx-rs/src/host/cli/cpu_generations.json, which the NVX CLI jobs compare with the pinned profiles; a revision that adds, revises, or removes a profile updates it in the gitlink commit.
  • In the pull request, name the OpenVMM pull request and the exact pinned SHA, the scope and any prerequisite revisions, and the validation you ran.
  • A gitlink change is an input of every CI suite, so its pull request runs the full matrix, including both OpenVMM suites.
  • When a rebase conflicts on the gitlink, do not pick either side: combining both changes usually needs a new OpenVMM revision.

The nvx-promote-openvmm-pin skill gives the complete procedure, including how to show that a replacement revision matches the reviewed one.

Chain rebases

A rebase moves the microVM chain onto a newer upstream main and publishes it as a new feature/microvm/release/vX.Y branch of microsoft/openvmm: the next minor version for a rebase, which replays the commits, or the next major version for a reorganization, which also folds fixups into the commits that they fix. The release branch's push runs OpenVMM CI. Once it passes, the previous release branch is fast-forwarded to the head of feature/microvm/dev, so that it keeps every revision of the old line that NVX pinned, and only then does feature/microvm/dev move to the new release, by a push with a lease. The NVX pull request that pins the release remaps the whole OpenVMM upstream roadmap. The nvx-rebase-openvmm-chain skill gives the complete procedure.