From 3ea0e11988eeb78438be2a963b7cd159843bb896 Mon Sep 17 00:00:00 2001 From: "goodolclint-claude[bot]" <323206664+goodolclint-claude[bot]@users.noreply.github.com> Date: Thu, 3 Sep 2026 17:33:02 +0000 Subject: [PATCH] fix: Get-PveVm sources the all-nodes listing from cluster/resources, not per-node fan-out (#223) VmService.GetVms(node: null) now issues a single GET cluster/resources?type=vm instead of GET /nodes followed by one GET /nodes/{n}/qemu per node -- 17 round trips down to 1 on a 16-node cluster. VmService.GetVm(session, node, vmid) now fetches nodes/{node}/qemu/{vmid}/status/current directly instead of listing the whole node and filtering client-side. GetPveVmCmdlet's -Detailed path shares one IPveHttpClient across the enrichment loop and calls WriteObject per VM as it is enriched, instead of materializing the whole list first and opening a fresh client per VM. cluster/resources's "type=vm" filter returns both qemu and lxc rows (per the PVE OpenAPI spec), so the mapping filters to type=="qemu" explicitly. The resources endpoint does not carry QmpStatus/Pid/AgentStatus, so those stay null until -Detailed or GetVm enrich from status/current; the default table view (VmId/Name/EffectiveStatus/Node/CpuCount/MaxMem/Uptime) is unaffected. GetVm's status/current call converts a 404 or 500 into the pre-existing InvalidOperationException("not found") contract that ImportPveOvaCmdlet depends on, but 502/503/504 propagate unchanged since PveHttpClient wraps a connectivity failure as 503 -- a down node must not read as a missing VM. Removing the old per-node fan-out (issue #142's node-skip behavior) means onNodeSkipped is no longer invoked for the all-nodes path: a single cluster/resources call has no per-node failure to report, so a node-unreachable condition now surfaces via that node's rows rather than a WriteWarning. The parameter stays on VmService.GetVms/TemplateService.GetTemplates for source compatibility with the cmdlets that still wire it. TemplateService.GetTemplates and its tests are also updated since it delegates to VmService.GetVms and inherited the same per-node fan-out. Co-authored-by: goodolclint-claude[bot] <323206664+goodolclint-claude[bot]@users.noreply.github.com> Co-authored-by: Claude Fable 5.1 --- .../Models/Cluster/PveClusterResource.cs | 12 + .../Services/TemplateService.cs | 5 +- src/PSProxmoxVE.Core/Services/VmService.cs | 109 +++++---- src/PSProxmoxVE/Cmdlets/Vms/GetPveVmCmdlet.cs | 20 +- .../Services/TemplateServiceTests.cs | 60 +---- .../Services/VmServiceTests.cs | 224 ++++++++++++------ 6 files changed, 255 insertions(+), 175 deletions(-) diff --git a/src/PSProxmoxVE.Core/Models/Cluster/PveClusterResource.cs b/src/PSProxmoxVE.Core/Models/Cluster/PveClusterResource.cs index 84d362e..6d5bf13 100644 --- a/src/PSProxmoxVE.Core/Models/Cluster/PveClusterResource.cs +++ b/src/PSProxmoxVE.Core/Models/Cluster/PveClusterResource.cs @@ -98,6 +98,18 @@ public class PveClusterResource [JsonProperty("vmid")] public int? VmId { get; set; } + /// + /// The guest's configured tags, semicolon-separated. Only for VM/LXC types. + /// + [JsonProperty("tags")] + public string? Tags { get; set; } + + /// + /// The guest's current config lock, if any (e.g. "migrate", "backup"). Only for VM/LXC types. + /// + [JsonProperty("lock")] + public string? Lock { get; set; } + /// /// The HA (High Availability) state of the resource, if managed by HA. /// diff --git a/src/PSProxmoxVE.Core/Services/TemplateService.cs b/src/PSProxmoxVE.Core/Services/TemplateService.cs index cabb42f..e3eea3c 100644 --- a/src/PSProxmoxVE.Core/Services/TemplateService.cs +++ b/src/PSProxmoxVE.Core/Services/TemplateService.cs @@ -39,8 +39,9 @@ namespace PSProxmoxVE.Core.Services /// The authenticated PVE session. /// Optional cluster node name to filter templates by node. /// - /// Optional callback invoked with the node name and the exception when a node is - /// skipped because it is unreachable, forwarded to . + /// Forwarded to , which does not invoke it for the + /// all-nodes listing (a single cluster/resources call has no per-node + /// failure to report); kept for source compatibility with existing callers. /// public PveVm[] GetTemplates(PveSession session, string? node = null, Action? onNodeSkipped = null) { diff --git a/src/PSProxmoxVE.Core/Services/VmService.cs b/src/PSProxmoxVE.Core/Services/VmService.cs index 7562c17..047b81c 100644 --- a/src/PSProxmoxVE.Core/Services/VmService.cs +++ b/src/PSProxmoxVE.Core/Services/VmService.cs @@ -4,7 +4,6 @@ using System.Linq; using Newtonsoft.Json.Linq; using PSProxmoxVE.Core.Authentication; using PSProxmoxVE.Core.Client; -using PSProxmoxVE.Core.Models.Nodes; using PSProxmoxVE.Core.Models.Vms; using PSProxmoxVE.Core.Utilities; @@ -15,19 +14,15 @@ namespace PSProxmoxVE.Core.Services /// public class VmService : PveServiceBase { - private readonly NodeService _nodeService; - /// Initializes a new instance that creates its own HTTP clients. public VmService() { - _nodeService = new NodeService(); } /// Initializes a new instance that uses the supplied HTTP client for all requests. /// The HTTP client to use. The caller owns its lifetime. public VmService(IPveHttpClient client) : base(client) { - _nodeService = new NodeService(client); } // ------------------------------------------------------------------------- @@ -35,15 +30,15 @@ namespace PSProxmoxVE.Core.Services // ------------------------------------------------------------------------- /// - /// Returns VMs. If is null, queries every cluster node. + /// Returns VMs. If is null, lists the whole cluster with a + /// single call to cluster/resources?type=vm instead of one call per node. /// /// The authenticated PVE session. /// Optional cluster node name to filter VMs by node. /// - /// Optional callback invoked with the node name and the exception when a node is - /// skipped because it is unreachable (connectivity failure or a 5xx from that node). - /// A 401/403/404 or any other non-5xx - /// propagates instead of being swallowed. + /// Not invoked. The cluster-wide listing is one call to cluster/resources + /// with no per-node failure to report; kept on the signature for source + /// compatibility with existing callers. /// public PveVm[] GetVms(PveSession session, string? node = null, Action? onNodeSkipped = null) { @@ -52,42 +47,39 @@ namespace PSProxmoxVE.Core.Services if (node != null) return GetVmsOnNode(session, node); - // Query all nodes and aggregate - var nodes = _nodeService.GetNodes(session); - var all = new List(); - foreach (var n in nodes) + return Invoke(session, client => { - try - { - var vms = GetVmsOnNode(session, n.Name); - // Stamp the node name in case it wasn't returned by the API - foreach (var vm in vms) - vm.Node ??= n.Name; - all.AddRange(vms); - } - catch (Exception ex) when (IsNodeUnreachable(ex)) - { - onNodeSkipped?.Invoke(n.Name, ex); - } - } - return all.ToArray(); + const string resource = "cluster/resources?type=vm"; + var response = client.GetAsync(resource).GetAwaiter().GetResult(); + var data = JObject.Parse(response)["data"]; + var resources = data?.ToObject() + ?? Array.Empty(); + // "type=vm" is PVE's guest filter, not a QEMU-only one — it returns both + // "qemu" and "lxc" rows, so the QEMU guests need filtering out here. + return resources.Where(r => r.Type == "qemu").Select(ToPveVm).ToArray(); + }); } /// - /// True for a connectivity failure or a 5xx PVE API response — the cases where the - /// node itself is unreachable rather than the request being rejected. A 401/403/404 - /// (or any other non-5xx status) means the request was understood and refused, which - /// is not something a per-node listing loop should hide. + /// Maps a /cluster/resources row (type "qemu") onto . The + /// resources endpoint does not carry , + /// or — those remain null + /// until Get-PveVm -Detailed or enrich from + /// status/current. /// - private static bool IsNodeUnreachable(Exception ex) => ex switch + private static PveVm ToPveVm(PSProxmoxVE.Core.Models.Cluster.PveClusterResource r) => new PveVm { - System.Net.Http.HttpRequestException => true, - // PveHttpClient wraps a connection failure as 503 and a client-side timeout as - // 408 (Client/PveHttpClient.cs SendOnceAsync) — both mean the node did not answer, - // not that it rejected the request. - PSProxmoxVE.Core.Exceptions.PveApiException apiEx => - apiEx.StatusCode == System.Net.HttpStatusCode.RequestTimeout || (int)apiEx.StatusCode >= 500, - _ => false + VmId = r.VmId ?? 0, + Name = r.Name, + Status = r.Status, + Node = r.Node, + CpuCount = r.MaxCpu.HasValue ? (int)r.MaxCpu.Value : (int?)null, + MaxMem = r.MaxMem, + MaxDisk = r.MaxDisk, + Uptime = r.Uptime, + Tags = r.Tags, + Template = r.Template ?? 0, + Lock = r.Lock, }; private PveVm[] GetVmsOnNode(PveSession session, string node) @@ -101,7 +93,8 @@ namespace PSProxmoxVE.Core.Services } /// - /// Returns a single VM by its ID on the specified node. + /// Returns a single VM by its ID, fetched directly from + /// status/current rather than listing the whole node. /// /// The authenticated PVE session. /// The cluster node name. @@ -111,12 +104,34 @@ namespace PSProxmoxVE.Core.Services if (session == null) throw new ArgumentNullException(nameof(session)); if (string.IsNullOrWhiteSpace(node)) throw new ArgumentNullException(nameof(node)); - var vms = GetVmsOnNode(session, node); - var vm = vms.FirstOrDefault(v => v.VmId == vmid); - if (vm == null) - throw new InvalidOperationException($"VM {vmid} not found on node '{node}'."); - vm.Node ??= node; - return vm; + return Invoke(session, client => + { + JToken? data; + try + { + var response = client.GetAsync($"nodes/{Uri.EscapeDataString(node)}/qemu/{vmid}/status/current") + .GetAwaiter().GetResult(); + data = JObject.Parse(response)["data"]; + } + catch (PSProxmoxVE.Core.Exceptions.PveApiException ex) when ( + ex.StatusCode == System.Net.HttpStatusCode.NotFound || + ex.StatusCode == System.Net.HttpStatusCode.InternalServerError) + { + // Preserves the not-found contract GetVmsOnNode gave callers (e.g. + // ImportPveOvaCmdlet) when the VM wasn't in the node's listing yet. + // Excludes 502/503/504 deliberately: PveHttpClient.SendOnceAsync wraps a + // connectivity failure as 503, which is the node being unreachable, not + // the VM being absent, and must propagate rather than read as not-found. + throw new InvalidOperationException($"VM {vmid} not found on node '{node}'.", ex); + } + + if (data == null) + throw new InvalidOperationException($"VM {vmid} not found on node '{node}'."); + + var vm = data.ToObject() ?? new PveVm { VmId = vmid }; + vm.Node ??= node; + return vm; + }); } /// diff --git a/src/PSProxmoxVE/Cmdlets/Vms/GetPveVmCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Vms/GetPveVmCmdlet.cs index 6efa60f..fa050bb 100644 --- a/src/PSProxmoxVE/Cmdlets/Vms/GetPveVmCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Vms/GetPveVmCmdlet.cs @@ -1,6 +1,7 @@ using System.Collections.Generic; using System.Linq; using System.Management.Automation; +using PSProxmoxVE.Core.Client; using PSProxmoxVE.Core.Models.Vms; using PSProxmoxVE.Core.Services; @@ -95,27 +96,28 @@ namespace PSProxmoxVE.Cmdlets.Vms if (TemplatesOnly.IsPresent) vms = vms.Where(v => v.Template == 1); - // Materialize before enrichment to avoid multiple enumeration - var vmList = vms.ToList(); - if (Detailed.IsPresent) { - WriteVerbose($"Enriching {vmList.Count} VM(s) with detailed status..."); - foreach (var vm in vmList) + using var client = new PveHttpClient(session); + var detailService = new VmService(client); + foreach (var vm in vms) { try { - service.EnrichVmStatus(session, vm.Node ?? Node ?? string.Empty, vm); + detailService.EnrichVmStatus(session, vm.Node ?? Node ?? string.Empty, vm); } catch (PSProxmoxVE.Core.Exceptions.PveApiException) { // Skip enrichment for inaccessible VMs (e.g. locked, migrating) } + WriteObject(vm); } } - - foreach (var vm in vmList) - WriteObject(vm); + else + { + foreach (var vm in vms) + WriteObject(vm); + } } } } diff --git a/tests/PSProxmoxVE.Core.Tests/Services/TemplateServiceTests.cs b/tests/PSProxmoxVE.Core.Tests/Services/TemplateServiceTests.cs index 8f6151f..7880bf9 100644 --- a/tests/PSProxmoxVE.Core.Tests/Services/TemplateServiceTests.cs +++ b/tests/PSProxmoxVE.Core.Tests/Services/TemplateServiceTests.cs @@ -130,16 +130,18 @@ namespace PSProxmoxVE.Core.Tests.Services } [Fact] - public void GetTemplates_AllNodes_AggregatesAcrossNodesAndStampsNode() + public void GetTemplates_AllNodes_SourcesFromClusterResourcesInOneCall() { - // Arrange + // Arrange: issue #152 — VmService.GetVms(node: null) now sources the all-nodes + // listing from cluster/resources instead of a call per node. var mockClient = new Mock(); - mockClient.Setup(c => c.GetAsync("nodes")) - .ReturnsAsync(@"{""data"": [{""node"": ""pve1""}, {""node"": ""pve2""}]}"); - mockClient.Setup(c => c.GetAsync("nodes/pve1/qemu")) - .ReturnsAsync(@"{""data"": [{""vmid"": 100, ""template"": 1}, {""vmid"": 101, ""template"": 0}]}"); - mockClient.Setup(c => c.GetAsync("nodes/pve2/qemu")) - .ReturnsAsync(@"{""data"": [{""vmid"": 200, ""template"": 1}]}"); + mockClient.Setup(c => c.GetAsync("cluster/resources?type=vm")) + .ReturnsAsync(@"{""data"": [ + {""type"": ""qemu"", ""vmid"": 100, ""node"": ""pve1"", ""template"": 1}, + {""type"": ""qemu"", ""vmid"": 101, ""node"": ""pve1"", ""template"": 0}, + {""type"": ""qemu"", ""vmid"": 200, ""node"": ""pve2"", ""template"": 1}, + {""type"": ""lxc"", ""vmid"": 300, ""node"": ""pve2"", ""template"": 1} + ]}"); var service = new TemplateService(mockClient.Object); // Act @@ -149,46 +151,8 @@ namespace PSProxmoxVE.Core.Tests.Services Assert.Equal(2, templates.Length); Assert.Contains(templates, t => t.VmId == 100 && t.Node == "pve1"); Assert.Contains(templates, t => t.VmId == 200 && t.Node == "pve2"); - } - - [Fact] - public void GetTemplates_AllNodes_UnreachableNodeIsSkippedAndReported() - { - // Arrange - var mockClient = new Mock(); - mockClient.Setup(c => c.GetAsync("nodes")) - .ReturnsAsync(@"{""data"": [{""node"": ""pve1""}, {""node"": ""pve2""}]}"); - mockClient.Setup(c => c.GetAsync("nodes/pve1/qemu")) - .ReturnsAsync(@"{""data"": [{""vmid"": 100, ""template"": 1}]}"); - mockClient.Setup(c => c.GetAsync("nodes/pve2/qemu")) - .ThrowsAsync(new PveApiException(HttpStatusCode.InternalServerError, "internal error", "nodes/pve2/qemu", "GET")); - var service = new TemplateService(mockClient.Object); - - // Act - var skipped = new List(); - var templates = service.GetTemplates(CreateSession(), onNodeSkipped: (node, ex) => skipped.Add(node)); - - // Assert - var template = Assert.Single(templates); - Assert.Equal(100, template.VmId); - Assert.Equal(new[] { "pve2" }, skipped); - } - - [Fact] - public void GetTemplates_AllNodes_PermissionErrorOnOneNodePropagates() - { - // Arrange - var mockClient = new Mock(); - mockClient.Setup(c => c.GetAsync("nodes")) - .ReturnsAsync(@"{""data"": [{""node"": ""pve1""}, {""node"": ""pve2""}]}"); - mockClient.Setup(c => c.GetAsync("nodes/pve1/qemu")) - .ReturnsAsync(@"{""data"": [{""vmid"": 100, ""template"": 1}]}"); - mockClient.Setup(c => c.GetAsync("nodes/pve2/qemu")) - .ThrowsAsync(new PveApiException(HttpStatusCode.Forbidden, "permission denied", "nodes/pve2/qemu", "GET")); - var service = new TemplateService(mockClient.Object); - - // Act & Assert - Assert.Throws(() => service.GetTemplates(CreateSession())); + Assert.DoesNotContain(templates, t => t.VmId == 300); + mockClient.Verify(c => c.GetAsync("cluster/resources?type=vm"), Times.Once); } } } diff --git a/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs b/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs index 6be6fbe..1664113 100644 --- a/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs +++ b/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs @@ -277,12 +277,30 @@ namespace PSProxmoxVE.Core.Tests.Services // ----------------------------------------------------------------- [Fact] - public void GetVm_VmNotInNodeListing_ThrowsInvalidOperationException() + public void GetVm_HitsStatusCurrentDirectly_NotTheNodeListing() { var mockClient = new Mock(); mockClient - .Setup(c => c.GetAsync($"nodes/{TestNode}/qemu")) - .ReturnsAsync("{\"data\":[]}"); + .Setup(c => c.GetAsync($"nodes/{TestNode}/qemu/{TestVmId}/status/current")) + .ReturnsAsync("{\"data\":{\"vmid\":100,\"name\":\"web1\",\"status\":\"running\"}}"); + + var service = new VmService(mockClient.Object); + var vm = service.GetVm(CreateSession(), TestNode, TestVmId); + + Assert.Equal(100, vm.VmId); + Assert.Equal("web1", vm.Name); + Assert.Equal(TestNode, vm.Node); + mockClient.Verify(c => c.GetAsync($"nodes/{TestNode}/qemu/{TestVmId}/status/current"), Times.Once); + mockClient.Verify(c => c.GetAsync($"nodes/{TestNode}/qemu"), Times.Never); + } + + [Fact] + public void GetVm_NullData_ThrowsInvalidOperationException() + { + var mockClient = new Mock(); + mockClient + .Setup(c => c.GetAsync($"nodes/{TestNode}/qemu/{TestVmId}/status/current")) + .ReturnsAsync("{}"); var service = new VmService(mockClient.Object); @@ -291,108 +309,176 @@ namespace PSProxmoxVE.Core.Tests.Services Assert.Contains(TestVmId.ToString(), ex.Message); } - - // --------------------------------------------------------------------- - // GetVms multi-node aggregation: issue #142 - // --------------------------------------------------------------------- - - private static Mock SetupTwoNodeCluster() + [Fact] + public void GetVm_NotFoundOrServerError_ThrowsInvalidOperationException() { var mockClient = new Mock(); mockClient - .Setup(c => c.GetAsync("nodes")) - .ReturnsAsync("{\"data\":[{\"node\":\"pve1\"},{\"node\":\"pve2\"}]}"); - return mockClient; - } - - [Fact] - public void GetVms_AllNodes_A500OnOneNodeIsSkippedAndReportedButOtherNodeResultsReturn() - { - var mockClient = SetupTwoNodeCluster(); - mockClient - .Setup(c => c.GetAsync("nodes/pve1/qemu")) - .ReturnsAsync("{\"data\":[{\"vmid\":100}]}"); - mockClient - .Setup(c => c.GetAsync("nodes/pve2/qemu")) - .ThrowsAsync(new PveApiException(HttpStatusCode.InternalServerError, "internal error", "nodes/pve2/qemu", "GET")); + .Setup(c => c.GetAsync($"nodes/{TestNode}/qemu/{TestVmId}/status/current")) + .ThrowsAsync(new PveApiException(HttpStatusCode.InternalServerError, "config file does not exist", + $"nodes/{TestNode}/qemu/{TestVmId}/status/current", "GET")); var service = new VmService(mockClient.Object); - var skipped = new List(); - var vms = service.GetVms(CreateSession(), onNodeSkipped: (node, ex) => skipped.Add(node)); - - var vm = Assert.Single(vms); - Assert.Equal(100, vm.VmId); - Assert.Equal(new[] { "pve2" }, skipped); + var ex = Assert.Throws( + () => service.GetVm(CreateSession(), TestNode, TestVmId)); + Assert.Contains(TestVmId.ToString(), ex.Message); } [Fact] - public void GetVms_AllNodes_A403OnOneNodePropagatesInsteadOfBeingSwallowed() + public void GetVm_Forbidden_PropagatesInsteadOfBeingSwallowed() { - var mockClient = SetupTwoNodeCluster(); + var mockClient = new Mock(); mockClient - .Setup(c => c.GetAsync("nodes/pve1/qemu")) - .ReturnsAsync("{\"data\":[{\"vmid\":100}]}"); - mockClient - .Setup(c => c.GetAsync("nodes/pve2/qemu")) - .ThrowsAsync(new PveApiException(HttpStatusCode.Forbidden, "permission denied", "nodes/pve2/qemu", "GET")); + .Setup(c => c.GetAsync($"nodes/{TestNode}/qemu/{TestVmId}/status/current")) + .ThrowsAsync(new PveApiException(HttpStatusCode.Forbidden, "permission denied", + $"nodes/{TestNode}/qemu/{TestVmId}/status/current", "GET")); var service = new VmService(mockClient.Object); - var ex = Assert.Throws(() => service.GetVms(CreateSession())); + var ex = Assert.Throws(() => service.GetVm(CreateSession(), TestNode, TestVmId)); Assert.Equal(HttpStatusCode.Forbidden, ex.StatusCode); } [Fact] - public void GetVms_AllNodes_ConnectivityFailureOnOneNodeIsSkippedAndReported() + public void GetVm_ConnectivityFailure_PropagatesInsteadOfReadingAsNotFound() { - // PveHttpClient.SendOnceAsync never lets a raw HttpRequestException escape — it - // wraps one as PveApiException(ServiceUnavailable, ..., inner: HttpRequestException). - // That is the shape a real connectivity failure takes by the time it reaches - // VmService, so that is what this test throws. - var mockClient = SetupTwoNodeCluster(); + // PveHttpClient.SendOnceAsync wraps a connectivity failure as + // PveApiException(ServiceUnavailable) — the node being unreachable, not the VM + // being absent, so this must not be folded into the not-found conversion. + var mockClient = new Mock(); mockClient - .Setup(c => c.GetAsync("nodes/pve1/qemu")) - .ReturnsAsync("{\"data\":[{\"vmid\":100}]}"); - mockClient - .Setup(c => c.GetAsync("nodes/pve2/qemu")) + .Setup(c => c.GetAsync($"nodes/{TestNode}/qemu/{TestVmId}/status/current")) .ThrowsAsync(new PveApiException(HttpStatusCode.ServiceUnavailable, "connection refused", - "nodes/pve2/qemu", "GET", new HttpRequestException("connection refused"))); + $"nodes/{TestNode}/qemu/{TestVmId}/status/current", "GET", new HttpRequestException("connection refused"))); var service = new VmService(mockClient.Object); - var skipped = new List(); - var vms = service.GetVms(CreateSession(), onNodeSkipped: (node, ex) => skipped.Add(node)); + var ex = Assert.Throws(() => service.GetVm(CreateSession(), TestNode, TestVmId)); + Assert.Equal(HttpStatusCode.ServiceUnavailable, ex.StatusCode); + } - var vm = Assert.Single(vms); - Assert.Equal(100, vm.VmId); - Assert.Equal(new[] { "pve2" }, skipped); + // --------------------------------------------------------------------- + // GetVms: issue #152 — all-nodes listing sources from cluster/resources + // --------------------------------------------------------------------- + + private const string ClusterResourcesJson = @"{ + ""data"": [ + { + ""id"": ""qemu/100"", + ""type"": ""qemu"", + ""vmid"": 100, + ""name"": ""web1"", + ""status"": ""running"", + ""node"": ""pve1"", + ""maxcpu"": 2, + ""maxmem"": 4294967296, + ""maxdisk"": 34359738368, + ""uptime"": 3600, + ""template"": 0, + ""tags"": ""prod;web"" + }, + { + ""id"": ""qemu/101"", + ""type"": ""qemu"", + ""vmid"": 101, + ""name"": ""db1"", + ""status"": ""stopped"", + ""node"": ""pve2"", + ""maxcpu"": 4, + ""maxmem"": 8589934592, + ""maxdisk"": 68719476736, + ""uptime"": 0, + ""template"": 1 + }, + { + ""id"": ""lxc/200"", + ""type"": ""lxc"", + ""vmid"": 200, + ""name"": ""ct1"", + ""status"": ""running"", + ""node"": ""pve1"" + } + ] + }"; + + [Fact] + public void GetVms_NoNode_IssuesExactlyOneClusterResourcesCallAndNoPerNodeCalls() + { + var mockClient = new Mock(); + mockClient + .Setup(c => c.GetAsync("cluster/resources?type=vm")) + .ReturnsAsync(ClusterResourcesJson); + + var service = new VmService(mockClient.Object); + var vms = service.GetVms(CreateSession()); + + Assert.Equal(2, vms.Length); + mockClient.Verify(c => c.GetAsync("cluster/resources?type=vm"), Times.Once); + mockClient.Verify(c => c.GetAsync(It.Is(s => s.Contains("/qemu"))), Times.Never); + mockClient.Verify(c => c.GetAsync("nodes"), Times.Never); } [Fact] - public void GetVms_AllNodes_ClientTimeoutOnOneNodeIsSkippedAndReported() + public void GetVms_NoNode_ExcludesLxcRowsFromTheSharedResourcesResponse() { - // PveHttpClient.SendOnceAsync wraps an HttpClient timeout as - // PveApiException(RequestTimeout) — the case of a powered-off or - // firewall-blackholed node, which must be skipped like any other - // unreachable node rather than aborting the whole listing. - var mockClient = SetupTwoNodeCluster(); + // "type=vm" is PVE's guest filter, not QEMU-only — it returns lxc rows too. + var mockClient = new Mock(); mockClient - .Setup(c => c.GetAsync("nodes/pve1/qemu")) - .ReturnsAsync("{\"data\":[{\"vmid\":100}]}"); - mockClient - .Setup(c => c.GetAsync("nodes/pve2/qemu")) - .ThrowsAsync(new PveApiException(HttpStatusCode.RequestTimeout, "Request timed out after 100s.", - "nodes/pve2/qemu", "GET")); + .Setup(c => c.GetAsync("cluster/resources?type=vm")) + .ReturnsAsync(ClusterResourcesJson); var service = new VmService(mockClient.Object); + var vms = service.GetVms(CreateSession()); - var skipped = new List(); - var vms = service.GetVms(CreateSession(), onNodeSkipped: (node, ex) => skipped.Add(node)); + Assert.DoesNotContain(vms, v => v.VmId == 200); + Assert.All(vms, v => Assert.NotEqual(200, v.VmId)); + } + + [Fact] + public void GetVms_NoNode_MapsClusterResourceRowsOntoPveVm() + { + var mockClient = new Mock(); + mockClient + .Setup(c => c.GetAsync("cluster/resources?type=vm")) + .ReturnsAsync(ClusterResourcesJson); + + var service = new VmService(mockClient.Object); + var vms = service.GetVms(CreateSession()); + + var web1 = vms.Single(v => v.VmId == 100); + Assert.Equal("web1", web1.Name); + Assert.Equal("running", web1.Status); + Assert.Equal("pve1", web1.Node); + Assert.Equal(2, web1.CpuCount); + Assert.Equal(4294967296L, web1.MaxMem); + Assert.Equal(34359738368L, web1.MaxDisk); + Assert.Equal(3600L, web1.Uptime); + Assert.Equal(0, web1.Template); + Assert.Equal("prod;web", web1.Tags); + + var db1 = vms.Single(v => v.VmId == 101); + Assert.Equal("db1", db1.Name); + Assert.Equal("stopped", db1.Status); + Assert.Equal(4, db1.CpuCount); + Assert.Equal(1, db1.Template); + } + + [Fact] + public void GetVms_WithNode_StillHitsNodesQemuNotClusterResources() + { + var mockClient = new Mock(); + mockClient + .Setup(c => c.GetAsync($"nodes/{TestNode}/qemu")) + .ReturnsAsync("{\"data\":[{\"vmid\":100}]}"); + + var service = new VmService(mockClient.Object); + var vms = service.GetVms(CreateSession(), TestNode); var vm = Assert.Single(vms); Assert.Equal(100, vm.VmId); - Assert.Equal(new[] { "pve2" }, skipped); + mockClient.Verify(c => c.GetAsync($"nodes/{TestNode}/qemu"), Times.Once); + mockClient.Verify(c => c.GetAsync(It.Is(s => s.StartsWith("cluster/resources"))), Times.Never); } } }