ncm-network: networkmanager module to configure using keyfile format #1587
Conversation
aka7
force-pushed
the
ncm_network_nm_support
branch
3 times, most recently
from
e757a81
to
b9bb43f
Compare
aka7
force-pushed
the
ncm_network_nm_support
branch
from
2afabd1
to
3f83015
Compare
starting a new module to manage networking using networkmanager keyfile format. using most of the existing logic. - slight tweak to file_dump and process_network to make it work with NM config. - single interface, and bonding is currenlty supported. - new nm_manage_dns option is added to schema to decided if NM should manage dns. NM by default manages dns which will intefere with ncm-resolver. - ethtool,vlan,bridge configuration not yet supported.
added to support policy routing config for networkmanegr keyfile. minor updates to comments add TODO comment to provide aliases support at later date.
aka7
force-pushed
the
ncm_network_nm_support
branch
from
3f83015
to
8bdc9d5
Compare
| my $fh = CAF::FileWriter->new($testcfg, log => $self, keeps_state => 1); | ||
| my $fh; | ||
| # files in nmconnection has to be mode 0400 otherwise networkmanager won't read it. | ||
| if ($file =~ /\.nmconnection$/) { |
There was a problem hiding this comment.
hmm, i don't like copy paste.
can you test
# files in nmconnection has to be mode 0400 otherwise networkmanager won't read it.
my $mode;
$mode = 0400 if ($file =~ /\.nmconnection$/);
my $fh = CAF::FileWriter->new($testcfg, log => $self, keeps_state => 1, mode => $mode);
| @@ -839,6 +846,14 @@ sub process_network | |||
| } | |||
| $iface->{my_inner_ipaddr} .= "/$iface->{my_inner_prefix}"; | |||
| } | |||
|
|
|||
| # Newly added used when generating the keyfile format config in networkmanager module | |||
| my $media = $nic->{media}; | |||
| @@ -839,6 +846,14 @@ sub process_network | |||
| } | |||
| $iface->{my_inner_ipaddr} .= "/$iface->{my_inner_prefix}"; | |||
| } | |||
|
|
|||
| # Newly added used when generating the keyfile format config in networkmanager module | |||
There was a problem hiding this comment.
i don't understand the comment
| # Newly added used when generating the keyfile format config in networkmanager module | ||
| my $media = $nic->{media}; | ||
| if ($media) { | ||
| $iface->{media} = $media; |
There was a problem hiding this comment.
is this something we should support in the schema? if you want to set (or is it preserve?) it, you should do this via the schema.
| $cmp->disable_networkmanager(1); | ||
| # check there a executed commands that match NetworkManager | ||
| ok(command_history_ok(undef, ['NetworkManager'])); | ||
|
|
There was a problem hiding this comment.
some more tests might be good ;)
| Readonly my $KEEPS_STATE => 3; | ||
|
|
||
|
|
||
| # NOTE: Had to pull in some fucntions from network.pm in order to satisfy location of new paths. |
There was a problem hiding this comment.
this shouldn't be required. if you redefine the readonly here, you should be able to use them in the parent. but the parent might need changes. i can look for an example where i managed to do this in perl.
There was a problem hiding this comment.
ok, it's coming back. readonly are real variables, as compared to the use constant. i would suggest to make subs like
sub _backup_dir { "whatever" }
and use $self->_backup_dir instead of $BAKCUP_DIR here or in the parent network.pm
| Readonly my $RESOLV_CONF_SAVE => '/etc/resolv.conf.save'; | ||
| Readonly my $RESOLV_SUFFIX => '.ncm-network'; | ||
|
|
||
| # need this to remain same value as what is in network.pm |
There was a problem hiding this comment.
you can import the values or perhaps use them straight away. there should be no need for this.
| return "$BACKUP_DIR/$back"; | ||
| } | ||
|
|
||
| # Look for existing interface configuration files (and symlinks) |
There was a problem hiding this comment.
what is diffeent here? is it only the NMCFG_DIR?
| return (\%exifiles, \%exilinks); | ||
| } | ||
|
|
||
| # Create new backupdir |
starting a new module to manage networking using networkmanager keyfile format. using most of the existing logic.
NOTE: opening this up for comments and suggestions, not expecting this PR to be merged. I have no doubt more work will be required on this. I've pulled in any functions that needs modification here purely so I don't make any changes to network.pm.
Ensure the title of this pull-request starts with the component name followed by a colon, e.g.