cloudinit/net: handle two different routes for the same ip

[esposem]

Proposed Commit Message

If we set a dhcp server side like this:
$ cat /var/tmp/cloud-init/cloud-init-dhcp-f0rie5tm/dhcp.leases
lease {
...
option classless-static-routes 31.169.254.169.254 0.0.0.0,31.169.254.169.254
    10.112.143.127,22.10.112.140 0.0.0.0,0 10.112.140.1;
...
}
cloud-init fails to configure the routes via 'ip route add' because to there are
two different routes for 169.254.169.254:

$ ip -4 route add 192.168.1.1/32 via 0.0.0.0 dev eth0
$ ip -4 route add 192.168.1.1/32 via 10.112.140.248 dev eth0

But NetworkManager can handle such scenario successfully as it uses "ip route append".
So change cloud-init to also use "ip route append" to fix the issue:

$ ip -4 route append 192.168.1.1/32 via 0.0.0.0 dev eth0
$ ip -4 route append 192.168.1.1/32 via 10.112.140.248 dev eth0

Signed-off-by: Emanuele Giuseppe Esposito <eesposit@redhat.com>

RHBZ: #2003231

Checklist:

• My code follows the process laid out in the documentation
• I have updated or added any unit tests accordingly
• I have updated or added any documentation accordingly

[TheRealFalcon]

@esposem , can you give me a little more context for this change? Why are you seeing the same route being added twice? How does using append fix it? Trying append twice still gives the same error for me:

$ ip -4 route append 192.168.1.1/32 dev eth0
$ ip -4 route append 192.168.1.1/32 dev eth0
RTNETLINK answers: File exists

This is in the Ephemeral IP network code, so if the route needed may already exist, does it make more sense to try the connectivity url instead?

[zhaohuijuan]

@esposem , can you give me a little more context for this change? Why are you seeing the same route being added twice? How does using append fix it? Trying append twice still gives the same error for me:

$ ip -4 route append 192.168.1.1/32 dev eth0
$ ip -4 route append 192.168.1.1/32 dev eth0
RTNETLINK answers: File exists

This is in the Ephemeral IP network code, so if the route needed may already exist, does it make more sense to try the connectivity url instead?

@TheRealFalcon , thanks for the review and questions. The context for this change comes from a specific configuration from customer.

1. It fixs the scenario as below:
   $ ip -4 route append 192.168.1.1/32 via 0.0.0.0 dev eth0
   $ ip -4 route append 192.168.1.1/32 via 10.112.140.248 dev eth0
   Not the same two routes as you mentioned below:
   $ ip -4 route append 192.168.1.1/32 dev eth0
   $ ip -4 route append 192.168.1.1/32 dev eth0

2. This bug comes from customer who set up routes in dhcp server side like this:
   $ cat /var/tmp/cloud-init/cloud-init-dhcp-f0rie5tm/dhcp.leases
   lease {
   ...
   option classless-static-routes 31.169.254.169.254 0.0.0.0,31.169.254.169.254 10.112.143.127,22.10.112.140 0.0.0.0,0 10.112.140.1;
   ...
   }
   For the " option classless-static-routes 31.169.254.169.254 0.0.0.0,31.169.254.169.254 10.112.143.127,22.10.112.140 0.0.0.0,0 10.112.140.1;", cloud-init failed to configure it via 'ip route add' due to there are two different routes for 169.254.169.254. But NetworkManager can handle such scenario successfully as it uses "ip route append". So cloud-init also changes to "ip route append" which can fix the issue.

3. For the scenario you mentioned:
   $ ip -4 route append 192.168.1.1/32 dev eth0
   $ ip -4 route append 192.168.1.1/32 dev eth0
   I think it is another scenario, they are exactly the same route, although it reports error in log when config the second route, but the first route has been set up successfully, so anyway cloud-init can config the route successfully. And I think the error log also makes sense because it can reminder users that they config duplicate routes.

[TheRealFalcon]

Thanks, this makes sense now.

[TheRealFalcon]

Sorry, shouldn't have hit that approve quite yet.

The change here looks good, but we still need an updated unit test as well. You should just be able to modify the currently failing one.

[esposem]

Thank you @zhaohuijuan for the explanation, and @TheRealFalcon for the review! I updated the failing test case, and gave a better PR description according to Huijuan comments.

[TheRealFalcon]

LGTM now, thanks! I pushed a rebase of main due to a dependency issue. I can merge once CI completes successfully.