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

["Fabian Grünbichler" <f.gruenbichler@proxmox.com>]

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!