diff --git a/src/PSProxmoxVE/Cmdlets/CloudInit/InvokePveCloudInitRegenerateCmdlet.cs b/src/PSProxmoxVE/Cmdlets/CloudInit/InvokePveCloudInitRegenerateCmdlet.cs index c013e1a..6a6da6b 100644 --- a/src/PSProxmoxVE/Cmdlets/CloudInit/InvokePveCloudInitRegenerateCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/CloudInit/InvokePveCloudInitRegenerateCmdlet.cs @@ -46,7 +46,7 @@ namespace PSProxmoxVE.Cmdlets.CloudInit if (Wait.IsPresent && !string.IsNullOrEmpty(upid)) { - task = new TaskService().WaitForTask(session, Node, upid, null, null, null); + task = new TaskService().WaitForTask(session, Node, upid); } WriteObject(task); diff --git a/src/PSProxmoxVE/Cmdlets/Containers/MovePveContainerCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Containers/MovePveContainerCmdlet.cs index 3505d98..f7ce0f0 100644 --- a/src/PSProxmoxVE/Cmdlets/Containers/MovePveContainerCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Containers/MovePveContainerCmdlet.cs @@ -54,7 +54,7 @@ namespace PSProxmoxVE.Cmdlets.Containers if (Wait.IsPresent) { var taskService = new TaskService(); - task = taskService.WaitForTask(session, Node, task.Upid, null, null, null); + task = taskService.WaitForTask(session, Node, task.Upid); } WriteObject(task); diff --git a/src/PSProxmoxVE/Cmdlets/Containers/MovePveContainerVolumeCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Containers/MovePveContainerVolumeCmdlet.cs index 90924c7..3ed2ec3 100644 --- a/src/PSProxmoxVE/Cmdlets/Containers/MovePveContainerVolumeCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Containers/MovePveContainerVolumeCmdlet.cs @@ -67,7 +67,7 @@ namespace PSProxmoxVE.Cmdlets.Containers if (Wait.IsPresent) { var taskService = new TaskService(); - task = taskService.WaitForTask(session, Node, task.Upid, null, null, null); + task = taskService.WaitForTask(session, Node, task.Upid); } WriteObject(task); diff --git a/src/PSProxmoxVE/Cmdlets/Containers/NewPveContainerCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Containers/NewPveContainerCmdlet.cs index 42b4d9d..56c67d4 100644 --- a/src/PSProxmoxVE/Cmdlets/Containers/NewPveContainerCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Containers/NewPveContainerCmdlet.cs @@ -209,7 +209,7 @@ namespace PSProxmoxVE.Cmdlets.Containers if (Wait.IsPresent) { var taskService = new TaskService(); - task = taskService.WaitForTask(session, task.Node ?? Node, task.Upid!, null, null, null); + task = taskService.WaitForTask(session, task.Node ?? Node, task.Upid!); } WriteObject(task); diff --git a/src/PSProxmoxVE/Cmdlets/Containers/RemovePveContainerCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Containers/RemovePveContainerCmdlet.cs index 7c6e5a3..e45a4de 100644 --- a/src/PSProxmoxVE/Cmdlets/Containers/RemovePveContainerCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Containers/RemovePveContainerCmdlet.cs @@ -68,7 +68,7 @@ namespace PSProxmoxVE.Cmdlets.Containers if (Wait.IsPresent) { var taskService = new TaskService(); - task = taskService.WaitForTask(session, task.Node ?? Node, task.Upid!, null, null, null); + task = taskService.WaitForTask(session, task.Node ?? Node, task.Upid!); } WriteObject(task); diff --git a/src/PSProxmoxVE/Cmdlets/Containers/ResumePveContainerCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Containers/ResumePveContainerCmdlet.cs index 67f3a1b..ccc814c 100644 --- a/src/PSProxmoxVE/Cmdlets/Containers/ResumePveContainerCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Containers/ResumePveContainerCmdlet.cs @@ -53,7 +53,7 @@ namespace PSProxmoxVE.Cmdlets.Containers if (Wait.IsPresent) { var taskService = new TaskService(); - task = taskService.WaitForTask(session, Node, task.Upid, null, null, null); + task = taskService.WaitForTask(session, Node, task.Upid); } WriteObject(task); diff --git a/src/PSProxmoxVE/Cmdlets/Containers/SuspendPveContainerCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Containers/SuspendPveContainerCmdlet.cs index 92927c8..97e74da 100644 --- a/src/PSProxmoxVE/Cmdlets/Containers/SuspendPveContainerCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Containers/SuspendPveContainerCmdlet.cs @@ -53,7 +53,7 @@ namespace PSProxmoxVE.Cmdlets.Containers if (Wait.IsPresent) { var taskService = new TaskService(); - task = taskService.WaitForTask(session, Node, task.Upid, null, null, null); + task = taskService.WaitForTask(session, Node, task.Upid); } WriteObject(task); diff --git a/src/PSProxmoxVE/Cmdlets/PveCmdletBase.cs b/src/PSProxmoxVE/Cmdlets/PveCmdletBase.cs index 0a32a99..3a3cc31 100644 --- a/src/PSProxmoxVE/Cmdlets/PveCmdletBase.cs +++ b/src/PSProxmoxVE/Cmdlets/PveCmdletBase.cs @@ -186,7 +186,7 @@ namespace PSProxmoxVE.Cmdlets var task = issueOperation(); return string.IsNullOrEmpty(task.Upid) ? task - : taskService.WaitForTask(session, node, task.Upid, null, null, null); + : taskService.WaitForTask(session, node, task.Upid); }, onRetry: ex => WriteVerbose($"Guest is locked, retrying: {ex.Message}")); } diff --git a/src/PSProxmoxVE/Cmdlets/Tasks/WaitPveTaskCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Tasks/WaitPveTaskCmdlet.cs index 6de8c12..1fc70e1 100644 --- a/src/PSProxmoxVE/Cmdlets/Tasks/WaitPveTaskCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Tasks/WaitPveTaskCmdlet.cs @@ -1,9 +1,9 @@ using System; using System.Management.Automation; -using Newtonsoft.Json.Linq; using PSProxmoxVE.Core.Client; using PSProxmoxVE.Core.Exceptions; using PSProxmoxVE.Core.Models.Vms; +using PSProxmoxVE.Core.Services; namespace PSProxmoxVE.Cmdlets.Tasks { @@ -20,6 +20,13 @@ namespace PSProxmoxVE.Cmdlets.Tasks [OutputType(typeof(PveTask))] public sealed class WaitPveTaskCmdlet : PveCmdletBase { + /// + /// TaskService.WaitForTask treats a null timeout as "use its own 10-minute default", + /// not "wait forever" — this cmdlet's own contract is the latter, so an omitted + /// -Timeout is passed through as this instead of null. + /// + private static readonly TimeSpan NoTimeout = TimeSpan.FromDays(36500); + /// The node on which the task is running. [Parameter(Mandatory = true, Position = 0, HelpMessage = "The PVE node name.")] public string Node { get; set; } = string.Empty; @@ -48,72 +55,40 @@ namespace PSProxmoxVE.Cmdlets.Tasks { var session = GetSession(); using var client = new PveHttpClient(session); + var taskService = new TaskService(client); - var poll = PollInterval ?? TimeSpan.FromSeconds(2); - var deadline = Timeout.HasValue ? DateTime.UtcNow + Timeout.Value : DateTime.MaxValue; - - var encodedUpid = Uri.EscapeDataString(Upid); - var statusResource = $"nodes/{Uri.EscapeDataString(Node)}/tasks/{encodedUpid}/status"; - - // Derive a short human-readable description from the UPID for progress display + var activityId = Math.Abs(Upid.GetHashCode()) % 1000 + 1; var taskDesc = Upid.Length > 50 ? Upid.Substring(0, 47) + "..." : Upid; - - var activityId = Math.Abs(Upid.GetHashCode()) % 1000 + 1; var progressRecord = new ProgressRecord(activityId, $"Waiting for task on {Node}", taskDesc) { - PercentComplete = -1 // indeterminate + PercentComplete = -1 }; + var startedAt = DateTime.UtcNow; + + void ReportProgress(PveTask task) + { + if (Timeout.HasValue) + { + var totalSecs = (int)Timeout.Value.TotalSeconds; + var elapsed = (int)(DateTime.UtcNow - startedAt).TotalSeconds; + progressRecord.PercentComplete = totalSecs > 0 + ? Math.Min(99, (elapsed * 100) / totalSecs) + : -1; + progressRecord.SecondsRemaining = Math.Max(0, totalSecs - elapsed); + } + WriteProgress(progressRecord); + } + try { - while (true) - { - if (DateTime.UtcNow >= deadline) - throw new PveTaskTimeoutException(Upid, Timeout!.Value); + var task = taskService.WaitForTask(session, Node, Upid, Timeout ?? NoTimeout, PollInterval, + ReportProgress); - WriteProgress(progressRecord); + progressRecord.RecordType = ProgressRecordType.Completed; + WriteProgress(progressRecord); - var elapsed = Timeout.HasValue - ? (int)((DateTime.UtcNow - (deadline - Timeout.Value)).TotalSeconds) - : -1; - if (Timeout.HasValue && elapsed >= 0) - { - var totalSecs = (int)Timeout.Value.TotalSeconds; - progressRecord.PercentComplete = totalSecs > 0 - ? Math.Min(99, (elapsed * 100) / totalSecs) - : -1; - progressRecord.SecondsRemaining = Math.Max(0, totalSecs - elapsed); - } - - System.Threading.Thread.Sleep((int)poll.TotalMilliseconds); - - var statusJson = client.GetAsync(statusResource).GetAwaiter().GetResult(); - var statusRoot = JObject.Parse(statusJson); - var data = statusRoot["data"]; - - var status = data?["status"]?.ToString(); - var exitStatus = data?["exitstatus"]?.ToString(); - - if (status == "stopped") - { - progressRecord.RecordType = ProgressRecordType.Completed; - WriteProgress(progressRecord); - - var task = new PveTask - { - Upid = Upid, - Node = Node, - Status = status, - ExitStatus = exitStatus - }; - - if (!task.IsSuccessful && !string.IsNullOrEmpty(exitStatus) && exitStatus != "OK") - throw new PveTaskFailedException(Upid, exitStatus!); - - WriteObject(task); - return; - } - } + WriteObject(task); } catch (PveTaskTimeoutException ex) { @@ -124,6 +99,8 @@ namespace PSProxmoxVE.Cmdlets.Tasks } catch (PveTaskFailedException ex) { + progressRecord.RecordType = ProgressRecordType.Completed; + WriteProgress(progressRecord); ThrowTerminatingError(new ErrorRecord( ex, "PveTaskFailed", ErrorCategory.OperationStopped, Upid)); } diff --git a/src/PSProxmoxVE/Cmdlets/Vms/ImportPveVmDiskCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Vms/ImportPveVmDiskCmdlet.cs index 4ad96bb..82b5854 100644 --- a/src/PSProxmoxVE/Cmdlets/Vms/ImportPveVmDiskCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Vms/ImportPveVmDiskCmdlet.cs @@ -85,7 +85,7 @@ namespace PSProxmoxVE.Cmdlets.Vms if (Wait.IsPresent && !string.IsNullOrEmpty(task.Upid)) { var taskService = new TaskService(); - task = taskService.WaitForTask(session, Node, task.Upid, null, null, null); + task = taskService.WaitForTask(session, Node, task.Upid); } WriteObject(task); diff --git a/src/PSProxmoxVE/Cmdlets/Vms/MovePveVmCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Vms/MovePveVmCmdlet.cs index b8e023d..383efe5 100644 --- a/src/PSProxmoxVE/Cmdlets/Vms/MovePveVmCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Vms/MovePveVmCmdlet.cs @@ -66,7 +66,7 @@ namespace PSProxmoxVE.Cmdlets.Vms if (Wait.IsPresent) { var taskService = new TaskService(); - task = taskService.WaitForTask(session, Node, task.Upid, null, null, null); + task = taskService.WaitForTask(session, Node, task.Upid); } WriteObject(task); diff --git a/src/PSProxmoxVE/Cmdlets/Vms/MovePveVmDiskCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Vms/MovePveVmDiskCmdlet.cs index 427af7a..70c63e4 100644 --- a/src/PSProxmoxVE/Cmdlets/Vms/MovePveVmDiskCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Vms/MovePveVmDiskCmdlet.cs @@ -74,7 +74,7 @@ namespace PSProxmoxVE.Cmdlets.Vms if (Wait.IsPresent) { var taskService = new TaskService(); - task = taskService.WaitForTask(session, Node, task.Upid, null, null, null); + task = taskService.WaitForTask(session, Node, task.Upid); } WriteObject(task); diff --git a/src/PSProxmoxVE/Cmdlets/Vms/RemovePveVmCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Vms/RemovePveVmCmdlet.cs index e5901ed..63a9bc7 100644 --- a/src/PSProxmoxVE/Cmdlets/Vms/RemovePveVmCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Vms/RemovePveVmCmdlet.cs @@ -68,7 +68,7 @@ namespace PSProxmoxVE.Cmdlets.Vms if (Wait.IsPresent) { var taskService = new TaskService(); - task = taskService.WaitForTask(session, Node, task.Upid, null, null, null); + task = taskService.WaitForTask(session, Node, task.Upid); } WriteObject(task); diff --git a/tests/PSProxmoxVE.Core.Tests/Services/TaskServiceTests.cs b/tests/PSProxmoxVE.Core.Tests/Services/TaskServiceTests.cs index c02017f..dd8f7d3 100644 --- a/tests/PSProxmoxVE.Core.Tests/Services/TaskServiceTests.cs +++ b/tests/PSProxmoxVE.Core.Tests/Services/TaskServiceTests.cs @@ -239,6 +239,103 @@ namespace PSProxmoxVE.Core.Tests.Services Assert.Equal(timeout, ex.Timeout); } + [Fact] + public void WaitForTask_AlreadyStopped_ChecksStatusBeforeSleeping() + { + // Arrange + var json = @"{ + ""data"": { + ""upid"": ""UPID:pve1:000ABC:00000001:5F1234AB:qmstart:100:root@pam:"", + ""status"": ""stopped"", + ""exitstatus"": ""OK"", + ""user"": ""root@pam"" + } + }"; + + var mockClient = new Mock(); + mockClient.Setup(c => c.GetAsync(It.IsAny())) + .ReturnsAsync(json); + + var service = new TaskService(mockClient.Object); + var session = CreateSession(); + + // Act — a long poll interval would dominate the elapsed time if the + // implementation slept before its first status check. + var stopwatch = System.Diagnostics.Stopwatch.StartNew(); + var task = service.WaitForTask(session, TestNode, TestUpid, + timeout: TimeSpan.FromSeconds(30), + pollInterval: TimeSpan.FromSeconds(10)); + stopwatch.Stop(); + + // Assert + Assert.True(task.IsSuccessful); + Assert.True(stopwatch.Elapsed < TimeSpan.FromSeconds(5), + $"Expected an immediate return for an already-stopped task, took {stopwatch.Elapsed}"); + mockClient.Verify(c => c.GetAsync(It.IsAny()), Times.Once); + } + + [Fact] + public void WaitForTask_PollIntervalBelowMinimum_IsClampedToOneSecond() + { + // Arrange — task reports "running" once, then "stopped". + var runningJson = @"{ ""data"": { ""status"": ""running"", ""user"": ""root@pam"" } }"; + var stoppedJson = @"{ + ""data"": { ""status"": ""stopped"", ""exitstatus"": ""OK"", ""user"": ""root@pam"" } + }"; + + var mockClient = new Mock(); + mockClient.SetupSequence(c => c.GetAsync(It.IsAny())) + .ReturnsAsync(runningJson) + .ReturnsAsync(stoppedJson); + + var service = new TaskService(mockClient.Object); + var session = CreateSession(); + + // Act — a zero poll interval must be clamped to the 1-second minimum, + // not passed through to Thread.Sleep(0). + var stopwatch = System.Diagnostics.Stopwatch.StartNew(); + var task = service.WaitForTask(session, TestNode, TestUpid, + timeout: TimeSpan.FromSeconds(30), + pollInterval: TimeSpan.Zero); + stopwatch.Stop(); + + // Assert + Assert.True(task.IsSuccessful); + Assert.True(stopwatch.Elapsed >= TimeSpan.FromMilliseconds(900), + $"Expected the clamp to force at least a ~1-second wait, took {stopwatch.Elapsed}"); + mockClient.Verify(c => c.GetAsync(It.IsAny()), Times.Exactly(2)); + } + + [Fact] + public void WaitForTask_ProgressCallback_InvokedOnEachPoll() + { + // Arrange — two polls report "running", the third reports "stopped". + var runningJson = @"{ ""data"": { ""status"": ""running"", ""user"": ""root@pam"" } }"; + var stoppedJson = @"{ + ""data"": { ""status"": ""stopped"", ""exitstatus"": ""OK"", ""user"": ""root@pam"" } + }"; + + var mockClient = new Mock(); + mockClient.SetupSequence(c => c.GetAsync(It.IsAny())) + .ReturnsAsync(runningJson) + .ReturnsAsync(runningJson) + .ReturnsAsync(stoppedJson); + + var service = new TaskService(mockClient.Object); + var session = CreateSession(); + var seenStatuses = new List(); + + // Act + var task = service.WaitForTask(session, TestNode, TestUpid, + timeout: TimeSpan.FromSeconds(30), + pollInterval: TimeSpan.FromSeconds(1), + progressCallback: t => seenStatuses.Add(t.Status)); + + // Assert + Assert.True(task.IsSuccessful); + Assert.Equal(new List { "running", "running", "stopped" }, seenStatuses); + } + [Fact] public void StopTask_CallsDeleteAsyncWithCorrectPath() {