From 2dff02f2bd24774f757532d6f87add4b3dad892f Mon Sep 17 00:00:00 2001 From: "goodolclint-claude[bot]" <323206664+goodolclint-claude[bot]@users.noreply.github.com> Date: Tue, 1 Sep 2026 22:53:26 -0500 Subject: [PATCH] feat: VLAN-aware bridges via New-PveNetwork and Set-PveNetwork PveNetwork already deserialised bridge_vlan_aware as BridgeVlanAware, so a VLAN-aware bridge could be read back but never created or changed. Both write paths now take a -BridgeVlanAware switch. Clearing the flag does not use bridge_vlan_aware=0. PVE merges the supplied keys onto the stored stanza and accepts that 0 without acting on it, so the obvious form is a silent no-op: an integration run against PVE 9 issued it and Get-PveNetwork still reported 1. The endpoint's delete list is what actually removes the key. The API schema advertises a plain boolean and gives no hint of this, which is why the behaviour is pinned by an integration test rather than inferred. Set-PveNetwork guards the switch on BoundParameters so an update that omits it leaves the flag alone; the create path follows the existing -Autostart form. Only bridge_vlan_aware is added. bridge_vids is an independent parameter that PVE defaults to 2-4094, and the issue asks only for the flag. Coverage: the integration suite pins create, disable, re-enable, and that an unrelated Set leaves the flag alone -- that last one kills a mutant that drops the BoundParameters guard, which every other test survives. A model test pins the read path the assertions depend on. The Pester unit tests assert only parameter metadata; the defect is server-side, so nothing offline can catch it. Closes #92 --- CHANGELOG.md | 4 ++ docs/cmdlets/New-PveNetwork.md | 19 +++++++- docs/cmdlets/Set-PveNetwork.md | 21 ++++++++- .../Cmdlets/Network/NewPveNetworkCmdlet.cs | 5 +++ .../Cmdlets/Network/SetPveNetworkCmdlet.cs | 11 +++++ .../Models/NetworkModelTests.cs | 12 ++++++ .../Integration/04_Network.Tests.ps1 | 43 +++++++++++++++++++ .../Network/Get-PveNetwork.Tests.ps1 | 18 ++++++++ 8 files changed, 131 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index abe9a77..fe0e944 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Conventional Commits](https://www.conventionalcommi ## [Unreleased] +### Added + +- `New-PveNetwork` and `Set-PveNetwork` gained `-BridgeVlanAware`, so a VLAN-aware Linux bridge can be created and toggled from the module instead of only being read back. The model already surfaced `bridge_vlan_aware` as `BridgeVlanAware`, so this closed a write-path gap. On `Set-PveNetwork` the switch is only sent when explicitly bound, so an update that omits it leaves the flag alone. Clearing it goes through the endpoint's `delete` list rather than `bridge_vlan_aware=0`: PVE merges supplied keys onto the stored stanza and accepts the `0` without acting on it, so the obvious form is a silent no-op — confirmed against a live PVE 9 cluster, where the bridge stayed VLAN-aware. `bridge_vids` is not covered; it is an independent parameter and PVE defaults to 2-4094. (#92) + ### Fixed - `Restart-PveVm` now uses PVE's native reboot endpoint (`POST {vmid}/status/reboot`) instead of composing a shutdown followed by a start. The two-call form raced Proxmox's own post-stop cleanup: the start won the guest's config lock, `qm cleanup` then held that lock for 30 seconds waiting on the newly started process, and the caller's next operation failed with `can't lock file '/var/lock/qemu-server/lock-.conf' - got timeout`. Reproduced in integration runs 183, 185 and 186 as a cascade of 4 failures. `Restart-PveContainer` is unchanged — LXC has no reboot endpoint. See `DECISIONS.md` D016. diff --git a/docs/cmdlets/New-PveNetwork.md b/docs/cmdlets/New-PveNetwork.md index 5ccb0c9..636a766 100644 --- a/docs/cmdlets/New-PveNetwork.md +++ b/docs/cmdlets/New-PveNetwork.md @@ -14,7 +14,8 @@ schema: 2.0.0 ``` New-PveNetwork [-Node] [-Iface] [-Type] [-Address ] [-Netmask ] - [-Gateway ] [-BridgePorts ] [-BondSlaves ] [-VlanId ] [-Mtu ] + [-Gateway ] [-BridgeVlanAware] [-BridgePorts ] [-BondSlaves ] [-VlanId ] + [-Mtu ] [-Autostart] [-Comments ] [-Session ] [-ProgressAction ] [-WhatIf] [-Confirm] [] ``` @@ -78,6 +79,22 @@ Accept pipeline input: False Accept wildcard characters: False ``` +### -BridgeVlanAware + +Enable VLAN-aware bridging (802.1Q) on this bridge. Applies to `bridge` type interfaces only. + +```yaml +Type: SwitchParameter +Parameter Sets: (All) +Aliases: + +Required: False +Position: Named +Default value: False +Accept pipeline input: False +Accept wildcard characters: False +``` + ### -BridgePorts Bridge ports (space-separated interface names). diff --git a/docs/cmdlets/Set-PveNetwork.md b/docs/cmdlets/Set-PveNetwork.md index 9c452f4..64ab163 100644 --- a/docs/cmdlets/Set-PveNetwork.md +++ b/docs/cmdlets/Set-PveNetwork.md @@ -14,7 +14,8 @@ schema: 2.0.0 ``` Set-PveNetwork [-Node] [-Iface] -Type [-Address ] [-Netmask ] - [-Gateway ] [-Address6 ] [-Netmask6 ] [-Gateway6 ] [-BridgePorts ] + [-Gateway ] [-Address6 ] [-Netmask6 ] [-Gateway6 ] [-BridgeVlanAware] + [-BridgePorts ] [-BondSlaves ] [-Mtu ] [-Autostart] [-Comments ] [-Session ] [-ProgressAction ] [-WhatIf] [-Confirm] [] ``` @@ -93,6 +94,24 @@ Accept pipeline input: False Accept wildcard characters: False ``` +### -BridgeVlanAware + +Enable or disable VLAN-aware bridging (802.1Q) on this bridge. Applies to `bridge` type interfaces +only. The flag is sent only when the switch is explicitly bound, so an update that omits it leaves +the current setting untouched; `-BridgeVlanAware:$false` clears it. + +```yaml +Type: SwitchParameter +Parameter Sets: (All) +Aliases: + +Required: False +Position: Named +Default value: False +Accept pipeline input: False +Accept wildcard characters: False +``` + ### -BridgePorts Bridge ports (space-separated interface names). diff --git a/src/PSProxmoxVE/Cmdlets/Network/NewPveNetworkCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Network/NewPveNetworkCmdlet.cs index b04b8a8..ccdfee9 100644 --- a/src/PSProxmoxVE/Cmdlets/Network/NewPveNetworkCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Network/NewPveNetworkCmdlet.cs @@ -42,6 +42,10 @@ namespace PSProxmoxVE.Cmdlets.Network [Parameter(Mandatory = false, HelpMessage = "IPv4 gateway address.")] public string? Gateway { get; set; } + /// Enable VLAN-aware bridging (802.1Q) on this bridge. + [Parameter(Mandatory = false, HelpMessage = "Enable VLAN-aware bridging on this bridge.")] + public SwitchParameter BridgeVlanAware { get; set; } + /// Bridge ports (space-separated interface names, for bridge type). [Parameter(Mandatory = false, HelpMessage = "Bridge ports (space-separated interface names).")] public string? BridgePorts { get; set; } @@ -85,6 +89,7 @@ namespace PSProxmoxVE.Cmdlets.Network if (!string.IsNullOrEmpty(Netmask)) data["netmask"] = Netmask!; if (!string.IsNullOrEmpty(Gateway)) data["gateway"] = Gateway!; if (!string.IsNullOrEmpty(BridgePorts)) data["bridge_ports"] = BridgePorts!; + if (BridgeVlanAware.IsPresent) data["bridge_vlan_aware"] = "1"; if (!string.IsNullOrEmpty(BondSlaves)) data["slaves"] = BondSlaves!; if (VlanId.HasValue) data["vlan-id"] = VlanId.Value.ToString(); if (Mtu.HasValue) data["mtu"] = Mtu.Value.ToString(); diff --git a/src/PSProxmoxVE/Cmdlets/Network/SetPveNetworkCmdlet.cs b/src/PSProxmoxVE/Cmdlets/Network/SetPveNetworkCmdlet.cs index 4405e81..e995834 100644 --- a/src/PSProxmoxVE/Cmdlets/Network/SetPveNetworkCmdlet.cs +++ b/src/PSProxmoxVE/Cmdlets/Network/SetPveNetworkCmdlet.cs @@ -52,6 +52,10 @@ namespace PSProxmoxVE.Cmdlets.Network [Parameter(Mandatory = false, HelpMessage = "IPv6 gateway address.")] public string? Gateway6 { get; set; } + /// Enable or disable VLAN-aware bridging (802.1Q) on this bridge. + [Parameter(Mandatory = false, HelpMessage = "Enable or disable VLAN-aware bridging on this bridge.")] + public SwitchParameter BridgeVlanAware { get; set; } + /// Bridge ports (space-separated interface names). [Parameter(Mandatory = false, HelpMessage = "Bridge ports (space-separated interface names).")] public string? BridgePorts { get; set; } @@ -93,6 +97,13 @@ namespace PSProxmoxVE.Cmdlets.Network if (Netmask6.HasValue) data["netmask6"] = Netmask6.Value.ToString(); if (!string.IsNullOrEmpty(Gateway6)) data["gateway6"] = Gateway6!; if (!string.IsNullOrEmpty(BridgePorts)) data["bridge_ports"] = BridgePorts!; + // PVE merges the supplied keys onto the stored stanza, and accepts + // bridge_vlan_aware=0 without acting on it. + if (MyInvocation.BoundParameters.ContainsKey(nameof(BridgeVlanAware))) + { + if (BridgeVlanAware.IsPresent) data["bridge_vlan_aware"] = "1"; + else data["delete"] = "bridge_vlan_aware"; + } if (!string.IsNullOrEmpty(BondSlaves)) data["slaves"] = BondSlaves!; if (Mtu.HasValue) data["mtu"] = Mtu.Value.ToString(); if (Autostart.IsPresent) data["autostart"] = "1"; diff --git a/tests/PSProxmoxVE.Core.Tests/Models/NetworkModelTests.cs b/tests/PSProxmoxVE.Core.Tests/Models/NetworkModelTests.cs index da7c80a..a5d2b9a 100644 --- a/tests/PSProxmoxVE.Core.Tests/Models/NetworkModelTests.cs +++ b/tests/PSProxmoxVE.Core.Tests/Models/NetworkModelTests.cs @@ -109,5 +109,17 @@ namespace PSProxmoxVE.Core.Tests.Models Assert.Equal("vmbr1", networks[1].Iface); Assert.Equal("10.0.0.1/24", networks[1].Cidr); } + + [Fact] + public void PveNetwork_Deserialize_Pve9_BridgeVlanAware_MapsAndIsNullWhenAbsent() + { + var json = TestHelper.LoadFixture("pve9_networks.json"); + var data = JObject.Parse(json)["data"]; + Assert.NotNull(data); + var networks = data.ToObject(); + Assert.NotNull(networks); + Assert.Equal(1, networks[1].BridgeVlanAware); + Assert.Null(networks[0].BridgeVlanAware); + } } } diff --git a/tests/PSProxmoxVE.Tests/Integration/04_Network.Tests.ps1 b/tests/PSProxmoxVE.Tests/Integration/04_Network.Tests.ps1 index 4879912..b8f25d3 100644 --- a/tests/PSProxmoxVE.Tests/Integration/04_Network.Tests.ps1 +++ b/tests/PSProxmoxVE.Tests/Integration/04_Network.Tests.ps1 @@ -3,6 +3,13 @@ BeforeAll { . $PSScriptRoot/_IntegrationHelper.ps1 Connect-TestPve + + function Get-PveBridge99 { + $bridge = Get-PveNetwork -Node $script:Node | + Where-Object { $_.Iface -eq 'vmbr99' } + $bridge | Should -Not -BeNullOrEmpty + $bridge + } } AfterAll { @@ -42,6 +49,7 @@ Describe 'Network — Integration' -Tag 'Integration' { -Iface 'vmbr99' ` -Type 'bridge' ` -Autostart ` + -BridgeVlanAware ` -ErrorAction Stop } | Should -Not -Throw } @@ -53,6 +61,35 @@ Describe 'Network — Integration' -Tag 'Integration' { Should -Not -BeNullOrEmpty } + It 'Should report the new bridge as VLAN aware' { + if (Skip-IfNoTarget) { return } + + $bridge = Get-PveBridge99 + $bridge.BridgeVlanAware | Should -Be 1 + } + + It 'Should disable VLAN awareness via Set-PveNetwork' { + if (Skip-IfNoTarget) { return } + + (Get-PveBridge99).BridgeVlanAware | Should -Be 1 + + Set-PveNetwork -Node $script:Node -Iface 'vmbr99' -Type 'bridge' ` + -BridgeVlanAware:$false -ErrorAction Stop + + (Get-PveBridge99).BridgeVlanAware | Should -BeIn @($null, 0) + } + + It 'Should re-enable VLAN awareness via Set-PveNetwork' { + if (Skip-IfNoTarget) { return } + + (Get-PveBridge99).BridgeVlanAware | Should -BeIn @($null, 0) + + Set-PveNetwork -Node $script:Node -Iface 'vmbr99' -Type 'bridge' ` + -BridgeVlanAware -ErrorAction Stop + + (Get-PveBridge99).BridgeVlanAware | Should -Be 1 + } + It 'Should update bridge comments' { if (Skip-IfNoTarget) { return } @@ -64,6 +101,12 @@ Describe 'Network — Integration' -Tag 'Integration' { -ErrorAction Stop } | Should -Not -Throw } + It 'Should leave VLAN awareness alone when the switch is not passed' { + if (Skip-IfNoTarget) { return } + + (Get-PveBridge99).BridgeVlanAware | Should -Be 1 + } + It 'Should remove the bridge' { if (Skip-IfNoTarget) { return } diff --git a/tests/PSProxmoxVE.Tests/Network/Get-PveNetwork.Tests.ps1 b/tests/PSProxmoxVE.Tests/Network/Get-PveNetwork.Tests.ps1 index bac8c21..e34d19a 100644 --- a/tests/PSProxmoxVE.Tests/Network/Get-PveNetwork.Tests.ps1 +++ b/tests/PSProxmoxVE.Tests/Network/Get-PveNetwork.Tests.ps1 @@ -117,6 +117,15 @@ Describe 'New-PveNetwork' { } } + Context 'VLAN-aware bridge' { + It 'Should expose BridgeVlanAware as a switch' { + Skip-IfMissing 'New-PveNetwork' + $script:Cmd.Parameters.ContainsKey('BridgeVlanAware') | Should -BeTrue + $script:Cmd.Parameters['BridgeVlanAware'].ParameterType | + Should -Be ([System.Management.Automation.SwitchParameter]) + } + } + Context 'Required parameters' { It 'Node should be Mandatory' { Skip-IfMissing 'New-PveNetwork' @@ -162,6 +171,15 @@ Describe 'Set-PveNetwork' { } } + Context 'VLAN-aware bridge' { + It 'Should expose BridgeVlanAware as a switch' { + Skip-IfMissing 'Set-PveNetwork' + $script:Cmd.Parameters.ContainsKey('BridgeVlanAware') | Should -BeTrue + $script:Cmd.Parameters['BridgeVlanAware'].ParameterType | + Should -Be ([System.Management.Automation.SwitchParameter]) + } + } + Context 'Required parameters' { It 'Node should be Mandatory' { Skip-IfMissing 'Set-PveNetwork'