[pve-devel] [PATCH pve-network 0/2] fix zones status

[pve-devel] [PATCH pve-network 0/2] fix zones status

[Alexandre Derumier]

fix reported bug by Fabian

https://lists.proxmox.com/pipermail/pve-devel/2023-June/057433.html

Alexandre Derumier (2):
  zones: fix status
  zones: status: add a special message if vnet is not generated

 src/PVE/Network/SDN/Zones.pm | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

-- 
2.30.2

[pve-devel] [PATCH pve-network 1/2] zones: fix status

[Alexandre Derumier]

Vnets were wrongly displayed in the main tree because of this,
and error was not set on zone

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 src/PVE/Network/SDN/Zones.pm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/PVE/Network/SDN/Zones.pm b/src/PVE/Network/SDN/Zones.pm
index f8e40b1..7d70e49 100644
--- a/src/PVE/Network/SDN/Zones.pm
+++ b/src/PVE/Network/SDN/Zones.pm
@@ -268,7 +268,7 @@ sub status {
 	if (@{$err_msg} > 0) {
 	    $vnet_status->{$id}->{status} = 'error';
 	    $vnet_status->{$id}->{statusmsg} = join(',', @{$err_msg});
-	    $zone_status->{$id}->{status} = 'error';
+	    $zone_status->{$zone}->{status} = 'error';
 	}
     }
 
-- 
2.30.2

[pve-devel] [PATCH pve-network 2/2] zones: status: add a special message if vnet is not generated

[Alexandre Derumier]

if vnet has not been generated (this should never happend)
warn the user to check if "source /etc/network/interfaces.d/sdn'
exist

Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
---
 src/PVE/Network/SDN/Zones.pm | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/src/PVE/Network/SDN/Zones.pm b/src/PVE/Network/SDN/Zones.pm
index 7d70e49..e26e21f 100644
--- a/src/PVE/Network/SDN/Zones.pm
+++ b/src/PVE/Network/SDN/Zones.pm
@@ -267,7 +267,11 @@ sub status {
 	my $err_msg = $plugin->status($plugin_config, $zone, $id, $vnet, $status);
 	if (@{$err_msg} > 0) {
 	    $vnet_status->{$id}->{status} = 'error';
-	    $vnet_status->{$id}->{statusmsg} = join(',', @{$err_msg});
+	    if (grep(/^missing ${id}$/, @$err_msg)) {
+		$vnet_status->{$id}->{statusmsg} = "vnet $id is not generated. Do you have included 'source /etc/network/interfaces.d/sdn' ?";
+	    } else {
+		$vnet_status->{$id}->{statusmsg} = join(',', @{$err_msg});
+	    }
 	    $zone_status->{$zone}->{status} = 'error';
 	}
     }
-- 
2.30.2

[pve-devel] applied: [PATCH pve-network 1/2] zones: fix status

[Fabian Grünbichler]

On June 12, 2023 7:43 pm, Alexandre Derumier wrote:
> Vnets were wrongly displayed in the main tree because of this,
> and error was not set on zone
> 
> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> ---
>  src/PVE/Network/SDN/Zones.pm | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Network/SDN/Zones.pm b/src/PVE/Network/SDN/Zones.pm
> index f8e40b1..7d70e49 100644
> --- a/src/PVE/Network/SDN/Zones.pm
> +++ b/src/PVE/Network/SDN/Zones.pm
> @@ -268,7 +268,7 @@ sub status {
>  	if (@{$err_msg} > 0) {
>  	    $vnet_status->{$id}->{status} = 'error';
>  	    $vnet_status->{$id}->{statusmsg} = join(',', @{$err_msg});
> -	    $zone_status->{$id}->{status} = 'error';
> +	    $zone_status->{$zone}->{status} = 'error';
>  	}
>      }
>  
> -- 
> 2.30.2
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

Re: [pve-devel] [PATCH pve-network 2/2] zones: status: add a special message if vnet is not generated

[Fabian Grünbichler]

On June 12, 2023 7:43 pm, Alexandre Derumier wrote:
> if vnet has not been generated (this should never happend)
> warn the user to check if "source /etc/network/interfaces.d/sdn'
> exist
> 
> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> ---
>  src/PVE/Network/SDN/Zones.pm | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/src/PVE/Network/SDN/Zones.pm b/src/PVE/Network/SDN/Zones.pm
> index 7d70e49..e26e21f 100644
> --- a/src/PVE/Network/SDN/Zones.pm
> +++ b/src/PVE/Network/SDN/Zones.pm
> @@ -267,7 +267,11 @@ sub status {
>  	my $err_msg = $plugin->status($plugin_config, $zone, $id, $vnet, $status);
>  	if (@{$err_msg} > 0) {
>  	    $vnet_status->{$id}->{status} = 'error';
> -	    $vnet_status->{$id}->{statusmsg} = join(',', @{$err_msg});
> +	    if (grep(/^missing ${id}$/, @$err_msg)) {
> +		$vnet_status->{$id}->{statusmsg} = "vnet $id is not generated. Do you have included 'source /etc/network/interfaces.d/sdn' ?";

these 'missing XXX' messages are generated by us in the plugins, so if
we want to expand them, we should do that where we generate them instead
of matching here.

what do you think about the following (in addition, or instead of
changing the 'statusmsg' contents):

diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
index 1ad85e5..8623fb8 100644
--- a/src/PVE/Network/SDN.pm
+++ b/src/PVE/Network/SDN.pm
@@ -6,6 +6,8 @@ use warnings;
 use Data::Dumper;
 use JSON;
 
+use PVE::INotify;
+
 use PVE::Network::SDN::Vnets;
 use PVE::Network::SDN::Zones;
 use PVE::Network::SDN::Controllers;
@@ -208,6 +210,16 @@ sub get_local_vnets {
 
 sub generate_zone_config {
     my $raw_config = PVE::Network::SDN::Zones::generate_etc_network_config();
+
+    if ($raw_config) {
+	eval {
+	    my $net_cfg = PVE::INotify::read_file('interfaces', 1);
+	    my $opts = $net_cfg->{data}->{options};
+	    warn "missing 'source /etc/network/interfaces.d/sdn' directive for SDN support!\n"
+		if ! grep { $_->[1] =~ m!^source /etc/network/interfaces.d/(:?sdn|\*)! } @$opts;
+	};
+	warn "Failed to read network interfaces definition - $@\n" if $@;
+    }
     PVE::Network::SDN::Zones::write_etc_network_config($raw_config);
 }

the check is not 100% reliable (i.e., there could be a wildcard but
slightly different format) unfortunately.

> +	    } else {
> +		$vnet_status->{$id}->{statusmsg} = join(',', @{$err_msg});
> +	    }
>  	    $zone_status->{$zone}->{status} = 'error';
>  	}
>      }
> -- 
> 2.30.2
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel
> 
> 
> 

Re: [pve-devel] [PATCH pve-network 2/2] zones: status: add a special message if vnet is not generated

[Fabian Grünbichler]

On June 13, 2023 1:21 pm, Fabian Grünbichler wrote:
> On June 12, 2023 7:43 pm, Alexandre Derumier wrote:
>> if vnet has not been generated (this should never happend)
>> warn the user to check if "source /etc/network/interfaces.d/sdn'
>> exist
>> 
>> Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
>> ---
>>  src/PVE/Network/SDN/Zones.pm | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
>> 
>> diff --git a/src/PVE/Network/SDN/Zones.pm b/src/PVE/Network/SDN/Zones.pm
>> index 7d70e49..e26e21f 100644
>> --- a/src/PVE/Network/SDN/Zones.pm
>> +++ b/src/PVE/Network/SDN/Zones.pm
>> @@ -267,7 +267,11 @@ sub status {
>>  	my $err_msg = $plugin->status($plugin_config, $zone, $id, $vnet, $status);
>>  	if (@{$err_msg} > 0) {
>>  	    $vnet_status->{$id}->{status} = 'error';
>> -	    $vnet_status->{$id}->{statusmsg} = join(',', @{$err_msg});
>> +	    if (grep(/^missing ${id}$/, @$err_msg)) {
>> +		$vnet_status->{$id}->{statusmsg} = "vnet $id is not generated. Do you have included 'source /etc/network/interfaces.d/sdn' ?";
> 
> these 'missing XXX' messages are generated by us in the plugins, so if
> we want to expand them, we should do that where we generate them instead
> of matching here.
> 
> what do you think about the following (in addition, or instead of
> changing the 'statusmsg' contents):
> 
> diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
> index 1ad85e5..8623fb8 100644
> --- a/src/PVE/Network/SDN.pm
> +++ b/src/PVE/Network/SDN.pm
> @@ -6,6 +6,8 @@ use warnings;
>  use Data::Dumper;
>  use JSON;
>  
> +use PVE::INotify;
> +
>  use PVE::Network::SDN::Vnets;
>  use PVE::Network::SDN::Zones;
>  use PVE::Network::SDN::Controllers;
> @@ -208,6 +210,16 @@ sub get_local_vnets {
>  
>  sub generate_zone_config {
>      my $raw_config = PVE::Network::SDN::Zones::generate_etc_network_config();
> +
> +    if ($raw_config) {
> +	eval {
> +	    my $net_cfg = PVE::INotify::read_file('interfaces', 1);
> +	    my $opts = $net_cfg->{data}->{options};
> +	    warn "missing 'source /etc/network/interfaces.d/sdn' directive for SDN support!\n"
> +		if ! grep { $_->[1] =~ m!^source /etc/network/interfaces.d/(:?sdn|\*)! } @$opts;
> +	};
> +	warn "Failed to read network interfaces definition - $@\n" if $@;
> +    }
>      PVE::Network::SDN::Zones::write_etc_network_config($raw_config);
>  }
> 
> the check is not 100% reliable (i.e., there could be a wildcard but
> slightly different format) unfortunately.

the above diff works even better if the 'warn' is replaced with
'log_warn' from PVE::RESTEnvironment - that way the network reload task
on the node is marked in the task log lists as well, quickly pointing
out that something is amiss!

Re: [pve-devel] [PATCH pve-network 2/2] zones: status: add a special message if vnet is not generated

[DERUMIER, Alexandre]

Le mardi 13 juin 2023 à 13:21 +0200, Fabian Grünbichler a écrit :
> On June 12, 2023 7:43 pm, Alexandre Derumier wrote:
> > if vnet has not been generated (this should never happend)
> > warn the user to check if "source /etc/network/interfaces.d/sdn'
> > exist
> > 
> > Signed-off-by: Alexandre Derumier <aderumier@odiso.com>
> > ---
> >  src/PVE/Network/SDN/Zones.pm | 6 +++++-
> >  1 file changed, 5 insertions(+), 1 deletion(-)
> > 
> > diff --git a/src/PVE/Network/SDN/Zones.pm
> > b/src/PVE/Network/SDN/Zones.pm
> > index 7d70e49..e26e21f 100644
> > --- a/src/PVE/Network/SDN/Zones.pm
> > +++ b/src/PVE/Network/SDN/Zones.pm
> > @@ -267,7 +267,11 @@ sub status {
> >         my $err_msg = $plugin->status($plugin_config, $zone, $id,
> > $vnet, $status);
> >         if (@{$err_msg} > 0) {
> >             $vnet_status->{$id}->{status} = 'error';
> > -           $vnet_status->{$id}->{statusmsg} = join(',',
> > @{$err_msg});
> > +           if (grep(/^missing ${id}$/, @$err_msg)) {
> > +               $vnet_status->{$id}->{statusmsg} = "vnet $id is not
> > generated. Do you have included 'source
> > /etc/network/interfaces.d/sdn' ?";
> 
> these 'missing XXX' messages are generated by us in the plugins, so
> if
> we want to expand them, we should do that where we generate them
> instead
> of matching here.
yes, it need to do in cleany, I'll be able to work on it next week.


> 
> what do you think about the following (in addition, or instead of
> changing the 'statusmsg' contents):
> 
> diff --git a/src/PVE/Network/SDN.pm b/src/PVE/Network/SDN.pm
> index 1ad85e5..8623fb8 100644
> --- a/src/PVE/Network/SDN.pm
> +++ b/src/PVE/Network/SDN.pm
> @@ -6,6 +6,8 @@ use warnings;
>  use Data::Dumper;
>  use JSON;
>  
> +use PVE::INotify;
> +
>  use PVE::Network::SDN::Vnets;
>  use PVE::Network::SDN::Zones;
>  use PVE::Network::SDN::Controllers;
> @@ -208,6 +210,16 @@ sub get_local_vnets {
>  
>  sub generate_zone_config {
>      my $raw_config =
> PVE::Network::SDN::Zones::generate_etc_network_config();
> +
> +    if ($raw_config) {
> +       eval {
> +           my $net_cfg = PVE::INotify::read_file('interfaces', 1);
> +           my $opts = $net_cfg->{data}->{options};
> +           warn "missing 'source /etc/network/interfaces.d/sdn'
> directive for SDN support!\n"
> +               if ! grep { $_->[1] =~ m!^source
> /etc/network/interfaces.d/(:?sdn|\*)! } @$opts;
> +       };
> +       warn "Failed to read network interfaces definition - $@\n" if
> $@;
> +    }
>      PVE::Network::SDN::Zones::write_etc_network_config($raw_config);
>  }
> 
> the check is not 100% reliable (i.e., there could be a wildcard but
> slightly different format) unfortunately.
> 

yes, I think it could be great indeed. I'll work on this for the v2.

> > +           } else {
> > +               $vnet_status->{$id}->{statusmsg} = join(',',
> > @{$err_msg});
> > +           }
> >             $zone_status->{$zone}->{status} = 'error';
> >         }
> >      }
> > -- 
> > 2.30.2
> > 
> > 
> > _______________________________________________
> > pve-devel mailing list
> > pve-devel@lists.proxmox.com
> > https://antiphishing.cetsi.fr/proxy/v3?i=SXVFem5DOGVpUU1rNjdmQuxbAYzjRE578NJDXO0bRW0&r=bWt1djZ5QzcyUms5R1Nzatwfz4p60Sh_bGp_TdGIYHovbc8XVtFiCyXKb5Z3syuM&f=Q3ZQNmU2SnpsRFlRbUF3dmj8RjqNbw-iHBi50S-iFw7wq1QYxMyfpOzIpg23W0yvT_m-5a44zDEurFGuVbLSbw&u=https%3A//lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel&k=syJL
> > 
> > 
> > 
> 
> 
> _______________________________________________
> pve-devel mailing list
> pve-devel@lists.proxmox.com
> https://antiphishing.cetsi.fr/proxy/v3?i=SXVFem5DOGVpUU1rNjdmQuxbAYzjRE578NJDXO0bRW0&r=bWt1djZ5QzcyUms5R1Nzatwfz4p60Sh_bGp_TdGIYHovbc8XVtFiCyXKb5Z3syuM&f=Q3ZQNmU2SnpsRFlRbUF3dmj8RjqNbw-iHBi50S-iFw7wq1QYxMyfpOzIpg23W0yvT_m-5a44zDEurFGuVbLSbw&u=https%3A//lists.proxmox.com/cgi-bin/mailman/listinfo/pve-devel&k=syJL
>