[pve-devel] [PATCH pve-manager] api2: network : check if ifupdown2 is proxmox version

[Alexandre DERUMIER]

hmm, I mean, seems a bit expensive.. Could we just ship a "flag file"
with ifupdown2 and just check if that does exist? E.g.:
die "..." if ! -e /usr/share/doc/ifupdown2/.proxmox-build;

>>Or, do we actually need the PVE build? Doesn't it just have to be recent enough?

we really need to proxmox patches,at least
0001-don-t-remove-tap-veth-fwpr-interfaces-from-bridge-on.patch
&&
0004-don-t-remove-bridge-is-tap-veth-are-still-plugged.patch

or this will break vm networking on reload (and I think this is exactly the user problem on the forum)
That's why I have use a die and not just a warning.


>>Maybe check the version? Not that we switch to upstream in Debian 11 Bullseye
>>and forget this ^^ We could now check for major version part >= 2, that wouldn't
>>be to hard. We get: "ifupdown2:2.0.1-1+pve1"

This is the problem with check version, if bullseye upgrade ifupdown2 to an bigger version and we forgot about it...
I really think we should have a proxmox flag somewhere.

why not put a comment in ifupdown2 python directly (/usr/share/ifupdown2/ifupdown2, for example), and read it ?
Like this, if something override it, we can be sure it's not proxmox version ?


----- Mail original -----
De: "Thomas Lamprecht" <t.lamprecht at proxmox.com>
À: "pve-devel" <pve-devel at pve.proxmox.com>, "aderumier" <aderumier at odiso.com>
Envoyé: Samedi 11 Janvier 2020 17:21:24
Objet: Re: [pve-devel] [PATCH pve-manager] api2: network : check if ifupdown2 is proxmox version

On 1/11/20 1:41 PM, Alexandre Derumier wrote: 
> 1 user forgot to add proxmox repo, and have installed debian version. 
> https://forum.proxmox.com/threads/bridge-cant-be-found-and-vm-failed-to-start.63138/ 
> 
> Signed-off-by: Alexandre Derumier <aderumier at odiso.com> 
> --- 
> PVE/API2/Network.pm | 5 +++++ 
> 1 file changed, 5 insertions(+) 
> 
> diff --git a/PVE/API2/Network.pm b/PVE/API2/Network.pm 
> index 01da0de1..94349998 100644 
> --- a/PVE/API2/Network.pm 
> +++ b/PVE/API2/Network.pm 
> @@ -575,6 +575,11 @@ __PACKAGE__->register_method({ 
> 
> die "you need ifupdown2 to reload networking\n" if !-e '/usr/share/ifupdown2'; 
> 
> + PVE::Tools::run_command(['ifreload', '-V'], outfunc => sub { 
> + my $line = shift; 
> + die "you need ifupdown2 package from proxmox repository" if $line !~ m/pve/; 
> + }); 
> + 
> if (-x '/usr/bin/ovs-vsctl') { 
> my $ovs_configured = sub { 
> my $ifaces = shift; 
> 

hmm, I mean, seems a bit expensive.. Could we just ship a "flag file" 
with ifupdown2 and just check if that does exist? E.g.: 
die "..." if ! -e /usr/share/doc/ifupdown2/.proxmox-build; 

Or, do we actually need the PVE build? Doesn't it just have to be recent enough? 
Maybe check the version? Not that we switch to upstream in Debian 11 Bullseye 
and forget this ^^ We could now check for major version part >= 2, that wouldn't 
be to hard. We get: "ifupdown2:2.0.1-1+pve1" 

So would something like: 

----8<---- 
diff --git a/PVE/API2/Network.pm b/PVE/API2/Network.pm 
index 2d201c0e..d692023b 100644 
--- a/PVE/API2/Network.pm 
+++ b/PVE/API2/Network.pm 
@@ -526,6 +526,25 @@ __PACKAGE__->register_method({ 
return $ifaces->{$param->{iface}}; 
}}); 

+sub ifupdown2_version { 
+ my $v; 
+ PVE::Tools::run_command(['ifreload', '-V'], outfunc => sub { $v //= shift }); 
+ return if !defined($v) || $v !~ /^\s*ifupdown2:(\S+)\s*$/; 
+ 
+ my ($major, $minor, $extra, $pve) = split(/\.|-/, $1); 
+ # ignore pve for now 
+ return $major * 100000 + $minor * 1000 + $extra * 10; 
+} 
+ 
+sub assert_ifupdown2_installed { 
+ die "you need ifupdown2 to reload networking\n" if !-e '/usr/share/ifupdown2'; 
+ 
+ my $v = ifupdown2_version; 
+ 
+ die "ifupdown2 version to old (< 2), do you installed from Proxmox repositories?\n" 
+ if $v < 2 * 100000; 
+} 
+ 
__PACKAGE__->register_method({ 
name => 'reload_network_config', 
path => '', 
@@ -554,7 +573,7 @@ __PACKAGE__->register_method({ 
my $current_config_file = "/etc/network/interfaces"; 
my $new_config_file = "/etc/network/interfaces.new"; 

- die "you need ifupdown2 to reload networking\n" if !-e '/usr/share/ifupdown2'; 
+ assert_ifupdown2_installed(); 

if (-x '/usr/bin/ovs-vsctl') { 
my $ovs_configured = sub { 
--- 

work to for you? 


I mean, a nice big warning if no working and current Proxmox VE/MG repo is 
setup is on my todo since a bit, that would help against this too..