* [pve-devel] [PATCH manager/network 0/2] do not apply FRR configuration when reloading host network configuration @ 2025-08-04 13:47 Stefan Hanreich 2025-08-04 13:47 ` [pve-devel] [PATCH pve-manager 1/1] api: network: default to not regenerating the frr configuration Stefan Hanreich 2025-08-04 13:47 ` [pve-devel] [PATCH pve-network 1/1] sdn: api: rename parameter from skip-frr to regenerate-frr Stefan Hanreich 0 siblings, 2 replies; 6+ messages in thread From: Stefan Hanreich @ 2025-08-04 13:47 UTC (permalink / raw) To: pve-devel The FRR apply function was called when omitting the skip_frr parameter, which is the case when saving the host network configuration via the Web UI, leading to an enabled FRR daemon after saving the network configuration. In a follow-up we should also consider checking if the previous configuration contained FRR entities, but the new one doesn't and disable the FRR daemon in this case. pve-manager: Stefan Hanreich (1): api: network: default to not regenerating the frr configuration PVE/API2/Network.pm | 7 ++++--- bin/pve-sdn-commit | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) pve-network: Stefan Hanreich (1): sdn: api: rename parameter from skip-frr to regenerate-frr src/PVE/API2/Network/SDN.pm | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) Summary over all repositories: 3 files changed, 10 insertions(+), 11 deletions(-) -- Generated by git-murpp 0.8.0 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* [pve-devel] [PATCH pve-manager 1/1] api: network: default to not regenerating the frr configuration 2025-08-04 13:47 [pve-devel] [PATCH manager/network 0/2] do not apply FRR configuration when reloading host network configuration Stefan Hanreich @ 2025-08-04 13:47 ` Stefan Hanreich 2025-08-04 14:07 ` Thomas Lamprecht 2025-08-04 13:47 ` [pve-devel] [PATCH pve-network 1/1] sdn: api: rename parameter from skip-frr to regenerate-frr Stefan Hanreich 1 sibling, 1 reply; 6+ messages in thread From: Stefan Hanreich @ 2025-08-04 13:47 UTC (permalink / raw) To: pve-devel Default to not regenerating the FRR configuration, unless explicitly requested. Otherwise applying the host network configuration would reload and enable the FRR service. Invert the boolean from skip to regenerate, since the logic is less convoluted this way. Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com> --- PVE/API2/Network.pm | 7 ++++--- bin/pve-sdn-commit | 4 ++-- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/PVE/API2/Network.pm b/PVE/API2/Network.pm index 93f441bab..b423e19b5 100644 --- a/PVE/API2/Network.pm +++ b/PVE/API2/Network.pm @@ -891,10 +891,11 @@ __PACKAGE__->register_method({ additionalProperties => 0, properties => { node => get_standard_option('pve-node'), - skip_frr => { + 'regenerate-frr' => { type => 'boolean', description => 'Whether FRR config generation should get skipped or not.', optional => 1, + default => 0, }, }, }, @@ -910,7 +911,7 @@ __PACKAGE__->register_method({ my $current_config_file = "/etc/network/interfaces"; my $new_config_file = "/etc/network/interfaces.new"; - my $skip_frr = extract_param($param, 'skip_frr'); + my $regenerate_frr = extract_param($param, 'regenerate-frr'); assert_ifupdown2_installed(); @@ -931,7 +932,7 @@ __PACKAGE__->register_method({ }; PVE::Tools::run_command(['ifreload', '-a'], errfunc => $err); - if ($have_sdn && !$skip_frr) { + if ($have_sdn && $regenerate_frr) { PVE::Network::SDN::generate_frr_config(1); } }; diff --git a/bin/pve-sdn-commit b/bin/pve-sdn-commit index 492963baf..6eeba301c 100644 --- a/bin/pve-sdn-commit +++ b/bin/pve-sdn-commit @@ -83,7 +83,7 @@ my $previous_config_has_frr = PVE::Network::SDN::running_config_has_frr(); PVE::Network::SDN::commit_config(); my $new_config_has_frr = PVE::Network::SDN::running_config_has_frr(); -my $skip_frr = !($previous_config_has_frr || $new_config_has_frr); +my $regenerate_frr = ($previous_config_has_frr || $new_config_has_frr); PVE::Network::SDN::generate_etc_network_config(); PVE::Network::SDN::generate_dhcp_config(); @@ -97,6 +97,6 @@ my $err = sub { PVE::Tools::run_command(['ifreload', '-a'], errfunc => $err); -PVE::Network::SDN::generate_frr_config(1) if !$skip_frr; +PVE::Network::SDN::generate_frr_config(1) if $regenerate_frr; exit 0; -- 2.47.2 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH pve-manager 1/1] api: network: default to not regenerating the frr configuration 2025-08-04 13:47 ` [pve-devel] [PATCH pve-manager 1/1] api: network: default to not regenerating the frr configuration Stefan Hanreich @ 2025-08-04 14:07 ` Thomas Lamprecht 2025-08-04 14:11 ` Stefan Hanreich 0 siblings, 1 reply; 6+ messages in thread From: Thomas Lamprecht @ 2025-08-04 14:07 UTC (permalink / raw) To: Proxmox VE development discussion, Stefan Hanreich Am 04.08.25 um 15:46 schrieb Stefan Hanreich: > Default to not regenerating the FRR configuration, unless explicitly > requested. Otherwise applying the host network configuration would > reload and enable the FRR service. Invert the boolean from skip to > regenerate, since the logic is less convoluted this way. and casually break API and reloading SDN during the upgrade, while triggering such updates might be not the best idea to do in the first place it would be at least good to call it out here. FWIW, if we'd walk the full mile here we probably would keep the old param as fallback and check the node version from the pmxcs in memory kv store in pve-network to determine what param to send to the other nodes. That said, I can live with skip doing that, it is a major release after all. Just ensure you have upgrades on your mind or, if you had, add a note for that next time. > > Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com> > --- > PVE/API2/Network.pm | 7 ++++--- > bin/pve-sdn-commit | 4 ++-- > 2 files changed, 6 insertions(+), 5 deletions(-) > > diff --git a/PVE/API2/Network.pm b/PVE/API2/Network.pm > index 93f441bab..b423e19b5 100644 > --- a/PVE/API2/Network.pm > +++ b/PVE/API2/Network.pm > @@ -891,10 +891,11 @@ __PACKAGE__->register_method({ > additionalProperties => 0, > properties => { > node => get_standard_option('pve-node'), > - skip_frr => { > + 'regenerate-frr' => { > type => 'boolean', > description => 'Whether FRR config generation should get skipped or not.', > optional => 1, > + default => 0, > }, > }, > }, > @@ -910,7 +911,7 @@ __PACKAGE__->register_method({ > my $current_config_file = "/etc/network/interfaces"; > my $new_config_file = "/etc/network/interfaces.new"; > > - my $skip_frr = extract_param($param, 'skip_frr'); > + my $regenerate_frr = extract_param($param, 'regenerate-frr'); > > assert_ifupdown2_installed(); > > @@ -931,7 +932,7 @@ __PACKAGE__->register_method({ > }; > PVE::Tools::run_command(['ifreload', '-a'], errfunc => $err); > > - if ($have_sdn && !$skip_frr) { > + if ($have_sdn && $regenerate_frr) { > PVE::Network::SDN::generate_frr_config(1); > } > }; > diff --git a/bin/pve-sdn-commit b/bin/pve-sdn-commit > index 492963baf..6eeba301c 100644 > --- a/bin/pve-sdn-commit > +++ b/bin/pve-sdn-commit > @@ -83,7 +83,7 @@ my $previous_config_has_frr = PVE::Network::SDN::running_config_has_frr(); > PVE::Network::SDN::commit_config(); > > my $new_config_has_frr = PVE::Network::SDN::running_config_has_frr(); > -my $skip_frr = !($previous_config_has_frr || $new_config_has_frr); > +my $regenerate_frr = ($previous_config_has_frr || $new_config_has_frr); > > PVE::Network::SDN::generate_etc_network_config(); > PVE::Network::SDN::generate_dhcp_config(); > @@ -97,6 +97,6 @@ my $err = sub { > > PVE::Tools::run_command(['ifreload', '-a'], errfunc => $err); > > -PVE::Network::SDN::generate_frr_config(1) if !$skip_frr; > +PVE::Network::SDN::generate_frr_config(1) if $regenerate_frr; > > exit 0; _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH pve-manager 1/1] api: network: default to not regenerating the frr configuration 2025-08-04 14:07 ` Thomas Lamprecht @ 2025-08-04 14:11 ` Stefan Hanreich 2025-08-04 14:13 ` Stefan Hanreich 0 siblings, 1 reply; 6+ messages in thread From: Stefan Hanreich @ 2025-08-04 14:11 UTC (permalink / raw) To: Thomas Lamprecht, Proxmox VE development discussion On 8/4/25 4:07 PM, Thomas Lamprecht wrote: > Am 04.08.25 um 15:46 schrieb Stefan Hanreich: >> Default to not regenerating the FRR configuration, unless explicitly >> requested. Otherwise applying the host network configuration would >> reload and enable the FRR service. Invert the boolean from skip to >> regenerate, since the logic is less convoluted this way. > > > and casually break API and reloading SDN during the upgrade, while > triggering such updates might be not the best idea to do in the first > place it would be at least good to call it out here. > > FWIW, if we'd walk the full mile here we probably would keep the old > param as fallback and check the node version from the pmxcs in memory > kv store in pve-network to determine what param to send to the other > nodes. > > That said, I can live with skip doing that, it is a major release > after all. Just ensure you have upgrades on your mind or, if you had, > add a note for that next time. The parameter has been introduced in PVE 9 with the SDN fabrics, so it should be fine to change this. Sorry, I should've at least mentioned it. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [pve-devel] [PATCH pve-manager 1/1] api: network: default to not regenerating the frr configuration 2025-08-04 14:11 ` Stefan Hanreich @ 2025-08-04 14:13 ` Stefan Hanreich 0 siblings, 0 replies; 6+ messages in thread From: Stefan Hanreich @ 2025-08-04 14:13 UTC (permalink / raw) To: Thomas Lamprecht, Proxmox VE development discussion On 8/4/25 4:10 PM, Stefan Hanreich wrote: > On 8/4/25 4:07 PM, Thomas Lamprecht wrote: >> Am 04.08.25 um 15:46 schrieb Stefan Hanreich: >>> Default to not regenerating the FRR configuration, unless explicitly >>> requested. Otherwise applying the host network configuration would >>> reload and enable the FRR service. Invert the boolean from skip to >>> regenerate, since the logic is less convoluted this way. >> >> >> and casually break API and reloading SDN during the upgrade, while >> triggering such updates might be not the best idea to do in the first >> place it would be at least good to call it out here. >> >> FWIW, if we'd walk the full mile here we probably would keep the old >> param as fallback and check the node version from the pmxcs in memory >> kv store in pve-network to determine what param to send to the other >> nodes. >> >> That said, I can live with skip doing that, it is a major release >> after all. Just ensure you have upgrades on your mind or, if you had, >> add a note for that next time. > > The parameter has been introduced in PVE 9 with the SDN fabrics, so it > should be fine to change this. Sorry, I should've at least mentioned it. Ah sorry, I misunderstood - in PVE 8 we actually did always execute generate_controller_config.. _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
* [pve-devel] [PATCH pve-network 1/1] sdn: api: rename parameter from skip-frr to regenerate-frr 2025-08-04 13:47 [pve-devel] [PATCH manager/network 0/2] do not apply FRR configuration when reloading host network configuration Stefan Hanreich 2025-08-04 13:47 ` [pve-devel] [PATCH pve-manager 1/1] api: network: default to not regenerating the frr configuration Stefan Hanreich @ 2025-08-04 13:47 ` Stefan Hanreich 1 sibling, 0 replies; 6+ messages in thread From: Stefan Hanreich @ 2025-08-04 13:47 UTC (permalink / raw) To: pve-devel Parameter has moved from skip_frr to regenerate-frr in pve-manager. Also invert the check for generating the boolean, since the logic is inverted now. Signed-off-by: Stefan Hanreich <s.hanreich@proxmox.com> --- src/PVE/API2/Network/SDN.pm | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/src/PVE/API2/Network/SDN.pm b/src/PVE/API2/Network/SDN.pm index 6645f28..d3b15f1 100644 --- a/src/PVE/API2/Network/SDN.pm +++ b/src/PVE/API2/Network/SDN.pm @@ -90,12 +90,10 @@ __PACKAGE__->register_method({ }); my $create_reload_network_worker = sub { - my ($nodename, $skip_frr) = @_; + my ($nodename, $regenerate_frr) = @_; my @command = ('pvesh', 'set', "/nodes/$nodename/network"); - if ($skip_frr) { - push(@command, '--skip_frr'); - } + push(@command, '--regenerate-frr') if $regenerate_frr; # FIXME: how to proxy to final node ? my $upid; @@ -140,14 +138,14 @@ __PACKAGE__->register_method({ PVE::Network::SDN::commit_config(); my $new_config_has_frr = PVE::Network::SDN::running_config_has_frr(); - my $skip_frr = !($previous_config_has_frr || $new_config_has_frr); + my $regenerate_frr = $previous_config_has_frr || $new_config_has_frr; my $code = sub { $rpcenv->{type} = 'priv'; # to start tasks in background PVE::Cluster::check_cfs_quorum(); my $nodelist = PVE::Cluster::get_nodelist(); for my $node (@$nodelist) { - my $pid = eval { $create_reload_network_worker->($node, $skip_frr) }; + my $pid = eval { $create_reload_network_worker->($node, $regenerate_frr) }; warn $@ if $@; } -- 2.47.2 _______________________________________________ pve-devel mailing list pve-devel@lists.proxmox.com https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2025-08-04 14:11 UTC | newest] Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2025-08-04 13:47 [pve-devel] [PATCH manager/network 0/2] do not apply FRR configuration when reloading host network configuration Stefan Hanreich 2025-08-04 13:47 ` [pve-devel] [PATCH pve-manager 1/1] api: network: default to not regenerating the frr configuration Stefan Hanreich 2025-08-04 14:07 ` Thomas Lamprecht 2025-08-04 14:11 ` Stefan Hanreich 2025-08-04 14:13 ` Stefan Hanreich 2025-08-04 13:47 ` [pve-devel] [PATCH pve-network 1/1] sdn: api: rename parameter from skip-frr to regenerate-frr Stefan Hanreich
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox