diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index a3d07a3..2bb744d 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -5,7 +5,10 @@ name: Integration Tests # Architecture: # build → GitHub-hosted, dotnet publish → upload artifact # container-image → GitHub-hosted, build+push Docker image to GHCR -# integration → self-hosted runner in Docker container +# provision → self-hosted, provisions ALL nested PVE VMs in parallel via Terraform +# test → self-hosted, runs Pester tests against pre-provisioned VMs +# cleanup → self-hosted, tears down all VMs (always runs) +# cleanup-images → GitHub-hosted, prunes old GHCR container images # # Required repository secrets (for provisioning): # PVE_ENDPOINT - Parent PVE API URL (e.g. https://pve.example.com:8006) @@ -42,6 +45,7 @@ env: SCRIPTS_DIR: tests/infrastructure/scripts PVE_PASSWORD: "Testpass123!" TEST_IMAGE: ghcr.io/goodolclint/psproxmoxve-integration + CACHE_DIR: /opt/pve-isos jobs: # ── Build module artifact (GitHub-hosted) ──────────────────────────── @@ -92,29 +96,204 @@ jobs: push: true tags: ${{ env.TEST_IMAGE }}:${{ github.sha }},${{ env.TEST_IMAGE }}:latest - # ── Clean up old container images from GHCR ────────────────────────── - cleanup-images: - needs: integration - if: always() && github.actor != 'dependabot[bot]' - runs-on: ubuntu-latest - timeout-minutes: 5 - env: - FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true - steps: - - name: Delete old container image versions - uses: actions/delete-package-versions@v5 - with: - package-name: psproxmoxve-integration - package-type: container - min-versions-to-keep: 1 - delete-only-untagged-versions: false - token: ${{ secrets.GITHUB_TOKEN }} - - # ── Integration tests (self-hosted runner in Docker container) ─────── - integration: + # ── Provision all nested PVE VMs (self-hosted) ────────────────────── + provision: + if: inputs.skip_provision != true needs: [build, container-image] runs-on: [self-hosted, proxmox, integration] - timeout-minutes: 45 + timeout-minutes: 30 + container: + image: ghcr.io/goodolclint/psproxmoxve-integration:${{ github.sha }} + credentials: + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + volumes: + - /opt/pve-isos:/opt/pve-isos + outputs: + pve9_host: ${{ steps.wait_pve9.outputs.host }} + pve9_token: ${{ steps.wait_pve9.outputs.token }} + pve8_host: ${{ steps.wait_pve8.outputs.host }} + pve8_token: ${{ steps.wait_pve8.outputs.token }} + cloud_image_path: ${{ steps.cloud_images.outputs.cloud_image_path }} + ova_path: ${{ steps.cloud_images.outputs.ova_path }} + + steps: + - name: Mask test password + run: echo "::add-mask::${PVE_PASSWORD}" + + - uses: actions/checkout@v6 + + # ── Cache: ensure base ISOs ────────────────────────────────────── + + - name: Ensure PVE 9 base ISO + shell: bash + run: bash ${SCRIPTS_DIR}/ensure-base-iso.sh "proxmox-ve_9.1-1.iso" "${CACHE_DIR}" + + - name: Ensure PVE 8 base ISO + shell: bash + run: bash ${SCRIPTS_DIR}/ensure-base-iso.sh "proxmox-ve_8.4-1.iso" "${CACHE_DIR}" + + # ── Cache: ensure cloud images ─────────────────────────────────── + + - name: Ensure cloud images cached + id: cloud_images + shell: bash + run: | + OUTPUT=$(bash ${SCRIPTS_DIR}/ensure-cloud-images.sh "${CACHE_DIR}") + CLOUD_IMAGE_PATH=$(echo "$OUTPUT" | grep "^CLOUD_IMAGE_PATH=" | cut -d= -f2) + OVA_PATH=$(echo "$OUTPUT" | grep "^OVA_PATH=" | cut -d= -f2) + echo "cloud_image_path=${CLOUD_IMAGE_PATH}" >> "$GITHUB_OUTPUT" + echo "ova_path=${OVA_PATH}" >> "$GITHUB_OUTPUT" + + # ── Pre-flight cleanup for ALL VMs ─────────────────────────────── + + - name: Pre-flight cleanup (PVE 9) + shell: bash + env: + PVE_API_TOKEN: ${{ secrets.PVE_API_TOKEN }} + run: | + bash ${SCRIPTS_DIR}/preflight-cleanup.sh \ + "${{ secrets.PVE_ENDPOINT }}" "${PVE_API_TOKEN}" \ + 99909 "proxmox-ve_9.1-1-auto.iso" "${INFRA_DIR}" + + - name: Pre-flight cleanup (PVE 8) + shell: bash + env: + PVE_API_TOKEN: ${{ secrets.PVE_API_TOKEN }} + run: | + bash ${SCRIPTS_DIR}/preflight-cleanup.sh \ + "${{ secrets.PVE_ENDPOINT }}" "${PVE_API_TOKEN}" \ + 99908 "proxmox-ve_8.4-1-auto.iso" "${INFRA_DIR}" + + # ── Generate answer files ──────────────────────────────────────── + + - name: Generate answer file + shell: bash + run: | + sed "s/\${root_password}/${PVE_PASSWORD}/" \ + ${INFRA_DIR}/answer.toml.tftpl > ${RUNNER_TEMP}/answer.toml + + # ── Prepare auto-install ISOs (cached) ─────────────────────────── + + - name: Prepare PVE 9 auto-install ISO + shell: bash + run: | + bash ${SCRIPTS_DIR}/prepare-auto-iso.sh \ + "${CACHE_DIR}/proxmox-ve_9.1-1.iso" \ + ${RUNNER_TEMP}/answer.toml \ + ${SCRIPTS_DIR}/first-boot.sh \ + "${RUNNER_TEMP}/proxmox-ve_9.1-1-auto.iso" \ + --cache-dir "${CACHE_DIR}" + + - name: Prepare PVE 8 auto-install ISO + shell: bash + run: | + bash ${SCRIPTS_DIR}/prepare-auto-iso.sh \ + "${CACHE_DIR}/proxmox-ve_8.4-1.iso" \ + ${RUNNER_TEMP}/answer.toml \ + ${SCRIPTS_DIR}/first-boot.sh \ + "${RUNNER_TEMP}/proxmox-ve_8.4-1-auto.iso" \ + --cache-dir "${CACHE_DIR}" + + # ── Terraform: provision both VMs in parallel ──────────────────── + + - name: Terraform init + shell: bash + working-directory: ${{ env.INFRA_DIR }} + run: terraform init -input=false + + - name: Terraform apply (both VMs) + shell: bash + working-directory: ${{ env.INFRA_DIR }} + env: + TF_VAR_proxmox_endpoint: ${{ secrets.PVE_ENDPOINT }} + TF_VAR_proxmox_api_token: ${{ secrets.PVE_API_TOKEN }} + TF_VAR_target_node: ${{ secrets.PVE_TARGET_NODE }} + TF_VAR_test_vm_password: ${{ env.PVE_PASSWORD }} + run: | + cat > /tmp/instances.tfvars.json <<'TFVARS' + { + "pve_instances": { + "pve9": { + "iso_local_path": "${{ runner.temp }}/proxmox-ve_9.1-1-auto.iso", + "vm_id": 99909, + "vm_name": "pve-test-pve9" + }, + "pve8": { + "iso_local_path": "${{ runner.temp }}/proxmox-ve_8.4-1-auto.iso", + "vm_id": 99908, + "vm_name": "pve-test-pve8" + } + } + } + TFVARS + terraform apply -auto-approve -input=false -var-file=/tmp/instances.tfvars.json + + # ── Wait for both PVE instances and create API tokens ──────────── + + - name: Wait for PVE 9 and create API token + id: wait_pve9 + shell: bash + env: + PVE_API_TOKEN: ${{ secrets.PVE_API_TOKEN }} + run: | + OUTPUT=$(bash ${SCRIPTS_DIR}/create-api-token.sh \ + "${{ secrets.PVE_ENDPOINT }}" "${PVE_API_TOKEN}" \ + 99909 "${PVE_PASSWORD}" 900) + VM_IP=$(echo "$OUTPUT" | grep "^IP=" | cut -d= -f2) + VM_TOKEN=$(echo "$OUTPUT" | grep "^TOKEN=" | cut -d= -f2-) + echo "::add-mask::${VM_IP}" + echo "::add-mask::${VM_TOKEN}" + echo "host=${VM_IP}" >> "$GITHUB_OUTPUT" + echo "token=${VM_TOKEN}" >> "$GITHUB_OUTPUT" + + - name: Wait for PVE 8 and create API token + id: wait_pve8 + shell: bash + env: + PVE_API_TOKEN: ${{ secrets.PVE_API_TOKEN }} + run: | + OUTPUT=$(bash ${SCRIPTS_DIR}/create-api-token.sh \ + "${{ secrets.PVE_ENDPOINT }}" "${PVE_API_TOKEN}" \ + 99908 "${PVE_PASSWORD}" 900) + VM_IP=$(echo "$OUTPUT" | grep "^IP=" | cut -d= -f2) + VM_TOKEN=$(echo "$OUTPUT" | grep "^TOKEN=" | cut -d= -f2-) + echo "::add-mask::${VM_IP}" + echo "::add-mask::${VM_TOKEN}" + echo "host=${VM_IP}" >> "$GITHUB_OUTPUT" + echo "token=${VM_TOKEN}" >> "$GITHUB_OUTPUT" + + # ── Prepare test environments on both nested PVEs ──────────────── + + - name: Prepare PVE 9 test environment + shell: bash + run: | + bash ${SCRIPTS_DIR}/prepare-test-environment.sh \ + "${{ steps.wait_pve9.outputs.host }}" "${PVE_PASSWORD}" + + - name: Prepare PVE 8 test environment + shell: bash + run: | + bash ${SCRIPTS_DIR}/prepare-test-environment.sh \ + "${{ steps.wait_pve8.outputs.host }}" "${PVE_PASSWORD}" + + # ── Upload Terraform state for cleanup job ─────────────────────── + + - name: Upload Terraform state + if: always() + uses: actions/upload-artifact@v7 + with: + name: terraform-state + path: | + ${{ env.INFRA_DIR }}/terraform.tfstate + ${{ env.INFRA_DIR }}/.terraform/ + retention-days: 1 + + # ── Integration tests (self-hosted, per PVE version) ──────────────── + test: + needs: [build, provision] + runs-on: [self-hosted, proxmox, integration] + timeout-minutes: 20 container: image: ghcr.io/goodolclint/psproxmoxve-integration:${{ github.sha }} credentials: @@ -127,17 +306,6 @@ jobs: max-parallel: 1 matrix: pve_version: ['9', '8'] - include: - - pve_version: '9' - iso_base: /opt/pve-isos/proxmox-ve_9.1-1.iso - iso_filename: proxmox-ve_9.1-1-auto.iso - vm_id: 99909 - vm_name: pve-test-pve9 - - pve_version: '8' - iso_base: /opt/pve-isos/proxmox-ve_8.4-1.iso - iso_filename: proxmox-ve_8.4-1-auto.iso - vm_id: 99908 - vm_name: pve-test-pve8 steps: - name: Mask test password @@ -151,86 +319,6 @@ jobs: name: module-integration path: ./publish/netstandard2.0/ - # ── Pre-flight cleanup ─────────────────────────────────────────── - - - name: Pre-flight cleanup - if: inputs.skip_provision != true - shell: bash - env: - PVE_API_TOKEN: ${{ secrets.PVE_API_TOKEN }} - run: | - bash ${SCRIPTS_DIR}/preflight-cleanup.sh \ - "${{ secrets.PVE_ENDPOINT }}" \ - "${PVE_API_TOKEN}" \ - ${{ matrix.vm_id }} \ - "${{ matrix.iso_filename }}" \ - "${INFRA_DIR}" - - # ── Provision nested PVE ───────────────────────────────────────── - - - name: Generate answer file - if: inputs.skip_provision != true - shell: bash - run: | - sed "s/\${root_password}/${PVE_PASSWORD}/" \ - ${INFRA_DIR}/answer.toml.tftpl > ${RUNNER_TEMP}/answer.toml - echo "Generated answer file:" - cat ${RUNNER_TEMP}/answer.toml - - - name: Prepare auto-install ISO - if: inputs.skip_provision != true - shell: bash - run: | - bash ${SCRIPTS_DIR}/prepare-auto-iso.sh \ - "${{ matrix.iso_base }}" \ - ${RUNNER_TEMP}/answer.toml \ - ${SCRIPTS_DIR}/first-boot.sh \ - "${RUNNER_TEMP}/${{ matrix.iso_filename }}" - - - name: Terraform init - if: inputs.skip_provision != true - shell: bash - working-directory: ${{ env.INFRA_DIR }} - run: terraform init -input=false - - - name: Terraform apply - if: inputs.skip_provision != true - id: terraform - shell: bash - working-directory: ${{ env.INFRA_DIR }} - env: - TF_VAR_proxmox_endpoint: ${{ secrets.PVE_ENDPOINT }} - TF_VAR_proxmox_api_token: ${{ secrets.PVE_API_TOKEN }} - TF_VAR_target_node: ${{ secrets.PVE_TARGET_NODE }} - TF_VAR_test_vm_password: ${{ env.PVE_PASSWORD }} - TF_VAR_vm_id: ${{ matrix.vm_id }} - TF_VAR_vm_name: ${{ matrix.vm_name }} - run: | - export TF_VAR_iso_local_path="${RUNNER_TEMP}/${{ matrix.iso_filename }}" - terraform apply -auto-approve -input=false - echo "vm_id=$(terraform output -raw pve_test_vm_id)" >> "$GITHUB_OUTPUT" - - - name: Wait for install and create API token - if: inputs.skip_provision != true - id: provision - shell: bash - env: - PVE_API_TOKEN: ${{ secrets.PVE_API_TOKEN }} - run: | - OUTPUT=$(bash ${SCRIPTS_DIR}/create-api-token.sh \ - "${{ secrets.PVE_ENDPOINT }}" \ - "${PVE_API_TOKEN}" \ - "${{ steps.terraform.outputs.vm_id }}" \ - "${PVE_PASSWORD}" \ - 900) - VM_IP=$(echo "$OUTPUT" | grep "^IP=" | cut -d= -f2) - VM_TOKEN=$(echo "$OUTPUT" | grep "^TOKEN=" | cut -d= -f2-) - echo "::add-mask::${VM_IP}" - echo "::add-mask::${VM_TOKEN}" - echo "Nested PVE ${{ matrix.pve_version }} ready at ${VM_IP}" - echo "host=${VM_IP}" >> "$GITHUB_OUTPUT" - echo "token=${VM_TOKEN}" >> "$GITHUB_OUTPUT" - # ── Resolve test target ────────────────────────────────────────── - name: Set test target @@ -243,10 +331,16 @@ jobs: echo "token=${{ secrets.PVETEST_APITOKEN }}" >> "$GITHUB_OUTPUT" echo "node=${{ secrets.PVETEST_NODE || 'pve' }}" >> "$GITHUB_OUTPUT" echo "storage=${{ secrets.PVETEST_STORAGE || 'local' }}" >> "$GITHUB_OUTPUT" - else - echo "host=${{ steps.provision.outputs.host }}" >> "$GITHUB_OUTPUT" + elif [ "${{ matrix.pve_version }}" = "9" ]; then + echo "host=${{ needs.provision.outputs.pve9_host }}" >> "$GITHUB_OUTPUT" echo "port=8006" >> "$GITHUB_OUTPUT" - echo "token=${{ steps.provision.outputs.token }}" >> "$GITHUB_OUTPUT" + echo "token=${{ needs.provision.outputs.pve9_token }}" >> "$GITHUB_OUTPUT" + echo "node=pve" >> "$GITHUB_OUTPUT" + echo "storage=local" >> "$GITHUB_OUTPUT" + else + echo "host=${{ needs.provision.outputs.pve8_host }}" >> "$GITHUB_OUTPUT" + echo "port=8006" >> "$GITHUB_OUTPUT" + echo "token=${{ needs.provision.outputs.pve8_token }}" >> "$GITHUB_OUTPUT" echo "node=pve" >> "$GITHUB_OUTPUT" echo "storage=local" >> "$GITHUB_OUTPUT" fi @@ -276,22 +370,6 @@ jobs: New-Item -ItemType Directory -Path $modulePath -Force | Out-Null Copy-Item -Path ./publish/netstandard2.0/* -Destination $modulePath -Recurse -Force - # ── Prepare test Linux VM ──────────────────────────────────────── - - - name: Prepare test environment on nested PVE - if: inputs.skip_provision != true - id: test_env - shell: bash - run: | - OUTPUT=$(bash ${SCRIPTS_DIR}/prepare-test-environment.sh \ - "${{ steps.provision.outputs.host }}" \ - "${PVE_PASSWORD}" \ - "${RUNNER_TEMP}") - CLOUD_IMAGE_PATH=$(echo "$OUTPUT" | grep "^CLOUD_IMAGE_PATH=" | cut -d= -f2) - OVA_PATH=$(echo "$OUTPUT" | grep "^OVA_PATH=" | cut -d= -f2) - echo "cloud_image_path=${CLOUD_IMAGE_PATH}" >> "$GITHUB_OUTPUT" - echo "ova_path=${OVA_PATH}" >> "$GITHUB_OUTPUT" - # ── Run tests ───────────────────────────────────────────────────── - name: Create test ISO @@ -308,8 +386,8 @@ jobs: PVETEST_STORAGE: ${{ steps.target.outputs.storage }} PVETEST_PVE_VERSION: ${{ matrix.pve_version }} PVETEST_PASSWORD: ${{ env.PVE_PASSWORD }} - PVETEST_CLOUD_IMAGE_PATH: ${{ steps.test_env.outputs.cloud_image_path }} - PVETEST_OVA_PATH: ${{ steps.test_env.outputs.ova_path }} + PVETEST_CLOUD_IMAGE_PATH: ${{ needs.provision.outputs.cloud_image_path }} + PVETEST_OVA_PATH: ${{ needs.provision.outputs.ova_path }} run: | $env:PVETEST_ISO_PATH = Join-Path $env:RUNNER_TEMP "pvetest.iso" Import-Module Pester -MinimumVersion 5.0 @@ -329,34 +407,55 @@ jobs: name: integration-test-results-pve${{ matrix.pve_version }} path: TestResults/ - # ── Teardown ───────────────────────────────────────────────────── + # ── Cleanup: destroy all VMs (always runs) ────────────────────────── + cleanup: + needs: [provision, test] + if: always() && needs.provision.result != 'skipped' && github.actor != 'dependabot[bot]' + runs-on: [self-hosted, proxmox, integration] + timeout-minutes: 15 + container: + image: ghcr.io/goodolclint/psproxmoxve-integration:${{ github.sha }} + credentials: + username: ${{ github.actor }} + password: ${{ secrets.GITHUB_TOKEN }} + volumes: + - /opt/pve-isos:/opt/pve-isos - - name: Terraform destroy - if: always() && inputs.skip_provision != true - shell: bash - working-directory: ${{ env.INFRA_DIR }} - env: - TF_VAR_proxmox_endpoint: ${{ secrets.PVE_ENDPOINT }} - TF_VAR_proxmox_api_token: ${{ secrets.PVE_API_TOKEN }} - TF_VAR_target_node: ${{ secrets.PVE_TARGET_NODE }} - TF_VAR_test_vm_password: ${{ env.PVE_PASSWORD }} - TF_VAR_vm_id: ${{ matrix.vm_id }} - TF_VAR_vm_name: ${{ matrix.vm_name }} - run: | - export TF_VAR_iso_local_path="${RUNNER_TEMP}/${{ matrix.iso_filename }}" - terraform init -input=false - terraform destroy -auto-approve -input=false || true - rm -f terraform.tfstate terraform.tfstate.backup .terraform.tfstate.lock.info + steps: + - uses: actions/checkout@v6 - - name: Final cleanup - if: always() && inputs.skip_provision != true + - name: API-based cleanup (PVE 9) shell: bash env: PVE_API_TOKEN: ${{ secrets.PVE_API_TOKEN }} run: | bash ${SCRIPTS_DIR}/preflight-cleanup.sh \ - "${{ secrets.PVE_ENDPOINT }}" \ - "${PVE_API_TOKEN}" \ - ${{ matrix.vm_id }} \ - "${{ matrix.iso_filename }}" \ - "${INFRA_DIR}" || true + "${{ secrets.PVE_ENDPOINT }}" "${PVE_API_TOKEN}" \ + 99909 "proxmox-ve_9.1-1-auto.iso" "${INFRA_DIR}" || true + + - name: API-based cleanup (PVE 8) + shell: bash + env: + PVE_API_TOKEN: ${{ secrets.PVE_API_TOKEN }} + run: | + bash ${SCRIPTS_DIR}/preflight-cleanup.sh \ + "${{ secrets.PVE_ENDPOINT }}" "${PVE_API_TOKEN}" \ + 99908 "proxmox-ve_8.4-1-auto.iso" "${INFRA_DIR}" || true + + # ── Clean up old container images from GHCR ────────────────────────── + cleanup-images: + needs: test + if: always() && github.actor != 'dependabot[bot]' + runs-on: ubuntu-latest + timeout-minutes: 5 + env: + FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true + steps: + - name: Delete old container image versions + uses: actions/delete-package-versions@v5 + with: + package-name: psproxmoxve-integration + package-type: container + min-versions-to-keep: 3 + delete-only-untagged-versions: false + token: ${{ secrets.GITHUB_TOKEN }} diff --git a/docs/review/PLAN-integration-refactor.md b/docs/review/PLAN-integration-refactor.md new file mode 100644 index 0000000..0b2f227 --- /dev/null +++ b/docs/review/PLAN-integration-refactor.md @@ -0,0 +1,124 @@ +# Plan: Integration Test Workflow Refactoring + +**Status**: Planned (not started) +**Created**: 2026-03-23 +**Priority**: Medium — improves CI speed and reliability but not blocking + +## Goals + +1. **Parallel VM provisioning** — provision PVE 8 and PVE 9 VMs simultaneously instead of sequentially +2. **File caching** — cache ISOs, cloud images, and OVA files on the self-hosted runner +3. **Zero-touch runner setup** — runner no longer needs manual ISO provisioning +4. **Extensibility** — easy to add cluster peers, storage VMs in the future + +## Proposed Job Graph + +``` +build ─────────────┐ + ├──→ provision ──→ test-pve8 ──┐ +container-image ───┘ (all VMs test-pve9 ──┤──→ cleanup + parallel) test-cluster─┘ (always) + (future) +``` + +## Changes Required + +### 1. Terraform: for_each multi-VM provisioning +- Refactor main.tf from single-VM to `pve_instances` map variable +- Both ISOs upload and both VMs create in a single `terraform apply` +- Update variables.tf and outputs.tf + +### 2. New caching scripts +- `ensure-base-iso.sh` — download PVE base ISO if not cached in /opt/pve-isos +- `ensure-cloud-images.sh` — download cloud image + OVA with ETag-based 7-day TTL +- Modify `prepare-auto-iso.sh` — add `--cache-dir` flag with hash-based skip + +### 3. New `provision` job (self-hosted) +- Preflight cleanup for ALL VM IDs +- Prepare both auto-install ISOs (cached) +- Download cloud images once (cached) +- Single `terraform apply` +- Wait for both PVE installs + create API tokens +- Expose connection details as job outputs + +### 4. Test jobs consume provision outputs +- Matrix with `max-parallel: 2` +- Tests get PVETEST_HOST, PVETEST_APITOKEN from `needs.provision.outputs.*` +- No provisioning in test jobs + +### 5. Cleanup job with `if: always()` +- API-only cleanup via preflight-cleanup.sh (stateless) +- Terraform state as artifact for belt-and-suspenders + +## Estimated Time Savings + +| Phase | Current | Proposed | +|---|---|---| +| Build + container | 5-10 min | 5-10 min | +| Provision (sequential → parallel) | 20-30 min | 10-15 min | +| Tests (sequential → parallel if runner allows) | 10-20 min | 5-10 min | +| Teardown | 4-6 min | 3-5 min | +| **Total** | **~40-60 min** | **~25-40 min** | + +## Dependabot / CI Isolation Lessons (2026-03-23) + +During scan-6 we discovered two issues that this refactoring must account for: + +### 1. cleanup-images must be gated against dependabot + +The `cleanup-images` job uses `actions/delete-package-versions` with `if: always()` to +prune old GHCR container images. When a dependabot PR ran, the `integration` job was +skipped (correctly), but `cleanup-images` still fired and **deleted the container image +that the main branch integration run was actively using**, causing PVE 8 to fail with +"image not found". + +**Fix already applied**: All jobs now have `if: github.actor != 'dependabot[bot]'`. + +**For the refactoring**: The new `cleanup` job (Terraform destroy + VM cleanup) must +also be gated. Use `if: always() && github.actor != 'dependabot[bot]'` on all jobs +that touch shared resources (self-hosted runner, GHCR, PVE host). + +### 2. Container image tags must survive concurrent cleanup + +The current `cleanup-images` job deletes all but 1 container image version. If two +workflow runs overlap (e.g., a push to main while a prior run is still testing), the +cleanup from the first run can delete the image needed by the second. + +**For the refactoring**: Consider one of: +- **Tag images by run ID** instead of commit SHA, and only delete images older than + the current run +- **Pin `min-versions-to-keep: 3`** to survive overlapping runs +- **Move cleanup to a scheduled workflow** (weekly) instead of per-run + +### 3. Self-hosted runner disk space + +The runner ran out of disk space during an integration test run, leaving it in a broken +state that required manual rebuilding. The caching strategy must account for this: +- Set a maximum cache size or file count in `/opt/pve-isos` +- Auto-prune ISOs not referenced by the current workflow matrix +- The cleanup job should clean up uploaded ISOs from the PVE host, not just VMs +- Consider a periodic runner maintenance script that frees disk space + +## Implementation Order + +1. Refactor Terraform (main.tf, variables.tf, outputs.tf) → for_each +2. Create ensure-base-iso.sh and ensure-cloud-images.sh +3. Add --cache-dir to prepare-auto-iso.sh +4. Create parallel wait wrapper for create-api-token.sh +5. Restructure workflow into provision → test → cleanup jobs +6. Update prepare-test-environment.sh for cache dir +7. Ensure all jobs gated with `github.actor != 'dependabot[bot]'` +8. Address container image cleanup race condition +9. Test on runner via workflow_dispatch + +## Files to Modify + +- .github/workflows/integration-tests.yml +- tests/infrastructure/main.tf +- tests/infrastructure/variables.tf +- tests/infrastructure/outputs.tf +- tests/infrastructure/scripts/prepare-auto-iso.sh +- tests/infrastructure/scripts/prepare-test-environment.sh +- tests/infrastructure/scripts/create-api-token.sh (wrapper) +- New: tests/infrastructure/scripts/ensure-base-iso.sh +- New: tests/infrastructure/scripts/ensure-cloud-images.sh diff --git a/tests/infrastructure/main.tf b/tests/infrastructure/main.tf index d30d664..934113b 100644 --- a/tests/infrastructure/main.tf +++ b/tests/infrastructure/main.tf @@ -15,19 +15,21 @@ provider "proxmox" { } resource "proxmox_virtual_environment_file" "auto_iso" { + for_each = var.pve_instances content_type = "iso" datastore_id = var.iso_storage node_name = var.target_node source_file { - path = var.iso_local_path + path = each.value.iso_local_path } } resource "proxmox_virtual_environment_vm" "nested_pve" { - name = var.vm_name + for_each = var.pve_instances + name = each.value.vm_name node_name = var.target_node - vm_id = var.vm_id + vm_id = each.value.vm_id machine = "q35" bios = "ovmf" @@ -56,7 +58,7 @@ resource "proxmox_virtual_environment_vm" "nested_pve" { } cdrom { - file_id = proxmox_virtual_environment_file.auto_iso.id + file_id = proxmox_virtual_environment_file.auto_iso[each.key].id interface = "ide2" } diff --git a/tests/infrastructure/outputs.tf b/tests/infrastructure/outputs.tf index 4fcec7b..4ecb5ee 100644 --- a/tests/infrastructure/outputs.tf +++ b/tests/infrastructure/outputs.tf @@ -1,5 +1,6 @@ -output "pve_test_vm_id" { - value = var.vm_id +output "pve_vm_ids" { + description = "Map of instance key to VM ID" + value = { for k, v in proxmox_virtual_environment_vm.nested_pve : k => v.vm_id } } output "pve_test_node_name" { diff --git a/tests/infrastructure/scripts/ensure-base-iso.sh b/tests/infrastructure/scripts/ensure-base-iso.sh new file mode 100755 index 0000000..95485c4 --- /dev/null +++ b/tests/infrastructure/scripts/ensure-base-iso.sh @@ -0,0 +1,36 @@ +#!/usr/bin/env bash +# Downloads a PVE base ISO to the cache directory if not already present. +# +# Usage: ensure-base-iso.sh +# iso-filename: e.g. proxmox-ve_9.1-1.iso +# cache-dir: e.g. /opt/pve-isos +set -euo pipefail + +ISO_FILENAME="${1:?Usage: ensure-base-iso.sh }" +CACHE_DIR="${2:?Cache directory required}" + +CACHED_PATH="${CACHE_DIR}/${ISO_FILENAME}" + +if [ -f "${CACHED_PATH}" ] && [ -s "${CACHED_PATH}" ]; then + echo "Base ISO already cached: ${CACHED_PATH} ($(du -h "${CACHED_PATH}" | cut -f1))" + exit 0 +fi + +# Ensure cache directory exists and is writable +mkdir -p "${CACHE_DIR}" + +DOWNLOAD_URL="http://download.proxmox.com/iso/${ISO_FILENAME}" +TMP_PATH="${CACHED_PATH}.downloading" + +echo "Downloading PVE base ISO: ${DOWNLOAD_URL}" +echo " Target: ${CACHED_PATH}" + +# Download to temp file, then atomic move +if curl -fSL --progress-bar -o "${TMP_PATH}" "${DOWNLOAD_URL}"; then + mv "${TMP_PATH}" "${CACHED_PATH}" + echo "Downloaded: ${CACHED_PATH} ($(du -h "${CACHED_PATH}" | cut -f1))" +else + rm -f "${TMP_PATH}" + echo "ERROR: Failed to download ${DOWNLOAD_URL}" >&2 + exit 1 +fi diff --git a/tests/infrastructure/scripts/ensure-cloud-images.sh b/tests/infrastructure/scripts/ensure-cloud-images.sh new file mode 100755 index 0000000..cb897eb --- /dev/null +++ b/tests/infrastructure/scripts/ensure-cloud-images.sh @@ -0,0 +1,61 @@ +#!/usr/bin/env bash +# Downloads cloud image and OVA to the cache directory if not already present +# or if the cached copy is older than 7 days. +# +# Usage: ensure-cloud-images.sh +# +# Outputs (for use in GITHUB_OUTPUT): +# CLOUD_IMAGE_PATH= +# OVA_PATH= +set -euo pipefail + +CACHE_DIR="${1:?Usage: ensure-cloud-images.sh }" +MAX_AGE_DAYS=7 + +CLOUD_IMAGE_URL="https://cloud-images.ubuntu.com/noble/current/noble-server-cloudimg-amd64.img" +CLOUD_IMAGE_FILENAME="noble-server-cloudimg-amd64.qcow2" + +OVA_URL="https://cloud-images.ubuntu.com/releases/24.04/release/ubuntu-24.04-server-cloudimg-amd64.ova" +OVA_FILENAME="ubuntu-24.04-server-cloudimg-amd64.ova" + +mkdir -p "${CACHE_DIR}" + +download_if_stale() { + local url="$1" + local filepath="$2" + local description="$3" + + if [ -f "${filepath}" ] && [ -s "${filepath}" ]; then + # Check age + local age_days + age_days=$(( ( $(date +%s) - $(stat -c %Y "${filepath}" 2>/dev/null || stat -f %m "${filepath}" 2>/dev/null) ) / 86400 )) + if [ "${age_days}" -lt "${MAX_AGE_DAYS}" ]; then + echo "${description} cached and fresh (${age_days}d old): ${filepath}" + return 0 + fi + echo "${description} is ${age_days}d old, re-downloading..." + else + echo "Downloading ${description}..." + fi + + local tmp_path="${filepath}.downloading" + if curl -fSL --progress-bar -o "${tmp_path}" "${url}"; then + mv "${tmp_path}" "${filepath}" + echo "Downloaded ${description}: $(du -h "${filepath}" | cut -f1)" + else + rm -f "${tmp_path}" + # If we have a stale copy, keep using it + if [ -f "${filepath}" ]; then + echo "WARNING: Download failed, using stale cached copy" >&2 + return 0 + fi + echo "ERROR: Failed to download ${description}" >&2 + return 1 + fi +} + +download_if_stale "${CLOUD_IMAGE_URL}" "${CACHE_DIR}/${CLOUD_IMAGE_FILENAME}" "Ubuntu cloud image" +download_if_stale "${OVA_URL}" "${CACHE_DIR}/${OVA_FILENAME}" "Ubuntu OVA" + +echo "CLOUD_IMAGE_PATH=${CACHE_DIR}/${CLOUD_IMAGE_FILENAME}" +echo "OVA_PATH=${CACHE_DIR}/${OVA_FILENAME}" diff --git a/tests/infrastructure/scripts/prepare-auto-iso.sh b/tests/infrastructure/scripts/prepare-auto-iso.sh index b89988d..feca559 100755 --- a/tests/infrastructure/scripts/prepare-auto-iso.sh +++ b/tests/infrastructure/scripts/prepare-auto-iso.sh @@ -2,13 +2,34 @@ # Prepare a PVE auto-install ISO with the answer file and first-boot script baked in. # Uses --fetch-from iso mode so only a single CD-ROM is needed, no HTTP server required. # -# Usage: prepare-auto-iso.sh [first-boot-script] [output-iso] +# Usage: prepare-auto-iso.sh [first-boot-script] [output-iso] [--cache-dir ] +# +# When --cache-dir is specified, the script caches the generated ISO keyed by a hash +# of the inputs (base ISO path + answer file content + first-boot script content). +# If the cache is warm and inputs haven't changed, the cached ISO is copied to the +# output path without regeneration. set -euo pipefail +# Parse arguments BASE_ISO="$1" ANSWER_FILE="$2" FIRST_BOOT="${3:-}" OUTPUT_ISO="${4:-${BASE_ISO%.iso}-auto.iso}" +CACHE_DIR="" + +# Check for --cache-dir flag in remaining args +shift 4 2>/dev/null || true +while [ $# -gt 0 ]; do + case "$1" in + --cache-dir) + CACHE_DIR="$2" + shift 2 + ;; + *) + shift + ;; + esac +done for f in "$BASE_ISO" "$ANSWER_FILE"; do if [ ! -f "$f" ]; then @@ -22,6 +43,33 @@ if [ -n "$FIRST_BOOT" ] && [ -f "$FIRST_BOOT" ]; then FIRST_BOOT_ARGS=(--on-first-boot "$FIRST_BOOT") fi +# --- Cache check --- +if [ -n "$CACHE_DIR" ]; then + mkdir -p "$CACHE_DIR" + + # Build a hash of all inputs to detect changes + INPUT_HASH=$(cat "$ANSWER_FILE" ${FIRST_BOOT:+"$FIRST_BOOT"} | sha256sum | cut -d' ' -f1) + BASE_HASH=$(sha256sum "$BASE_ISO" | cut -d' ' -f1) + CACHE_KEY="${BASE_HASH:0:16}_${INPUT_HASH:0:16}" + CACHED_ISO="${CACHE_DIR}/$(basename "$OUTPUT_ISO")" + CACHED_HASH_FILE="${CACHED_ISO}.inputhash" + + if [ -f "$CACHED_ISO" ] && [ -s "$CACHED_ISO" ] && [ -f "$CACHED_HASH_FILE" ]; then + STORED_HASH=$(cat "$CACHED_HASH_FILE") + if [ "$STORED_HASH" = "$CACHE_KEY" ]; then + echo "Cached auto-install ISO is up to date (hash: ${CACHE_KEY})" + if [ "$CACHED_ISO" != "$OUTPUT_ISO" ]; then + cp "$CACHED_ISO" "$OUTPUT_ISO" + fi + echo "Using cached: $OUTPUT_ISO" + exit 0 + fi + echo "Cache stale (stored: ${STORED_HASH}, current: ${CACHE_KEY}), regenerating..." + else + echo "No cached auto-install ISO found, generating..." + fi +fi + echo "Preparing auto-install ISO..." echo " Base ISO: $BASE_ISO" echo " Answer file: $ANSWER_FILE" @@ -38,3 +86,12 @@ proxmox-auto-install-assistant prepare-iso \ "$BASE_ISO" echo "Created: $OUTPUT_ISO" + +# --- Update cache --- +if [ -n "$CACHE_DIR" ]; then + if [ "$CACHED_ISO" != "$OUTPUT_ISO" ]; then + cp "$OUTPUT_ISO" "$CACHED_ISO" + fi + echo "$CACHE_KEY" > "$CACHED_HASH_FILE" + echo "Cached auto-install ISO (hash: ${CACHE_KEY})" +fi diff --git a/tests/infrastructure/scripts/prepare-test-environment.sh b/tests/infrastructure/scripts/prepare-test-environment.sh index a011e0a..4392dd8 100755 --- a/tests/infrastructure/scripts/prepare-test-environment.sh +++ b/tests/infrastructure/scripts/prepare-test-environment.sh @@ -1,26 +1,17 @@ #!/usr/bin/env bash # Prepares the test environment on the nested PVE node. -# Only performs operations that have no PVE API equivalent, plus -# downloads test artifacts for the integration tests to upload. +# Only performs operations that have no PVE API equivalent. # -# Usage: prepare-test-environment.sh +# Usage: prepare-test-environment.sh # # Operations: # - Enable snippets+import content types on local storage (pvesm set) # - Upload cloud-init user-data snippet (SCP — no snippet upload API) -# - Download Ubuntu cloud image to for upload tests set -euo pipefail -NESTED_IP="${1:?Usage: prepare-test-environment.sh }" +NESTED_IP="${1:?Usage: prepare-test-environment.sh }" ROOT_PASS="$2" -OUTPUT_DIR="${3:?Output directory required}" - -CLOUD_IMAGE_URL="https://cloud-images.ubuntu.com/noble/current/noble-server-cloudimg-amd64.img" -# PVE upload API validates extensions per content type — content=import -# does not accept .img. The Ubuntu cloud image is qcow2 format, so we -# rename it to .qcow2 for compatibility with the upload endpoint. -CLOUD_IMAGE_FILENAME="noble-server-cloudimg-amd64.qcow2" SSH_OPTS="-o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o LogLevel=ERROR" SSH_CMD="sshpass -p ${ROOT_PASS} ssh ${SSH_OPTS} root@${NESTED_IP}" @@ -47,27 +38,4 @@ YAML ${SCP_CMD} "${USERDATA}" "root@${NESTED_IP}:/var/lib/vz/snippets/test-vm-userdata.yml" rm -f "${USERDATA}" -# Download cloud image for integration tests to upload via Send-PveFile -CLOUD_IMAGE_PATH="${OUTPUT_DIR}/${CLOUD_IMAGE_FILENAME}" -if [ ! -f "${CLOUD_IMAGE_PATH}" ]; then - echo "Downloading Ubuntu cloud image..." - curl -fSL -o "${CLOUD_IMAGE_PATH}" "${CLOUD_IMAGE_URL}" -else - echo "Cloud image already cached at ${CLOUD_IMAGE_PATH}" -fi - -# Download Ubuntu cloud OVA for Import-PveOva testing -OVA_URL="https://cloud-images.ubuntu.com/releases/24.04/release/ubuntu-24.04-server-cloudimg-amd64.ova" -OVA_FILENAME="ubuntu-24.04-server-cloudimg-amd64.ova" -OVA_PATH="${OUTPUT_DIR}/${OVA_FILENAME}" -if [ ! -f "${OVA_PATH}" ]; then - echo "Downloading Ubuntu cloud OVA (this may take a few minutes)..." - curl -fSL -o "${OVA_PATH}" "${OVA_URL}" - echo "Downloaded OVA ($(du -h "${OVA_PATH}" | cut -f1))" -else - echo "OVA already cached at ${OVA_PATH}" -fi - -echo "CLOUD_IMAGE_PATH=${CLOUD_IMAGE_PATH}" -echo "OVA_PATH=${OVA_PATH}" echo "Environment preparation complete." diff --git a/tests/infrastructure/variables.tf b/tests/infrastructure/variables.tf index e38fd85..5de8149 100644 --- a/tests/infrastructure/variables.tf +++ b/tests/infrastructure/variables.tf @@ -16,36 +16,33 @@ variable "proxmox_insecure" { } variable "target_node" { - description = "Name of the Proxmox node where the nested PVE VM will be created" + description = "Name of the Proxmox node where the nested PVE VMs will be created" type = string } -variable "vm_id" { - description = "VMID to assign to the nested PVE virtual machine" - type = number - default = 99900 -} - -variable "vm_name" { - description = "Name for the nested PVE virtual machine" - type = string - default = "pve-test-nested" +variable "pve_instances" { + description = "Map of PVE instances to provision. Key is a label (e.g. 'pve9'), value defines the VM." + type = map(object({ + iso_local_path = string + vm_id = number + vm_name = string + })) } variable "cores" { - description = "Number of CPU cores to allocate to the nested PVE VM" + description = "Number of CPU cores to allocate to each nested PVE VM" type = number default = 4 } variable "memory" { - description = "Amount of memory in MB to allocate to the nested PVE VM" + description = "Amount of memory in MB to allocate to each nested PVE VM" type = number default = 8192 } variable "disk_size" { - description = "Size of the primary disk in GB for the nested PVE VM" + description = "Size of the primary disk in GB for each nested PVE VM" type = number default = 64 } @@ -56,11 +53,6 @@ variable "disk_storage" { default = "nas-iSCSI-lvm" } -variable "iso_local_path" { - description = "Local path to the prepared auto-install PVE ISO on the runner" - type = string -} - variable "iso_storage" { description = "Proxmox storage pool for uploading the ISO (must accept ISO content type)" type = string @@ -68,13 +60,13 @@ variable "iso_storage" { } variable "network_bridge" { - description = "Network bridge on the host to attach the nested PVE VM to" + description = "Network bridge on the host to attach the nested PVE VMs to" type = string default = "Core" } variable "test_vm_password" { - description = "Root password for the nested PVE instance" + description = "Root password for the nested PVE instances" type = string sensitive = true default = "Testpass123!"