Skip to content

feat(system-tests): boot nested VMs on the local backend - #11265

Draft
basvandijk wants to merge 13 commits into
masterfrom
bas/local-backend-nested
Draft

feat(system-tests): boot nested VMs on the local backend#11265
basvandijk wants to merge 13 commits into
masterfrom
bas/local-backend-nested

Conversation

@basvandijk

@basvandijk basvandijk commented Aug 21, 2026

Copy link
Copy Markdown
Collaborator

Problem

A nested system-test (rs/tests/nested) drives the whole node deployment chain: SetupOS installs HostOS onto a blank disk, HostOS boots a nested GuestOS VM, and that GuestOS registers with the NNS. All five non-bare-metal targets were pinned to Farm, each carrying the same TODO, because setup_and_start_nested_vms had only a Farm code path.

What was actually missing

Less than the TODO suggested. allocate_resources already had a Local arm; write_nested_vm's address derivation already lines up with vm_mac/create_vm; create_setupos_config_image already writes the gateway that create_group puts on the bridge (and HostOS pings it through each candidate NIC before accepting one, so it has to be real); the group bridge already learns all three MACs that end up behind one TAP; and HostOS's nftables already accepts SSH from 2a00:fb01:400::/56, the prefix #11234 moved this backend to for unrelated reasons.

Nor was any boot-order plumbing needed: the shape the backend already produces is the one Farm relies on — the primary disk carries bootindex=1 but is all zeros, so the firmware falls through to SetupOS, and install-hostos.sh's IC-OS UEFI entry then wins out of the persisted per-VM varstore. Confirmed in the log: BdsDxe: loading Boot0004 "IC-OS" ... Booting linux from system A.

Four things were missing in the driver, plus one hard blocker in SetupOS.

The driver. The install target was built as a DiskImage::Url whose _URL variable run_systest.sh never sets under the local backend; setup_and_start_nested_vms gets its Local arm; NESTED_CONFIG_IMAGE_PATH claimed .zst for a file nothing compresses, which the local backend would have run unzstd over; and the nested node's NNS URL had to stop being the ic-gateway, which locally serves a self-signed cert for a domain the group's resolver knows nothing about, with no trust-anchor knob on a nested GuestOS. An unregistered node cannot pass the replicas' firewall either, hence with_group_wide_firewall_whitelist() under Local.

Also: pad_to_request_alignment now refuses a GPT image rather than silently displacing its backup header (the SetupOS installer disk is the first boot disk attached this way; it happens to be 1 MiB-aligned), and the nested VM drops from 32 GiB to 16 GiB — the floor, since the driver takes 8 GiB off and HostOS then takes 4 more, leaving the GuestOS the same 4 GiB every other node VM gets. That reduction applies on Farm too; all 20 Farm nested targets pass with it.

The blocker: SetupOS halts because chrony can never synchronise

check-ntp.sh is the one SetupOS check not gated on ic.setupos.run_checks, so setupos-disable-checks does not skip it. It waits 60s for NTPSynchronized and then halts the installation foreverlog_and_halt_installation_on_error ends in sleep infinity. The nested VM sat in SetupOS for the full 15-minute registration budget and never installed anything.

The obvious fix — serve NTP to the group ourselves — does not work, and the first two commits here do exactly that before it is reverted. chrony's authselectmode defaults to mix, and sources.c then marks the authenticated sources required as soon as both authenticated and unauthenticated sources are configured — which is precisely what chrony.conf does in pairing fourteen nts servers with two plain pools. Sources configured from names that never resolve still count towards that. So the clock can only ever be synchronised from one of those fourteen NTS servers. The driver's SNTP server was polled every 64s for the whole run and never selected; chronyd said so in the node journals:

Can't synchronise: no required source in selectable sources

Verified against chronyd 4.8 directly: given that config shape it polls a reachable plain server for 100s without selecting it, and selects it in 4s once authselectmode ignore is added. Satisfying the check for real would mean serving NTS under one of those fourteen hostnames with a certificate the guest trusts, which no test harness can arrange.

So the check is gated instead, like the four that also need connectivity, and the reasoning is recorded on check_ntp so this is not attempted a third time. Nothing is lost: every test VM takes its clock from the hypervisor via -rtc base=utc.

A note on the two reverts in the history

Both are kept rather than squashed, because each records a plausible approach and the measurement that kills it — worth more to the next reader than a clean diff.

  1. serve_ntp_task (the SNTP server above), reverted for the authselectmode reason.
  2. Ordering setupos.service after serial-getty@ttyS0.service, an attempt to fix a second defect found on the way: the SetupOS install log vanishes from the serial console 1.02s into setupos.sh and never returns, which is why the hang above took an investigation rather than one glance at a console. output-wrapper.sh holds a single tee descriptor on /dev/ttyS0, and the getty starting on that tty hangs it up, so every later write fails with EIO; tee drops the console and the install runs on silently. Ordering does not help, measured: serial-getty@.service is itself Type=idle, so its start job completes at fork time, long before its child does the tty setup — and agetty calls vhangup() on its own anyway, so no unit directive can prevent it. Fixing it means rewriting the wrapper every IC-OS install path depends on, so it is left for a change of its own rather than smuggled in here.

Scope

registration and both *_upgrade_smoke_test targets are un-pinned. backend is per target definition, so that also un-pins the three _head_nns variants system_test_nns generates; all six were run locally before dropping the pins.

The two *_from_latest_release_to_current targets stay pinned: they install from a prebuilt mainnet SetupOS image, of which setupos-disable-checks can only rewrite the grub boot args, so their check-ntp.sh predates this gate. Their TODOs now say that instead of "unverified".

One thing for reviewers to weigh: this takes the non-manual nested _local targets from two to six, and system_tests.bzl:342 reserves CPU only — ceil(cpus / cpus_oversubscription_factor) = 9 of the 27 these declare — with no accounting for memory or disk. Three can therefore run concurrently on a 32-core runner, for ~84 GiB of nominal VM memory and ~60 GiB of qcow2 overlays. cpus_oversubscription_factor = 1 would serialise them if that turns out to matter; no target in the repo sets it today, so this is left at the default.

Testing

  • All six nested _local targets pass on a local dev box, run one at a time:

    target
    registration_local PASSED in 892.4s
    registration_head_nns_local PASSED in 740.8s
    guestos_upgrade_smoke_test_local PASSED in 947.9s
    guestos_upgrade_smoke_test_head_nns_local PASSED in 835.9s
    hostos_upgrade_smoke_test_local PASSED in 1297.2s
    hostos_upgrade_smoke_test_head_nns_local PASSED in 1247.1s

    SetupOS installs, configure_efi writes the IC-OS entry, the VM reboots into HostOS, and the nested GuestOS onboards: "Join request successful! The node has successfully joined the Internet Computer". The upgrade targets are real upgrades, not re-runs of registration: GuestOS goes 0000...00000000...0000-test after fetching the update image from http://[<group files>]:8080/<hash>, and the HostOS one reaches the HostOS over SSH from the driver — the first thing to exercise that path here, and it works unchanged because HostOS's nftables dfinity_dcs set already covers the group prefix.

  • All 20 Farm nested targets pass on CI Main, including registration (633.5s) and both upgrade smoke tests, with the 16 GiB change.

  • cargo check --all-targets --all-features -p ic-system-test-driver, cargo fmt, ./ci/scripts/rust-lint.sh — clean.

  • bazel build //... --nobuild, bazel run //:buildifier, bazel run //:shfmt-format — clean.

  • //ic-os/components:check_unused_components_test, //ic-os/components/setupos:test_setupos, //rs/tests/driver:unit_tests — pass.

rs/tests/driver:generic_workload_engine is renamed to unit_tests along the way: crate = compiles the whole library in test mode, so it was already running driver::report, driver::subprocess_ipc, driver::task and driver::timeout too, not just the workload engine it was named after.

🤖 Generated with Claude Code

basvandijk and others added 2 commits August 21, 2026 14:28
The nested system-tests install a node from SetupOS, and SetupOS refuses to
install onto a machine whose clock is not synchronized. `check-ntp.sh` waits 60s
for `timedatectl show -p NTPSynchronized` to become `yes`, then halts the
installation forever -- `log_and_halt_installation_on_error` ends in
`sleep infinity`. It is the one SetupOS check that is *not* gated on
`ic.setupos.run_checks`, so the `setupos-disable-checks` pass that turns a dev
image into a test image, which does skip the age, hardware, network and
guestos-elected checks, leaves this one running.

On the local backend chrony can never satisfy it. `chrony.conf` names fourteen
public NTS servers and two NTP pools; the backend runs in a network namespace
with no external connectivity and its `dnsmasq` runs `--no-resolv`, so every one
of those names comes back REFUSED. No source, no sync, no installation.

Gating the check like the other four would have been a one-line change, but it
only reaches images we build ourselves: `guestos_upgrade_from_latest_release_to_current`
and its HostOS sibling boot a prebuilt *mainnet* SetupOS image, and
`create_test_img` can only rewrite that image's grub boot args. So serve NTP
instead. That works with any image, and it removes a divergence rather than
hiding one: HostOS and GuestOS chrony now report a synchronized clock on the
local backend as they do on Farm, instead of running permanently unsynchronized
behind a clock that only happens to be right because every VM reads the host's
through `-rtc base=utc`.

`serve_ntp_task` answers SNTP on UDP 123 at the group's gateway, and
`start_dnsmasq` seeds its hosts-file with the two pool names pointing there. The
gateway already carries the resolver addresses GuestOS is hard-coded to query, so
this puts the group's NTP service on the one address every guest talks to anyway.
A dedicated address was not an option: the `/64`'s subnet-id field is two bits
wide and all four values are taken -- nodes, management, journald and the file
server.

The fourteen NTS names are deliberately left unresolvable. Answering them would
mean terminating an NTS-KE TLS handshake on port 4460, and `prefer` only biases
chrony's selection, so an unreachable preferred source does not stop it selecting
the pool source; a REFUSED lookup is the cheaper outcome.

Seeding the records in `start_dnsmasq` rather than through `add_dns_record` means
they are there for the first guest to boot, with no SIGHUP to race, and
`add_dns_record` appends, so the records tests register later are unaffected.
Binding a privileged port needs no new capability work: the driver created the
user namespace it runs in, so it holds `CAP_NET_BIND_SERVICE` in its effective
set, and task subprocesses fork rather than exec. The task itself is modelled on
`serve_files_task` down to the plan wiring, so the scheduler kills it with the
subtree instead of treating its non-return as a failure.

The two DNS records are seeded for *every* local group, not just the nested ones.
That is deliberate -- nothing else in the local suite resolves these names, and
`dnsmasq` already REFUSES every other external name -- and it is what makes the
clock right for ordinary GuestOS nodes too.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A nested test (`rs/tests/nested`) drives the whole node-deployment chain: SetupOS
installs HostOS onto a blank disk, HostOS boots a nested GuestOS VM, and that
GuestOS registers with the NNS. All five non-bare-metal targets were pinned to
Farm because `setup_and_start_nested_vms` had only a Farm code path.

Most of what the local backend needed was already there, and the parts that
matched did so exactly rather than by luck. `allocate_resources` already had a
`Local` arm. `write_nested_vm` derives the HostOS and GuestOS addresses by
SLAAC'ing deterministic MACs into the allocated VM's `/64`, which the backend's
`vm_mac` and `create_vm` feed unchanged. `create_setupos_config_image` writes
`ipv6_gateway = <prefix>::1`, which is `group_gateway_ipv6` on the group bridge --
and HostOS pings that gateway through each candidate NIC before accepting it, so
the address has to be real, not just consistent. Three MACs end up behind one TAP
(SetupOS, then HostOS, which writes its derived MAC onto the outer NIC, then the
inner GuestOS), which a Linux bridge learns without being told. And HostOS's
nftables only accepts SSH from its `dfinity_dcs` set, which contains
`2a00:fb01:400::/56` -- the prefix this backend moved to in 4d4ab2c, for
unrelated reasons.

Four things were actually missing.

`get_resource_request_for_nested_nodes` built the install target as a
`DiskImage::Url` unconditionally, and `run_systest.sh` never sets
`ENV_DEPS__EMPTY_DISK_IMG_URL` under the local backend (it only hashes the file),
so the URL was absent -- and `start_vm` panics on a URL primary image anyway. Both
that image and the SetupOS image now go through `ic_images` getters shaped like
`get_guestos_disk_image`, which resolve to a local path when there is one.

`setup_and_start_nested_vms` gets its `Local` arm, mirroring the sibling
`setup_and_start_vms`: attach the SetupOS image and the config image, then start.
No boot-order plumbing is needed, because the shape the backend already produces
is the one Farm relies on -- the primary disk carries `bootindex=1` but is all
zeros, so the firmware falls through to SetupOS, and once `install-hostos.sh` has
written HostOS onto it and created the `IC-OS` UEFI entry (persisted in the
per-VM `OVMF_VARS.fd`), that entry wins.

`NESTED_CONFIG_IMAGE_PATH` claimed to be `config.img.zst`, but
`build-setupos-config-image.sh` writes a plain FAT image -- nothing compresses it.
On Farm the name is only a multipart form field, so the lie was invisible; the
local backend dispatches on the extension and would have run `unzstd` over a FAT
filesystem. SetupOS finds the image by its `OVERRIDE` label either way, so the
name is now honest.

The nested node's NNS URL had to change. On Farm it is the ic-gateway, and that is
not incidental: a nested node is not in the registry yet, so it cannot pass the
replicas' firewall, while the gateway fronts an API boundary node that can.
Neither of the gateway's properties survives locally -- it serves a self-signed
certificate for a `.local` domain the group's `dnsmasq` knows nothing about, and a
nested GuestOS has no knob for either, since `make_bootstrap_options` gives it no
trust anchors the way a driver-managed node's config image does. So point it at
the NNS node over plain HTTP, exactly as `create_config_disk_image` does for every
other node in the group, and have the test open the group's range with
`with_group_wide_firewall_whitelist` so an unregistered node can reach it. Keeping
that opt-in per test preserves the deliberate narrowing of the default whitelist
to the driver's own addresses.

Two smaller things. `pad_to_request_alignment` ran over every extra disk, and its
own doc-comment warned that boot disks must not be padded, because a GPT keeps its
backup header in the last sector; the SetupOS installer disk is the first boot disk
to be attached this way. Its partitions sit on 1 MiB boundaries, so no padding
happens today -- but silently, so it now detects the GPT signature and refuses
rather than corrupting a partition table. And the nested VM's memory comes down
from the Farm default of 32 GiB to 16 GiB on the local backend, where all VMs
share one host. 16 GiB is the floor, not a guess: the driver takes
`HOSTOS_MEMORY_RESERVED_GIB` (8) off before writing `dev_vm_resources.memory`, and
HostOS then takes `UPGRADE_VM_MEMORY_GIB` (4) off for the upgrade VM, so 16 leaves
the GuestOS the same 4 GiB every other node VM gets, while 12 would size it 0 GiB
and libvirt would refuse the domain.

Only `registration` is un-pinned. The four upgrade targets have not been run on the
local backend yet -- they additionally download an update image from the per-group
file server and survive a reboot inside the test's timeouts -- so their TODOs now
say that, rather than claiming the backend cannot boot a nested VM.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added the feat label Aug 21, 2026
@basvandijk
basvandijk requested a balanced review from Copilot August 21, 2026 14:31
@basvandijk basvandijk added the CI_ALL_BAZEL_TARGETS Runs all bazel targets label Aug 21, 2026
Empty commit to re-run CI now that the PR carries `CI_ALL_BAZEL_TARGETS`. The
label sets `skip_long_tests=false`, which is what these nested targets need: they
are tagged `long_test`, so the default PR run skips them entirely.

It reaches both halves of the split. `ci-main` excludes `local_system_test`, so it
runs the Farm `//rs/tests/nested:registration` -- the regression check that the new
`SystemTestBackend::Local` arms did not disturb the Farm path they branch away
from. `ci-rbe-evaluation` is the mirror image: it excludes `farm_system_test` and
so runs `//rs/tests/nested:registration_local` on the Namespace cluster, which is
the end-to-end verification of the change itself.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Enables nested VM system tests on the local QEMU backend while preserving Farm behavior.

Changes:

  • Adds local SetupOS boot, resource, networking, and NNS registration support.
  • Provides local DNS/SNTP services required by SetupOS.
  • Enables the registration test locally and adds safety around GPT image alignment.

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.

Show a summary per file
File Description
rs/tests/nested/src/util.rs Whitelists local nested-node registration traffic.
rs/tests/nested/src/lib.rs Reduces local nested VM memory allocation.
rs/tests/nested/BUILD.bazel Enables local registration testing and updates TODOs.
rs/tests/driver/src/driver/test_env_api.rs Removes obsolete empty-disk URL helpers.
rs/tests/driver/src/driver/serve_ntp_task.rs Implements and tests the local SNTP server.
rs/tests/driver/src/driver/resource.rs Selects backend-appropriate empty disks.
rs/tests/driver/src/driver/nested.rs Corrects the raw config-image extension.
rs/tests/driver/src/driver/mod.rs Exposes the NTP task module.
rs/tests/driver/src/driver/local_backend.rs Adds NTP DNS records and GPT-safe alignment.
rs/tests/driver/src/driver/ic_images.rs Adds backend-aware SetupOS and empty-disk selection.
rs/tests/driver/src/driver/group.rs Runs NTP as a supervised local task.
rs/tests/driver/src/driver/bootstrap.rs Boots nested VMs locally and selects a reachable NNS URL.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread rs/tests/driver/src/driver/bootstrap.rs Outdated
Comment thread rs/tests/nested/src/lib.rs Outdated
basvandijk and others added 2 commits August 22, 2026 11:40
Two review comments.

`get_setupos_disk_image` had exactly one caller, in a branch that had already
matched on `SystemTestBackend::Local` -- so it built a `DiskImage`, returned it
through a Farm arm that could not be reached, and forced the caller to destructure
it with a `let ... else { bail! }` for a case that could not happen. Read
`ENV_DEPS__SETUPOS_DISK_IMG_PATH` at the call site instead, which is also what the
existing `Local` arms in `resource.rs` and `universal_vm.rs` do. `get_empty_disk_image`
stays: it is genuinely called for both backends, because
`get_resource_request_for_nested_nodes` needs a `DiskImage` either way.

The 16 GiB nested VM is now the default for Farm too, rather than a local-backend
override, which deletes the override and its explanation. Farm was giving the
nested GuestOS 20 GiB (32 minus the driver's 8 and HostOS's 4) where every other
node VM in a test gets `DEFAULT_MEMORY_KIB_PER_VM`, 4 GiB. A nested node runs the
same replica as its unnested peers, so there was nothing for the extra 16 GiB to
do -- the figure looks inherited from production sizing, where a real node has
512 GiB. The only other consumer of the default is
`//rs/tests/node:launch_single_host`; `rs/tests/nested/nns_recovery` sets both
vcpus and memory explicitly (40 / 48 GiB) and is unaffected.

While there: two `Default` impls in `rs/ic_os/config` claimed to be
`(HOSTOS_VCPUS_PER_VM / 2, HOSTOS_MEMORY_KIB_PER_VM / 2)`. That was already wrong
for vcpus (8 / 2 = 4, not the 16 they set) and wrong for both, since nested tests
get `total - reserved` rather than half. Halving the memory constant would have
made it wronger still, so say what those defaults actually are: a fallback for a
`deployment.json` that omits the field, which nested tests never take.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment thread rs/tests/driver/src/driver/nested.rs Outdated
Comment on lines +34 to +37
// `build-setupos-config-image.sh` writes a raw FAT image here -- despite what an
// earlier `.zst` in this name suggested, nothing compresses it. The Local backend
// dispatches on the extension when it attaches the image
// (`LocalBackend::extract_image`), so the name has to be honest.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Don't mention the previous situation. Just remove the whole comment.

Suggested change
// `build-setupos-config-image.sh` writes a raw FAT image here -- despite what an
// earlier `.zst` in this name suggested, nothing compresses it. The Local backend
// dispatches on the extension when it attaches the image
// (`LocalBackend::extract_image`), so the name has to be honest.

Comment thread rs/tests/driver/src/driver/resource.rs Outdated
Comment on lines +45 to +46
/// on one host. 12 GiB would be too little -- the GuestOS would come out at 0 GiB
/// and libvirt would refuse the domain. Tests that need more say so with

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Why mention the 12 GiB? Just remove that sentence:

Suggested change
/// on one host. 12 GiB would be too little -- the GuestOS would come out at 0 GiB
/// and libvirt would refuse the domain. Tests that need more say so with
/// on one host. Tests that need more say so with

basvandijk and others added 7 commits August 22, 2026 12:14
SetupOS output stops one second into `setupos.sh` and never resumes. What is lost
is everything that matters: the per-check progress, and the
"INTERNET COMPUTER - SETUP - FAILED" banner that
`log_and_halt_installation_on_error` prints -- including the chrony service log
and `chronyc sources` dump that `check-ntp.sh` attaches to it -- right before it
goes to `sleep infinity`. The installer looks like it hung for no reason, and the
VM has to be reverse-engineered from the outside.

The cause is not the installer. `output-wrapper.sh` runs
`setupos.sh 2>&1 | tee /dev/ttyS0`, so `tee` holds a single file descriptor on
that tty for the whole install. `serial-getty@.service` carries
`TTYVHangup=yes`, so systemd issues `TIOCVHANGUP` on /dev/ttyS0 when the getty on
it starts; the kernel then swaps every *other* descriptor on that tty to
`hung_up_tty_fops`, whose `write` returns EIO. `tee` drops the failing output,
keeps writing to the one that still works, and the install runs on -- silently.
Nor does tee's stdout help: `StandardOutput=tty` is /dev/console, which the boot
args' trailing `console=tty0` points at the virtual console, and a VM with no VGA
registers that as `Console: colour dummy device`.

`Type=idle` was meant to prevent exactly this by keeping the unit behind the
gettys, but it only defers while the job queue is busy. This unit waits for
systemd-networkd-wait-online and serial-getty@ttyS0 waits for
systemd-user-sessions; the network releases both at once and the installer wins
by about a second, which is precisely the wrong side of the hangup. Seen at +1.0s
in every run so far -- one local, three CI attempts.

So order the unit after the getty, and let `tee` open the tty once the hangup is
already behind it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`check-ntp.sh` is the one SetupOS check not gated on `ic.setupos.run_checks`, so
the `setupos-disable-checks` pass that turns a dev image into a test image --
which does skip the age, hardware, network and guestos-elected checks -- leaves
this one running. In a hermetic test environment it cannot pass, and failing it is
fatal: `log_and_halt_installation_on_error` ends in `sleep infinity`, so the node
is never installed. This is what blocks the nested system-tests on the local
backend, whose network namespace has no external connectivity.

The obvious remedy -- serving NTP to the test network -- does not work. chrony's
`authselectmode` defaults to `mix`, and `sources.c` then marks the *authenticated*
sources `require`d as soon as both authenticated and unauthenticated sources are
configured, which is exactly what `chrony.conf` does in pairing fourteen `nts`
servers with two plain pools. Sources configured from names that never resolve
still count towards that. So the clock can only ever be synchronized from one of
those fourteen NTS servers, and a test-local NTP server is never selected however
correct its replies: chronyd reports "Can't synchronise: no required source in
selectable sources" and `NTPSynchronized` stays `no`. Verified against chronyd
4.8 -- with that config shape it polls a reachable plain server for 100s without
selecting it, and selects it in 4s once `authselectmode ignore` is added.

Satisfying the check for real would mean serving NTS under one of those hostnames
with a certificate the guest trusts, which no test harness can arrange. So gate
it, like the four checks that also need connectivity. Nothing is lost: every test
VM takes its clock from the hypervisor.

`set_hwclock_utc` stays unconditional -- it needs no network.

The two `*_from_latest_release_to_current` targets install from a prebuilt
mainnet SetupOS image, of which `setupos-disable-checks` can only rewrite the
grub boot args, so they keep hanging until this gate has shipped in a release.
Their TODOs now say so rather than "not verified yet".

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This reverts commit e9d4fba.

`serve_ntp_task` cannot do what it was added for. It rested on `prefer` merely
biasing chrony's selection, so that an unreachable preferred NTS source would not
stop chrony selecting the plain pool source pointed at the driver. That much is
true of `prefer` -- but it is not the whole story. `authselectmode` defaults to
`mix`, under which chrony marks the authenticated sources `require`d as soon as
both kinds are configured, so no unauthenticated source is ever selected. In
`registration_local` the guests polled the driver's server every 64s for the
entire run and stayed unsynchronized throughout, with chronyd reporting
"Can't synchronise: no required source in selectable sources".

The previous commit gates `check-ntp.sh` instead, which is what actually unblocks
the nested tests, so none of this is needed: no local test requires a synchronized
clock, and the two DNS records seeded here existed only to point chrony at the
server. The comment on `check_ntp` now records why serving NTP is a dead end, so
this is not attempted a second time.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This reverts commit 117c1f7, which ordered `setupos.service` after
`serial-getty@ttyS0.service` so that `output-wrapper.sh`'s `tee` would open
/dev/ttyS0 only once the getty's hangup was behind it. Measured on
`registration_local`, it changes nothing: the console still goes dark 1.02s after
`setupos.sh - Start`, exactly as before, with the getty's terminal probe and
issue banner as the last thing on the wire.

Two reasons, either of which is fatal to the approach. `serial-getty@.service` is
itself `Type=idle`, so its start job completes when systemd forks the child, well
before that child waits out the idle pipe, sets the tty up and execs -- the
ordering is satisfied within microseconds of the fork and says nothing about when
the hangup happens. And the hangup is not only systemd's `TTYVHangup=yes`: agetty
calls `vhangup()` on its own (`/dev/%s: vhangup() failed: %m` is in the binary),
so no unit directive can prevent it either.

Losing the install log is worth fixing -- it is what made the `check-ntp.sh` hang
take a full investigation instead of one glance at a console -- but it has to be
fixed where the descriptor lives, in `output-wrapper.sh`, not by ordering. Doing
that means rewriting the wrapper every IC-OS install path depends on, so it is
left for a change of its own rather than smuggled in here.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

Comment on lines +51 to +54
if check_cmdline_var ic.setupos.run_checks; then
check_ntp
else
echo "* NTP synchronization check skipped by request via kernel command line"
Comment on lines +582 to +584
SystemTestBackend::Local => env
.topology_snapshot()
.root_subnet()
…ackend

`guestos_upgrade_smoke_test` and `hostos_upgrade_smoke_test` were pinned to Farm
pending verification: on top of installing a node from SetupOS they download an
update image from the per-group file server and have to survive a reboot inside
the test's timeouts. Both pass, so drop the pin.

`backend` is per target definition, so this un-pins the `_head_nns` variants
`system_test_nns` generates as well. All six were run locally, one at a time:

  //rs/tests/nested:registration_local                        PASSED in  892.4s
  //rs/tests/nested:registration_head_nns_local               PASSED in  740.8s
  //rs/tests/nested:guestos_upgrade_smoke_test_local          PASSED in  947.9s
  //rs/tests/nested:guestos_upgrade_smoke_test_head_nns_local PASSED in  835.9s
  //rs/tests/nested:hostos_upgrade_smoke_test_local           PASSED in 1297.2s
  //rs/tests/nested:hostos_upgrade_smoke_test_head_nns_local  PASSED in 1247.1s

Each upgrade target is a real upgrade rather than a re-run of `registration`: the
GuestOS one goes from version `0000...0000` to `0000...0000-test` after fetching
the update image from `http://[<group files>]:8080/<hash>`, and the HostOS one
additionally reaches the HostOS over SSH from the driver -- the first time
anything has exercised that path on this backend, and it works unchanged because
HostOS's nftables `dfinity_dcs` set already covers the group prefix. Against
budgets of 30 min per test and 40 min overall there is comfortable headroom.

Note this takes the non-manual nested `_local` targets from two to six, and
`system_tests.bzl` reserves CPU only -- `ceil(cpus / cpus_oversubscription_factor)`
= 9 of the 27 these declare -- with no accounting for memory or disk. Three can
therefore run concurrently on a 32-core runner, for ~84 GiB of nominal VM memory
and ~60 GiB of qcow2 overlays. If that starts biting, `cpus_oversubscription_factor
= 1` on these targets serialises them.

Still pinned are the two `*_from_latest_release_to_current` targets, which install
from a prebuilt mainnet SetupOS image whose `check-ntp.sh` predates the
`ic.setupos.run_checks` gate; see their TODOs.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Suppressed comments (1)

rs/tests/driver/src/driver/bootstrap.rs:586

  • The Local branch panics before reaching the dummy fallback when no IC/NNS was created: TopologySnapshot::root_subnet() calls root_subnet_id(), which explicitly panics if the registry has no root subnet (test_env_api.rs:547-559). This breaks the existing //rs/tests/node:launch_single_host_local setup, which starts only NestedNodes and intentionally creates no IC infrastructure. Guard the topology lookup (for example with env.prep_dir("")) and use the dummy URL when no prep state exists.
        SystemTestBackend::Local => env
            .topology_snapshot()
            .root_subnet()
            .nodes()
            .next()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CI_ALL_BAZEL_TARGETS Runs all bazel targets feat

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants