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.

Sign up for GitHub

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

Jump to bottom

Routing rules cannot refer to routing tables by name #506

Closed
bsteinb opened this issue Jun 1, 2022 · 2 comments
Closed

Routing rules cannot refer to routing tables by name #506

bsteinb opened this issue Jun 1, 2022 · 2 comments

Comments

@bsteinb
Copy link

bsteinb commented Jun 1, 2022

Thank you for adding support for routing rules. The feature is working well for us. However, as it is, it does not support referring to routing tables by name, only by their numerical id, e.g.:

      routing_rule:
        - priority: 99
          family: ipv4
          iif: eno1
          action: to-table
          table: main

results in task "Configure networking connection profiles" to fail with:

  msg: 'fatal error: Must be number, not str'

while it works with table: 254.

It is possible to refer to tables by name when specifying ordinary routes on an interface (ip.routes.table) and indeed both code paths use ArgValidatorRouteTable to validate the table value, which does allow int or str. However, that validator class does not normalize the value to number by looking up the mapping. That happens later in ArgValidator_ListConnections.validate_route_tables which only works on tables specified as part of ordinary routes, not on routing rules. Either this method validate_route_tables should be modified to also normalize tables specified as part of routing rules or maybe the normalization can be moved into ArgValidatorRouteTable.
@liangwen12year
Copy link
Collaborator

Sorry for replying late, thanks for catching this, I will propose a fix soon.

@liangwen12year
Copy link
Collaborator

Fixed by #526

liangwen12year added a commit to liangwen12year/network that referenced this issue Sep 28, 2022
@liangwen12year
Support looking up named route table in routing rule
03f88e2 
The user may need to define the named route table in the routing rule
besides the table id, add support for that.

The commit fixes
linux-system-roles#506.

Signed-off-by: Wen Liang <liangwen12year@gmail.com>
richm added a commit to richm/linux-system-roles-network that referenced this issue Nov 1, 2022
@richm
Version 1.10.0 - CHANGELOG.md [citest skip]
0c8a597 
[1.10.0] - 2022-11-01
--------------------

### New Features

- Support looking up named route table in routing rule

The user may need to define the named route table in the routing rule
besides the table id, add support for that.

The commit fixes
linux-system-roles#506.

- Support 'route_metric4' for initscripts provider

The user want to change the metric for the default route, add support
for that.

https://bugzilla.redhat.com/show_bug.cgi?id=2134201

- Support the DNS priority

The users want to configure the priority of DNS servers, add support for
that.

Fixes linux-system-roles#505.

Signed-off-by: Wen Liang <liangwen12year@gmail.com>

### Bug Fixes

- bond: improve the validation for setting peer_notif_delay

Synchronize with NM, the default value of peer_notif_delay in NM is 0,
which is not considered as enabling the setting or specifying the
delay.

- bond: test arp_all_targets only when arp_interval is enabled

Kernel allows to set `arp_all_targets` when `arp_interval` is disabled
(disable ARP monitoring). But `arp_all_targets` specifies the quantity
of `arp_ip_targets` that must be reachable in order for the ARP monitor
to consider a slave as being up. It makes more sense to only set the
`arp_all_targets` while enabling the `arp_interval`.

- bond: attach ports when creating the bonding connection

When ports are not attached, the bonding connection may risk not in fully
connected state, e.g. connecting (getting IP configuration). Therefore,
attach ports for the bonding connection.

### Other Changes

- Set the route metric when testing the 'auto_gateway'

For initscripts provider, the metric for the default route defaults to
0, as a result, the default route can take precedence or blindly ingore
other routes. Adding a higher route metric value to honor other routes
during the route selection.

- Fix markdownlint 'unordered list indentation' issue

- add ip.route_metric4: 65535 to failing bond tests

When creating a bond, the bond also creates a default route with a
default metric of 0.  This causes test failures on CI systems as
it overrides the system default route.  Use the new `ip.route_metric4`
parameter to set a high metric value so as not to override the
default system route.
Some systems cannot use a metric value of 32 bit unsigned int max value.  To ensure
the broadest possible support, use a metric value of 16 bit signed int max value,
which should be high enough to ensure the routes always have the lowest priority.

- use rpm -i instead of yum install for epel7

On BaseOS CI systems, `yum install` for the epel7 rpm does not work.
Instead, use `rpm -i` which should work on any system.  We should not
need to use `yum install` here because the epel7 setup rpm does not have
additional dependencies.
In addition, the rpm download sometimes returns 403 - I think it is because
multiple tests run in parallel in BaseOS CI, resulting in too many
download attempts in too short a period of time - so introduce a retry
to mitigate the situation.

Signed-off-by: Rich Megginson <rmeggins@redhat.com>
@richm richm mentioned this issue Nov 1, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@bsteinb @liangwen12year