From c12bfe9183eaa67ad5f18671dede14e5aa5dfd6a Mon Sep 17 00:00:00 2001 From: "goodolclint-claude[bot]" <323206664+goodolclint-claude[bot]@users.noreply.github.com> Date: Wed, 2 Sep 2026 16:34:52 +0000 Subject: [PATCH] fix: plumb skiplock parameter through Remove-PveVm and Remove-PveContainer When -Force is specified, both cmdlets now pass skiplock=1 to PVE, which bypasses locks. PVE honours the skiplock parameter for root@pam only. Also updated help text on both cmdlets to clarify the limitation and behaviour. Fixes #136. --- .../Services/ContainerService.cs | 12 +++-- src/PSProxmoxVE.Core/Services/VmService.cs | 12 +++-- .../Containers/RemovePveContainerCmdlet.cs | 6 +-- .../Cmdlets/Vms/RemovePveVmCmdlet.cs | 6 +-- .../Services/VmServiceTests.cs | 52 +++++++++++++++++++ 5 files changed, 76 insertions(+), 12 deletions(-) diff --git a/src/PSProxmoxVE.Core/Services/ContainerService.cs b/src/PSProxmoxVE.Core/Services/ContainerService.cs index 374b8ba..e57af92 100644 --- a/src/PSProxmoxVE.Core/Services/ContainerService.cs +++ b/src/PSProxmoxVE.Core/Services/ContainerService.cs @@ -329,16 +329,22 @@ namespace PSProxmoxVE.Core.Services PveSession session, string node, int vmid, - bool purge = false) + bool purge = false, + bool skipLock = false) { if (session == null) throw new ArgumentNullException(nameof(session)); if (string.IsNullOrWhiteSpace(node)) throw new ArgumentNullException(nameof(node)); - var purgeParam = purge ? "?purge=1" : "?purge=0"; + var queryParams = new List(); + queryParams.Add(purge ? "purge=1" : "purge=0"); + if (skipLock) + queryParams.Add("skiplock=1"); + var queryString = "?" + string.Join("&", queryParams); + IPveHttpClient client = _injectedClient ?? new PveHttpClient(session); try { - var response = client.DeleteAsync($"nodes/{Uri.EscapeDataString(node)}/lxc/{vmid}{purgeParam}") + var response = client.DeleteAsync($"nodes/{Uri.EscapeDataString(node)}/lxc/{vmid}{queryString}") .GetAwaiter().GetResult(); return ParseTask(response, node); } diff --git a/src/PSProxmoxVE.Core/Services/VmService.cs b/src/PSProxmoxVE.Core/Services/VmService.cs index bc409e7..d350088 100644 --- a/src/PSProxmoxVE.Core/Services/VmService.cs +++ b/src/PSProxmoxVE.Core/Services/VmService.cs @@ -394,16 +394,22 @@ namespace PSProxmoxVE.Core.Services /// The cluster node name. /// The VM ID. /// If true, also removes all associated backup files and jobs. - public PveTask RemoveVm(PveSession session, string node, int vmid, bool purge = false) + /// If true, bypasses locks (PVE honours this for root@pam only). + public PveTask RemoveVm(PveSession session, string node, int vmid, bool purge = false, bool skipLock = false) { if (session == null) throw new ArgumentNullException(nameof(session)); if (string.IsNullOrWhiteSpace(node)) throw new ArgumentNullException(nameof(node)); - var purgeParam = purge ? "?purge=1" : "?purge=0"; + var queryParams = new List(); + queryParams.Add(purge ? "purge=1" : "purge=0"); + if (skipLock) + queryParams.Add("skiplock=1"); + var queryString = "?" + string.Join("&", queryParams); + IPveHttpClient client = _injectedClient ?? new PveHttpClient(session); try { - var response = client.DeleteAsync($"nodes/{Uri.EscapeDataString(node)}/qemu/{vmid}{purgeParam}") + var response = client.DeleteAsync($"nodes/{Uri.EscapeDataString(node)}/qemu/{vmid}{queryString}") .GetAwaiter().GetResult(); return ParseTask(response, node); } diff --git a/src/PSProxmoxVE/Cmdlets/Containers/RemovePveContainerCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Containers/RemovePveContainerCmdlet.cs index 58fdfcd..bdadf5d 100644 --- a/src/PSProxmoxVE/Cmdlets/Containers/RemovePveContainerCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Containers/RemovePveContainerCmdlet.cs @@ -42,10 +42,10 @@ namespace PSProxmoxVE.Cmdlets.Containers /// /// - /// When specified, bypasses locks and forces removal even if a lock is set on the container. + /// When specified, sends skiplock=1 to PVE, which bypasses locks. PVE honours this for root@pam only. /// /// - [Parameter(Mandatory = false, HelpMessage = "Force the operation without additional checks.")] + [Parameter(Mandatory = false, HelpMessage = "Bypass locks (root@pam only); sends skiplock=1 to PVE.")] public SwitchParameter Force { get; set; } /// @@ -63,7 +63,7 @@ namespace PSProxmoxVE.Cmdlets.Containers var containerService = new ContainerService(); WriteVerbose($"Removing container {VmId} from node '{Node}'..."); - var task = containerService.RemoveContainer(session, Node, VmId, Purge.IsPresent); + var task = containerService.RemoveContainer(session, Node, VmId, Purge.IsPresent, Force.IsPresent); if (Wait.IsPresent) { diff --git a/src/PSProxmoxVE/Cmdlets/Vms/RemovePveVmCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Vms/RemovePveVmCmdlet.cs index 785433b..b047c25 100644 --- a/src/PSProxmoxVE/Cmdlets/Vms/RemovePveVmCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Vms/RemovePveVmCmdlet.cs @@ -42,10 +42,10 @@ namespace PSProxmoxVE.Cmdlets.Vms /// /// - /// When specified, bypasses locks and forces removal even if a lock is set on the VM. + /// When specified, sends skiplock=1 to PVE, which bypasses locks. PVE honours this for root@pam only. /// /// - [Parameter(Mandatory = false, HelpMessage = "Force the operation without additional checks.")] + [Parameter(Mandatory = false, HelpMessage = "Bypass locks (root@pam only); sends skiplock=1 to PVE.")] public SwitchParameter Force { get; set; } /// @@ -63,7 +63,7 @@ namespace PSProxmoxVE.Cmdlets.Vms var vmService = new VmService(); WriteVerbose($"Removing VM {VmId} from node '{Node}'..."); - var task = vmService.RemoveVm(session, Node, VmId, Purge.IsPresent); + var task = vmService.RemoveVm(session, Node, VmId, Purge.IsPresent, Force.IsPresent); if (Wait.IsPresent) { diff --git a/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs b/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs index 230ce2e..2c0147d 100644 --- a/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs +++ b/tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs @@ -165,5 +165,57 @@ namespace PSProxmoxVE.Core.Tests.Services Assert.Empty(captured!); } + [Fact] + public void RemoveVm_WithSkipLockTrue_IncludesSkiplockInQueryString() + { + string? resource = null; + var mockClient = new Mock(); + mockClient + .Setup(c => c.DeleteAsync(It.IsAny())) + .Callback(r => resource = r) + .ReturnsAsync("{\"data\":\"UPID:pve1:00001234:00005678:6A970AAB:qmremove:100:root@pam:\"}"); + + var service = new VmService(mockClient.Object); + service.RemoveVm(CreateSession(), TestNode, TestVmId, purge: false, skipLock: true); + + Assert.NotNull(resource); + Assert.Contains("skiplock=1", resource!); + } + + [Fact] + public void RemoveVm_WithSkipLockFalse_OmitsSkiplockFromQueryString() + { + string? resource = null; + var mockClient = new Mock(); + mockClient + .Setup(c => c.DeleteAsync(It.IsAny())) + .Callback(r => resource = r) + .ReturnsAsync("{\"data\":\"UPID:pve1:00001234:00005678:6A970AAB:qmremove:100:root@pam:\"}"); + + var service = new VmService(mockClient.Object); + service.RemoveVm(CreateSession(), TestNode, TestVmId, purge: false, skipLock: false); + + Assert.NotNull(resource); + Assert.DoesNotContain("skiplock", resource!); + } + + [Fact] + public void RemoveVm_WithPurgeAndSkipLock_IncludesBothInQueryString() + { + string? resource = null; + var mockClient = new Mock(); + mockClient + .Setup(c => c.DeleteAsync(It.IsAny())) + .Callback(r => resource = r) + .ReturnsAsync("{\"data\":\"UPID:pve1:00001234:00005678:6A970AAB:qmremove:100:root@pam:\"}"); + + var service = new VmService(mockClient.Object); + service.RemoveVm(CreateSession(), TestNode, TestVmId, purge: true, skipLock: true); + + Assert.NotNull(resource); + Assert.Contains("purge=1", resource!); + Assert.Contains("skiplock=1", resource!); + } + } }