Merge pull request #93 from GoodOlClint/test/cluster-join-link0

fix: New-PveCluster -Wait blocks until quorate; make cluster failures diagnosable

Adds ClusterConfigService.WaitForQuorum with a -Timeout parameter (D014), so
Add-PveClusterMember no longer hits "cluster not ready - no quorum?" after a
completed create task. Adds diagnose-cluster.sh, which captures corosync and
pmxcfs state plus PVE task logs from both nodes before cleanup destroys them.

.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:

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

DECISIONS.md

@@ -407,3 +407,57 @@ public List<Dictionary<string, object?>>? 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 ...
+```

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);
+
         /// <summary>
         /// Initializes a new instance of the <see cref="ClusterConfigService"/> class.
         /// </summary>
@@ -378,6 +383,48 @@ namespace PSProxmoxVE.Core.Services
             }
         }
 
+        /// <summary>
+        /// Blocks until the cluster reports quorum (GET /cluster/status, quorate = 1).
+        /// </summary>
+        /// <param name="session">The authenticated PVE session.</param>
+        /// <param name="timeout">Maximum time to wait. Defaults to 60 seconds.</param>
+        /// <remarks>
+        /// 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.
+        /// </remarks>
+        /// <exception cref="TimeoutException">Quorum was not reached before the deadline.</exception>
+        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);
+            }
+        }
+
         /// <summary>
         /// Returns the next available VM/CT ID (GET /cluster/nextid).
         /// </summary>

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; }
 
-        /// <summary>Wait for the cluster creation task to complete.</summary>
-        [Parameter(Mandatory = false, HelpMessage = "Wait for the task to complete before returning.")]
+        /// <summary>Wait for the cluster creation task to complete and the cluster to reach quorum.</summary>
+        [Parameter(Mandatory = false, HelpMessage = "Wait for the task to complete and the cluster to reach quorum before returning.")]
         public SwitchParameter Wait { get; set; }
 
+        /// <summary>Seconds to wait for the cluster to reach quorum when -Wait is used.</summary>
+        [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);

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<ArgumentNullException>(() => 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<IPveHttpClient>();
+            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<IPveHttpClient>();
+            mockClient.Setup(c => c.GetAsync("cluster/status")).ReturnsAsync(notQuorate);
+            var service = new ClusterConfigService(mockClient.Object);
+
+            Assert.Throws<TimeoutException>(() =>
+                service.WaitForQuorum(CreateSession(), TimeSpan.FromMilliseconds(1)));
+        }
+
+        [Fact]
+        public void WaitForQuorum_RetriesWhileTheApiIsRestarting()
+        {
+            var quorate = @"{""data"": [{""type"": ""cluster"", ""name"": ""c1"", ""quorate"": 1}]}";
+            var mockClient = new Mock<IPveHttpClient>();
+            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));
+        }
     }
 }

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
         }

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