Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Networkmanager #136

Merged
merged 17 commits into from Apr 13, 2023
Merged

Networkmanager #136

merged 17 commits into from Apr 13, 2023

Conversation

eb4x
Copy link
Contributor

@eb4x eb4x commented Sep 16, 2022

network-scripts is deprecated (and removed from el9 releases if I'm not mistaken). In el8 networkmanager does a pretty good job of reading network-scripts, so might aswell try using it.

For el9 support we'll also need to convert the network-scripts files to nmconnection files.

@markgoddard
Copy link
Collaborator

@bbezak

Copy link
Collaborator

@markgoddard markgoddard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @eb4x. My colleague @bbezak is also looking at this, for Rocky Linux 9 support.

I wanted to leave some comments, but it might be best to discuss with @bbezak on how best to proceed.

defaults/main.yml Outdated Show resolved Hide resolved
handlers/main.yml Outdated Show resolved Hide resolved
@bbezak
Copy link
Contributor

bbezak commented Oct 7, 2022

RHEL 9 still supports old network scripts and it can also do ifup/ifdown with NetworkManager-initscripts-updown package. I've tested it on Rocky 9, and it works well - bbezak@31ce70e

I've also started adding ethernet template for native NM keyfile format, however that still needs some work - bbezak@8aad4f9

https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/9/html/considerations_in_adopting_rhel_9/assembly_networking_considerations-in-adopting-rhel-9#ref_networkmanager-networking_assembly_networking

https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/9/html-single/9.0_release_notes/index#BZ-2082303

@bbezak
Copy link
Contributor

bbezak commented Oct 7, 2022

RHEL 9 still supports old network scripts and it can also do ifup/ifdown with NetworkManager-initscripts-updown package. I've tested it on Rocky 9, and it works well - bbezak@31ce70e

I've also started adding ethernet template for native NM keyfile format, however that still needs some work - bbezak@8aad4f9

https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/9/html/considerations_in_adopting_rhel_9/assembly_networking_considerations-in-adopting-rhel-9#ref_networkmanager-networking_assembly_networking

https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/9/html-single/9.0_release_notes/index#BZ-2082303

It is even more interesting, when one creates bonding with nmcli then only old school network script got created:

nmcli connection add type bond con-name bond42 ifname bond42 ipv4.method manual ipv4.addresses '192.0.2.1/24' ipv4.gateway '192.0.2.254' ipv4.dns '192.0.2.253' bond.options "mode=active-backup,miimon=1000"

cat /etc/sysconfig/network-scripts/ifcfg-bond42
BONDING_OPTS="mode=active-backup miimon=1000"
TYPE=Bond
BONDING_MASTER=yes
PROXY_METHOD=none
BROWSER_ONLY=no
BOOTPROTO=none
IPADDR=192.0.2.1
PREFIX=24
GATEWAY=192.0.2.254
DNS1=192.0.2.253
DEFROUTE=yes
IPV4_FAILURE_FATAL=no
IPV6INIT=yes
IPV6_AUTOCONF=yes
IPV6_DEFROUTE=yes
IPV6_FAILURE_FATAL=no
IPV6_ADDR_GEN_MODE=stable-privacy
NAME=bond42
UUID=98f785d3-9c56-4aa7-9f89-ddd275e03132
DEVICE=bond42
ONBOOT=yes

@eb4x
Copy link
Contributor Author

eb4x commented Oct 7, 2022

RHEL 9 still supports old network scripts and it can also do ifup/ifdown with NetworkManager-initscripts-updown package. I've tested it on Rocky 9, and it works well - bbezak@31ce70e

Could this get merged then? Installing that package seems like a good enough option to tie us over until we actually need nmconnection files.

I've also started adding ethernet template for native NM keyfile format, however that still needs some work - bbezak@8aad4f9

Cool, I've written some nmconnection files just to get a feel for it, I'll have a look at your work :)

@eb4x
Copy link
Contributor Author

eb4x commented Oct 21, 2022

Not necessarily feature-complete, but this works for my setup.

[eno1,eno2] -> bond0 -> br-provision {ipv4 address, multiple routes}
bond0.1000 -> br-internet {dhcp}

handlers/main.yml Show resolved Hide resolved
tasks/bond_configuration.yml Outdated Show resolved Hide resolved
tasks/bond_configuration.yml Outdated Show resolved Hide resolved
tasks/bond_configuration.yml Outdated Show resolved Hide resolved
@eb4x
Copy link
Contributor Author

eb4x commented Nov 29, 2022

Really starting to like this eb4x/ansible-role-interfaces@3215b4a type of solution.

@eb4x
Copy link
Contributor Author

eb4x commented Dec 6, 2022

Updated with smol modifications. will test if they still work. (I suspect the named regex groups should have \g and not \g when single-quoted.)

@eb4x eb4x force-pushed the networkmanager branch 3 times, most recently from 4a36f95 to 1dd77c0 Compare December 6, 2022 13:37
tasks/bond_configuration.yml Show resolved Hide resolved
tasks/debian.yml Show resolved Hide resolved
@eb4x
Copy link
Contributor Author

eb4x commented Dec 9, 2022

The bounce networkmanager devices needs more thought/work. I think it needs to be a shellscript like the other one and run "con down/up" for certain interfaces to have their changes take.

@eb4x eb4x force-pushed the networkmanager branch 2 times, most recently from 35ad095 to e26d36f Compare December 10, 2022 14:06
@markgoddard
Copy link
Collaborator

The bounce networkmanager devices needs more thought/work. I think it needs to be a shellscript like the other one and run "con down/up" for certain interfaces to have their changes take.

Can you give more information about this - did it not work with a simple service restart, or was there some undesirable side-effect?

@eb4x
Copy link
Contributor Author

eb4x commented Dec 14, 2022

Can you give more information about this - did it not work with a simple service restart, or was there some undesirable side-effect?

Once the interfaces are up, if there are changes to them later in the form of new/additional ips, those kinds of changes aren't effected.

@markgoddard
Copy link
Collaborator

Can you give more information about this - did it not work with a simple service restart, or was there some undesirable side-effect?

Once the interfaces are up, if there are changes to them later in the form of new/additional ips, those kinds of changes aren't effected.

Ok, that's a problem. Let's go with the other approach then.

@eb4x
Copy link
Contributor Author

eb4x commented Dec 20, 2022

I've merged a fix from Pierre. Just modified the when clause to interfaces_use_nmconnection. I'm currently using NetworkManager with network-scripts store on EL8, (which is a possibility) and I'd rather not have any surprises.

@eb4x
Copy link
Contributor Author

eb4x commented Dec 20, 2022

Looking at ifcfg removal some more, it didn't seem quite right. Changed the with_items to a list of all interfaces we manage. Needs testing.

@eb4x
Copy link
Contributor Author

eb4x commented Dec 20, 2022

Well, that cleared out some dummy-files I created from network-scripts. So I suppose that works.

@priteau ab2d048 looks ok to you?

@eb4x eb4x force-pushed the networkmanager branch 2 times, most recently from 1e78008 to bc11343 Compare January 18, 2023 14:54
templates/bond_Debian.j2 Outdated Show resolved Hide resolved
@eb4x eb4x requested a review from markgoddard March 31, 2023 10:13
Copy link
Collaborator

@markgoddard markgoddard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking really close. Let's get @bbezak's PR merged in and fix these last few bits, then we can finally merge and move on...

templates/bond_Debian.j2 Outdated Show resolved Hide resolved
templates/bond_Debian.j2 Outdated Show resolved Hide resolved
templates/ethernet_nmconnection.j2 Show resolved Hide resolved
@markgoddard
Copy link
Collaborator

Merge conflicts

eb4x and others added 17 commits April 13, 2023 10:29
This is what NetworkManager expects the type to be, and it is
backwards compatible with network-scripts.
This keeps the NetworkManager connection names short and sweet.
NetworkManager is able to read legacy network-scripts files and
configure interfaces as intended with just a simple `nmcli
connection reload`
Shouldn't do any harm to existing usage, it'll continue to match
as before, but now we can use the groups in regex_replace
statements.
We're doing this for readability wrt. the upcoming nmconnection
patch.
Putting these nested structures into vars allows us to load only the
distribution specific values that we need.

This uses a first_found lookup instead of a with_first_found loop so that the
'paths' parameter can be used. This ensures that only vars from the role are
included, and not vars from a parent calling role. This can happen when a
parent has a higher priority vars file available for inclusion than the role
it calls.
EL9 (almalinux,centos,rhel,rocky) have changed where the default storage of
network-configuration files. And also the format, from old network-scripts to
NetworkManager keyfiles.

These new templates implement (atleast partial) functionality of
network-scripts in NetworkManager keyfile style.

https://www.redhat.com/en/blog/rhel-9-networking-say-goodbye-ifcfg-files-and-hello-keyfiles
This path can be hardcoded, it's very redhat/network-scripts
specific. And clears out any stale configuration in case of
interfaces_net_path being set to NetworkManager/system-connections.
Existing ifcfg files in /etc/sysconfig/network-scripts (for example
generated by cloud-init) shadow any nmconnection file we generate.
Remove them when using NetworkManager with system-connections store
to manage the same interfaces.
`nmcli connection reload` was all well and good for unconfigured
devices, but didn't work in most cases of reconfiguring devices,
like adding/updating IP addresses. This borrows heavily from the
previous handler for bouncing network devices, using the suggested
RedHat ordering.
By defaulting route to an empty list (when it's not defined),
there's no output from the for-loop, skipping the need for an
if defined check, which reduces nesting.
An attempt at supporting additional routing options. The example
under tests/interfaces.yml is a list of strings, 'onlink' being the
primary/only example. nmconnection wants this in the format of
option=value (e.g. onlink=true). So we're introducing the possibility
of using simple dicts as options. (i.e. '- onlink: true') while
also maintaining backwards compatability with plain strings.
Adding the option to send rules as a dictionary, we seem to need the
table_id when using nmconnections. As a dict, it's atleast feasible
to try looking up an id from a name in the templates.

We're not attempting to find the table name from a string in this
iteration. So if using nmconnections the table name needs to be
swapped for id.

It looks like the older network-scripts rules on EL-systems start
priority at 32765 and decrement as more are added, so we're mimicking
that behavior. It is also possible to override with a priority added
to the dict rule.

Also making old rule_RedHat.j2 template compatible with dicts.

Basing this on previous templates and documentation [1]
[1] https://access.redhat.com/documentation/en-us/red_hat_enterprise_linux/9/html/configuring_and_managing_networking/configuring-policy-based-routing-to-define-alternative-routes_configuring-and-managing-networking
'nmcli connection up' works asynchronously so we need to check
if interfaces are up.
Network scripts custom route needed to use dev with options.
Network manager don't need it. Ignoring dev for backwards
compatibility.
@eb4x
Copy link
Contributor Author

eb4x commented Apr 13, 2023

Thanks for catching the details here 👍

I've applied fixups to the commits introducing/containing erroneous code.

Copy link
Collaborator

@markgoddard markgoddard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Let's wait for a pass on https://review.opendev.org/c/openstack/kayobe/+/869977 then merge.

Copy link
Collaborator

@markgoddard markgoddard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great work @eb4x, @bbezak & everyone involved - well done for persevering on this one.

@markgoddard markgoddard merged commit dc80e62 into michaelrigart:master Apr 13, 2023
@markgoddard
Copy link
Collaborator

https://github.com/michaelrigart/ansible-role-interfaces/releases/tag/v1.14.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants