diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index ff6be01..f506962 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -53,6 +53,11 @@ on: required: false type: boolean default: false + keep_vms: + description: 'Skip cleanup so the nested PVE VMs survive for inspection' + required: false + type: boolean + default: false permissions: contents: read @@ -183,6 +188,13 @@ jobs: PVETEST_PASSWORD: ${{ secrets.PVE_TEST_PASSWORD }} run: bash ${SCRIPTS_DIR}/run-integration.sh test ${{ matrix.pve_version }} + - name: Diagnose cluster state + if: failure() + 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 @@ -193,7 +205,7 @@ jobs: # ── Cleanup: destroy all VMs (always runs) ────────────────────── cleanup: needs: [provision, test] - if: always() && needs.provision.result != 'skipped' && github.actor != 'dependabot[bot]' + if: always() && needs.provision.result != 'skipped' && github.actor != 'dependabot[bot]' && !inputs.keep_vms runs-on: psproxmoxve timeout-minutes: 15 container: diff --git a/CHANGELOG.md b/CHANGELOG.md index 8a8b0f0..f6a1a20 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Conventional Commits](https://www.conventionalcommi ## [Unreleased] +### Fixed + +- `New-PveCluster -Wait` now blocks until the cluster reports quorum, not merely until the creation task finishes. PVE's create task returns before corosync converges (~6s earlier in testing), so the natural `New-PveCluster -Wait` → `Add-PveClusterMember` sequence failed with `cluster not ready - no quorum?`. Adds `-Timeout` (seconds, default 60, range 1-3600) following the `-Wait` timeout convention used by `Stop-PveContainer` and `Reset-PveVm`. See `DECISIONS.md` D014. + ## [0.2.0] - 2026-05-22 ### Added diff --git a/DECISIONS.md b/DECISIONS.md index 8393e51..b24c13f 100644 --- a/DECISIONS.md +++ b/DECISIONS.md @@ -407,3 +407,57 @@ public List>? Members { get; set; } // or [OutputType(typeof(PSObject))] ``` + +--- + +## D014 — New-PveCluster -Wait blocks until the cluster is quorate + +**Status**: Active +**Finding refs**: (none — found via integration run 172, 2026-09-01) +**Resolved in scan**: n/a + +### Decision +`New-PveCluster -Wait` returns only after the cluster reports quorum, not merely when the +create task completes. `ClusterConfigService.WaitForQuorum` implements the wait; it polls +`GET /cluster/status` for the `cluster` entry with `quorate = 1` and tolerates transient API +errors while corosync and pmxcfs restart. + +The wait is bounded and throws `TimeoutException` on expiry. It follows the `-Wait` timeout +convention already used by `Stop-PveContainer`, `Reset-PveVm` and `New-PveBackup`: +`[ValidateRange(1, 3600)] public int Timeout` with a default (60 here), **no `0 = infinite`**. + +Note there are two distinct timeout conventions in this module; do not mix them: +- **`-Wait` waits** (`Timeout`, `int` with a default, range 1-3600, no infinite) — task/state waits. +- **HTTP client timeouts** (`TimeoutSeconds`, `int?`, range 0-int.MaxValue, `0 = infinite`) — + `Connect-PveServer`, `Send-PveFile`, `Invoke-PveStorageDownload`, which set `HttpClient.Timeout`. + +A single-node cluster reaches quorum in seconds (~6 s observed), so a node still not quorate +after 60 s is broken rather than slow. + +`-Wait` on every other cmdlet still means "wait for the task". Cluster creation is the +exception because the task completing does not make the cluster usable. + +### Rationale +PVE's cluster-create task returns before corosync converges. Until the node is quorate it +rejects a join with `cluster not ready - no quorum?`, so the natural sequence +`New-PveCluster -Wait` → `Add-PveClusterMember` fails intermittently for every caller. + +Observed on node A in integration run 172: the create task returned, corosync started ~1 s +later, and `node has quorum` appeared ~6 s after that. The integration test had guarded this +with `Start-Sleep -Seconds 5` — a fixed sleep against a longer, variable convergence — which +is why the cluster tests had never passed. + +### Anti-pattern (do not reintroduce) +```powershell +# NEVER guard cluster convergence with a fixed sleep +New-PveCluster -ClusterName 'c1' -Wait +Start-Sleep -Seconds 5 +Add-PveClusterMember ... +``` + +### Correct pattern +```powershell +# -Wait already guarantees quorum; join immediately +New-PveCluster -ClusterName 'c1' -Wait +Add-PveClusterMember ... +``` diff --git a/src/PSProxmoxVE.Core/Services/ClusterConfigService.cs b/src/PSProxmoxVE.Core/Services/ClusterConfigService.cs index a66f603..e288816 100644 --- a/src/PSProxmoxVE.Core/Services/ClusterConfigService.cs +++ b/src/PSProxmoxVE.Core/Services/ClusterConfigService.cs @@ -1,8 +1,10 @@ using System; using System.Collections.Generic; +using System.Threading; using Newtonsoft.Json.Linq; using PSProxmoxVE.Core.Authentication; using PSProxmoxVE.Core.Client; +using PSProxmoxVE.Core.Exceptions; using PSProxmoxVE.Core.Models.Cluster; using PSProxmoxVE.Core.Utilities; @@ -16,6 +18,9 @@ namespace PSProxmoxVE.Core.Services { private readonly IPveHttpClient? _injectedClient; + private static readonly TimeSpan DefaultQuorumTimeout = TimeSpan.FromSeconds(60); + private static readonly TimeSpan QuorumPollInterval = TimeSpan.FromSeconds(2); + /// /// Initializes a new instance of the class. /// @@ -378,6 +383,48 @@ namespace PSProxmoxVE.Core.Services } } + /// + /// Blocks until the cluster reports quorum (GET /cluster/status, quorate = 1). + /// + /// The authenticated PVE session. + /// Maximum time to wait. Defaults to 60 seconds. + /// + /// The cluster-create task completes before corosync converges; until the node + /// is quorate it rejects joins with "cluster not ready - no quorum?". API errors + /// during that window are transient and are retried until the deadline. + /// + /// Quorum was not reached before the deadline. + public void WaitForQuorum(PveSession session, TimeSpan? timeout = null) + { + if (session == null) throw new ArgumentNullException(nameof(session)); + + var effectiveTimeout = timeout ?? DefaultQuorumTimeout; + var deadline = DateTime.UtcNow.Add(effectiveTimeout); + + while (true) + { + try + { + foreach (var entry in GetClusterStatus(session)) + { + if (string.Equals(entry.Type, "cluster", StringComparison.OrdinalIgnoreCase) + && entry.Quorate == 1) + return; + } + } + catch (PveApiException) + { + // pmxcfs and corosync restart while the cluster forms. + } + + if (DateTime.UtcNow >= deadline) + throw new TimeoutException( + $"Cluster did not reach quorum within {effectiveTimeout.TotalSeconds:0} seconds."); + + Thread.Sleep(QuorumPollInterval); + } + } + /// /// Returns the next available VM/CT ID (GET /cluster/nextid). /// diff --git a/src/PSProxmoxVE/Cmdlets/Cluster/NewPveClusterCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Cluster/NewPveClusterCmdlet.cs index 13621c7..4a485a9 100644 --- a/src/PSProxmoxVE/Cmdlets/Cluster/NewPveClusterCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Cluster/NewPveClusterCmdlet.cs @@ -1,3 +1,4 @@ +using System; using System.Management.Automation; using PSProxmoxVE.Core.Models.Vms; using PSProxmoxVE.Core.Services; @@ -36,10 +37,15 @@ namespace PSProxmoxVE.Cmdlets.Cluster [Parameter(Mandatory = false, HelpMessage = "Corosync link addresses as key=value strings (e.g. 'link0=10.0.0.1').")] public string[]? Links { get; set; } - /// Wait for the cluster creation task to complete. - [Parameter(Mandatory = false, HelpMessage = "Wait for the task to complete before returning.")] + /// Wait for the cluster creation task to complete and the cluster to reach quorum. + [Parameter(Mandatory = false, HelpMessage = "Wait for the task to complete and the cluster to reach quorum before returning.")] public SwitchParameter Wait { get; set; } + /// Seconds to wait for the cluster to reach quorum when -Wait is used. + [Parameter(Mandatory = false, HelpMessage = "Timeout in seconds for -Wait (default 60).")] + [ValidateRange(1, 3600)] + public int Timeout { get; set; } = 60; + protected override void ProcessRecord() { if (!ShouldProcess($"cluster '{ClusterName}'", "Create new cluster")) @@ -61,6 +67,11 @@ namespace PSProxmoxVE.Cmdlets.Cluster { var taskService = new TaskService(); task = taskService.WaitForTask(session, node, upid); + + // The create task returns before corosync converges; the cluster + // rejects joins until it is quorate. + WriteVerbose("Waiting for cluster to reach quorum..."); + service.WaitForQuorum(session, TimeSpan.FromSeconds(Timeout)); } WriteObject(task); diff --git a/tests/PSProxmoxVE.Core.Tests/Services/ClusterConfigServiceTests.cs b/tests/PSProxmoxVE.Core.Tests/Services/ClusterConfigServiceTests.cs index f5d65f8..e5db43a 100644 --- a/tests/PSProxmoxVE.Core.Tests/Services/ClusterConfigServiceTests.cs +++ b/tests/PSProxmoxVE.Core.Tests/Services/ClusterConfigServiceTests.cs @@ -1,11 +1,13 @@ using System; using System.Collections.Generic; +using System.Net; using System.Threading.Tasks; using Moq; using Newtonsoft.Json.Linq; using Xunit; using PSProxmoxVE.Core.Authentication; using PSProxmoxVE.Core.Client; +using PSProxmoxVE.Core.Exceptions; using PSProxmoxVE.Core.Services; namespace PSProxmoxVE.Core.Tests.Services @@ -466,5 +468,49 @@ namespace PSProxmoxVE.Core.Tests.Services { Assert.Throws(() => new ClusterConfigService(null!)); } + + [Fact] + public void WaitForQuorum_ReturnsOnceClusterIsQuorate() + { + var notQuorate = @"{""data"": [{""type"": ""cluster"", ""name"": ""c1"", ""quorate"": 0}]}"; + var quorate = @"{""data"": [{""type"": ""cluster"", ""name"": ""c1"", ""quorate"": 1}]}"; + var mockClient = new Mock(); + mockClient.SetupSequence(c => c.GetAsync("cluster/status")) + .ReturnsAsync(notQuorate) + .ReturnsAsync(quorate); + var service = new ClusterConfigService(mockClient.Object); + + service.WaitForQuorum(CreateSession(), TimeSpan.FromSeconds(30)); + + mockClient.Verify(c => c.GetAsync("cluster/status"), Times.Exactly(2)); + } + + [Fact] + public void WaitForQuorum_ThrowsWhenQuorumNeverReached() + { + var notQuorate = @"{""data"": [{""type"": ""cluster"", ""name"": ""c1"", ""quorate"": 0}]}"; + var mockClient = new Mock(); + mockClient.Setup(c => c.GetAsync("cluster/status")).ReturnsAsync(notQuorate); + var service = new ClusterConfigService(mockClient.Object); + + Assert.Throws(() => + service.WaitForQuorum(CreateSession(), TimeSpan.FromMilliseconds(1))); + } + + [Fact] + public void WaitForQuorum_RetriesWhileTheApiIsRestarting() + { + var quorate = @"{""data"": [{""type"": ""cluster"", ""name"": ""c1"", ""quorate"": 1}]}"; + var mockClient = new Mock(); + mockClient.SetupSequence(c => c.GetAsync("cluster/status")) + .ThrowsAsync(new PveApiException(HttpStatusCode.InternalServerError, + "cluster not ready", "cluster/status", "GET")) + .ReturnsAsync(quorate); + var service = new ClusterConfigService(mockClient.Object); + + service.WaitForQuorum(CreateSession(), TimeSpan.FromSeconds(30)); + + mockClient.Verify(c => c.GetAsync("cluster/status"), Times.Exactly(2)); + } } } diff --git a/tests/PSProxmoxVE.Tests/Integration/16_Cluster.Tests.ps1 b/tests/PSProxmoxVE.Tests/Integration/16_Cluster.Tests.ps1 index a14a26f..d982d70 100644 --- a/tests/PSProxmoxVE.Tests/Integration/16_Cluster.Tests.ps1 +++ b/tests/PSProxmoxVE.Tests/Integration/16_Cluster.Tests.ps1 @@ -112,8 +112,11 @@ Describe 'Cluster Config & HA Lifecycle — Integration' -Tag 'Integration' { $result | Should -Not -BeNullOrEmpty $script:ClusterCreated = $true - # Allow corosync to fully stabilize - Start-Sleep -Seconds 5 + # New-PveCluster -Wait returns only once the cluster is quorate (D014), + # which is what makes the join below safe without a sleep. + $cluster = @(Get-PveClusterStatus -ErrorAction Stop) | + Where-Object { $_.Type -eq 'cluster' } | Select-Object -First 1 + $cluster.Quorate | Should -Be 1 -Because 'node A must be quorate before node B can join' } It 'Get-PveClusterStatus shows cluster formed' { @@ -216,6 +219,12 @@ Describe 'Cluster Config & HA Lifecycle — Integration' -Tag 'Integration' { Start-Sleep -Seconds 3 } while ([DateTime]::UtcNow -lt $deadline) + $cluster = @($status) | Where-Object { $_.Type -eq 'cluster' } | Select-Object -First 1 + Write-Host "cluster: quorate=$($cluster.Quorate) nodes=$($cluster.Nodes); node B answers on $($script:HostB)" + foreach ($n in $nodeEntries) { + Write-Host " node=$($n.Name) nodeid=$($n.NodeId) ring0=$($n.Ip) online=$($n.Online) local=$($n.Local)" + } + @($nodeEntries).Count | Should -BeGreaterOrEqual 2 @($onlineNodes).Count | Should -BeGreaterOrEqual 2 } diff --git a/tests/infrastructure/scripts/diagnose-cluster.sh b/tests/infrastructure/scripts/diagnose-cluster.sh new file mode 100644 index 0000000..ade5509 --- /dev/null +++ b/tests/infrastructure/scripts/diagnose-cluster.sh @@ -0,0 +1,106 @@ +#!/usr/bin/env bash +# Dump corosync state from both nested PVE nodes after a cluster test failure. +# +# Usage: diagnose-cluster.sh [8|9] +# +# The PVE API reports a joined-but-offline node as online=0 with no further +# detail; corosync's own view lives only on the nodes, which the cleanup job +# destroys minutes later. Best-effort: never fails the caller. +# +# Required env vars: +# PVE_PASSWORD Root password for the nested PVE instances +# +# Optional env vars: +# CONFIG_FILE Test config JSON (default: $CACHE_DIR/work/config.json) +# CACHE_DIR Shared cache mount (default: /opt/pve-integration) + +VERSION="${1:-9}" +CACHE_DIR="${CACHE_DIR:-/opt/pve-integration}" +CONFIG_FILE="${CONFIG_FILE:-$CACHE_DIR/work/config.json}" + +if [[ ! -f "$CONFIG_FILE" ]]; then + echo "diagnose-cluster: no config at $CONFIG_FILE — nothing to inspect" + exit 0 +fi + +if [[ -z "${PVE_PASSWORD:-}" ]]; then + echo "diagnose-cluster: PVE_PASSWORD unset — cannot reach the nodes" + exit 0 +fi + +SSH_OPTS=(-o StrictHostKeyChecking=no -o UserKnownHostsFile=/dev/null -o LogLevel=ERROR -o ConnectTimeout=10) + +dump_node() { + local label="$1" ip="$2" + echo + echo "══════════ $label ($ip) ══════════" + if [[ -z "$ip" || "$ip" == "null" ]]; then + echo " no address in $CONFIG_FILE" + return + fi + + sshpass -p "$PVE_PASSWORD" ssh "${SSH_OPTS[@]}" "root@${ip}" bash -s <<'REMOTE' 2>&1 || echo " ssh to $ip failed (rc=$?)" +set +e +echo "--- hostname / resolution ---" +hostname -f +echo "hostname -i: $(hostname -i 2>&1)" +grep -vE '^\s*#' /etc/hosts | grep -vE '^\s*$' +echo +echo "--- addresses ---" +ip -4 -o addr show scope global +echo +echo "--- pmxcfs mode: cluster or local? ---" +# /etc/pve/corosync.conf is database-backed; pmxcfs only creates it when it +# starts with no config.db and imports /etc/corosync/corosync.conf. A surviving +# standalone config.db means silent local mode with corosync otherwise healthy. +dpkg-query -W pve-cluster corosync 2>&1 +tr '\0' ' ' < "/proc/$(systemctl show pve-cluster -p MainPID --value)/cmdline" 2>&1; echo +findmnt --target /etc/pve --output TARGET,SOURCE,FSTYPE 2>&1 +echo ".members: $(cat /etc/pve/.members 2>&1 | tr -d '\n')" +ls -la /var/lib/pve-cluster/ 2>&1 +ls -la /var/lib/pve-cluster/backup/ 2>&1 +if command -v sqlite3 >/dev/null 2>&1; then + sqlite3 -readonly /var/lib/pve-cluster/config.db \ + "PRAGMA quick_check; SELECT name,version,writer,mtime,length(data) FROM tree WHERE name='corosync.conf';" 2>&1 +else + echo "sqlite3 absent; corosync.conf occurrences in config.db: $(strings /var/lib/pve-cluster/config.db 2>/dev/null | grep -c '^corosync\.conf$')" +fi +echo +echo "--- corosync-cpgtool (pmxcfs joins dcdb/status CPG groups when clustered) ---" +corosync-cpgtool 2>&1 +echo +echo "--- corosync.conf ---" +cat /etc/pve/corosync.conf 2>&1 || cat /etc/corosync/corosync.conf 2>&1 +echo +echo "--- corosync-cfgtool -s ---" +corosync-cfgtool -s 2>&1 +echo +echo "--- pvecm status ---" +pvecm status 2>&1 +echo +echo "--- corosync service ---" +systemctl is-active corosync pve-cluster 2>&1 +echo +echo "--- journalctl -u corosync (last 60) ---" +journalctl -u corosync -n 60 --no-pager 2>&1 +echo +echo "--- journalctl -u pve-cluster (last 30) ---" +journalctl -u pve-cluster -n 30 --no-pager 2>&1 +echo +echo "--- cluster task logs ---" +# "Cluster join aborted!" is generic; the reason is only in the task log. +find /var/log/pve/tasks -type f \( -name '*clusterjoin*' -o -name '*clustercreate*' \) \ + -exec echo "== {} ==" \; -exec cat {} \; 2>&1 | tail -80 +REMOTE +} + +echo "=== Cluster diagnostics for PVE $VERSION ===" +node_a="$(jq -r ".pve${VERSION}.nodes.a.host // empty" "$CONFIG_FILE")" +node_b="$(jq -r ".pve${VERSION}.nodes.b.host // empty" "$CONFIG_FILE")" + +dump_node "node A" "$node_a" +dump_node "node B" "$node_b" + +echo +echo "=== End cluster diagnostics ===" +exit 0