From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from firstgate.proxmox.com (firstgate.proxmox.com [IPv6:2a01:7e0:0:424::9]) by lore.proxmox.com (Postfix) with ESMTPS id C75131FF17C for ; Wed, 3 Sep 2025 11:36:51 +0200 (CEST) Received: from firstgate.proxmox.com (localhost [127.0.0.1]) by firstgate.proxmox.com (Proxmox) with ESMTP id CD9812F972; Wed, 3 Sep 2025 11:37:06 +0200 (CEST) Mime-Version: 1.0 Date: Wed, 03 Sep 2025 11:37:02 +0200 Message-Id: To: "Proxmox Datacenter Manager development discussion" , "Stefan Hanreich" X-Mailer: aerc 0.20.0 References: <20250902140956.228031-1-s.hanreich@proxmox.com> <20250902140956.228031-10-s.hanreich@proxmox.com> In-Reply-To: <20250902140956.228031-10-s.hanreich@proxmox.com> From: "Shannon Sterz" X-Bm-Milter-Handled: 55990f41-d878-4baa-be0a-ee34c49e34d2 X-Bm-Transport-Timestamp: 1756892207209 X-SPAM-LEVEL: Spam detection results: 0 AWL 0.025 Adjusted score from AWL reputation of From: address BAYES_00 -1.9 Bayes spam probability is 0 to 1% DMARC_MISSING 0.1 Missing DMARC policy KAM_DMARC_STATUS 0.01 Test Rule for DKIM or SPF Failure with Strict Alignment RCVD_IN_MSPIKE_H2 0.001 Average reputation (+2) SPF_HELO_NONE 0.001 SPF: HELO does not publish an SPF Record SPF_PASS -0.001 SPF: sender matches SPF record Subject: Re: [pdm-devel] [PATCH pve-network v3 6/6] api: zones: update schema of endpoints X-BeenThere: pdm-devel@lists.proxmox.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Proxmox Datacenter Manager development discussion List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Reply-To: Proxmox Datacenter Manager development discussion Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Errors-To: pdm-devel-bounces@lists.proxmox.com Sender: "pdm-devel" small comments in-line On Tue Sep 2, 2025 at 4:09 PM CEST, Stefan Hanreich wrote: > The possible properties returned by the zone endpoints were only > partly documented. Add all missing properties and improve descriptions > for existing properties. > > Extract all duplicate properties into a separate variable, so we > don't have to rewrite the whole API definition for every endpoint. > > Signed-off-by: Stefan Hanreich > --- > src/PVE/API2/Network/SDN/Zones.pm | 203 +++++++++++++++++++++-- > src/PVE/Network/SDN/Zones/EvpnPlugin.pm | 22 ++- > src/PVE/Network/SDN/Zones/QinQPlugin.pm | 6 +- > src/PVE/Network/SDN/Zones/VlanPlugin.pm | 1 + > src/PVE/Network/SDN/Zones/VxlanPlugin.pm | 15 +- > 5 files changed, 218 insertions(+), 29 deletions(-) > -->8 snip 8<-- > + 'vxlan-port' => { > + description => > + "UDP port that should be used for the VXLAN tunnel (default 4789). VXLAN zone only.", wouldn't it make sense to define a `default` property here then? so: default => 4789, -->8 snip 8<-- > 'rt-import' => { > type => 'string', > - description => "Route-Target import", > + description => > + 'List of Route Targets that should be imported into the VRF of the zone', nit: you added a period to all other descriptions here, so end this sentence with one too? > optional => 1, > format => 'pve-sdn-bgp-rt-list', > }, > diff --git a/src/PVE/Network/SDN/Zones/QinQPlugin.pm b/src/PVE/Network/SDN/Zones/QinQPlugin.pm > index 5806e69..3c72d35 100644 > --- a/src/PVE/Network/SDN/Zones/QinQPlugin.pm > +++ b/src/PVE/Network/SDN/Zones/QinQPlugin.pm > @@ -18,11 +18,11 @@ sub properties { > tag => { > type => 'integer', > minimum => 0, > - description => "Service-VLAN Tag", > + description => "Service-VLAN Tag (outer VLAN)", > }, since this is so short, i think it's probably fine to leave that without a period here. > mtu => { > type => 'integer', > - description => "MTU", > + description => "MTU of the zone, will be used for the created VNet bridges.", > optional => 1, > }, > 'vlan-protocol' => { > @@ -30,6 +30,8 @@ sub properties { > enum => ['802.1q', '802.1ad'], > default => '802.1q', > optional => 1, > + description => > + "Which VLAN protocol should be used for the creation of the QinQ zone", nit: add a period here too? > }, > }; > } > diff --git a/src/PVE/Network/SDN/Zones/VlanPlugin.pm b/src/PVE/Network/SDN/Zones/VlanPlugin.pm > index 90f16bf..9d6932f 100644 > --- a/src/PVE/Network/SDN/Zones/VlanPlugin.pm > +++ b/src/PVE/Network/SDN/Zones/VlanPlugin.pm > @@ -27,6 +27,7 @@ sub properties { > return { > 'bridge' => { > type => 'string', > + description => 'the bridge for which VLANs should be managed', nit: capitalize beginning of sentence and end it in a period? > }, > 'bridge-disable-mac-learning' => { > type => 'boolean', > diff --git a/src/PVE/Network/SDN/Zones/VxlanPlugin.pm b/src/PVE/Network/SDN/Zones/VxlanPlugin.pm > index 8f6fba0..7ab89da 100644 > --- a/src/PVE/Network/SDN/Zones/VxlanPlugin.pm > +++ b/src/PVE/Network/SDN/Zones/VxlanPlugin.pm > @@ -27,21 +27,22 @@ sub type { > sub properties { > return { > 'peers' => { > - description => "peers address list.", > + description => > + "Comma-separated list of peers, that are part of the VXLAN zone. Usually the IPs of the nodes.", > type => 'string', > format => 'ip-list', > }, > - fabric => { > - description => "SDN fabric to use as underlay for this VXLAN zone.", > - type => 'string', > - format => 'pve-sdn-fabric-id', > - }, > 'vxlan-port' => { > - description => "Vxlan tunnel udp port (default 4789).", > + description => "UDP port that should be used for the VXLAN tunnel (default 4789).", > minimum => 1, > maximum => 65536, could also define the default here too? > type => 'integer', > }, > + fabric => { > + description => "SDN fabric to use as underlay for this VXLAN zone.", > + type => 'string', > + format => 'pve-sdn-fabric-id', > + }, > }; > } > _______________________________________________ pdm-devel mailing list pdm-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pdm-devel