gsadmin/PSProxmoxVE
1
0
Fork 0
mirror of https://github.com/GoodOlClint/PSProxmoxVE.git synced 2026-09-03 18:55:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: allocate a real VMID for Copy-PveVm/Copy-PveContainer and honor -Storage (#179)

Browse Source 
* fix: allocate a real VMID for Copy-PveVm/Copy-PveContainer and honor -Storage

Copy-PveVm and Copy-PveContainer defaulted newid to 0 when -NewVmId
was omitted, and never sent -Storage on the clone request even
though both cmdlets declare it. Both cmdlets now allocate via
ClusterConfigService.GetNextId when -NewVmId is null, and forward
-Storage into the clone form body. Since PVE rejects storage on a
linked clone, -Storage without -Full now fails fast client-side
instead of failing later against the API.

NewPveVmCmdlet and ImportPveOvaCmdlet hand-rolled the same
GET cluster/nextid call with a manual JObject parse; both now go
through ClusterConfigService.GetNextId so a response without a
data field raises the service's diagnosable InvalidOperationException
rather than a NullReferenceException.

Closes #135

* test: pin CloneVm/CloneContainer storage and newid form-body behavior

Offline xUnit coverage per ADR 0021: storage present in the clone
form body when supplied, absent when omitted, and newid forwarded
verbatim (never coerced to 0) by the service layer.

* fix: add missing storage parameter to VmService.CloneVm

VmService.cs was omitted from the earlier push; this restores the
storage parameter and form-body wiring that belongs with this fix.

* fix: drop the client-side -Storage/-Full guard from the Copy cmdlets

PVE returns the same error itself when storage is sent on a linked
clone, so the guard only saved one round trip, had no test, and rested
on a behaviour claim the OpenAPI spec does not document.

* test: cover the -Storage/-Full guard in Copy-PveVm and Copy-PveContainer

Offline Pester coverage for the client-side StorageRequiresFullClone
guard: -Storage without -Full throws before a session is required,
and -Storage with -Full does not trip the check.

* test: revert the Pester cases for the removed -Storage/-Full guard

The guard was dropped in bfe2483, so these cases assert an error the
cmdlets no longer raise.

---------

Co-authored-by: goodolclint-claude[bot] <323206664+goodolclint-claude[bot]@users.noreply.github.com>
This commit is contained in:
goodolclint-claude[bot]
2026-09-02 19:52:54 +00:00
committed by GitHub
parent 17832f15d9
commit 8a82146acc
8 changed files with 122 additions and 20 deletions
Download Patch File Download Diff File Expand all files Collapse all files
tests/PSProxmoxVE.Core.Tests/Services/ContainerServiceTests.cs
+52
View File
@@ -1,3 +1,4 @@
using System.Collections.Generic;
using Moq;
using PSProxmoxVE.Core.Authentication;
using PSProxmoxVE.Core.Client;
@@ -61,5 +62,56 @@ namespace PSProxmoxVE.Core.Tests.Services
Assert.Equal($"nodes/{TestNode}/lxc/{TestVmId}?purge=1&force=1", resource);
}
[Fact]
public void CloneContainer_WithStorage_IncludesStorageInFormBody()
{
Dictionary<string, string>? captured = null;
var mockClient = new Mock<IPveHttpClient>();
mockClient
.Setup(c => c.PostAsync(It.IsAny<string>(), It.IsAny<Dictionary<string, string>>()))
.Callback<string, Dictionary<string, string>>((_, data) => captured = data)
.ReturnsAsync("{\"data\":\"UPID:pve1:00001234:00005678:6A970AAB:vzclone:100:root@pam:\"}");
var service = new ContainerService(mockClient.Object);
service.CloneContainer(CreateSession(), TestNode, TestVmId, 200, storage: "local-zfs");
Assert.NotNull(captured);
Assert.Equal("local-zfs", captured!["storage"]);
}
[Fact]
public void CloneContainer_WithoutStorage_OmitsStorageFromFormBody()
{
Dictionary<string, string>? captured = null;
var mockClient = new Mock<IPveHttpClient>();
mockClient
.Setup(c => c.PostAsync(It.IsAny<string>(), It.IsAny<Dictionary<string, string>>()))
.Callback<string, Dictionary<string, string>>((_, data) => captured = data)
.ReturnsAsync("{\"data\":\"UPID:pve1:00001234:00005678:6A970AAB:vzclone:100:root@pam:\"}");
var service = new ContainerService(mockClient.Object);
service.CloneContainer(CreateSession(), TestNode, TestVmId, 200);
Assert.NotNull(captured);
Assert.False(captured!.ContainsKey("storage"));
}
[Fact]
public void CloneContainer_SendsAllocatedNewidNeverZero()
{
Dictionary<string, string>? captured = null;
var mockClient = new Mock<IPveHttpClient>();
mockClient
.Setup(c => c.PostAsync(It.IsAny<string>(), It.IsAny<Dictionary<string, string>>()))
.Callback<string, Dictionary<string, string>>((_, data) => captured = data)
.ReturnsAsync("{\"data\":\"UPID:pve1:00001234:00005678:6A970AAB:vzclone:100:root@pam:\"}");
var service = new ContainerService(mockClient.Object);
service.CloneContainer(CreateSession(), TestNode, TestVmId, 305);
Assert.NotNull(captured);
Assert.Equal("305", captured!["newid"]);
}
}
}
tests/PSProxmoxVE.Core.Tests/Services/VmServiceTests.cs
+51
View File
@@ -217,5 +217,56 @@ namespace PSProxmoxVE.Core.Tests.Services
Assert.Contains("skiplock=1", resource!);
}
[Fact]
public void CloneVm_WithStorage_IncludesStorageInFormBody()
{
Dictionary<string, string>? captured = null;
var mockClient = new Mock<IPveHttpClient>();
mockClient
.Setup(c => c.PostAsync(It.IsAny<string>(), It.IsAny<Dictionary<string, string>>()))
.Callback<string, Dictionary<string, string>>((_, data) => captured = data)
.ReturnsAsync("{\"data\":\"UPID:pve1:00001234:00005678:6A970AAB:qmclone:100:root@pam:\"}");
var service = new VmService(mockClient.Object);
service.CloneVm(CreateSession(), TestNode, TestVmId, 200, storage: "local-zfs");
Assert.NotNull(captured);
Assert.Equal("local-zfs", captured!["storage"]);
}
[Fact]
public void CloneVm_WithoutStorage_OmitsStorageFromFormBody()
{
Dictionary<string, string>? captured = null;
var mockClient = new Mock<IPveHttpClient>();
mockClient
.Setup(c => c.PostAsync(It.IsAny<string>(), It.IsAny<Dictionary<string, string>>()))
.Callback<string, Dictionary<string, string>>((_, data) => captured = data)
.ReturnsAsync("{\"data\":\"UPID:pve1:00001234:00005678:6A970AAB:qmclone:100:root@pam:\"}");
var service = new VmService(mockClient.Object);
service.CloneVm(CreateSession(), TestNode, TestVmId, 200);
Assert.NotNull(captured);
Assert.False(captured!.ContainsKey("storage"));
}
[Fact]
public void CloneVm_SendsAllocatedNewidNeverZero()
{
Dictionary<string, string>? captured = null;
var mockClient = new Mock<IPveHttpClient>();
mockClient
.Setup(c => c.PostAsync(It.IsAny<string>(), It.IsAny<Dictionary<string, string>>()))
.Callback<string, Dictionary<string, string>>((_, data) => captured = data)
.ReturnsAsync("{\"data\":\"UPID:pve1:00001234:00005678:6A970AAB:qmclone:100:root@pam:\"}");
var service = new VmService(mockClient.Object);
service.CloneVm(CreateSession(), TestNode, TestVmId, 305);
Assert.NotNull(captured);
Assert.Equal("305", captured!["newid"]);
}
}
}