[pve-devel] [PATCH pve-network] fix total removal of zones|vnets|controllers

[Alexandre DERUMIER]

>>Why do we prefer always the new one, would it be nicer to always get the
>>current applied one? And on changes which result in a .new we apply it then
>>ASAP and rename it (ideally atomic)?
>>
>>Or what's your current update + .new file rename strategy?

Well, The .new files are renamed at the same time for vnet/zones/controllers
when we apply configuration + push local config on each node.

We can't really rename it "atomically" after apply it on local nodes, because config is push on multiple nodes,
a node could be not responding,.... (or we should manage rollback of all local nodes configuration, if 1 node is failing).
So currently we rename it in any case

The current strategy, is that pvestatd compare local configuration, and report errors if configuration is different than datacenter sdn configuration.

Maybe it could be improve, because currently I'm looking file creation timestamp to known if datacenter sdn configuration is newer than local configuration.
Maybe with a global versionning counter increase in /etc/pve/sdn/version, increased at each config change of any sdn/*.cfg
Then we could remove .new files. (without rollback feature, but anyway, I think it's difficult to manage relationship between vnet/zones/controllers if we rollback
only 1 of the component)

What do you think about this ?


Current local status code:


Zones.pm
https://git.proxmox.com/?p=pve-network.git;a=blob;f=PVE/Network/SDN/Zones.pm;h=87915f22cdbd44ee4fdbc249cdb9d35cd4c7fddd;hb=HEAD


 168 sub status {
 169 
 170     my $cluster_vnet_file = "/etc/pve/sdn/vnets.cfg";
 171     my $cluster_zone_file = "/etc/pve/sdn/zones.cfg";
 172     my $local_sdn_file = "/etc/network/interfaces.d/sdn";
 173     my $err_config = undef;
 174 
 175     return if !-e $cluster_vnet_file && !-e $cluster_zone_file;
 176 
 177     if (!-e $local_sdn_file) {
 178 
 179         $err_config = "local sdn network configuration is not yet generated, please reload";
 180         warn "$err_config\n";
 181     } else {
 182         # fixme : use some kind of versioning info?
 183         my $cluster_vnet_timestamp = (stat($cluster_vnet_file))[9];
 184         my $cluster_zone_timestamp = (stat($cluster_zone_file))[9];
 185         my $local_sdn_timestamp = (stat($local_sdn_file))[9];
 186 
 187         if ($local_sdn_timestamp < $cluster_vnet_timestamp || $local_sdn_timestamp < $cluster_zone_timestamp) {
 188             $err_config = "local sdn network configuration is too old, please reload";
 189             warn "$err_config\n";
 190         }
 191     }


----- Mail original -----
De: "Thomas Lamprecht" <t.lamprecht at proxmox.com>
À: "pve-devel" <pve-devel at pve.proxmox.com>, "aderumier" <aderumier at odiso.com>
Envoyé: Jeudi 9 Avril 2020 08:01:03
Objet: Re: [pve-devel] [PATCH pve-network] fix total removal of zones|vnets|controllers

On 3/16/20 9:44 AM, Alexandre Derumier wrote: 
> Signed-off-by: Alexandre Derumier <aderumier at odiso.com> 
> --- 
> PVE/Network/SDN/Controllers.pm | 3 ++- 
> PVE/Network/SDN/Vnets.pm | 2 +- 
> PVE/Network/SDN/Zones.pm | 3 ++- 
> 3 files changed, 5 insertions(+), 3 deletions(-) 
> 
> diff --git a/PVE/Network/SDN/Controllers.pm b/PVE/Network/SDN/Controllers.pm 
> index 16c664d..1741c95 100644 
> --- a/PVE/Network/SDN/Controllers.pm 
> +++ b/PVE/Network/SDN/Controllers.pm 
> @@ -33,7 +33,8 @@ sub sdn_controllers_config { 
> 
> sub config { 
> my $config = cfs_read_file("sdn/controllers.cfg.new"); 
> - $config = cfs_read_file("sdn/controllers.cfg") if !keys %{$config->{ids}}; 
> + $config = cfs_read_file("sdn/controllers.cfg") if !-e "/etc/pve/sdn/controllers.cfg.new"; 
> + 

Why do we prefer always the new one, would it be nicer to always get the 
current applied one? And on changes which result in a .new we apply it then 
ASAP and rename it (ideally atomic)? 

Or what's your current update + .new file rename strategy? 

> return $config; 
> } 
> 
> diff --git a/PVE/Network/SDN/Vnets.pm b/PVE/Network/SDN/Vnets.pm 
> index 725605b..ca6e1d0 100644 
> --- a/PVE/Network/SDN/Vnets.pm 
> +++ b/PVE/Network/SDN/Vnets.pm 
> @@ -23,7 +23,7 @@ sub sdn_vnets_config { 
> 
> sub config { 
> my $config = cfs_read_file("sdn/vnets.cfg.new"); 
> - $config = cfs_read_file("sdn/vnets.cfg") if !keys %{$config->{ids}}; 
> + $config = cfs_read_file("sdn/vnets.cfg") if !-e "/etc/pve/sdn/vnets.cfg.new"; 

If we still want to do this then I'd rather like: 

my $config; 
if (-e "/etc/pve/sdn/vnets.cfg.new") { 
# always prefer new for <reason> 
$config = cfs_read_file("sdn/vnets.cfg.new"); 
} else { 
$config = cfs_read_file("sdn/vnets.cfg"); 
} 

> return $config; 
> } 
> 
> diff --git a/PVE/Network/SDN/Zones.pm b/PVE/Network/SDN/Zones.pm 
> index 17ef507..ae31fea 100644 
> --- a/PVE/Network/SDN/Zones.pm 
> +++ b/PVE/Network/SDN/Zones.pm 
> @@ -39,7 +39,8 @@ sub sdn_zones_config { 
> 
> sub config { 
> my $config = cfs_read_file("sdn/zones.cfg.new"); 
> - $config = cfs_read_file("sdn/zones.cfg") if !keys %{$config->{ids}}; 
> + $config = cfs_read_file("sdn/zones.cfg") if !-e "/etc/pve/sdn/zones.cfg.new"; 
> + 
> return $config; 
> } 
> 
>