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

[Alexandre DERUMIER]

Hi Thomas,

I just send patches to avoiding using of .new files + .version file.

I think it's better/cleaner like this.

Feel free to comment !


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

>>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; 
> } 
> 
>