rib_priority should be an option

[rcgoodfellow]

In #359 local_pref was renamed to rib_priority and went from an Option<u32> to a u8. This has cause a misalignment with Omicron where route priorities are optional.

When nexus has a route to reconcile to maghemite that has no specified priority, since rib_priority is not optional it forces Nexus into choosing a sentinel when sending routes to maghemite. Then on the next reconciler run, nexus sees that the route it has in the database has no priority, but the route that it sees form maghemite has a priority, so it deletes the route and recreates it. We see this in the following logs

Deletes:

21:53:39.577Z INFO 1a90c686-13b4-4d8f-8ecb-7f6bd4a040cf (ServerContext): deleting static routes
    background_task = switch_port_config_manager
    file = nexus/src/app/background/tasks/sync_switch_configuration.rs:457
    rack_id = 73cdd12c-a64a-43cc-a5c0-12c3add70029
    routes = {Switch1: DeleteStaticRouteRequest { v4: DeleteStaticRoute4Request { routes: StaticRoute4List { list: [StaticRoute4 { nexthop: 172.20.15.65, prefix: Prefix4 { value: 0.0.0.0, length: 0 }, rib_priority: 1, vlan_id: None }] } }, v6: DeleteStaticRoute6Request { routes: StaticRoute6List { list: [] } } }, Switch0: DeleteStaticRouteRequest { v4: DeleteStaticRoute4Request { routes: StaticRoute4List { list: [StaticRoute4 { nexthop: 172.20.15.65, prefix: Prefix4 { value: 0.0.0.0, length: 0 }, rib_priority: 1, vlan_id: None }] } }, v6: DeleteStaticRoute6Request { routes: StaticRoute6List { list: [] } } }}
21:53:39.577Z INFO 1a90c686-13b4-4d8f-8ecb-7f6bd4a040cf (ServerContext): removing static routes
    background_task = switch_port_config_manager
    file = nexus/src/app/background/tasks/sync_switch_configuration.rs:2343
    rack_id = 73cdd12c-a64a-43cc-a5c0-12c3add70029
    request = DeleteStaticRouteRequest { v4: DeleteStaticRoute4Request { routes: StaticRoute4List { list: [StaticRoute4 { nexthop: 172.20.15.65, prefix: Prefix4 { value: 0.0.0.0, length: 0 }, rib_priority: 1, vlan_id: None }] } }, v6: DeleteStaticRoute6Request { routes: StaticRoute6List { list: [] } } }
    switch_location = Switch1
21:53:39.577Z INFO 1a90c686-13b4-4d8f-8ecb-7f6bd4a040cf (ServerContext): removing static routes
    background_task = switch_port_config_manager
    file = nexus/src/app/background/tasks/sync_switch_configuration.rs:2343
    rack_id = 73cdd12c-a64a-43cc-a5c0-12c3add70029
    request = DeleteStaticRouteRequest { v4: DeleteStaticRoute4Request { routes: StaticRoute4List { list: [StaticRoute4 { nexthop: 172.20.15.65, prefix: Prefix4 { value: 0.0.0.0, length: 0 }, rib_priority: 1, vlan_id: None }] } }, v6: DeleteStaticRoute6Request { routes: StaticRoute6List { list: [] } } }
    switch_location = Switch0

Recreates:

21:53:39.578Z INFO 1a90c686-13b4-4d8f-8ecb-7f6bd4a040cf (ServerContext): adding static routes
    background_task = switch_port_config_manager
    file = nexus/src/app/background/tasks/sync_switch_configuration.rs:463
    rack_id = 73cdd12c-a64a-43cc-a5c0-12c3add70029
    routes = {Switch0: AddStaticRouteRequest { v4: AddStaticRoute4Request { routes: StaticRoute4List { list: [StaticRoute4 { nexthop: 172.20.15.65, prefix: Prefix4 { value: 0.0.0.0, length: 0 }, rib_priority: 1, vlan_id: None }] } }, v6: AddStaticRoute6Request { routes: StaticRoute6List { list: [] } } }, Switch1: AddStaticRouteRequest { v4: AddStaticRoute4Request { routes: StaticRoute4List { list: [StaticRoute4 { nexthop: 172.20.15.65, prefix: Prefix4 { value: 0.0.0.0, length: 0 }, rib_priority: 1, vlan_id: None }] } }, v6: AddStaticRoute6Request { routes: StaticRoute6List { list: [] } } }}
21:53:39.578Z INFO 1a90c686-13b4-4d8f-8ecb-7f6bd4a040cf (ServerContext): adding static routes
    background_task = switch_port_config_manager
    file = nexus/src/app/background/tasks/sync_switch_configuration.rs:2388
    rack_id = 73cdd12c-a64a-43cc-a5c0-12c3add70029
    request = AddStaticRouteRequest { v4: AddStaticRoute4Request { routes: StaticRoute4List { list: [StaticRoute4 { nexthop: 172.20.15.65, prefix: Prefix4 { value: 0.0.0.0, length: 0 }, rib_priority: 1, vlan_id: None }] } }, v6: AddStaticRoute6Request { routes: StaticRoute6List { list: [] } } }
    switch_location = Switch0
21:53:39.579Z INFO 1a90c686-13b4-4d8f-8ecb-7f6bd4a040cf (ServerContext): adding static routes
    background_task = switch_port_config_manager
    file = nexus/src/app/background/tasks/sync_switch_configuration.rs:2388
    rack_id = 73cdd12c-a64a-43cc-a5c0-12c3add70029
    request = AddStaticRouteRequest { v4: AddStaticRoute4Request { routes: StaticRoute4List { list: [StaticRoute4 { nexthop: 172.20.15.65, prefix: Prefix4 { value: 0.0.0.0, length: 0 }, rib_priority: 1, vlan_id: None }] } }, v6: AddStaticRoute6Request { routes: StaticRoute6List { list: [] } } }
    switch_location = Switch1

Where the difference is in the priority

21:53:39.577Z INFO 1a90c686-13b4-4d8f-8ecb-7f6bd4a040cf (ServerContext): retrieved existing routes
    background_task = switch_port_config_manager
    file = nexus/src/app/background/tasks/sync_switch_configuration.rs:433
    rack_id = 73cdd12c-a64a-43cc-a5c0-12c3add70029
    routes = {Switch1: {V4(SwitchStaticRouteV4 { nexthop: 172.20.15.65, prefix: Prefix4 { value: 0.0.0.0, length: 0 }, vlan: None, priority: Some(1) })}, Switch0: {V4(SwitchStaticRouteV4 { nexthop: 172.20.15.65, prefix: Prefix4 { value: 0.0.0.0, length: 0 }, vlan: None, priority: Some(1) })}}
21:53:39.577Z INFO 1a90c686-13b4-4d8f-8ecb-7f6bd4a040cf (ServerContext): retrieved desired routes
    background_task = switch_port_config_manager
    file = nexus/src/app/background/tasks/sync_switch_configuration.rs:437
    rack_id = 73cdd12c-a64a-43cc-a5c0-12c3add70029
    routes = {Switch1: {V4(SwitchStaticRouteV4 { nexthop: 172.20.15.65, prefix: Prefix4 { value: 0.0.0.0, length: 0 }, vlan: None, priority: None })}, Switch0: {V4(SwitchStaticRouteV4 { nexthop: 172.20.15.65, prefix: Prefix4 { value: 0.0.0.0, length: 0 }, vlan: None, priority: None })}}

While this could be fixed in nexus to recognize extended equality over sentinel values, that seems quite fraught. We should have maghemite line up with omicron in terms of having route priorities be optional.

[jmpesp]

In #359 local_pref was renamed to rib_priority and went from an Option to a u8. This has cause a misalignment with Omicron where route priorities are optional.

When nexus has a route to reconcile to maghemite that has no specified priority, since rib_priority is not optional it forces Nexus into choosing a sentinel when sending routes to maghemite.

Why not resolve this in Nexus? the change of rib_priority from an Option to a u8 could have bubbled up to Nexus too, and would resolve the misalignment. IMO Nexus should also change from an Option to a u8, and the existing rows in the database should have their column set to non-null as well (and filled with said sentinel). I worry that this bug will turn up a long way down the road unless we do this as the underlying misalignment still exists.

[rcgoodfellow]

If we bubble non-optionality all the way up to nexus, that would likely also mean bubbling up to the external API which means users need to explicitly specify a priority for every static route. While we could have a default plumbed in somewhere in Omicron, that still runs the risk of a disconnect like we have now as things evolve. I think we should be consistent across the board and not rely on sentinel values.

[jmpesp]

I think we're saying the same thing? namely that users should need to explicitly specify a priority and the default introduces the disconnect

[rcgoodfellow]

What I'm saying is that users should not have to explicitly specify a priority. And that the bestbath algorithm instead explicitly handles the case where there is no user-defined priority.

[internet-diglett]

Some additional discussion took place between Ry, Trey, and I last week. Putting the key takeaways here:

• We need to account for the idea that the default values for a routing protocol's rib_priority may change in the future
• In the future, we need to allow operators the ability to define customize what those default values are. The defaults for rib priority (often called "Administrative Distance" by some vendors) are not the same from one vendor to the next, which means what the customer expects these values to be might be different than whatever default we chose. Some network operators may have very strong opinions about these values because keeping these values consistent helps them avoid routing loops in their network topology.
• These configurable defaults will need to be stored in the bootstore so the route priorities will be consistent pre and post Nexus
• I believe a determination has not been made as to whether we will resolve the value for the default in Nexus or Maghemite, but it seems like it's actually okay in either place as long as we don't erase the critical information in that the user wants us to apply the default rib_priority value for that route, whatever it may be.

For the time being, to resolve the route flapping in the Nexus reconciler, we will resolve the None to the proper default value before comparing what we have in Nexus to what is on Maghemite:

oxidecomputer/omicron#9907