Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions build/nix/grafana.nix
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,10 @@ in
security = {
admin_user = "admin";
admin_password = cfg.adminPassword;
# Newer NixOS grafana modules assert this is set explicitly (no default).
# The dev microvm has no real secrets to protect — the file already
# warns this is for local development only.
secret_key = "pcp-microvm-dev-only-not-a-real-secret";
};

# Disable analytics/phone-home
Expand Down
51 changes: 45 additions & 6 deletions build/nix/tests/test-all-microvms.nix
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ pkgs.writeShellApplication {
gnugrep
procps
nix
iproute2
];
text = ''
set +e # Don't exit on error - we want to continue and report all results
Expand All @@ -91,6 +92,8 @@ pkgs.writeShellApplication {
SSH_RETRY_DELAY=${toString constants.test.sshRetryDelaySeconds}
BASE_SSH_PORT=${toString constants.ports.sshForward}
TAP_VM_IP="${constants.network.vmIp}"
TAP_BRIDGE="${constants.network.bridge}"
TAP_DEVICE="${constants.network.tap}"

# Service warmup: wait for services to fully start after SSH connects
# pmlogger takes ~9s, Grafana HTTP takes ~17s after service activation
Expand Down Expand Up @@ -259,6 +262,16 @@ pkgs.writeShellApplication {
run_ssh_check "$host" "$port" "pminfo kernel.all.load" "pminfo -f kernel.all.load"
}

# Check whether host TAP networking has been set up.
# Returns 0 if both the bridge and TAP device exist on the host.
# Note: operational state may be DOWN when no VM is attached — that is
# the normal pre-test condition, so we only check for existence.
check_tap_network() {
ip link show "$TAP_BRIDGE" >/dev/null 2>&1 || return 1
ip link show "$TAP_DEVICE" >/dev/null 2>&1 || return 1
return 0
}

# Check Grafana HTTP endpoint (with retry - Grafana needs extra startup time)
check_grafana_http() {
local host="$1"
Expand Down Expand Up @@ -546,6 +559,24 @@ pkgs.writeShellApplication {
# Ensure clean state
stop_all_vms

# Preflight: detect whether host TAP networking is set up.
# When it is not, tap variants are skipped with an actionable hint
# instead of failing with an opaque VM_START_FAILED.
TAP_NETWORK_READY=true
if check_tap_network; then
log "TAP networking detected ($TAP_BRIDGE, $TAP_DEVICE)"
else
TAP_NETWORK_READY=false
log_section "TAP networking not set up — tap variants will be skipped"
echo "Bridge '$TAP_BRIDGE' or TAP device '$TAP_DEVICE' is not present/up."
echo ""
echo "To enable tap variants (base-tap, eval-tap, grafana-tap), run:"
echo " nix run .#pcp-check-host"
echo " sudo nix run .#pcp-network-setup"
echo ""
echo "Then re-run this test. To suppress this message, pass --skip-tap."
fi
Comment on lines +562 to +578
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Preflight check should be conditional on --skip-tap flag.

The preflight check currently runs unconditionally, but the PR description explicitly states "no preflight banner" when --skip-tap is passed. Users who invoke --skip-tap will still see either the detection log (line 567) or the setup banner (lines 570-577), which is confusing—why tell users to set up TAP networking when they explicitly asked to skip it?

Proposed fix to make preflight conditional
   # Ensure clean state
   stop_all_vms
 
-  # Preflight: detect whether host TAP networking is set up.
-  # When it is not, tap variants are skipped with an actionable hint
-  # instead of failing with an opaque VM_START_FAILED.
-  TAP_NETWORK_READY=true
-  if check_tap_network; then
-    log "TAP networking detected ($TAP_BRIDGE, $TAP_DEVICE)"
-  else
-    TAP_NETWORK_READY=false
-    log_section "TAP networking not set up — tap variants will be skipped"
-    echo "Bridge '$TAP_BRIDGE' or TAP device '$TAP_DEVICE' is not present/up."
-    echo ""
-    echo "To enable tap variants (base-tap, eval-tap, grafana-tap), run:"
-    echo "    nix run .#pcp-check-host"
-    echo "    sudo nix run .#pcp-network-setup"
-    echo ""
-    echo "Then re-run this test. To suppress this message, pass --skip-tap."
+  # Preflight: detect whether host TAP networking is set up (unless --skip-tap).
+  # When it is not, tap variants are skipped with an actionable hint
+  # instead of failing with an opaque VM_START_FAILED.
+  TAP_NETWORK_READY=true
+  if [[ "$SKIP_TAP" == "false" ]]; then
+    if check_tap_network; then
+      log "TAP networking detected ($TAP_BRIDGE, $TAP_DEVICE)"
+    else
+      TAP_NETWORK_READY=false
+      log_section "TAP networking not set up — tap variants will be skipped"
+      echo "Bridge '$TAP_BRIDGE' or TAP device '$TAP_DEVICE' is not present/up."
+      echo ""
+      echo "To enable tap variants (base-tap, eval-tap, grafana-tap), run:"
+      echo "    nix run .#pcp-check-host"
+      echo "    sudo nix run .#pcp-network-setup"
+      echo ""
+      echo "Then re-run this test. To suppress this message, pass --skip-tap."
+    fi
   fi
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Preflight: detect whether host TAP networking is set up.
# When it is not, tap variants are skipped with an actionable hint
# instead of failing with an opaque VM_START_FAILED.
TAP_NETWORK_READY=true
if check_tap_network; then
log "TAP networking detected ($TAP_BRIDGE, $TAP_DEVICE)"
else
TAP_NETWORK_READY=false
log_section "TAP networking not set up — tap variants will be skipped"
echo "Bridge '$TAP_BRIDGE' or TAP device '$TAP_DEVICE' is not present/up."
echo ""
echo "To enable tap variants (base-tap, eval-tap, grafana-tap), run:"
echo " nix run .#pcp-check-host"
echo " sudo nix run .#pcp-network-setup"
echo ""
echo "Then re-run this test. To suppress this message, pass --skip-tap."
fi
# Ensure clean state
stop_all_vms
# Preflight: detect whether host TAP networking is set up (unless --skip-tap).
# When it is not, tap variants are skipped with an actionable hint
# instead of failing with an opaque VM_START_FAILED.
TAP_NETWORK_READY=true
if [[ "$SKIP_TAP" == "false" ]]; then
if check_tap_network; then
log "TAP networking detected ($TAP_BRIDGE, $TAP_DEVICE)"
else
TAP_NETWORK_READY=false
log_section "TAP networking not set up — tap variants will be skipped"
echo "Bridge '$TAP_BRIDGE' or TAP device '$TAP_DEVICE' is not present/up."
echo ""
echo "To enable tap variants (base-tap, eval-tap, grafana-tap), run:"
echo " nix run .#pcp-check-host"
echo " sudo nix run .#pcp-network-setup"
echo ""
echo "Then re-run this test. To suppress this message, pass --skip-tap."
fi
fi
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@build/nix/tests/test-all-microvms.nix` around lines 562 - 578, Wrap the TAP
preflight block so it only runs when the user did not pass the skip-tap flag:
check the parsed --skip-tap indicator (e.g., SKIP_TAP or skip_tap) and only
execute the existing logic that calls check_tap_network and sets
TAP_NETWORK_READY / prints messages about TAP_BRIDGE and TAP_DEVICE if the flag
is false; if skip-tap is true, skip the whole detection and banner so no TAP
setup hints are shown. Ensure you reference the same symbols already used
(check_tap_network, TAP_NETWORK_READY, TAP_BRIDGE, TAP_DEVICE) and do not change
the existing messages aside from guarding their execution behind the skip-tap
condition.


# Define test order (NOTE: BCC is deprecated - use BPF PMDA instead)
VARIANT_ORDER=(base base-tap eval eval-tap grafana grafana-tap bpf)

Expand All @@ -555,12 +586,20 @@ pkgs.writeShellApplication {
continue
fi

# Skip TAP variants if requested
if [[ "$SKIP_TAP" == "true" ]] && [[ "$key" == *"-tap" ]]; then
RESULTS[$key]="SKIPPED"
DURATIONS[$key]=0
TOTAL_SKIPPED=$((TOTAL_SKIPPED + 1))
continue
# Skip TAP variants when requested or when host networking is not set up.
if [[ "$key" == *"-tap" ]]; then
if [[ "$SKIP_TAP" == "true" ]]; then
RESULTS[$key]="SKIPPED (--skip-tap)"
DURATIONS[$key]=0
TOTAL_SKIPPED=$((TOTAL_SKIPPED + 1))
continue
fi
if [[ "$TAP_NETWORK_READY" == "false" ]]; then
RESULTS[$key]="SKIPPED (tap network not set up)"
DURATIONS[$key]=0
TOTAL_SKIPPED=$((TOTAL_SKIPPED + 1))
continue
fi
fi

# Get variant properties
Expand Down
20 changes: 10 additions & 10 deletions flake.lock

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

Loading