route: correctly compare the route's next-hop

In nmstate routes without next-hop are represented with "0.0.0.0" or
"::". This is because we need to differentiate the case "I don't care
about the next-hop for routes matching" (next-hop=None) and "This route
doesn't have next-hop" (next-hop=0.0.0.0).

However, NM doesn't do the same because it doesn't need that
differentiation. If a NM profile contains a direct route, without
next-hop, it will just leave it empty, thus we get next-hop=None.

This causes that comparing an NmIpRoute created from a nmstate route
with an NmIpRoute created from an existing NM profile will fail because
of this difference. Fix it by converting "0.0.0.0" / "::" to None when
creating a NmIpRoute from an nmstate route. This is the right
representation for it at `nm` layer, anyway.

Signed-off-by: Íñigo Huguet <ihuguet@redhat.com>

rust/src/lib/nm/settings/route.rs

@@ -6,6 +6,9 @@ use crate::{
     ip::is_ipv6_addr, InterfaceIpAddr, NmstateError, RouteEntry, RouteType,
 };
 
+const IPV4_EMPTY_NEXT_HOP: &str = "0.0.0.0";
+const IPV6_EMPTY_NEXT_HOP: &str = "::";
+
 pub(crate) fn gen_nm_ip_routes(
     routes: &[RouteEntry],
     is_ipv6: bool,
@@ -31,7 +34,15 @@ pub(crate) fn gen_nm_ip_routes(
             Some(i) => Some(i),
             None => None,
         };
-        nm_route.next_hop = route.next_hop_addr.as_ref().cloned();
+        // Empty next-hop is represented by 0.0.0.0 or :: in nmstate, but NM and
+        // the kernel just leave it undefined.
+        nm_route.next_hop = route
+            .next_hop_addr
+            .as_ref()
+            .filter(|&nh| {
+                nh != IPV4_EMPTY_NEXT_HOP && nh != IPV6_EMPTY_NEXT_HOP
+            })
+            .cloned();
         nm_route.src = route.source.as_ref().cloned();
         if let Some(weight) = route.weight {
             nm_route.weight = Some(weight as u32);

tests/integration/route_test.py

@@ -19,8 +19,10 @@
 from .testlib import cmdlib
 from .testlib import iprule
 from .testlib.bridgelib import linux_bridge
+from .testlib.dummy import dummy_interface
 from .testlib.env import nm_minor_version
 from .testlib.genconf import gen_conf_apply
+from .testlib.iproutelib import ip_monitor_assert_stable_link_up
 from .testlib.route import assert_routes
 from .testlib.route import assert_routes_missing
 from .testlib.servicelib import disable_service
@@ -112,6 +114,12 @@ def clean_up_route_rule():
     )
 
 
+@pytest.fixture(scope="function")
+def dummy0_up(test_env_setup):
+    with dummy_interface("dummy0") as ifstate:
+        yield ifstate
+
+
 @pytest.mark.tier1
 def test_add_static_routes(static_eth1_with_routes):
     routes = _get_ipv4_test_routes() + _get_ipv6_test_routes()
@@ -2189,3 +2197,43 @@ def test_kernel_mode_static_route_and_remove(cleanup_veth1_kernel_mode):
     assert_routes_missing(
         desired_state[Route.KEY][Route.CONFIG], cur_state, nic="veth1"
     )
+
+
+# https://issues.redhat.com/browse/RHEL-64707
+# ip_monitor_assert_stable_link_up doesn't work with ethernet, use dummy
+@ip_monitor_assert_stable_link_up("dummy0")
+def test_apply_routes_twice_only_reapplies(dummy0_up):
+    dummy0_state = dummy0_up[Interface.KEY][0]
+    dummy0_state[Interface.IPV4] = ETH1_INTERFACE_STATE[Interface.IPV4]
+    dummy0_state[Interface.IPV6] = ETH1_INTERFACE_STATE[Interface.IPV6]
+    routes = [
+        {
+            Route.NEXT_HOP_INTERFACE: "dummy0",
+            Route.DESTINATION: IPV4_TEST_NET1,
+            Route.NEXT_HOP_ADDRESS: IPV4_EMPTY_ADDRESS,
+            Route.METRIC: 100,
+            Route.TABLE_ID: 254,
+        },
+        {
+            Route.NEXT_HOP_INTERFACE: "dummy0",
+            Route.DESTINATION: IPV6_TEST_NET1,
+            Route.NEXT_HOP_ADDRESS: IPV6_EMPTY_ADDRESS,
+            Route.METRIC: 100,
+            Route.TABLE_ID: 254,
+        },
+    ]
+    state = {
+        Interface.KEY: [dummy0_state],
+        Route.KEY: {Route.CONFIG: routes},
+    }
+    libnmstate.apply(state)
+
+    # Apply a second time to test that empty next-hop is treated
+    # correctly. Per issue RHEL-64707, the next-hop didn't match between the
+    # nmstate generated routes and those retrieved from NM, causing nmstate to
+    # incorrectly think that there were routes to remove.
+    # This caused deactivate & activate instead of reapply.
+    libnmstate.apply(state)
+
+    cur_state = libnmstate.show()
+    assert_routes(routes, cur_state)

tests/integration/testlib/dummy.py

@@ -5,6 +5,7 @@
 import libnmstate
 from libnmstate.schema import Interface
 from libnmstate.schema import InterfaceState
+from libnmstate.schema import InterfaceType
 
 from . import cmdlib
 
@@ -32,3 +33,30 @@ def nm_unmanaged_dummy(name):
         except Exception:
             # dummy1 might not became managed by NM, hence removal might fail
             cmdlib.exec_cmd(f"ip link del {name}".split())
+
+
+@contextmanager
+def dummy_interface(ifname):
+    desired_state = {
+        Interface.KEY: [
+            {
+                Interface.NAME: ifname,
+                Interface.TYPE: InterfaceType.DUMMY,
+                Interface.STATE: InterfaceState.UP,
+            }
+        ]
+    }
+    libnmstate.apply(desired_state)
+    try:
+        yield desired_state
+    finally:
+        libnmstate.apply(
+            {
+                Interface.KEY: [
+                    {
+                        Interface.NAME: ifname,
+                        Interface.STATE: InterfaceState.ABSENT,
+                    }
+                ]
+            }
+        )