diff --git a/.github/workflows/package-currency.yml b/.github/workflows/package-currency.yml new file mode 100644 index 0000000..af5beea --- /dev/null +++ b/.github/workflows/package-currency.yml @@ -0,0 +1,249 @@ +name: PVE Package Currency + +# Lane 2 of the two-lane CI split (see docs/lane2-change-plan.md). +# +# Lane 1 (integration-tests.yml) pins the nested nodes to what the ISO ships +# and never upgrades — that pin is what makes it a stable merge gate. +# This lane does the opposite: it dist-upgrades the nested nodes to current +# PVE, reboots onto the new kernel, records the package set, and runs the full +# suite against it. +# +# REPORT-ONLY. Test failures here do NOT fail the job — this lane never gates +# a merge, and a red weekly cron nobody can action becomes noise. The signal is +# the recorded package set and (from commit 3) the rolling issue, not the check +# colour. A failure of the lane's own machinery — provisioning, upgrade, reboot +# — still fails the job, because that means the lane learned nothing. +# +# Shares the `integration-tests` concurrency group: both lanes drive the same +# nested VMIDs on the same parent node, so they must never run at once. + +concurrency: + group: integration-tests + cancel-in-progress: false + +on: + schedule: + # Mondays 06:00 UTC. Weekly is deliberate: this lane exists to notice + # upstream drift, and PVE's no-subscription repo does not move hourly. + - cron: '0 6 * * 1' + workflow_dispatch: + inputs: + keep_vms: + description: 'Skip cleanup so the nested PVE VMs survive for inspection' + required: false + type: boolean + default: false + +# packages: write is granted per-job to container-image, the only job that +# pushes. The self-hosted jobs get read-only tokens: they SSH into freshly +# upgraded nodes and should not be able to overwrite published GHCR images. +permissions: + contents: read + packages: read + +env: + SCRIPTS_DIR: tests/infrastructure/scripts + TEST_IMAGE: ghcr.io/goodolclint/psproxmoxve-integration + CACHE_DIR: /opt/pve-integration + PVE_VERSIONS: '9' + PVE9_ISO: proxmox-ve_9.2-1.iso + +jobs: + # ── Build module artifact (GitHub-hosted) ──────────────────────── + build: + runs-on: ubuntu-latest + timeout-minutes: 10 + steps: + - uses: actions/checkout@v7 + + - name: Setup .NET + uses: actions/setup-dotnet@v6 + with: + dotnet-version: '10.0.x' + + - name: Build module + run: dotnet publish src/PSProxmoxVE/PSProxmoxVE.csproj --configuration Release --framework netstandard2.0 --output ./publish/netstandard2.0 + + - name: Clean publish output + run: rm -f ./publish/netstandard2.0/*.deps.json + + - name: Upload module artifact + uses: actions/upload-artifact@v7 + with: + name: module-currency + path: ./publish/netstandard2.0/ + + # ── Build test container image (GitHub-hosted) ─────────────────────── + container-image: + runs-on: ubuntu-latest + timeout-minutes: 20 + permissions: + contents: read + packages: write + steps: + - uses: actions/checkout@v7 + + - name: Log in to GHCR + uses: docker/login-action@v4 + with: + registry: ghcr.io + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + + - name: Build and push + uses: docker/build-push-action@v7 + with: + context: . + file: tests/Dockerfile.test + target: dev-infra + push: true + tags: ${{ env.TEST_IMAGE }}:${{ github.sha }} + + # ── Provision, dist-upgrade, reboot (self-hosted) ──────────────────── + provision: + needs: [build, container-image] + runs-on: psproxmoxve + timeout-minutes: 60 + container: + image: ghcr.io/goodolclint/psproxmoxve-integration:${{ github.sha }} + credentials: + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + volumes: + - /opt/pve-integration:/opt/pve-integration + + steps: + # persist-credentials: false on the self-hosted jobs — nothing downstream + # uses git, and the token would otherwise be written into .git/config on + # a runner that outlives the job's container. + - uses: actions/checkout@v7 + with: + persist-credentials: false + + - name: Provision PVE instances and dist-upgrade + shell: bash + env: + PVE_ENDPOINT: ${{ secrets.PVE_ENDPOINT }} + PVE_API_TOKEN: ${{ secrets.PVE_API_TOKEN }} + PVE_TARGET_NODE: ${{ vars.PVE_TARGET_NODE }} + PVE_PASSWORD: ${{ secrets.PVE_TEST_PASSWORD }} + STORAGE_VM_FQDN: ${{ vars.STORAGE_VM_FQDN }} + TF_VAR_disk_storage: ${{ vars.DISK_STORAGE }} + TF_VAR_iso_storage: ${{ vars.ISO_STORAGE }} + TF_VAR_network_bridge: ${{ vars.NETWORK_BRIDGE }} + TF_VAR_pool_id: ${{ vars.POOL_ID }} + # The one line that separates this lane from lane 1. + PVE_DIST_UPGRADE: '1' + run: bash ${SCRIPTS_DIR}/run-integration.sh provision + + # success() not always(): when provisioning succeeded the package file + # must exist, and if-no-files-found: error enforces that invariant. When + # provisioning failed the job is already red, and a missing-artifact + # error would only bury the real cause. + - name: Upload recorded package set + if: success() + uses: actions/upload-artifact@v7 + with: + name: pve-package-set + path: ${{ env.CACHE_DIR }}/work/*-packages.txt + if-no-files-found: error + + # ── Integration tests against the upgraded nodes ───────────────────── + test: + needs: [build, provision] + runs-on: psproxmoxve + timeout-minutes: 20 + container: + image: ghcr.io/goodolclint/psproxmoxve-integration:${{ github.sha }} + credentials: + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + volumes: + - /opt/pve-integration:/opt/pve-integration + strategy: + fail-fast: false + matrix: + pve_version: ['9'] + + steps: + - uses: actions/checkout@v7 + with: + persist-credentials: false + + - name: Download module artifact + uses: actions/download-artifact@v8 + with: + name: module-currency + path: ./publish/netstandard2.0/ + + # Report-only applies to TEST failures, not to the lane's own machinery. + # cmd_test returns 3 for genuine Pester failures and 4 when it cannot + # reach or authenticate to a node. A blanket continue-on-error would + # swallow 4 as well — and an unreachable node is exactly the symptom of a + # reboot gone wrong, so the lane would go green having learned nothing. + - name: Run integration tests (PVE ${{ matrix.pve_version }}) + id: suite + shell: bash + env: + PVE_PASSWORD: ${{ secrets.PVE_TEST_PASSWORD }} + MODULE_ARTIFACT: ./publish/netstandard2.0 + run: | + set +e + bash ${SCRIPTS_DIR}/run-integration.sh test ${{ matrix.pve_version }} + rc=$? + echo "suite_rc=${rc}" >> "$GITHUB_OUTPUT" + if [ "${rc}" -eq 0 ]; then + echo "Suite passed against current PVE." + elif [ "${rc}" -eq 3 ]; then + echo "::warning title=Suite failed against current PVE::Report-only - tests failed (rc=3). See the uploaded results." + else + echo "::error title=Currency lane machinery failed::run-integration.sh exited ${rc}, which is not a test failure. The lane learned nothing about package currency." + fi + [ "${rc}" -eq 0 ] || [ "${rc}" -eq 3 ] + + - name: Diagnose cluster state + if: always() && steps.suite.outputs.suite_rc != '0' + shell: bash + env: + PVE_PASSWORD: ${{ secrets.PVE_TEST_PASSWORD }} + run: bash ${SCRIPTS_DIR}/diagnose-cluster.sh ${{ matrix.pve_version }} + + - name: Upload test results + if: always() + uses: actions/upload-artifact@v7 + with: + name: currency-test-results-pve${{ matrix.pve_version }} + path: TestResults/ + + # ── Cleanup: destroy all VMs (always runs) ────────────────────── + cleanup: + needs: [provision, test] + if: always() && needs.provision.result != 'skipped' && !inputs.keep_vms + runs-on: psproxmoxve + timeout-minutes: 15 + container: + image: ghcr.io/goodolclint/psproxmoxve-integration:${{ github.sha }} + credentials: + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + volumes: + - /opt/pve-integration:/opt/pve-integration + + steps: + - uses: actions/checkout@v7 + with: + persist-credentials: false + + - name: Cleanup PVE instances + shell: bash + env: + PVE_ENDPOINT: ${{ secrets.PVE_ENDPOINT }} + PVE_API_TOKEN: ${{ secrets.PVE_API_TOKEN }} + PVE_TARGET_NODE: ${{ vars.PVE_TARGET_NODE }} + PVE_PASSWORD: ${{ secrets.PVE_TEST_PASSWORD }} + STORAGE_VM_FQDN: ${{ vars.STORAGE_VM_FQDN }} + TF_VAR_disk_storage: ${{ vars.DISK_STORAGE }} + TF_VAR_iso_storage: ${{ vars.ISO_STORAGE }} + TF_VAR_network_bridge: ${{ vars.NETWORK_BRIDGE }} + TF_VAR_pool_id: ${{ vars.POOL_ID }} + run: bash ${SCRIPTS_DIR}/run-integration.sh force-cleanup diff --git a/tests/infrastructure/scripts/prepare-test-environment.sh b/tests/infrastructure/scripts/prepare-test-environment.sh index ff6c437..b19047c 100755 --- a/tests/infrastructure/scripts/prepare-test-environment.sh +++ b/tests/infrastructure/scripts/prepare-test-environment.sh @@ -35,20 +35,58 @@ if [[ "${DIST_UPGRADE}" == "1" ]]; then if [[ -n "${PKG_OUT}" ]]; then echo "Recording package set to ${PKG_OUT}..." - ${SSH_CMD} "dpkg-query -W -f='\${binary:Package}\t\${Version}\n' | sort" > "${PKG_OUT}" + # pipefail must be set in the REMOTE shell. ssh returns the remote + # pipeline's status, which is sort's — and sort succeeds on the empty + # input a failed dpkg-query produces, so a broken query would otherwise + # leave a zero-byte file and still exit 0. + ${SSH_CMD} "set -o pipefail; dpkg-query -W -f='\${binary:Package}\t\${Version}\n' | sort" > "${PKG_OUT}" + if [[ ! -s "${PKG_OUT}" ]]; then + echo "ERROR: empty package set from ${NESTED_IP}" >&2 + exit 1 + fi fi # A PVE dist-upgrade pulls proxmox-kernel-*; without a reboot the node runs # new userspace on the old kernel. Reboot unconditionally rather than # testing /var/run/reboot-required — that file comes from # update-notifier-common, which is not guaranteed on a PVE node. + # + # boot_id is the evidence that the reboot happened. Without it the `|| true` + # below swallows every ssh failure, the node stays up, and wait-for-api.sh + # matches the still-running pre-reboot pveproxy on its first poll. + boot_before="$(${SSH_CMD} "cat /proc/sys/kernel/random/boot_id")" + echo "Rebooting after dist-upgrade..." ${SSH_CMD} "systemctl reboot" || true - # The API stays up for a few seconds after the reboot is issued, so polling - # immediately would match the pre-reboot node and return at once. - sleep 30 + # Order matters: prove the reboot first (ssh returns before pveproxy does), + # then wait for the API, then for pmxcfs. + boot_after="" + for _ in $(seq 1 60); do + boot_after="$(${SSH_CMD} "cat /proc/sys/kernel/random/boot_id" 2>/dev/null || true)" + [[ -n "${boot_after}" && "${boot_after}" != "${boot_before}" ]] && break + sleep 5 + done + if [[ -z "${boot_after}" || "${boot_after}" == "${boot_before}" ]]; then + echo "ERROR: ${NESTED_IP} did not reboot (boot_id unchanged)" >&2 + exit 1 + fi + bash "${SCRIPT_DIR}/wait-for-api.sh" "${NESTED_IP}" 8006 600 + + # wait-for-api.sh only proves pveproxy answers. `pvesm set` below writes + # /etc/pve/storage.cfg, which needs pmxcfs to have mounted /etc/pve — on a + # freshly rebooted node those are seconds apart. + for _ in $(seq 1 30); do + ${SSH_CMD} "test -f /etc/pve/storage.cfg" 2>/dev/null && break + sleep 5 + done + + # printf, not echo: bash's builtin echo does not interpret \t without -e, + # which would make this the one row in the file without a real tab. + if [[ -n "${PKG_OUT}" ]]; then + ${SSH_CMD} "printf '# running-kernel\t%s\n' \"\$(uname -r)\"" >> "${PKG_OUT}" + fi fi # Enable snippets and import content types on local storage diff --git a/tests/infrastructure/scripts/prepare-test-environment.test.sh b/tests/infrastructure/scripts/prepare-test-environment.test.sh index 7e44d31..d34ec3d 100755 --- a/tests/infrastructure/scripts/prepare-test-environment.test.sh +++ b/tests/infrastructure/scripts/prepare-test-environment.test.sh @@ -1,9 +1,12 @@ #!/usr/bin/env bash # Self-check for prepare-test-environment.sh's opt-in dist-upgrade branch. # -# Stubs sshpass/curl/sleep on PATH so both paths run offline in ~0s, then +# Stubs sshpass/curl/sleep on PATH so every path runs offline in ~0s, then # asserts on the commands the script actually issued. # +# The stub is deliberately stateful: boot_id must differ across the reboot, and +# case 3 pins the failure by returning the SAME boot_id twice. +# # Run: bash tests/infrastructure/scripts/prepare-test-environment.test.sh set -euo pipefail @@ -16,19 +19,45 @@ trap 'rm -rf "$TMP"' EXIT mkdir -p "$TMP/bin" -# Fake sshpass: log every invocation, emit a plausible dpkg-query result. +# Fake sshpass. Logs every invocation; emits plausible output per command. +# BOOT_ID_STUCK=1 makes it return an unchanging boot_id, simulating a node that +# never rebooted. cat > "$TMP/bin/sshpass" <<'STUB' #!/usr/bin/env bash echo "$*" >> "$STUB_LOG" case "$*" in - *dpkg-query*) echo -e "proxmox-kernel-6.14\t6.14.11-1\npve-manager\t9.2.1" ;; + *boot_id*) + if [[ "${BOOT_ID_STUCK:-0}" == "1" ]]; then + echo "11111111-1111-1111-1111-111111111111" + else + n=0 + [[ -f "$STUB_STATE/boot_calls" ]] && n=$(cat "$STUB_STATE/boot_calls") + n=$((n + 1)) + echo "$n" > "$STUB_STATE/boot_calls" + if [[ "$n" -le 1 ]]; then + echo "11111111-1111-1111-1111-111111111111" + else + echo "22222222-2222-2222-2222-222222222222" + fi + fi + ;; + *dpkg-query*) + # DPKG_EMPTY=1 simulates a failed query whose output is swallowed by + # the remote `| sort`, which succeeds on empty input. + [[ "${DPKG_EMPTY:-0}" == "1" ]] || printf 'proxmox-kernel-6.14\t6.14.11-1\npve-manager\t9.2.1\n' + ;; + *uname*) + printf '# running-kernel\t6.14.11-1-pve\n' + ;; esac exit 0 STUB -# wait-for-api.sh greps curl output for "version"; sleep must not really sleep. +# wait-for-api.sh greps curl output for "version"; log the call so the test can +# assert the wait actually ran. sleep must not really sleep. cat > "$TMP/bin/curl" <<'STUB' #!/usr/bin/env bash +echo "curl $*" >> "$STUB_LOG" echo '{"data":{"version":"9.2.1"}}' STUB cat > "$TMP/bin/sleep" <<'STUB' @@ -38,11 +67,13 @@ STUB chmod +x "$TMP/bin/"* export PATH="$TMP/bin:$PATH" +export STUB_STATE="$TMP" fail=0 check() { local desc="$1" haystack="$2" needle="$3" want="$4" - if grep -q -- "$needle" "$haystack"; then found=yes; else found=no; fi + local found=no + grep -q -- "$needle" "$haystack" && found=yes if [[ "$found" == "$want" ]]; then echo " ok: $desc" else @@ -50,41 +81,91 @@ check() { fail=1 fi } +pass() { echo " ok: $1"; } +fatal() { echo " FAIL: $1"; fail=1; } echo "case 1: no dist-upgrade argument — lane 1 path must be untouched" export STUB_LOG="$TMP/log1" -: > "$STUB_LOG" +: > "$STUB_LOG"; rm -f "$TMP/boot_calls" bash "$TARGET" 10.0.0.1 secret > "$TMP/out1" 2>&1 check "no dist-upgrade issued" "$STUB_LOG" "dist-upgrade" no check "no reboot issued" "$STUB_LOG" "systemctl reboot" no check "no package set recorded" "$STUB_LOG" "dpkg-query" no +check "no boot_id probe" "$STUB_LOG" "boot_id" no check "storage still configured" "$STUB_LOG" "pvesm set local" yes echo "case 2: dist-upgrade requested" export STUB_LOG="$TMP/log2" -: > "$STUB_LOG" +: > "$STUB_LOG"; rm -f "$TMP/boot_calls" bash "$TARGET" 10.0.0.1 secret 1 "$TMP/packages.txt" > "$TMP/out2" 2>&1 check "dist-upgrade issued" "$STUB_LOG" "dist-upgrade" yes check "reboot issued" "$STUB_LOG" "systemctl reboot" yes check "package set recorded" "$STUB_LOG" "dpkg-query" yes +check "boot_id checked" "$STUB_LOG" "boot_id" yes +check "waited for the API" "$STUB_LOG" "api2/json/version" yes +check "waited for pmxcfs" "$STUB_LOG" "/etc/pve/storage.cfg" yes check "storage still configured" "$STUB_LOG" "pvesm set local" yes +# The stub replaces the remote shell, so it cannot observe what that shell does +# with a command — only which command was sent. These two assert at that level, +# because both defects live in the command string itself: +# - without `set -o pipefail`, a failed remote dpkg-query is masked by `sort`, +# which succeeds on empty input and makes ssh return 0. +# - bash's builtin `echo` does not interpret \t without -e, so `echo` here +# would write the one row in the file lacking a real tab. +check "query sets remote pipefail" "$STUB_LOG" "set -o pipefail" yes +check "kernel capture uses printf" "$STUB_LOG" "printf '# running-kernel" yes + # The reboot must be issued after the upgrade, or the node records a package # set it never booted. upgrade_line=$(grep -n "dist-upgrade" "$STUB_LOG" | head -1 | cut -d: -f1) reboot_line=$(grep -n "systemctl reboot" "$STUB_LOG" | head -1 | cut -d: -f1) -if [[ "$reboot_line" -gt "$upgrade_line" ]]; then - echo " ok: reboot ordered after dist-upgrade" +api_line=$(grep -n "api2/json/version" "$STUB_LOG" | head -1 | cut -d: -f1) +[[ "$reboot_line" -gt "$upgrade_line" ]] \ + && pass "reboot ordered after dist-upgrade" \ + || fatal "reboot ordered before dist-upgrade" +[[ "$api_line" -gt "$reboot_line" ]] \ + && pass "API wait ordered after reboot" \ + || fatal "API wait ordered before reboot" + +# The running-kernel row must carry a REAL tab, like every dpkg-query row. +# `echo "...\t..."` in bash emits a literal backslash-t and would fail here. +if grep -q '^# running-kernel' "$TMP/packages.txt"; then + pass "running kernel recorded" + if grep -qP '^# running-kernel\t' "$TMP/packages.txt" 2>/dev/null \ + || awk -F'\t' '/^# running-kernel/ && NF == 2 {found=1} END {exit !found}' "$TMP/packages.txt"; then + pass "running-kernel row uses a real tab" + else + fatal "running-kernel row has a literal backslash-t, not a tab" + fi else - echo " FAIL: reboot ordered before dist-upgrade" - fail=1 + fatal "running kernel not recorded" fi -if [[ -s "$TMP/packages.txt" ]]; then - echo " ok: package file non-empty" +[[ -s "$TMP/packages.txt" ]] && pass "package file non-empty" || fatal "package file empty or missing" + +echo "case 3: node never rebooted — must be fatal" +export STUB_LOG="$TMP/log3" +: > "$STUB_LOG"; rm -f "$TMP/boot_calls" +if BOOT_ID_STUCK=1 bash "$TARGET" 10.0.0.1 secret 1 "$TMP/packages3.txt" > "$TMP/out3" 2>&1; then + fatal "script exited 0 despite an unchanged boot_id" else - echo " FAIL: package file empty or missing" - fail=1 + pass "unchanged boot_id fails the run" + grep -q "did not reboot" "$TMP/out3" \ + && pass "failure names the cause" \ + || fatal "failure message does not mention the reboot" +fi + +echo "case 4: dpkg-query produced nothing — must be fatal" +export STUB_LOG="$TMP/log4" +: > "$STUB_LOG"; rm -f "$TMP/boot_calls" +if DPKG_EMPTY=1 bash "$TARGET" 10.0.0.1 secret 1 "$TMP/packages4.txt" > "$TMP/out4" 2>&1; then + fatal "script exited 0 despite an empty package set" +else + pass "empty package set fails the run" + grep -q "empty package set" "$TMP/out4" \ + && pass "failure names the cause" \ + || fatal "failure message does not mention the package set" fi if [[ "$fail" -eq 0 ]]; then diff --git a/tests/infrastructure/scripts/run-integration.sh b/tests/infrastructure/scripts/run-integration.sh index 8b9594b..351cc9f 100644 --- a/tests/infrastructure/scripts/run-integration.sh +++ b/tests/infrastructure/scripts/run-integration.sh @@ -238,6 +238,11 @@ cmd_provision() { ci_mask "$PVE_PASSWORD" mkdir -p "$WORK_DIR" "$CACHE_DIR" + # WORK_DIR is a persistent shared mount. Stale package sets from an earlier + # run would otherwise be uploaded as if they were current — the artifact + # guard checks existence, not freshness. + rm -f "$WORK_DIR"/*-packages.txt + # Ensure base ISOs (one per version, not per node) for v in $provision_versions; do log "Ensuring base ISO for PVE $v..." @@ -523,7 +528,9 @@ cmd_test() { -d "username=root@pam&password=${PVETEST_PASSWORD}" \ "https://${PVETEST_HOST}:${PVETEST_PORT}/api2/json/access/ticket" | grep -q '"ticket"'; then ci_error "Cannot authenticate to PVE $v node A at ${PVETEST_HOST}:${PVETEST_PORT}" - overall_exit=3 + # 4, not 3: this is a machinery failure, not a test failure. The + # currency lane suppresses 3 (report-only) but must fail on 4. + overall_exit=4 continue fi @@ -534,7 +541,7 @@ cmd_test() { -d "username=root@pam&password=${PVETEST_PASSWORD}" \ "https://${PVETEST_HOST_B}:${PVETEST_PORT}/api2/json/access/ticket" | grep -q '"ticket"'; then ci_error "Cannot authenticate to PVE $v node B at ${PVETEST_HOST_B}:${PVETEST_PORT}" - overall_exit=3 + overall_exit=4 continue fi fi @@ -733,6 +740,7 @@ cmd_force_cleanup() { # Remove work artifacts, including locally cached auto-install ISOs rm -f "$CONFIG_FILE" "$WORK_DIR"/instances.tfvars.json "$TARGET_NODE_FILE" rm -f "$WORK_DIR"/*-auto-*.iso "$WORK_DIR"/*-http-auto.iso + rm -f "$WORK_DIR"/*-packages.txt log "Force cleanup complete. Next provision will start from scratch." }