Skip to content

net: Document use of ip route append to add routes#2130

Merged
TheRealFalcon merged 3 commits into
canonical:mainfrom
holmanb:holmanb/route-add
Apr 21, 2023
Merged

net: Document use of ip route append to add routes#2130
TheRealFalcon merged 3 commits into
canonical:mainfrom
holmanb:holmanb/route-add

Conversation

@holmanb
Copy link
Copy Markdown
Member

@holmanb holmanb commented Apr 19, 2023

net: Document use of `ip route append` to add routes

Update
I am changing this PR to document the reason that the current code is needed for better context.

Support for rfc3442 routes was added to cloud-init in 07b172. In this
commit, `ip route append` was used to add the route. This succeeds at adding
static routes, but its documented purpose is related to multipath routing. To
avoid confusion around the undocumented behavior being used, switch to
`ip route add`, which does the same thing. This will also simplify an
abstraction to support distros that do not use iproute2.
Copy link
Copy Markdown
Contributor

@cjp256 cjp256 left a comment

Choose a reason for hiding this comment

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

LGTM

@TheRealFalcon
Copy link
Copy Markdown
Contributor

TheRealFalcon commented Apr 20, 2023

This was intentionally changed in #1124, though the code has moved files. I can't say I 100% understand the use case, but I think changing this could break some (or at least one) use cases.

@holmanb
Copy link
Copy Markdown
Member Author

holmanb commented Apr 20, 2023

This was intentionally changed in #1124, though the code has moved files. I can't say I 100% understand the use case, but I think changing this could break some (or at least one) use cases.

Great context, thanks for pointing out the requirement for this behavior @TheRealFalcon.

I just reverted the previous commit and updated this PR to include context alongside the code, since that is currently not obvious.

@holmanb holmanb changed the title net: Use ip route add to add static routes net: Document use of ip route append to add routes Apr 20, 2023
Copy link
Copy Markdown
Contributor

@TheRealFalcon TheRealFalcon left a comment

Choose a reason for hiding this comment

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

Thanks!

@TheRealFalcon TheRealFalcon merged commit 34637a4 into canonical:main Apr 21, 2023
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.

3 participants