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

Fix ValidateExpectedRoute with non default routes and nil GW #887

Merged
merged 1 commit into from Apr 24, 2023

Conversation

champtar
Copy link
Contributor

@champtar champtar commented Apr 20, 2023

Using ptp plugin with non default routes, we get the following error when cri-o call CheckNetworkList():

Expected Route {Dst:{IP:198.18.128.0 Mask:ffff8000} GW:<nil>} not found in routing table

Using cniVersion 0.3.1 to bypass the check, we can see that the route is added with a gateway

$ ip r
198.18.0.0/17 via 198.18.0.1 dev eth0 src 198.18.3.102
198.18.0.1 dev eth0 scope link src 198.18.3.102
198.18.128.0/17 via 198.18.0.1 dev eth0

If GW is nil only check if we have a route with a DST that matches, and ignore the GW.

Fixes #886

Using ptp plugin with non default routes, we get the following error
when cri-o call CheckNetworkList():
```
Expected Route {Dst:{IP:198.18.128.0 Mask:ffff8000} GW:<nil>} not found in routing table
```
Using cniVersion 0.3.1 to bypass the check, we can see that the
route is added with a gateway
```
$ ip r
198.18.0.0/17 via 198.18.0.1 dev eth0 src 198.18.3.102
198.18.0.1 dev eth0 scope link src 198.18.3.102
198.18.128.0/17 via 198.18.0.1 dev eth0
```

If GW is nil only check if we have a route with a DST that matches, and
ignore the GW.

Fixes containernetworking#886
Signed-off-by: Etienne Champetier <e.champetier@ateme.com>
@s1061123
Copy link
Contributor

lgtm!

Copy link
Member

@squeed squeed left a comment

Choose a reason for hiding this comment

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

Good catch, thanks!

@dcbw dcbw merged commit 00b82fb into containernetworking:main Apr 24, 2023
5 checks passed
@champtar champtar deleted the route-CHECK branch April 24, 2023 20:09
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.

CHECK fails with ptp plugin and non default routes
5 participants