Skip to content

Commit

Permalink
ncm-network: support for managing network using nmstate (quattor#1601)
Browse files Browse the repository at this point in the history
* ncm-network: support for managing network using nmstate

A new module to manage networking using nmstate declarative approach.
Reuses functions from network.pm as much as possible.
Changes made in network.pm are minimal and should be backward compatible for existing configs.

The following is supported in the initial tech preview:
- single interface and bonding
- vlan interface
- policy routing
- ethtool setting via cli support only.
- new nm_manage_dns option is added to schema to decided if NM should manage DNS, we use ncm-resolver so to avoid clash. NM by default manages DNS which will interfere with ncm-resolver.
- aliases, bridge etc configuration not yet supported, and possibly many other advanced options I haven't thought about yet.
---------
Co-authored-by: Abdul Karim <Abdul.Karim@morganstanley.com>
Co-authored-by: stdweird <stijn.deweirdt@ugent.be>
  • Loading branch information
aka7 authored and jrha committed Aug 22, 2023
1 parent a599c26 commit 2988cd0
Show file tree
Hide file tree
Showing 6 changed files with 954 additions and 40 deletions.
14 changes: 14 additions & 0 deletions ncm-network/src/main/pan/components/network/config-nmstate.pan
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# ${license-info}
# ${developer-info}
# ${author-info}

unique template components/network/config-nmstate;

include 'components/network/config';

prefix "/software/components/network";
"ncm-module" = "nmstate";

# Add dependency that can't be added to rpm directly
prefix '/software/packages';
'nmstate' = dict();
14 changes: 8 additions & 6 deletions ncm-network/src/main/pan/components/network/core-schema.pan
Original file line number Diff line number Diff line change
Expand Up @@ -49,11 +49,11 @@ type structure_route = {
};
if (ipv6) {
if (!is_ipv6_prefix_length(pref)) {
error(format("Prefix %s is not a valid IPv6 prefix", pref));
error("Prefix %s is not a valid IPv6 prefix", pref);
};
} else {
if (!is_ipv4_prefix_length(pref)) {
error(format("Prefix %s is not a valid IPv4 prefix", pref));
error("Prefix %s is not a valid IPv4 prefix", pref);
};
};
};
Expand Down Expand Up @@ -345,12 +345,12 @@ type structure_interface = {
};
if (exists(SELF['ip']) && exists(SELF['netmask'])) {
if (exists(SELF['gateway']) && ! ip_in_network(SELF['gateway'], SELF['ip'], SELF['netmask'])) {
error(format('networkinterface has gateway %s not reachable from ip %s with netmask %s',
SELF['gateway'], SELF['ip'], SELF['netmask']));
error('networkinterface has gateway %s not reachable from ip %s with netmask %s',
SELF['gateway'], SELF['ip'], SELF['netmask']);
};
if (exists(SELF['broadcast']) && ! ip_in_network(SELF['broadcast'], SELF['ip'], SELF['netmask'])) {
error(format('networkinterface has broadcast %s not reachable from ip %s with netmask %s',
SELF['broadcast'], SELF['ip'], SELF['netmask']));
error('networkinterface has broadcast %s not reachable from ip %s with netmask %s',
SELF['broadcast'], SELF['ip'], SELF['netmask']);
};
};
if (exists(SELF['plugin']) && exists(SELF['plugin']['vxlan']) && ! exists(SELF['physdev'])) {
Expand Down Expand Up @@ -418,6 +418,8 @@ type structure_network = {
"set_hwaddr" ? boolean
"nmcontrolled" ? boolean
"allow_nm" ? boolean
@{let NetworkManager manage the dns (only for nmstate)}
"nm_manage_dns" : boolean = false
"primary_ip" ? string
"routers" ? structure_router{}
"ipv6" ? structure_ipv6
Expand Down
117 changes: 83 additions & 34 deletions ncm-network/src/main/perl/network.pm
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ An example:
=cut

use 5.10.1;
use parent qw(NCM::Component CAF::Path);

our $EC = LC::Exception::Context->new->will_store_all;
Expand Down Expand Up @@ -144,10 +145,10 @@ Readonly my $BRIDGECMD => '/usr/sbin/brctl';
Readonly my $IPADDR => [qw(ip addr show)];
Readonly my $IPROUTE => [qw(ip route show)];
Readonly my $OVS_VCMD => '/usr/bin/ovs-vsctl';
Readonly my $HOSTNAME_CMD => '/usr/bin/hostnamectl';
Readonly our $HOSTNAME_CMD => '/usr/bin/hostnamectl';
Readonly my $ROUTING_TABLE => '/etc/iproute2/rt_tables';

Readonly my $NETWORK_PATH => '/system/network';
Readonly our $NETWORK_PATH => '/system/network';
Readonly my $HARDWARE_PATH => '/hardware/cards/nic';

# Regexp for the supported ifcfg-<device> devices.
Expand Down Expand Up @@ -177,22 +178,38 @@ Readonly my $DEVICE_REGEXP => qr{
$
}x;

Readonly my $IFCFG_DIR => "/etc/sysconfig/network-scripts";
Readonly my $NETWORKCFG => "/etc/sysconfig/network";
Readonly our $NETWORKCFG => "/etc/sysconfig/network";

Readonly my $RESOLV_CONF => '/etc/resolv.conf';
Readonly my $RESOLV_CONF_SAVE => '/etc/resolv.conf.save';
Readonly my $RESOLV_SUFFIX => '.ncm-network';
Readonly our $RESOLV_CONF => '/etc/resolv.conf';
Readonly our $RESOLV_CONF_SAVE => '/etc/resolv.conf.save';
Readonly our $RESOLV_SUFFIX => '.ncm-network';

Readonly my $FAILED_SUFFIX => '-failed';
Readonly my $BACKUP_DIR => "$IFCFG_DIR/.quattorbackup";
Readonly our $FAILED_SUFFIX => '-failed';

Readonly my $REMOVE => -1;
Readonly my $NOCHANGES => 0;
Readonly my $UPDATED => 1;
Readonly my $NEW => 2;
Readonly our $BACKUP_DIR_SUFFIX => '.quattorbackup';

Readonly our $REMOVE => -1;
Readonly our $NOCHANGES => 0;
Readonly our $UPDATED => 1;
Readonly our $NEW => 2;
# changes to file, but same config (eg for new file formats)
Readonly my $KEEPS_STATE => 3;
Readonly our $KEEPS_STATE => 3;

# automatic exports of readonlys
our @EXPORT = qw($FAILED_SUFFIX
$REMOVE $NOCHANGES $UPDATED $NEW $KEEPS_STATE
$RESOLV_CONF $RESOLV_CONF_SAVE $RESOLV_SUFFIX
$NETWORKCFG $NETWORK_PATH $HOSTNAME_CMD
);

# list of constants to allow inheritance via $self->CONSTANTNAME
use constant IFCFG_DIR => "/etc/sysconfig/network-scripts";

sub backup_dir
{
my ($self) = @_;
return $self->IFCFG_DIR . "/$BACKUP_DIR_SUFFIX";
}


# wrapper around -x for easy unittesting
Expand Down Expand Up @@ -266,6 +283,12 @@ sub ethtool_get_current
return %current;
}

sub iface_filename
{
my ($self, $iface) = @_;
return $self->IFCFG_DIR . "/ifcfg-$iface";
}

# backup_filename: returns backup filename for given file
sub backup_filename
{
Expand All @@ -274,7 +297,7 @@ sub backup_filename
my $back = "$file";
$back =~ s/\//_/g;

return "$BACKUP_DIR/$back";
return $self->backup_dir() . "/$back";
}

# Generate the filename to hold the test configuration data
Expand Down Expand Up @@ -447,6 +470,13 @@ sub test_network_ccm_fetch
}
}


sub get_current_config_post
{
my ($self) = @_;
return "";
}

# Gather current network configuration using available tools
# Is gathered for debugging in case of failure.
sub get_current_config
Expand All @@ -456,8 +486,8 @@ sub get_current_config
my $fh = CAF::FileReader->new($NETWORKCFG, log => $self);
my $output = "$NETWORKCFG\n$fh";

$output .= "\nls -lrt $IFCFG_DIR\n";
$output .= $self->runrun(['ls', '-ltr', $IFCFG_DIR]);
$output .= "\nls -lrt " . $self->IFCFG_DIR . "\n";
$output .= $self->runrun(['ls', '-ltr', $self->IFCFG_DIR]);

$output .= "\n@$IPADDR\n";
$output .= $self->runrun($IPADDR);
Expand All @@ -483,6 +513,8 @@ sub get_current_config
$output .= "\nMissing $OVS_VCMD executable or socket.\n";
};

$output .= $self->get_current_config_post();

return $output;
}

Expand Down Expand Up @@ -757,6 +789,21 @@ sub process_network
if (defined($opts) && keys %$opts) {
$iface->{$attr} = [map {"$_=$opts->{$_}"} sort keys %$opts];
$self->debug(1, "Replaced $attr with ", join(' ', @{$iface->{$attr}}), " for interface $ifname");

# for bonding_opts, we need linkagregation settings for nmstate.
# this should not impact existing configs as it adds interface/$name/link_aggregation
if ($attr eq "bonding_opts"){
foreach my $opt (sort keys %$opts){
$iface->{link_aggregation} ||= {};
my $la = $iface->{link_aggregation};
if ($opt ne 'mode') {
$la->{options} ||= {};
$la = $la->{options};
}
$la->{$opt} = $opts->{$opt};
}
}
# TODO for briging_opts
}
}

Expand Down Expand Up @@ -854,16 +901,16 @@ sub gather_existing
my (%exifiles, %exilinks);

# read current config
my $files = $self->listdir($IFCFG_DIR, test => sub { return $self->is_valid_interface($_[0]); });
my $files = $self->listdir($self->IFCFG_DIR, test => sub { return $self->is_valid_interface($_[0]); });
foreach my $filename (@$files) {
if ($filename =~ m/^([:\w.-]+)$/) {
$filename = $1; # untaint
} else {
$self->warn("Cannot untaint filename $IFCFG_DIR/$filename. Skipping");
$self->warn("Cannot untaint filename " . $self->IFCFG_DIR . "/$filename. Skipping");
next;
}

my $file = "$IFCFG_DIR/$filename";
my $file = $self->IFCFG_DIR . "/$filename";

my $msg;
if ($self->is_symlink($file)) {
Expand Down Expand Up @@ -1307,7 +1354,7 @@ sub make_ifdown
my $valid = $self->is_valid_interface($file);
if ($valid) {
my ($iface, $ifupdownname) = @$valid;

my $cfg_filename = $self->iface_filename($iface);
# ifdown: all devices that have files with non-zero status
if ($value == $NOCHANGES) {
$self->verbose("No changes for interface $iface (cfg file $file)");
Expand Down Expand Up @@ -1344,7 +1391,7 @@ sub make_ifdown
$ifdown{$attached} = 1;
}
}
} elsif ($file eq "$IFCFG_DIR/ifcfg-$iface" && $self->any_exists($file)) {
} elsif ($file eq "$cfg_filename" && $self->any_exists($file)) {
# here's the tricky part: see if it used to be a slave.
# the bond-master must be restarted if a device was removed from the bond.
# TODO: why read from backup?
Expand Down Expand Up @@ -1386,8 +1433,10 @@ sub make_ifup
# and have state other than REMOVE
# e.g. master with NOCHANGES state can be added here
# when a slave had modifications
if (exists($exifiles->{"$IFCFG_DIR/ifcfg-$iface"}) &&
$exifiles->{"$IFCFG_DIR/ifcfg-$iface"} == $REMOVE) {
my $cfg_filename = $self->iface_filename($iface);

if (exists($exifiles->{"$cfg_filename"}) &&
$exifiles->{"$cfg_filename"} == $REMOVE) {
$self->verbose("Not starting $iface scheduled for removal");
} else {
if ($ifaces->{$iface}->{master}) {
Expand Down Expand Up @@ -1583,7 +1632,7 @@ sub recover

$self->error("Network restart failed. Reverting back to original config. ",
"Failed modified configfiles can be found in ",
"$BACKUP_DIR with suffix $FAILED_SUFFIX. ",
$self->backup_dir() . " with suffix $FAILED_SUFFIX. ",
"(If there aren't any, it means only some devices were removed.)");

# stop/recover/start whole network is the only thing that should always work.
Expand Down Expand Up @@ -1673,12 +1722,12 @@ sub init_backupdir
{
my $self = shift;

if (!defined($self->cleanup($BACKUP_DIR, undef, keeps_state => 1))) {
$self->error("Failed to cleanup previous backup directory $BACKUP_DIR: $self->{fail}");
if (!defined($self->cleanup($self->backup_dir(), undef, keeps_state => 1))) {
$self->error("Failed to cleanup previous backup directory " . $self->backup_dir() . ": $self->{fail}");
return;
}
if (!defined($self->directory($BACKUP_DIR, mode => oct(700), keeps_state => 1))) {
$self->error("Failed to create backup directory $BACKUP_DIR: $self->{fail}");
if (!defined($self->directory($self->backup_dir(), mode => oct(700), keeps_state => 1))) {
$self->error("Failed to create backup directory " . $self->backup_dir() . ": $self->{fail}");
return;
}

Expand Down Expand Up @@ -1992,7 +2041,7 @@ sub Configure
my $iface = $ifaces->{$ifacename};
my $text = $self->make_ifcfg($ifacename, $iface, $ipv6);

my $file_name = "$IFCFG_DIR/ifcfg-$ifacename";
my $file_name = $self->iface_filename($ifacename);
$exifiles->{$file_name} = $self->file_dump($file_name, $text);

$self->default_broadcast_keeps_state($file_name, $ifacename, $iface, $exifiles, 0);
Expand All @@ -2013,13 +2062,13 @@ sub Configure
# pass device, not system interface name
my $text = $self->$method($flavour, $iface->{device} || $ifacename, $iface->{$flavour});

my $file_name = "$IFCFG_DIR/$flavour-$ifacename";
my $file_name = $self->IFCFG_DIR . "/$flavour-$ifacename";
$exifiles->{$file_name} = $self->file_dump($file_name, $text);
}
}

# legacy IPv4 format
$file_name = "$IFCFG_DIR/route-$ifacename";
$file_name = $self->IFCFG_DIR . "/route-$ifacename";
if (exists($exifiles->{$file_name}) && $exifiles->{$file_name} == $UPDATED) {
# IPv4 route data was modified.
# Check if it was due to conversion of legacy format or
Expand All @@ -2035,7 +2084,7 @@ sub Configure
my $al_iface = $iface->{aliases}->{$al};
my $text = $self->make_ifcfg_alias($al_dev, $al_iface);

my $file_name = "$IFCFG_DIR/ifcfg-$ifacename:$al";
my $file_name = $self->IFCFG_DIR . "/ifcfg-$ifacename:$al";
$exifiles->{$file_name} = $self->file_dump($file_name, $text);

$self->default_broadcast_keeps_state($file_name, $al_dev, $al_iface, $exifiles, 1);
Expand All @@ -2048,7 +2097,7 @@ sub Configure
# Problem is, we want both
# Adding symlinks however is not the best thing to do.

my $file_name_sym = "$IFCFG_DIR/ifcfg-$al_dev";
my $file_name_sym = $self->IFCFG_DIR . "/ifcfg-$al_dev";
if ($iface->{vlan} &&
$file_name_sym ne $file_name &&
! $self->any_exists($file_name_sym)) { # TODO: should check target with readlink
Expand Down
Loading

0 comments on commit 2988cd0

Please sign in to comment.