quattor / configuration-modules-core

Node Configuration Manager Components for Everyone
www.quattor.org
Other
6 stars 54 forks source link

ncm-network: nmstate default gateway issue #1655

Closed wdpypere closed 5 months ago

wdpypere commented 6 months ago

@aka7 I found another discrepancy with nmstate, with gateways. on our site we only set /system/network/default_gateway. something like:

+-/system/network
  $ default_gateway : '10.141.10.250'
  $ domainname : 'domain'
  $ hostname : 'hostname'
  +-interfaces
    +-eth0
      $ broadcast : '10.141.31.255'
      $ driver : 'bnx2'
      $ ip : '10.141.10.62'
      $ netmask : '255.255.224.0'
      +-route
        +-0
...

but in the code I see:

    if ($default_gw) {
        # create only default gw entry if gw entry match interface gateway defined                                                                           
        # otherwise this interface is not the default gw interface.                                                                                          
    if ((defined($iface->{gateway})) and ($iface->{gateway} eq $default_gw)) {
            $default_rt{destination} = '0.0.0.0/0';
            $default_rt{'next-hop-address'} = $default_gw;
            $default_rt{'next-hop-interface'} = $device;
    }
    }

the defined($iface->{gateway}) always fails, so no default gateway is set. My perl is quite poor but it seems the code makes the assumption that a gateway is set on interface level. while network.pm does not. Unless I'm mistaken.

I would expect a route like the following to show up:

Destination     Gateway         Genmask         Flags Metric Ref    Use Iface
0.0.0.0         10.141.10.250   0.0.0.0         UG    0      0        0 eth0
aka7 commented 6 months ago

@wdpypere probably worth understanding your profile settings. so if I understood this right, you have a default gateway defined only, and non of the interface config has gw defined? I found this to be tricky with nmstate.
To add a route via nmstate it needs next-hope-interface. So you can't just add a default route without a next-hop-interface, in nmstate config, sadly. All our profile configs has gw on interface level and a default_gateway defined, which is why I havent yet notice this issue.

So what this part of code currently supports if default_gateway is defined and tries match the same with the interface gw settings so it can pick the next-hope-interface. I'm not sure how we can fix this, other than you making sure your interface config has gw? This highlights that we may need to add a check in the schema to fail compile. Something to discuss on our calls.

wdpypere commented 6 months ago

@aka7 yes, we are on the same page. :smile: none of our interfaces have a gw defined. we define gateways on routes and a default gateway.

My preference is of course backwards compatibility on the profiles, but if that is not possible then yes, it would be good if it would fail to compile. a system should have at least a fuctional default gateway.

stdweird commented 6 months ago

i will have a look at this. default gateway should be something we can set/configure.

stdweird commented 6 months ago

@aka7 the schema already has following code

} with {
    if (exists(SELF['default_gateway'])) {
        reachable = false;
        # is there any interface that can reach it?
        foreach (name; data; SELF['interfaces']) {
            if (exists(data['ip']) && exists(data['netmask']) &&
                ip_in_network(SELF['default_gateway'], data['ip'], data['netmask'])) {
                reachable = true;
            };
        };
        if (!reachable) {
            error("No interface with ip/mask found to reach default gateway");
        };
    };
    true;
};

i guess we would need something similar in perl or in our profiles to add a /0 route to the interface.

i looked into nmstate and networkmanager a bit, and it's not that clear how to set it (eg https://askubuntu.com/questions/1073692/network-manager-no-default-route). looks like you need to set the "gateway" value in the networkmanger config file instead of only adding a route. not sure how to do that with nmstate.

@wdpypere i guess the easiest way for now is to add some "run-last" template that based on code above adds a route with prefix 0 to the interface that it can reach.

i do think that this is better done in perl though, but given on the test above, it's not that hard in pan.

aka7 commented 6 months ago

@stdweird I already had chat with Redhat about this, if we can add default route (or any route) without having to add next-hop-interface in nmstate and answer was no. However if we think its valid requirement perhaps I can push them to create a RFE for it.

Thinking about this a bit more, we can perhaps change the perl code to add the next-hop-interface to default DGW entry if DGW falls within the subnet boundary of that interface? instead of what it does currently.

alternatively, we may need to add the support for command option in nmstate too.

aka7 commented 6 months ago

Thinking about this a bit more, we can perhaps change the perl code to add the next-hop-interface to default DGW entry >>if DGW falls within the subnet boundary of that interface? instead of what it does currently.

@wdpypere made a change to pick the right interface without looking at gateway on the interface. I think should fix the issue.
This appears to work for me, well, its a noop for me now, so looks good. Therefore pull this and try it, please?

https://github.com/quattor/configuration-modules-core/pull/1656

Other commits of this is in my other PR which is yet to be merged but are needed here.

wdpypere commented 6 months ago

As I commented in the PR, it does fix this issue.