mirror of
https://github.com/GoodOlClint/PSProxmoxVE.git
synced 2026-08-18 00:57:55 +00:00
fix: replace infinite WaitForTask loops with TaskService, add timeout to guest exec
Four cmdlets (Invoke-PveNetworkApply, New/Restore/Remove-PveSnapshot) had private WaitForTask methods with bare while(true) loops and no timeout or failure detection. Replaced with TaskService.WaitForTask which has a 10-minute default timeout, exit status validation, and throws PveTaskTimeoutException or PveTaskFailedException on failure. Invoke-PveVmGuestExec had a similar unbounded do/while polling loop. Added a -Timeout parameter (default 300s, range 1-3600) with Stopwatch enforcement. Also fixed bare catch in DecodeBase64 to catch FormatException specifically. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -1,8 +1,8 @@
|
||||
using System;
|
||||
using System.Management.Automation;
|
||||
using Newtonsoft.Json.Linq;
|
||||
using PSProxmoxVE.Core.Client;
|
||||
using PSProxmoxVE.Core.Models.Vms;
|
||||
using PSProxmoxVE.Core.Services;
|
||||
|
||||
namespace PSProxmoxVE.Cmdlets.Network
|
||||
{
|
||||
@@ -16,7 +16,7 @@ namespace PSProxmoxVE.Cmdlets.Network
|
||||
/// </summary>
|
||||
[Cmdlet(VerbsLifecycle.Invoke, "PveNetworkApply", SupportsShouldProcess = true)]
|
||||
[OutputType(typeof(PveTask))]
|
||||
public class InvokePveNetworkApplyCmdlet : PveCmdletBase
|
||||
public sealed class InvokePveNetworkApplyCmdlet : PveCmdletBase
|
||||
{
|
||||
/// <summary>The Proxmox VE node on which to apply network changes.</summary>
|
||||
[Parameter(Mandatory = true, Position = 0, HelpMessage = "The PVE node name.")]
|
||||
@@ -43,25 +43,11 @@ namespace PSProxmoxVE.Cmdlets.Network
|
||||
|
||||
if (Wait.IsPresent && !string.IsNullOrEmpty(upid))
|
||||
{
|
||||
task = WaitForTask(client, Node, upid);
|
||||
var taskService = new TaskService();
|
||||
task = taskService.WaitForTask(session, Node, upid);
|
||||
}
|
||||
|
||||
WriteObject(task);
|
||||
}
|
||||
|
||||
private static PveTask WaitForTask(PveHttpClient client, string node, string upid)
|
||||
{
|
||||
var encodedUpid = Uri.EscapeDataString(upid);
|
||||
var statusResource = $"nodes/{node}/tasks/{encodedUpid}/status";
|
||||
while (true)
|
||||
{
|
||||
System.Threading.Thread.Sleep(2000);
|
||||
var statusJson = client.GetAsync(statusResource).GetAwaiter().GetResult();
|
||||
var statusRoot = JObject.Parse(statusJson);
|
||||
var d = statusRoot["data"];
|
||||
if (d?["status"]?.ToString() == "stopped")
|
||||
return new PveTask { Upid = upid, Node = node, Status = "stopped", ExitStatus = d["exitstatus"]?.ToString() };
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,9 +1,9 @@
|
||||
using System;
|
||||
using System.Collections.Generic;
|
||||
using System.Management.Automation;
|
||||
using Newtonsoft.Json.Linq;
|
||||
using PSProxmoxVE.Core.Client;
|
||||
using PSProxmoxVE.Core.Models.Vms;
|
||||
using PSProxmoxVE.Core.Services;
|
||||
|
||||
namespace PSProxmoxVE.Cmdlets.Snapshots
|
||||
{
|
||||
@@ -16,7 +16,7 @@ namespace PSProxmoxVE.Cmdlets.Snapshots
|
||||
/// </summary>
|
||||
[Cmdlet(VerbsCommon.New, "PveSnapshot", SupportsShouldProcess = true)]
|
||||
[OutputType(typeof(PveTask))]
|
||||
public class NewPveSnapshotCmdlet : PveCmdletBase
|
||||
public sealed class NewPveSnapshotCmdlet : PveCmdletBase
|
||||
{
|
||||
/// <summary>The Proxmox VE node name.</summary>
|
||||
[Parameter(Mandatory = true, Position = 0, HelpMessage = "The PVE node name.")]
|
||||
@@ -67,25 +67,11 @@ namespace PSProxmoxVE.Cmdlets.Snapshots
|
||||
|
||||
if (Wait.IsPresent && !string.IsNullOrEmpty(upid))
|
||||
{
|
||||
task = WaitForTask(client, Node, upid);
|
||||
var taskService = new TaskService();
|
||||
task = taskService.WaitForTask(session, Node, upid);
|
||||
}
|
||||
|
||||
WriteObject(task);
|
||||
}
|
||||
|
||||
private static PveTask WaitForTask(PveHttpClient client, string node, string upid)
|
||||
{
|
||||
var encodedUpid = Uri.EscapeDataString(upid);
|
||||
var statusResource = $"nodes/{node}/tasks/{encodedUpid}/status";
|
||||
while (true)
|
||||
{
|
||||
System.Threading.Thread.Sleep(2000);
|
||||
var statusJson = client.GetAsync(statusResource).GetAwaiter().GetResult();
|
||||
var statusRoot = JObject.Parse(statusJson);
|
||||
var d = statusRoot["data"];
|
||||
if (d?["status"]?.ToString() == "stopped")
|
||||
return new PveTask { Upid = upid, Node = node, Status = "stopped", ExitStatus = d["exitstatus"]?.ToString() };
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
using System;
|
||||
using System.Management.Automation;
|
||||
using Newtonsoft.Json.Linq;
|
||||
using PSProxmoxVE.Core.Client;
|
||||
using PSProxmoxVE.Core.Models.Vms;
|
||||
using PSProxmoxVE.Core.Services;
|
||||
|
||||
namespace PSProxmoxVE.Cmdlets.Snapshots
|
||||
{
|
||||
@@ -15,7 +15,7 @@ namespace PSProxmoxVE.Cmdlets.Snapshots
|
||||
/// </summary>
|
||||
[Cmdlet(VerbsCommon.Remove, "PveSnapshot", SupportsShouldProcess = true, ConfirmImpact = ConfirmImpact.High)]
|
||||
[OutputType(typeof(PveTask))]
|
||||
public class RemovePveSnapshotCmdlet : PveCmdletBase
|
||||
public sealed class RemovePveSnapshotCmdlet : PveCmdletBase
|
||||
{
|
||||
/// <summary>The Proxmox VE node name.</summary>
|
||||
[Parameter(Mandatory = true, Position = 0, HelpMessage = "The PVE node name.")]
|
||||
@@ -54,25 +54,11 @@ namespace PSProxmoxVE.Cmdlets.Snapshots
|
||||
|
||||
if (Wait.IsPresent && !string.IsNullOrEmpty(upid))
|
||||
{
|
||||
task = WaitForTask(client, Node, upid);
|
||||
var taskService = new TaskService();
|
||||
task = taskService.WaitForTask(session, Node, upid);
|
||||
}
|
||||
|
||||
WriteObject(task);
|
||||
}
|
||||
|
||||
private static PveTask WaitForTask(PveHttpClient client, string node, string upid)
|
||||
{
|
||||
var encodedUpid = Uri.EscapeDataString(upid);
|
||||
var statusResource = $"nodes/{node}/tasks/{encodedUpid}/status";
|
||||
while (true)
|
||||
{
|
||||
System.Threading.Thread.Sleep(2000);
|
||||
var statusJson = client.GetAsync(statusResource).GetAwaiter().GetResult();
|
||||
var statusRoot = JObject.Parse(statusJson);
|
||||
var d = statusRoot["data"];
|
||||
if (d?["status"]?.ToString() == "stopped")
|
||||
return new PveTask { Upid = upid, Node = node, Status = "stopped", ExitStatus = d["exitstatus"]?.ToString() };
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
using System;
|
||||
using System.Management.Automation;
|
||||
using Newtonsoft.Json.Linq;
|
||||
using PSProxmoxVE.Core.Client;
|
||||
using PSProxmoxVE.Core.Models.Vms;
|
||||
using PSProxmoxVE.Core.Services;
|
||||
|
||||
namespace PSProxmoxVE.Cmdlets.Snapshots
|
||||
{
|
||||
@@ -18,7 +18,7 @@ namespace PSProxmoxVE.Cmdlets.Snapshots
|
||||
SupportsShouldProcess = true,
|
||||
ConfirmImpact = ConfirmImpact.High)]
|
||||
[OutputType(typeof(PveTask))]
|
||||
public class RestorePveSnapshotCmdlet : PveCmdletBase
|
||||
public sealed class RestorePveSnapshotCmdlet : PveCmdletBase
|
||||
{
|
||||
/// <summary>The Proxmox VE node name.</summary>
|
||||
[Parameter(Mandatory = true, Position = 0, HelpMessage = "The PVE node name.")]
|
||||
@@ -57,25 +57,11 @@ namespace PSProxmoxVE.Cmdlets.Snapshots
|
||||
|
||||
if (Wait.IsPresent && !string.IsNullOrEmpty(upid))
|
||||
{
|
||||
task = WaitForTask(client, Node, upid);
|
||||
var taskService = new TaskService();
|
||||
task = taskService.WaitForTask(session, Node, upid);
|
||||
}
|
||||
|
||||
WriteObject(task);
|
||||
}
|
||||
|
||||
private static PveTask WaitForTask(PveHttpClient client, string node, string upid)
|
||||
{
|
||||
var encodedUpid = Uri.EscapeDataString(upid);
|
||||
var statusResource = $"nodes/{node}/tasks/{encodedUpid}/status";
|
||||
while (true)
|
||||
{
|
||||
System.Threading.Thread.Sleep(2000);
|
||||
var statusJson = client.GetAsync(statusResource).GetAwaiter().GetResult();
|
||||
var statusRoot = JObject.Parse(statusJson);
|
||||
var d = statusRoot["data"];
|
||||
if (d?["status"]?.ToString() == "stopped")
|
||||
return new PveTask { Upid = upid, Node = node, Status = "stopped", ExitStatus = d["exitstatus"]?.ToString() };
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
using System;
|
||||
using System.Diagnostics;
|
||||
using System.Management.Automation;
|
||||
using PSProxmoxVE.Core.Services;
|
||||
|
||||
@@ -32,6 +34,11 @@ namespace PSProxmoxVE.Cmdlets.Vms
|
||||
[Parameter(Mandatory = false, HelpMessage = "Arguments to pass to the command.")]
|
||||
public string[]? Args { get; set; }
|
||||
|
||||
/// <summary>Maximum time in seconds to wait for the command to complete. Defaults to 300 (5 minutes).</summary>
|
||||
[Parameter(Mandatory = false, HelpMessage = "Timeout in seconds to wait for command completion. Defaults to 300.")]
|
||||
[ValidateRange(1, 3600)]
|
||||
public int Timeout { get; set; } = 300;
|
||||
|
||||
protected override void ProcessRecord()
|
||||
{
|
||||
if (!ShouldProcess($"VM {VmId} on node '{Node}'", $"Execute guest command: {Command}"))
|
||||
@@ -43,11 +50,15 @@ namespace PSProxmoxVE.Cmdlets.Vms
|
||||
WriteVerbose($"Executing command on VM {VmId} via guest agent...");
|
||||
var pid = service.ExecuteGuestCommand(session, Node, VmId, Command, Args);
|
||||
|
||||
// Poll for completion
|
||||
// Poll for completion with timeout
|
||||
var sw = Stopwatch.StartNew();
|
||||
var deadline = TimeSpan.FromSeconds(Timeout);
|
||||
Newtonsoft.Json.Linq.JObject result;
|
||||
do
|
||||
{
|
||||
System.Threading.Thread.Sleep(1000);
|
||||
if (sw.Elapsed >= deadline)
|
||||
throw new TimeoutException($"Guest command did not complete within {Timeout} seconds.");
|
||||
result = service.GetGuestExecStatus(session, Node, VmId, pid);
|
||||
} while (result["exited"]?.ToObject<int>() != 1);
|
||||
|
||||
@@ -68,7 +79,7 @@ namespace PSProxmoxVE.Cmdlets.Vms
|
||||
var bytes = System.Convert.FromBase64String(encoded);
|
||||
return System.Text.Encoding.UTF8.GetString(bytes);
|
||||
}
|
||||
catch
|
||||
catch (FormatException)
|
||||
{
|
||||
return encoded!;
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user