[pve-devel] [PATCH pve-manager] API2 : Network : add network config reload

[Wolfgang Bumiller]

On Fri, Jun 22, 2018 at 03:06:26AM +0200, Alexandre Derumier wrote:
> This add a new api to online reload networking configuration
> with ifupdown2.
> 
> This work with native ifupdown2 modules, as ifupdown2 have
> interface dependency relationships.
> 
> an optional "restart" param is available, for non native modules
> like openvswitch.

I'm not sure that's a good idea. In theory `restart` would have worked
before as well, but the consequence in this case is *always* that the
API call fails due to a connection loss.

IMO we should only ever do 'reloads' - they too can kill the connection
of course, but restart *always* does.
And it would probably look better for the caller (UI) if it returned a
task UPID and runs in the background. That way the call can always
finish first (the actual execution of the reload command should probably
be delayed by a few seconds to be sure), and we can later see the task
in the tasklog - if it works. This would probably make it easier to
handle on the web UI side.

> ---
>  PVE/API2/Network.pm | 65 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 65 insertions(+)
> 
> diff --git a/PVE/API2/Network.pm b/PVE/API2/Network.pm
> index 92256863..3fb71617 100644
> --- a/PVE/API2/Network.pm
> +++ b/PVE/API2/Network.pm
> @@ -471,6 +471,71 @@ __PACKAGE__->register_method({
>     }});
>  
>  __PACKAGE__->register_method({
> +    name => 'reload_network_config', 
> +    path => '', 
> +    method => 'PUT',
> +    permissions => {
> +	check => ['perm', '/nodes/{node}', [ 'Sys.Modify' ]],
> +    },
> +    description => "Reload network configuration",
> +    protected => 1,
> +    proxyto => 'node',
> +    parameters => {
> +    	additionalProperties => 0,
> +	properties => {
> +	    node => get_standard_option('pve-node'),
> +	    restart => {
> +		type => 'boolean',
> +		description => "restart networking.",
> +		optional => 1,
> +	    }
> +	},
> +    },
> +    returns => { type => "null" },
> +    code => sub {
> +
> +	my ($param) = @_;
> +
> +	my $action = $param->{restart} ? "restart" : "reload";
> +	my $current_config_file = "/etc/network/interfaces";
> +	my $new_config_file = "/etc/network/interfaces.new";
> +
> +	raise_param_exc({ config => "you need ifupdown2 to $action networking" }) if !-e '/usr/share/ifupdown2';
> +	raise_param_exc({ config => "no new network config to apply" }) if !-e $new_config_file;
> +
> +	my $tmp = PVE::INotify::read_file('interfaces', 1);
> +	my $config = $tmp->{data};
> +	my $changes = $tmp->{changes};
> +
> +	raise_param_exc({ config => "no changes detected" }) if !$changes;
> +
> +	my $ovs_changes = undef;
> +	my $bridges_delete = {};
> +	my @lines = split(/\n/, $changes);
> +	foreach my $line (@lines) {
> +	   if($line =~ m/^\-iface\s(vmbr(\S+))/) {
> +		$bridges_delete->{$1} = 1;
> +	   } elsif ($line =~ m/ovs_type/) {
> +		$ovs_changes = 1;
> +	   } 
> +	}
> +
> +	raise_param_exc({ config => "reloading config with ovs changes is not possible currently, please use restart" }) 
> +	    if $ovs_changes && !$param->{restart};
> +
> +	#fixme : restart : check if vm are running on bridge, and try to reattach them ?
> +	raise_param_exc({ config => "restart config with bridge delete is not possible currently, please use reload" }) 
> +	    if keys %$bridges_delete && $param->{restart};
> +
> +	PVE::Tools::file_copy($new_config_file, $current_config_file);
> +	unlink $new_config_file;
> +
> +	PVE::Tools::run_command(['systemctl', $action, 'networking']);
> +
> +	return undef;
> +   }});
> +
> +__PACKAGE__->register_method({
>      name => 'delete_network', 
>      path => '{iface}', 
>      method => 'DELETE',
> -- 
> 2.11.0