Add Advertised_Route_Status to report one route state on one chassis, with the following columns: - desired_status: advertised or withdrawn - withdrawal_reasons: identifies the controller input that caused withdrawal; the map can carry additional inputs without adding new desired-state values - operational_status: the kernel route reconciliation result (installed, withdrawn, or unknown) - error: describes a reconciliation failure Route and chassis references are weak. Copied route UUID and chassis name fields provide the index and RBAC identity.
Each controller can only create, delete and update status rows that bear its own chassis name. Controller shutdown and ovn-sbctl chassis-del remove status rows for the chassis. Guard status cleanup when the system ID is unavailable. Desired status records are sorted by route UUID and looked up via binary search while publishing results. Missing rows are recreated, modified rows are repaired and unexpected local rows are removed. The status-table monitor condition is limited to the local chassis name. If the SB server does not have Advertised_Route_Status, route handling continues without status publication. System-test targets generate the existing TLS test PKI used by the controller RBAC coverage. The SB schema version changes from 21.11.0 to 21.12.0. Signed-off-by: Dmitrii Shcherbakov <[email protected]> --- controller/chassis.c | 27 ++- controller/chassis.h | 4 +- controller/ovn-controller.c | 272 ++++++++++++++++++++++++++++++- controller/route-exchange.c | 38 ++++- controller/route-exchange.h | 5 +- controller/route.c | 63 +++++++ controller/route.h | 20 +++ northd/ovn-northd.c | 17 ++ ovn-sb.ovsschema | 34 +++- ovn-sb.xml | 92 +++++++++++ tests/automake.mk | 6 +- tests/ovn-controller.at | 59 +++++++ tests/ovn-inc-proc-graph-dump.at | 2 + tests/ovn-northd.at | 11 ++ tests/ovn-sbctl.at | 7 +- tests/system-ovn.at | 184 +++++++++++++++++++++ utilities/ovn-sbctl.8.xml | 5 +- utilities/ovn-sbctl.c | 17 ++ 18 files changed, 842 insertions(+), 21 deletions(-) diff --git a/controller/chassis.c b/controller/chassis.c index 0e3bd37d8..07d375a81 100644 --- a/controller/chassis.c +++ b/controller/chassis.c @@ -1181,14 +1181,26 @@ chassis_cleanup(struct ovsdb_idl_txn *ovs_idl_txn, struct ovsdb_idl_txn *ovnsb_idl_txn, const struct ovsrec_open_vswitch_table *ovs_table, const struct sbrec_chassis *chassis_rec, - const struct sbrec_chassis_private *chassis_private_rec) + const struct sbrec_chassis_private *chassis_private_rec, + const struct sbrec_advertised_route_status_table *status_table) { + const char *chassis_name = get_ovs_chassis_id(ovs_table); + bool has_route_status = false; + if (status_table && chassis_name) { + const struct sbrec_advertised_route_status *status; + SBREC_ADVERTISED_ROUTE_STATUS_TABLE_FOR_EACH (status, status_table) { + if (!strcmp(status->chassis_name, chassis_name)) { + has_route_status = true; + break; + } + } + } + if (!chassis_rec && !chassis_private_rec && - !is_chassis_idx_stored(ovs_table)) { + !is_chassis_idx_stored(ovs_table) && !has_route_status) { return true; } - const char *chassis_name = get_ovs_chassis_id(ovs_table); if (ovs_idl_txn) { ovsdb_idl_txn_add_comment( ovs_idl_txn, @@ -1207,6 +1219,15 @@ chassis_cleanup(struct ovsdb_idl_txn *ovs_idl_txn, if (chassis_private_rec) { sbrec_chassis_private_delete(chassis_private_rec); } + if (status_table && chassis_name) { + const struct sbrec_advertised_route_status *status; + SBREC_ADVERTISED_ROUTE_STATUS_TABLE_FOR_EACH_SAFE ( + status, status_table) { + if (!strcmp(status->chassis_name, chassis_name)) { + sbrec_advertised_route_status_delete(status); + } + } + } } return false; } diff --git a/controller/chassis.h b/controller/chassis.h index 45dd9537a..3be39c957 100644 --- a/controller/chassis.h +++ b/controller/chassis.h @@ -29,6 +29,7 @@ struct ovsrec_open_vswitch_table; struct sbrec_chassis; struct sbrec_chassis_table; struct sbrec_chassis_private_table; +struct sbrec_advertised_route_status_table; struct sset; struct eth_addr; struct smap; @@ -50,7 +51,8 @@ bool chassis_cleanup(struct ovsdb_idl_txn *ovs_idl_txn, struct ovsdb_idl_txn *ovnsb_idl_txn, const struct ovsrec_open_vswitch_table *, const struct sbrec_chassis *, - const struct sbrec_chassis_private *); + const struct sbrec_chassis_private *, + const struct sbrec_advertised_route_status_table *); bool chassis_get_mac(const struct sbrec_chassis *chassis, const char *bridge_mapping, struct eth_addr *chassis_mac); diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 4dcb4b895..6551a4e30 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -175,6 +175,7 @@ struct controller_engine_ctx { struct if_status_mgr *if_mgr; const unsigned int *ovnsb_expected_cond_seqno; const bool *sb_monitor_all; + const struct sbrec_chassis *chassis; }; /* Pending packet to be injected into connected OVS. */ @@ -221,6 +222,7 @@ static char *get_file_system_id(void) static unsigned int update_sb_monitors(struct ovsdb_idl *ovnsb_idl, const struct sbrec_chassis *chassis, + const char *chassis_id, const struct simap *local_ifaces, const struct shash *local_bindings, struct hmap *local_datapaths, @@ -241,7 +243,8 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, * * Monitor ECMP_Nexthop for local datapaths. * - * Monitor Advertised/Learned_Route for local datapaths. + * Monitor Advertised/Learned_Route for local datapaths and + * Advertised_Route_Status for the local chassis. * * We always monitor patch ports because they allow us to see the linkages * between related logical datapaths. That way, when we know that we have @@ -261,6 +264,7 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, struct ovsdb_idl_condition tv = OVSDB_IDL_CONDITION_INIT(&tv); struct ovsdb_idl_condition nh = OVSDB_IDL_CONDITION_INIT(&nh); struct ovsdb_idl_condition ar = OVSDB_IDL_CONDITION_INIT(&ar); + struct ovsdb_idl_condition ars = OVSDB_IDL_CONDITION_INIT(&ars); struct ovsdb_idl_condition lr = OVSDB_IDL_CONDITION_INIT(&lr); struct ovsdb_idl_condition amb = OVSDB_IDL_CONDITION_INIT(&amb); @@ -297,6 +301,7 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, ovsdb_idl_condition_add_clause_true(&tv); ovsdb_idl_condition_add_clause_true(&nh); ovsdb_idl_condition_add_clause_true(&ar); + ovsdb_idl_condition_add_clause_true(&ars); ovsdb_idl_condition_add_clause_true(&amb); goto out; } @@ -365,6 +370,16 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, * ones. */ ovsdb_idl_condition_add_clause_true(&ar); } + + /* The configured name is available before the Chassis row and also + * matches status rows whose chassis weak reference was cleared when the + * Chassis row was deleted. */ + const char *status_chassis_name = chassis ? chassis->name : chassis_id; + if (status_chassis_name) { + sbrec_advertised_route_status_add_clause_chassis_name( + &ars, OVSDB_F_EQ, status_chassis_name); + } + if (local_ifaces) { const char *name; @@ -434,6 +449,7 @@ out:; sbrec_chassis_template_var_set_condition(ovnsb_idl, &tv), sbrec_ecmp_nexthop_set_condition(ovnsb_idl, &nh), sbrec_advertised_route_set_condition(ovnsb_idl, &ar), + sbrec_advertised_route_status_set_condition(ovnsb_idl, &ars), sbrec_learned_route_set_condition(ovnsb_idl, &lr), sbrec_advertised_mac_binding_set_condition(ovnsb_idl, &amb), }; @@ -457,6 +473,7 @@ out:; ovsdb_idl_condition_destroy(&tv); ovsdb_idl_condition_destroy(&nh); ovsdb_idl_condition_destroy(&ar); + ovsdb_idl_condition_destroy(&ars); ovsdb_idl_condition_destroy(&lr); ovsdb_idl_condition_destroy(&amb); return expected_cond_seqno; @@ -768,7 +785,8 @@ update_sb_db(struct ovsdb_idl *ovs_idl, struct ovsdb_idl *ovnsb_idl, * extra cost. Instead, it is called after the engine execution only * when it is necessary. */ unsigned int next_cond_seqno = - update_sb_monitors(ovnsb_idl, NULL, NULL, NULL, NULL, true); + update_sb_monitors(ovnsb_idl, NULL, chassis_id, + NULL, NULL, NULL, true); if (sb_cond_seqno) { *sb_cond_seqno = next_cond_seqno; } @@ -5277,6 +5295,12 @@ struct ed_type_route { /* Contains struct advertise_datapath_entry */ struct hmap announce_routes; + + /* Contains struct advertised_route_status recorded by route_run() + * and published in Advertised_Route_Status by + * route_exchange_run(). */ + struct vector advertised_route_status; + struct ovsdb_idl *ovnsb_idl; }; @@ -5334,9 +5358,11 @@ en_route_run(struct engine_node *node, void *data) .filtered_ports = &re_data->filtered_ports, .tracked_ports_remote = &re_data->tracked_ports_remote, .announce_routes = &re_data->announce_routes, + .advertised_route_status = &re_data->advertised_route_status, }; route_cleanup(&re_data->announce_routes); + advertised_route_status_clear(&re_data->advertised_route_status); tracked_datapaths_clear(r_ctx_out.tracked_re_datapaths); sset_clear(r_ctx_out.tracked_ports_local); sset_clear(r_ctx_out.tracked_ports_remote); @@ -5345,7 +5371,7 @@ en_route_run(struct engine_node *node, void *data) sset_clear(r_ctx_out.filtered_ports); route_run(&r_ctx_in, &r_ctx_out); - + advertised_route_status_sort(&re_data->advertised_route_status); return EN_UPDATED; } @@ -5363,6 +5389,8 @@ en_route_init(struct engine_node *node OVS_UNUSED, uuidset_init(&data->relevant_service_monitors); sset_init(&data->filtered_ports); hmap_init(&data->announce_routes); + data->advertised_route_status = + VECTOR_EMPTY_INITIALIZER(struct advertised_route_status); data->ovnsb_idl = arg->sb_idl; return data; @@ -5381,6 +5409,7 @@ en_route_cleanup(void *data) sset_destroy(&re_data->filtered_ports); route_cleanup(&re_data->announce_routes); hmap_destroy(&re_data->announce_routes); + vector_destroy(&re_data->advertised_route_status); } static enum engine_input_handler_result @@ -5710,10 +5739,113 @@ struct ed_type_route_exchange { bool sb_changes_pending; }; +static void +advertised_route_get_withdrawal_reasons( + const struct advertised_route_status *desired, + struct smap *withdrawal_reasons) +{ + smap_init(withdrawal_reasons); + if (desired->withdrawal_reason) { + smap_add(withdrawal_reasons, desired->withdrawal_reason, + desired->withdrawal_reason_value); + } +} + +static char * +advertised_route_get_operational_state( + struct ed_type_route *route_data, + const struct advertised_route_status *desired, + const char **operational_status) +{ + struct advertise_datapath_entry *ad = + advertise_datapath_find(&route_data->announce_routes, + desired->route->datapath); + *operational_status = ad && ad->routes_synced + ? (!strcmp(desired->desired_status, "advertised") + ? "installed" : "withdrawn") + : "unknown"; + + if (!ad || !ad->route_error_description) { + return NULL; + } + return ad->route_error + ? xasprintf("%s: %s", ad->route_error_description, + ovs_strerror(ad->route_error)) + : xstrdup(ad->route_error_description); +} + +static enum engine_input_handler_result +route_exchange_sb_advertised_route_status_handler( + struct engine_node *node, void *data OVS_UNUSED) +{ + struct controller_engine_ctx *ctrl_ctx = + engine_get_context()->client_ctx; + if (!ctrl_ctx->chassis) { + return EN_HANDLED_UNCHANGED; + } + + struct ed_type_route *route_data = + engine_get_input_data("route", node); + const struct sbrec_advertised_route_status_table *status_table = + EN_OVSDB_GET(engine_get_input("SB_advertised_route_status", node)); + const struct sbrec_advertised_route_status *sb_status; + + SBREC_ADVERTISED_ROUTE_STATUS_TABLE_FOR_EACH_TRACKED (sb_status, + status_table) { + if (strcmp(sb_status->chassis_name, ctrl_ctx->chassis->name)) { + continue; + } + + const struct advertised_route_status *desired = + advertised_route_status_find( + &route_data->advertised_route_status, + sb_status->advertised_route); + if (sbrec_advertised_route_status_is_deleted(sb_status)) { + if (desired) { + return EN_UNHANDLED; + } + continue; + } + + const struct uuid *route_uuid = desired + ? &desired->route->header_.uuid : NULL; + if (!desired || + sb_status->advertised_route != desired->route || + !uuid_equals(&sb_status->advertised_route_uuid, route_uuid) || + sb_status->chassis != ctrl_ctx->chassis) { + return EN_UNHANDLED; + } + + const char *operational_status; + char *error = advertised_route_get_operational_state( + route_data, desired, &operational_status); + struct smap withdrawal_reasons; + advertised_route_get_withdrawal_reasons( + desired, &withdrawal_reasons); + bool matches = + !strcmp(sb_status->desired_status, desired->desired_status) && + smap_equal(&sb_status->withdrawal_reasons, + &withdrawal_reasons) && + !strcmp(sb_status->operational_status, operational_status) && + nullable_string_is_equal(sb_status->error, error); + smap_destroy(&withdrawal_reasons); + free(error); + + if (!matches) { + return EN_UNHANDLED; + } + } + + return EN_HANDLED_UNCHANGED; +} + static enum engine_node_state en_route_exchange_run(struct engine_node *node, void *data) { struct ed_type_route_exchange *re = data; + struct controller_engine_ctx *ctrl_ctx = + engine_get_context()->client_ctx; + struct ovsdb_idl_index *sbrec_learned_route_by_datapath = engine_ovsdb_node_get_index( engine_get_input("SB_learned_route", node), @@ -5723,6 +5855,11 @@ en_route_exchange_run(struct engine_node *node, void *data) engine_ovsdb_node_get_index( engine_get_input("SB_port_binding", node), "name"); + struct ovsdb_idl_index *sbrec_advertised_route_status_by_chassis_name = + engine_ovsdb_node_get_index( + engine_get_input("SB_advertised_route_status", node), + "chassis_name"); + struct ed_type_route *route_data = engine_get_input_data("route", node); struct ed_type_route_table_notify *rt_notify = @@ -5762,6 +5899,106 @@ en_route_exchange_run(struct engine_node *node, void *data) }; route_exchange_run(&r_ctx_in, &r_ctx_out); + + /* Both references are weak. chassis_name is used by RBAC and cleanup. */ + if (r_ctx_in.ovnsb_idl_txn && ctrl_ctx->chassis && + sbrec_server_has_advertised_route_status_table(re->sb_idl)) { + struct uuidset published = UUIDSET_INITIALIZER(&published); + const struct sbrec_advertised_route_status *sb_status; + struct sbrec_advertised_route_status *status_filter = + sbrec_advertised_route_status_index_init_row( + sbrec_advertised_route_status_by_chassis_name); + sbrec_advertised_route_status_index_set_chassis_name( + status_filter, ctrl_ctx->chassis->name); + + SBREC_ADVERTISED_ROUTE_STATUS_FOR_EACH_EQUAL ( + sb_status, status_filter, + sbrec_advertised_route_status_by_chassis_name) { + const struct advertised_route_status *desired = + advertised_route_status_find( + &route_data->advertised_route_status, + sb_status->advertised_route); + + const struct uuid *route_uuid = sb_status->advertised_route + ? &sb_status->advertised_route->header_.uuid : NULL; + if (!desired || + !uuid_equals(&sb_status->advertised_route_uuid, route_uuid) || + sb_status->chassis != ctrl_ctx->chassis || + (route_uuid && uuidset_contains(&published, route_uuid))) { + sbrec_advertised_route_status_delete(sb_status); + continue; + } + + uuidset_insert(&published, route_uuid); + const char *operational_status; + char *error = advertised_route_get_operational_state( + route_data, desired, &operational_status); + struct smap withdrawal_reasons; + advertised_route_get_withdrawal_reasons( + desired, &withdrawal_reasons); + + if (strcmp(sb_status->desired_status, + desired->desired_status)) { + sbrec_advertised_route_status_set_desired_status( + sb_status, desired->desired_status); + } + if (!smap_equal(&sb_status->withdrawal_reasons, + &withdrawal_reasons)) { + sbrec_advertised_route_status_set_withdrawal_reasons( + sb_status, &withdrawal_reasons); + } + if (strcmp(sb_status->operational_status, + operational_status)) { + sbrec_advertised_route_status_set_operational_status( + sb_status, operational_status); + } + if (!nullable_string_is_equal(sb_status->error, error)) { + sbrec_advertised_route_status_set_error(sb_status, error); + } + smap_destroy(&withdrawal_reasons); + free(error); + } + sbrec_advertised_route_status_index_destroy_row(status_filter); + + struct advertised_route_status *desired; + VECTOR_FOR_EACH_PTR (&route_data->advertised_route_status, desired) { + const struct uuid *route_uuid = &desired->route->header_.uuid; + if (uuidset_contains(&published, route_uuid)) { + continue; + } + + struct sbrec_advertised_route_status *new_status = + sbrec_advertised_route_status_insert(r_ctx_in.ovnsb_idl_txn); + sbrec_advertised_route_status_set_advertised_route( + new_status, desired->route); + sbrec_advertised_route_status_set_advertised_route_uuid( + new_status, *route_uuid); + sbrec_advertised_route_status_set_chassis(new_status, + ctrl_ctx->chassis); + sbrec_advertised_route_status_set_chassis_name( + new_status, ctrl_ctx->chassis->name); + sbrec_advertised_route_status_set_desired_status( + new_status, desired->desired_status); + struct smap withdrawal_reasons; + advertised_route_get_withdrawal_reasons( + desired, &withdrawal_reasons); + sbrec_advertised_route_status_set_withdrawal_reasons( + new_status, &withdrawal_reasons); + smap_destroy(&withdrawal_reasons); + + const char *operational_status; + char *error = advertised_route_get_operational_state( + route_data, desired, &operational_status); + sbrec_advertised_route_status_set_operational_status( + new_status, operational_status); + if (error) { + sbrec_advertised_route_status_set_error(new_status, error); + } + free(error); + } + uuidset_destroy(&published); + } + route_table_notify_update(&rt_notify->watches); re->sb_changes_pending = r_ctx_out.sb_changes_pending; @@ -6982,6 +7219,7 @@ evpn_arp_vtep_binding_handler(struct engine_node *node, void *data OVS_UNUSED) SB_NODE(chassis_template_var) \ SB_NODE(acl_id) \ SB_NODE(advertised_route) \ + SB_NODE(advertised_route_status) \ SB_NODE(learned_route) \ SB_NODE(advertised_mac_binding) \ SB_NODE(service_monitor) @@ -7127,6 +7365,8 @@ inc_proc_ovn_controller_init( engine_noop_handler); engine_add_input(&en_route_exchange, &en_sb_port_binding, engine_noop_handler); + engine_add_input(&en_route_exchange, &en_sb_advertised_route_status, + route_exchange_sb_advertised_route_status_handler); engine_add_input(&en_route_exchange, &en_route_table_notify, NULL); engine_add_input(&en_route_exchange, &en_route_exchange_status, NULL); engine_add_input(&en_route_exchange, &en_sb_ro, @@ -7471,6 +7711,16 @@ inc_proc_ovn_controller_init( &sbrec_learned_route_col_datapath); engine_ovsdb_node_add_index(&en_sb_learned_route, "datapath", sbrec_learned_route_index_by_datapath); + + struct ovsdb_idl_index + *sbrec_advertised_route_status_by_chassis_name = + ovsdb_idl_index_create1( + sb_idl_loop->idl, + &sbrec_advertised_route_status_col_chassis_name); + engine_ovsdb_node_add_index( + &en_sb_advertised_route_status, "chassis_name", + sbrec_advertised_route_status_by_chassis_name); + struct ovsdb_idl_index *sbrec_advertised_mac_binding_index_by_dp = ovsdb_idl_index_create1(sb_idl_loop->idl, &sbrec_advertised_mac_binding_col_datapath); @@ -7743,7 +7993,8 @@ main(int argc, char *argv[]) ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_chassis_private_col_external_ids); - update_sb_monitors(ovnsb_idl_loop.idl, NULL, NULL, NULL, NULL, false); + update_sb_monitors(ovnsb_idl_loop.idl, NULL, NULL, + NULL, NULL, NULL, false); stopwatch_create(CONTROLLER_LOOP_STOPWATCH_NAME, SW_MS); stopwatch_create(OFCTRL_PUT_STOPWATCH_NAME, SW_MS); @@ -8078,6 +8329,7 @@ main(int argc, char *argv[]) &chassis_private, sbrec_encaps_index_by_ip_and_type); } + ctrl_engine_ctx.chassis = chassis; /* If any OVS feature support changed, force a full recompute. * 'br_int_dp' is valid only if an OVS transaction is possible. @@ -8286,7 +8538,7 @@ main(int argc, char *argv[]) ovnsb_expected_cond_seqno; ovnsb_expected_cond_seqno = update_sb_monitors( - ovnsb_idl_loop.idl, chassis, + ovnsb_idl_loop.idl, chassis, chassis_id, &runtime_data->local_lports, &runtime_data->lbinding_data.bindings, &runtime_data->local_datapaths, @@ -8560,11 +8812,19 @@ loop_done: ? chassis_private_lookup_by_name( sbrec_chassis_private_by_name, chassis_id) : NULL); + const struct sbrec_advertised_route_status_table *status_table = + sbrec_server_has_advertised_route_status_table( + ovnsb_idl_loop.idl) + ? sbrec_advertised_route_status_table_get( + ovnsb_idl_loop.idl) + : NULL; + /* Run all of the cleanup functions, even if one of them returns * false. We're done if all of them return true. */ done = binding_cleanup(ovnsb_idl_txn, port_binding_table, chassis); done = chassis_cleanup(ovs_idl_txn, ovnsb_idl_txn, ovs_table, - chassis, chassis_private) && done; + chassis, chassis_private, + status_table) && done; done = encaps_cleanup(ovs_idl_txn, br_int) && done; done = igmp_group_cleanup(ovnsb_idl_txn, sbrec_igmp_group, chassis) && done; diff --git a/controller/route-exchange.c b/controller/route-exchange.c index 027375071..24d5b5875 100644 --- a/controller/route-exchange.c +++ b/controller/route-exchange.c @@ -321,13 +321,19 @@ route_exchange_run(const struct route_exchange_ctx_in *r_ctx_in, int error; CLEAR_ROUTE_EXCHANGE_NL_STATUS(); - const struct advertise_datapath_entry *ad; + struct advertise_datapath_entry *ad; HMAP_FOR_EACH (ad, node, r_ctx_in->announce_routes) { + ad->routes_synced = false; + ad->route_error = 0; + ad->route_error_description = NULL; + uint32_t table_id = route_get_table_id(ad->db); if (!TABLE_ID_VALID(table_id)) { VLOG_WARN_RL(&rl, "Unable to sync routes for datapath "UUID_FMT": " "invalid table id: %"PRIu32, UUID_ARGS(&ad->db->header_.uuid), table_id); + ad->route_error = EINVAL; + ad->route_error_description = "invalid route table ID"; continue; } @@ -340,6 +346,8 @@ route_exchange_run(const struct route_exchange_ctx_in *r_ctx_in, UUID_FMT": %s.", ad->vrf_name, UUID_ARGS(&ad->db->header_.uuid), ovs_strerror(error)); + ad->route_error = error; + ad->route_error_description = "unable to create VRF"; SET_ROUTE_EXCHANGE_NL_STATUS(error); continue; } @@ -363,6 +371,9 @@ route_exchange_run(const struct route_exchange_ctx_in *r_ctx_in, "routes on routing table %"PRIu32, table_id); entry->can_sync = false; + ad->route_error = EBUSY; + ad->route_error_description = + "multiple datapaths use the same route table"; } else { entry->routes = &ad->routes; } @@ -383,6 +394,9 @@ route_exchange_run(const struct route_exchange_ctx_in *r_ctx_in, } if (!entry->can_sync) { + ad->route_error = EBUSY; + ad->route_error_description = + "multiple datapaths use the same route table"; continue; } @@ -392,6 +406,7 @@ route_exchange_run(const struct route_exchange_ctx_in *r_ctx_in, struct advertised_routes_entry *arte; HMAP_FOR_EACH_POP (arte, node, &advertised_routes) { maintained_route_table_add(arte->table_id); + error = 0; if (arte->can_sync) { struct vector received_routes = VECTOR_EMPTY_INITIALIZER(struct re_nl_received_route_node); @@ -425,6 +440,27 @@ route_exchange_run(const struct route_exchange_ctx_in *r_ctx_in, vector_destroy(&received_routes); } + struct hmapx_node *dp_node; + HMAPX_FOR_EACH (dp_node, &arte->datapaths) { + struct advertise_datapath_entry *adpe = + advertise_datapath_find(r_ctx_in->announce_routes, + dp_node->data); + if (!adpe) { + continue; + } + if (!arte->can_sync) { + adpe->route_error = EBUSY; + adpe->route_error_description = + "multiple datapaths use the same route table"; + } else if (error) { + adpe->route_error = error; + adpe->route_error_description = + "route table reconciliation failed"; + } else { + adpe->routes_synced = true; + } + } + hmapx_destroy(&arte->datapaths); free(arte); } diff --git a/controller/route-exchange.h b/controller/route-exchange.h index a1ef4a359..7ecc66671 100644 --- a/controller/route-exchange.h +++ b/controller/route-exchange.h @@ -26,8 +26,9 @@ struct route_exchange_ctx_in { struct ovsdb_idl_index *sbrec_learned_route_by_datapath; const struct sbrec_chassis *chassis; - /* Contains struct advertise_datapath_entry */ - const struct hmap *announce_routes; + /* Contains struct advertise_datapath_entry. route_exchange_run() records + * the reconciliation result in each entry. */ + struct hmap *announce_routes; }; struct route_exchange_ctx_out { diff --git a/controller/route.c b/controller/route.c index 0230e372a..c6accfba3 100644 --- a/controller/route.c +++ b/controller/route.c @@ -578,6 +578,58 @@ advertised_datapath_alloc(const struct sbrec_datapath_binding *datapath) return ad; } +static void +route_record_status(struct route_ctx_out *r_ctx_out, + const struct sbrec_advertised_route *route, + const char *desired_status, + const char *withdrawal_reason, + const char *withdrawal_reason_value) +{ + struct advertised_route_status status = { + .route = route, + .desired_status = desired_status, + .withdrawal_reason = withdrawal_reason, + .withdrawal_reason_value = withdrawal_reason_value, + }; + vector_push(r_ctx_out->advertised_route_status, &status); +} + +void +advertised_route_status_clear(struct vector *statuses) +{ + vector_clear(statuses); +} + +static int +advertised_route_status_cmp(const void *a_, const void *b_) +{ + const struct advertised_route_status *a = a_; + const struct advertised_route_status *b = b_; + + return uuid_compare_3way(&a->route->header_.uuid, + &b->route->header_.uuid); +} + +void +advertised_route_status_sort(struct vector *statuses) +{ + vector_qsort(statuses, advertised_route_status_cmp); +} + +const struct advertised_route_status * +advertised_route_status_find(const struct vector *statuses, + const struct sbrec_advertised_route *route) +{ + if (!route) { + return NULL; + } + + const struct advertised_route_status key = { + .route = route, + }; + return vector_bsearch(statuses, &key, advertised_route_status_cmp); +} + void route_run(struct route_ctx_in *r_ctx_in, struct route_ctx_out *r_ctx_out) @@ -736,14 +788,25 @@ route_run(struct route_ctx_in *r_ctx_in, if (distributed_lb && !smap_get_bool(&route->external_ids, "enabled", true)) { + route_record_status(r_ctx_out, route, "withdrawn", + "administrative-policy", "disabled"); continue; } int gate = lb_route_gate_decision(&lb_route_gates, route); if (gate == 0) { + if (distributed_lb) { + route_record_status(r_ctx_out, route, + "withdrawn", "service-monitor", + "no-online-backend"); + } continue; } + if (distributed_lb) { + route_record_status(r_ctx_out, route, "advertised", NULL, NULL); + } + struct in6_addr nexthop = IN6_IS_ADDR_V4MAPPED(&prefix) ? ad->ipv4_nexthop : ad->ipv6_nexthop; if (advertise_route_find(priority, &prefix, plen, &nexthop, diff --git a/controller/route.h b/controller/route.h index 5ddc94456..cf575f0a9 100644 --- a/controller/route.h +++ b/controller/route.h @@ -68,6 +68,21 @@ struct route_ctx_out { /* Contains struct advertise_datapath_entry */ struct hmap *announce_routes; + + /* Contains struct advertised_route_status entries recorded by + * route_run(), sorted by Advertised_Route UUID for publication in + * Advertised_Route_Status by route_exchange_run(). */ + struct vector *advertised_route_status; +}; + +/* Per-route advertisement decision recorded by route_run() so that + * route_exchange_run() (which holds a writable SB txn) can publish it for the + * local chassis in Advertised_Route_Status. */ +struct advertised_route_status { + const struct sbrec_advertised_route *route; + const char *desired_status; + const char *withdrawal_reason; + const char *withdrawal_reason_value; }; struct advertise_datapath_entry { @@ -116,6 +131,11 @@ struct advertise_route_entry advertise_route_from_route_data(const struct route_data *); void route_run(struct route_ctx_in *, struct route_ctx_out *); void route_cleanup(struct hmap *announce_routes); +void advertised_route_status_clear(struct vector *statuses); +void advertised_route_status_sort(struct vector *statuses); +const struct advertised_route_status *advertised_route_status_find( + const struct vector *statuses, + const struct sbrec_advertised_route *route); uint32_t route_get_table_id(const struct sbrec_datapath_binding *); struct advertise_route_entry * advertise_route_find(unsigned int priority, const struct in6_addr *prefix, diff --git a/northd/ovn-northd.c b/northd/ovn-northd.c index 499cf9edd..dbb9b7a26 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -132,6 +132,10 @@ static const char *rbac_learned_route_auth[] = {""}; static const char *rbac_learned_route_update[] = {"datapath", "logical_port", "ip_prefix", "nexthop", "external_ids"}; +static const char *rbac_advertised_route_status_auth[] = + {"chassis_name"}; +static const char *rbac_advertised_route_status_update[] = + {"desired_status", "withdrawal_reasons", "operational_status", "error"}; static struct rbac_perm_cfg { const char *table; @@ -222,6 +226,14 @@ static struct rbac_perm_cfg { .update = rbac_bfd_update, .n_update = ARRAY_SIZE(rbac_bfd_update), .row = NULL + },{ + .table = "Advertised_Route_Status", + .auth = rbac_advertised_route_status_auth, + .n_auth = ARRAY_SIZE(rbac_advertised_route_status_auth), + .insdel = true, + .update = rbac_advertised_route_status_update, + .n_update = ARRAY_SIZE(rbac_advertised_route_status_update), + .row = NULL },{ .table = "Learned_Route", .auth = rbac_learned_route_auth, @@ -941,6 +953,11 @@ main(int argc, char *argv[]) /* Omit unused columns. */ ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_sb_global_col_connections); ovsdb_idl_omit(ovnsb_idl_loop.idl, &sbrec_sb_global_col_ssl); + for (size_t i = 0; + i < SBREC_ADVERTISED_ROUTE_STATUS_N_COLUMNS; i++) { + ovsdb_idl_omit(ovnsb_idl_loop.idl, + &sbrec_advertised_route_status_columns[i]); + } /* Disable alerting for pure write-only columns. */ ovsdb_idl_omit_alert(ovnsb_idl_loop.idl, &sbrec_sb_global_col_nb_cfg); diff --git a/ovn-sb.ovsschema b/ovn-sb.ovsschema index 6b703f328..3587024bf 100644 --- a/ovn-sb.ovsschema +++ b/ovn-sb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Southbound", - "version": "21.11.0", - "cksum": "4289271680 36997", + "version": "21.12.0", + "cksum": "1561088612 38543", "tables": { "SB_Global": { "columns": { @@ -682,6 +682,36 @@ "indexes": [["datapath", "logical_port", "ip_prefix", "tracked_port"]], "isRoot": true}, + "Advertised_Route_Status": { + "columns": { + "advertised_route": { + "type": {"key": {"type": "uuid", + "refTable": "Advertised_Route", + "refType": "weak"}, + "min": 0, "max": 1}}, + "advertised_route_uuid": {"type": "uuid"}, + "chassis": { + "type": {"key": {"type": "uuid", + "refTable": "Chassis", + "refType": "weak"}, + "min": 0, "max": 1}}, + "chassis_name": {"type": "string"}, + "desired_status": { + "type": {"key": {"type": "string", + "enum": ["set", ["advertised", + "withdrawn"]]}}}, + "withdrawal_reasons": { + "type": {"key": "string", "value": "string", + "min": 0, "max": "unlimited"}}, + "operational_status": { + "type": {"key": {"type": "string", + "enum": ["set", ["installed", + "withdrawn", + "unknown"]]}}}, + "error": { + "type": {"key": "string", "min": 0, "max": 1}}}, + "indexes": [["advertised_route_uuid", "chassis_name"]], + "isRoot": true}, "Learned_Route": { "columns": { "datapath": {"type": {"key": {"type": "uuid", diff --git a/ovn-sb.xml b/ovn-sb.xml index 037954800..d5c5210dc 100644 --- a/ovn-sb.xml +++ b/ovn-sb.xml @@ -5538,6 +5538,98 @@ tcp.flags = RST; </dl> </column> </table> + + <table name="Advertised_Route_Status"> + <p> + Each record reports the desired and kernel route state of one + distributed Load Balancer <ref table="Advertised_Route"/> on one + chassis. Several chassis can report the same route. + </p> + + <p> + A controller creates a row for each + <code>external_ids:distributed-lb=true</code> route selected for its + chassis, including withdrawn routes. Other route types have no status + rows. A missing row means that the route was not selected for the + chassis or that the controller has not published its state. Status rows + do not indicate controller liveness. + </p> + + <p> + <code>ovn-controller</code> manages these rows. RBAC restricts each + controller to its chassis name. Normal controller shutdown and + <code>ovn-sbctl chassis-del</code> remove the corresponding rows. A + controller failure followed by generic database deletion of the Chassis + can leave rows with cleared weak references. The copied route UUID and + chassis name remain available for identifying those rows. + </p> + + <column name="advertised_route"> + The route for which the chassis made the advertisement decision. + </column> + + <column name="advertised_route_uuid"> + A copy of the UUID of + <ref table="Advertised_Route_Status" column="advertised_route"/>. It + remains after the weak reference is cleared and avoids duplicate empty + values in the status index when several routes are deleted. + </column> + + <column name="chassis"> + The chassis that made the advertisement decision. + </column> + + <column name="chassis_name"> + A copy of the name of + <ref table="Advertised_Route_Status" column="chassis"/>. RBAC uses this + value to authorize controller writes. It remains after the weak + chassis reference is cleared. + </column> + + <column name="desired_status"> + The chassis's current decision. <code>advertised</code> means the + controller intends to install the route for export, and + <code>withdrawn</code> means that it does not. The + <ref table="Advertised_Route_Status" column="withdrawal_reasons"/> + map describes why a route is withdrawn. + </column> + + <column name="withdrawal_reasons"> + Machine-readable reasons for the current + <ref table="Advertised_Route_Status" column="desired_status"/>. + The map is empty when the desired state is <code>advertised</code>. + Initially, <code>administrative-policy=disabled</code> reports an + <ref table="Advertised_Route" column="external_ids"/> + <code>enabled=false</code> override, and + <code>service-monitor=no-online-backend</code> reports that the + relevant <ref table="Service_Monitor"/> records prevented + advertisement. Future controller inputs can add reason keys without + changing the desired-state values. Readers must ignore unknown keys. + </column> + + <column name="operational_status"> + The result of reconciling the desired state with the chassis's kernel + routing table. <code>installed</code> means reconciliation completed + with this route selected. <code>withdrawn</code> means reconciliation + completed with this route excluded. <code>unknown</code> means + reconciliation has not yet completed + or failed. <ref table="Advertised_Route_Status" + column="error"/> can contain more detail. + + <p> + This column reports OVN's kernel route. It does not report whether an + external routing daemon exported the prefix. Equivalent + <ref table="Advertised_Route"/> rows with the same prefix, nexthop and + priority can share one kernel route. + </p> + </column> + + <column name="error"> + A description of the current route-table reconciliation failure. It is + cleared after successful reconciliation. + </column> + </table> + <table name="Learned_Route"> <p> Each record represents a route that learned by ovn using some dynamic diff --git a/tests/automake.mk b/tests/automake.mk index 8084357b5..d4c1d3710 100644 --- a/tests/automake.mk +++ b/tests/automake.mk @@ -190,17 +190,17 @@ check-userspace-valgrind: all $(valgrind_wrappers) $(check_DATA) check-helgrind: all $(valgrind_wrappers) $(check_DATA) -$(SHELL) '$(TESTSUITE)' -C tests CHECK_VALGRIND=true VALGRIND='$(HELGRIND)' AUTOTEST_PATH='tests/valgrind:$(AUTOTEST_PATH)' -d $(TESTSUITEFLAGS) -check-system-dpdk: all +check-system-dpdk: all $(check_DATA) set $(SHELL) '$(SYSTEM_DPDK_TESTSUITE)' -C tests AUTOTEST_PATH='$(AUTOTEST_PATH)'; \ $(SUDO) "$$@" $(TESTSUITEFLAGS) -j1 || (test X'$(RECHECK)' = Xyes && $(SUDO) "$$@" --recheck) # Run kmod tests. Assume kernel modules has been installed or linked into the kernel -check-kernel: all +check-kernel: all $(check_DATA) set $(SHELL) '$(SYSTEM_KMOD_TESTSUITE)' -C tests AUTOTEST_PATH='$(AUTOTEST_PATH)'; \ $(SUDO) "$$@" $(TESTSUITEFLAGS) -j1 || (test X'$(RECHECK)' = Xyes && $(SUDO) "$$@" --recheck) -check-system-userspace: all +check-system-userspace: all $(check_DATA) set $(SHELL) '$(SYSTEM_USERSPACE_TESTSUITE)' -C tests AUTOTEST_PATH='$(AUTOTEST_PATH)'; \ $(SUDO) "$$@" $(TESTSUITEFLAGS) -j1 || (test X'$(RECHECK)' = Xyes && $(SUDO) "$$@" --recheck) diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index e17ebea76..8af3be8d9 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -3006,6 +3006,65 @@ OVN_CLEANUP([hv1]) AT_CLEANUP ]) +OVN_FOR_EACH_NORTHD([ +AT_SETUP([ovn-controller - advertised route status schema compatibility]) +AT_KEYWORDS([ovn]) + +ovn_start + +# Connect a new ovn-controller to an SB database without +# Advertised_Route_Status. +$PYTHON -c ' +import json +import sys + +with open(sys.argv[[1]], encoding="utf-8") as stream: + schema = json.load(stream) +schema[["tables"]].pop("Advertised_Route_Status") +schema[["version"]] = "21.10.0" +schema.pop("cksum", None) +json.dump(schema, sys.stdout) +' "$abs_top_srcdir/ovn-sb.ovsschema" > old-sb.ovsschema +check ovsdb-client convert $OVN_SB_DB old-sb.ovsschema +OVS_WAIT_UNTIL([ + test X"`ovsdb-client get-schema-version $OVN_SB_DB OVN_Southbound`" \ + = X21.10.0 +]) + +# Restart ovsdb-server so that the _Server database reports the old schema. +as ovn-sb +OVS_APP_EXIT_AND_WAIT([ovsdb-server]) +mv "$ovs_base"/ovn-sb/ovsdb-server.log \ + "$ovs_base"/ovn-sb/ovsdb-server-before-schema-restart.log +as ovn-sb start_daemon ovsdb-server \ + -vjsonrpc \ + --remote=punix:"$ovs_base"/ovn-sb/ovn-sb.sock \ + --remote=db:OVN_Southbound,SB_Global,connections \ + --private-key="$PKIDIR"/testpki-test-privkey.pem \ + --certificate="$PKIDIR"/testpki-test-cert.pem \ + --ca-cert="$PKIDIR"/testpki-cacert.pem \ + "$ovs_base"/ovn-sb/ovn-sb.db +PARSE_LISTENING_PORT([ovn-sb/ovsdb-server.log], [TCP_PORT]) +SSL_OVN_SB_DB=ssl:127.0.0.1:$TCP_PORT +export SSL_OVN_SB_DB + +net_add n1 +sim_add hv1 +as hv1 +ovs-vsctl add-br br-phys +ovn_attach n1 br-phys 192.168.0.1 + +# The controller registers and omits status publication. +wait_row_count Chassis 1 name=hv1 +OVS_WAIT_FOR_OUTPUT([ovn-appctl -t ovn-controller connection-status], + [0], [connected +]) + +OVN_CLEANUP([hv1 +/lacks Advertised_Route_Status table/d]) +AT_CLEANUP +]) + AT_SETUP([ovn-controller - ssl/tls ciphers using command line options]) AT_KEYWORDS([ovn]) AT_SKIP_IF([test "$HAVE_OPENSSL" = no]) diff --git a/tests/ovn-inc-proc-graph-dump.at b/tests/ovn-inc-proc-graph-dump.at index 81f8c151f..ea70293ce 100644 --- a/tests/ovn-inc-proc-graph-dump.at +++ b/tests/ovn-inc-proc-graph-dump.at @@ -463,6 +463,7 @@ digraph "Incremental-Processing-Engine" { SB_datapath_binding -> route [[label="route_sb_datapath_binding_handler"]]; SB_service_monitor -> route [[label="route_sb_service_monitor_handler"]]; SB_learned_route [[style=filled, shape=box, fillcolor=white, label="SB_learned_route"]]; + SB_advertised_route_status [[style=filled, shape=box, fillcolor=white, label="SB_advertised_route_status"]]; route_table_notify [[style=filled, shape=box, fillcolor=white, label="route_table_notify"]]; route_exchange_status [[style=filled, shape=box, fillcolor=white, label="route_exchange_status"]]; route_exchange [[style=filled, shape=box, fillcolor=white, label="route_exchange"]]; @@ -471,6 +472,7 @@ digraph "Incremental-Processing-Engine" { route -> route_exchange [[label=""]]; SB_learned_route -> route_exchange [[label="engine_noop_handler"]]; SB_port_binding -> route_exchange [[label="engine_noop_handler"]]; + SB_advertised_route_status -> route_exchange [[label="route_exchange_sb_advertised_route_status_handler"]]; route_table_notify -> route_exchange [[label=""]]; route_exchange_status -> route_exchange [[label=""]]; sb_ro -> route_exchange [[label="route_exchange_sb_ro_handler"]]; diff --git a/tests/ovn-northd.at b/tests/ovn-northd.at index 8a7707362..e7912108a 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -15860,6 +15860,17 @@ AT_SETUP([RBAC -- Recover builtin role and permissions]) ovn_start wait_row_count RBAC_Role 1 name=ovn-controller +# Route status has a dedicated chassis-owned table. Do not grant +# ovn-controller column-wide access to the northd-owned Advertised_Route map, +# where administrative keys such as external_ids:enabled live. +check_row_count RBAC_Permission 0 table=Advertised_Route +check_column '[chassis_name]' RBAC_Permission authorization \ + table=Advertised_Route_Status +check_column true RBAC_Permission insert_delete \ + table=Advertised_Route_Status +check_column 'desired_status error operational_status withdrawal_reasons' \ + RBAC_Permission update \ + table=Advertised_Route_Status RBR_BUILTIN_PNAMES=$(fetch_column RBAC_Role permissions name=ovn-controller | uuidfilt | sed -e 's/<[[^<>]]>*//g' | tr -d '=,{}') declare -A RBR_BUILTIN_PERM diff --git a/tests/ovn-sbctl.at b/tests/ovn-sbctl.at index 5e9754ffd..2ce682870 100644 --- a/tests/ovn-sbctl.at +++ b/tests/ovn-sbctl.at @@ -96,6 +96,11 @@ ch2_uuid=$(ovn-sbctl -d bare --no-headings --columns _uuid find chassis name=ch2 check_uuid ovn-sbctl create Chassis_Private name=ch2 chassis=$ch2_uuid check_row_count Chassis_Private 1 +check_uuid ovn-sbctl create Advertised_Route_Status \ + advertised_route_uuid=$ch2_uuid chassis=$ch2_uuid chassis_name=ch2 \ + desired_status=advertised operational_status=installed +check_row_count Advertised_Route_Status 1 chassis_name=ch2 + AT_CHECK([ovn-sbctl -f csv -d bare --no-headings --columns ip,type list encap | sort], [0], [dnl 1.2.3.5,geneve @@ -110,6 +115,7 @@ AT_CHECK([ovn-sbctl -f csv -d bare --no-headings --columns ip,type list encap | 1.2.3.5,vxlan ]) check_row_count Chassis_Private 0 +check_row_count Advertised_Route_Status 0 chassis_name=ch2 as ovn-sb OVS_APP_EXIT_AND_WAIT([ovsdb-server]) @@ -322,4 +328,3 @@ AT_CHECK([ovn-sbctl count-flows wrongDatapath], [0], [dnl Total number of logical flows = 0 ]) ]) - diff --git a/tests/system-ovn.at b/tests/system-ovn.at index b28f4c991..ee5c2b810 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -22725,6 +22725,47 @@ wait_row_count sb:Advertised_Route 1 ip_prefix='"172.16.1.10"' lb_ar=$(ovn-sbctl --bare --columns=_uuid find Advertised_Route \ ip_prefix=172.16.1.10) test -n "$lb_ar" +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar \ + advertised_route_uuid=$lb_ar chassis_name=hv1 \ + desired_status=withdrawn operational_status=withdrawn +check_column 'service-monitor=no-online-backend' \ + Advertised_Route_Status withdrawal_reasons \ + advertised_route=$lb_ar chassis_name=hv1 + +# The hv1 role can change its status columns. The checks below reject writes +# to another chassis, identity columns and Advertised_Route. +if test -n "$SSL_OVN_SB_DB"; then + # Rejected writes are checked below with their expected nonzero status. + rbac_hv1_sbctl() { + command ovn-sbctl --db=$SSL_OVN_SB_DB \ + --private-key=$PKIDIR/testpki-hv1-privkey.pem \ + --certificate=$PKIDIR/testpki-hv1-cert.pem \ + --ca-cert=$PKIDIR/testpki-cacert.pem "$@" + } + + hv1_status=$(ovn-sbctl --bare --columns=_uuid \ + find Advertised_Route_Status advertised_route=$lb_ar \ + chassis_name=hv1) + hv2_status=$(ovn-sbctl create Advertised_Route_Status \ + advertised_route=$lb_ar advertised_route_uuid=$lb_ar \ + chassis_name=hv2 desired_status=advertised \ + operational_status=unknown) + + AT_CHECK([rbac_hv1_sbctl set Advertised_Route_Status $hv2_status \ + desired_status=withdrawn], [1], [ignore], [ignore]) + AT_CHECK([rbac_hv1_sbctl destroy Advertised_Route_Status $hv2_status], + [1], [ignore], [ignore]) + check rbac_hv1_sbctl set Advertised_Route_Status $hv1_status \ + withdrawal_reasons:rbac-test=allowed + AT_CHECK([rbac_hv1_sbctl set Advertised_Route_Status $hv1_status \ + chassis_name=hv2], [1], [ignore], [ignore]) + AT_CHECK([rbac_hv1_sbctl set Advertised_Route_Status $hv1_status \ + advertised_route_uuid=$hv2_status], [1], [ignore], [ignore]) + AT_CHECK([rbac_hv1_sbctl set Advertised_Route $lb_ar \ + external_ids:enabled=false], [1], [ignore], [ignore]) + + check ovn-sbctl destroy Advertised_Route_Status $hv2_status +fi # The offline monitor withdraws the route. AT_CHECK([ @@ -22740,6 +22781,7 @@ central_ar=$(ovn-sbctl --bare --columns=_uuid find Advertised_Route \ test -n "$central_ar" AT_CHECK([! ovn-sbctl --bare get Advertised_Route $central_ar external_ids | grep -q 'distributed-lb']) +wait_row_count sb:Advertised_Route_Status 0 chassis_name=hv1 OVS_WAIT_UNTIL([ ip route list vrf ovnvrf1339 | grep -q "blackhole 172.16.1.10"]) @@ -22756,6 +22798,9 @@ lb_ar=$(ovn-sbctl --bare --columns=_uuid find Advertised_Route \ test -n "$lb_ar" AT_CHECK([ovn-sbctl --bare get Advertised_Route $lb_ar external_ids | grep -q 'distributed-lb.*true']) +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar \ + advertised_route_uuid=$lb_ar chassis_name=hv1 \ + desired_status=withdrawn operational_status=withdrawn OVS_WAIT_UNTIL([ ! ip route list vrf ovnvrf1339 | grep -q "blackhole 172.16.1.10"]) @@ -22773,6 +22818,9 @@ check ovn-sbctl chassis-add hv2 geneve 169.0.0.2 hv2=$(fetch_column Chassis _uuid name=hv2) check ovn-sbctl set Port_Binding be0 chassis=$hv2 wait_row_count Service_Monitor 1 logical_port=be0 status=offline +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar \ + advertised_route_uuid=$lb_ar chassis_name=hv1 \ + desired_status=withdrawn operational_status=withdrawn AT_CHECK([ ip route list vrf ovnvrf1339 | grep -c "blackhole 172.16.1.10" || true ], [0], [0 @@ -22783,6 +22831,7 @@ AT_CHECK([ # recompute hv1's route node. check ovn-nbctl --wait=hv set Logical_Router_Port lr-origin-share \ options:dynamic-routing-redistribute-local-only=true +wait_row_count sb:Advertised_Route_Status 0 chassis_name=hv1 sm_v4=$(ovn-sbctl --bare --columns=_uuid find Service_Monitor \ logical_port=be0 ip='"192.168.0.10"' port=80 protocol=tcp) @@ -22818,12 +22867,23 @@ OVS_WAIT_UNTIL([ route_recompute_ct=$(ovn-appctl inc-engine/show-stats route recompute) test "$route_recompute_ct" -ne 0 ]) +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar \ + advertised_route_uuid=$lb_ar chassis_name=hv1 \ + desired_status=advertised operational_status=installed +check_column '' Advertised_Route_Status withdrawal_reasons \ + advertised_route=$lb_ar chassis_name=hv1 OVS_WAIT_UNTIL([ ip route list vrf ovnvrf1339 | grep -q "blackhole 172.16.1.10"]) # Stop the backend and wait for route withdrawal. kill `cat $be0_pid_file` wait_row_count Service_Monitor 1 logical_port=be0 status=offline +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar \ + advertised_route_uuid=$lb_ar chassis_name=hv1 \ + desired_status=withdrawn operational_status=withdrawn +check_column 'service-monitor=no-online-backend' \ + Advertised_Route_Status withdrawal_reasons \ + advertised_route=$lb_ar chassis_name=hv1 OVS_WAIT_UNTIL([ ! ip route list vrf ovnvrf1339 | grep -q "blackhole 172.16.1.10"]) @@ -22887,6 +22947,9 @@ OVS_WAIT_UNTIL([ route_recompute_ct=$(ovn-appctl inc-engine/show-stats route recompute) test "$route_recompute_ct" -ne 0 ]) +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar \ + advertised_route_uuid=$lb_ar chassis_name=hv1 \ + desired_status=withdrawn operational_status=withdrawn OVS_WAIT_UNTIL([ ! ip route list vrf ovnvrf1339 | grep -q "blackhole 172.16.1.10"]) check ovn-sbctl destroy Service_Monitor $unrelated_sm @@ -22920,6 +22983,9 @@ AT_CHECK([ ip -6 route list vrf ovnvrf1339 | grep -c "blackhole fd00::200" || true ], [0], [0 ]) +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar_v6 \ + advertised_route_uuid=$lb_ar_v6 chassis_name=hv1 \ + desired_status=withdrawn operational_status=withdrawn # Pause northd so it cannot overwrite the monitor ip, then rewrite the # ip to its expanded textual form. A forced recompute makes the @@ -22944,6 +23010,12 @@ AT_CHECK([ ip -6 route list vrf ovnvrf1339 | grep -c "blackhole fd00::200" || true ], [0], [0 ]) +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar_v6 \ + advertised_route_uuid=$lb_ar_v6 chassis_name=hv1 \ + desired_status=withdrawn operational_status=withdrawn +check_column 'service-monitor=no-online-backend' \ + Advertised_Route_Status withdrawal_reasons \ + advertised_route=$lb_ar_v6 chassis_name=hv1 # Resume northd. It repairs the monitor ip back to compressed form. check as northd ovn-appctl -t ovn-northd resume wait_row_count Service_Monitor 1 logical_port=be0 ip='"fd00::10"' \ @@ -22953,6 +23025,9 @@ wait_row_count Service_Monitor 1 logical_port=be0 ip='"fd00::10"' \ # unconditional. check ovn-nbctl clear Load_Balancer lb0 health_check wait_row_count Service_Monitor 0 logical_port=be0 +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar \ + advertised_route_uuid=$lb_ar chassis_name=hv1 \ + desired_status=advertised operational_status=installed OVS_WAIT_UNTIL([ ip route list vrf ovnvrf1339 | grep -q "blackhole 172.16.1.10"]) @@ -22972,10 +23047,21 @@ check test "$route_recompute_ct" -eq 0 check ovn-sbctl destroy Service_Monitor $ungated_sm check as northd ovn-appctl -t ovn-northd resume +# Add a second VIP so both routes can be deleted in one transaction. +check ovn-nbctl --wait=hv lb-add lb0 172.16.1.11:80 192.168.0.10:80 +lb_ar2=$(ovn-sbctl --bare --columns=_uuid find Advertised_Route \ + ip_prefix=172.16.1.11) +test -n "$lb_ar2" +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar2 \ + advertised_route_uuid=$lb_ar2 chassis_name=hv1 \ + desired_status=advertised operational_status=installed + # Remove the routes. check ovn-nbctl --wait=hv lr-lb-del lr-target lb0 wait_row_count sb:Advertised_Route 0 ip_prefix='"172.16.1.10"' +wait_row_count sb:Advertised_Route 0 ip_prefix='"172.16.1.11"' wait_row_count sb:Advertised_Route 0 ip_prefix='"fd00::200"' +wait_row_count sb:Advertised_Route_Status 0 chassis_name=hv1 OVS_APP_EXIT_AND_WAIT([ovn-controller]) @@ -23432,12 +23518,28 @@ lb_ar=$(ovn-sbctl --columns=_uuid,external_ids find Advertised_Route \ /source.*lb/ { print uuid; exit } ') test -n "$lb_ar" +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar \ + advertised_route_uuid=$lb_ar chassis_name=hv1 \ + desired_status=withdrawn operational_status=withdrawn + +# The NAT route has no status row. +nat_ar=$(ovn-sbctl --columns=_uuid,tracked_port find Advertised_Route \ + ip_prefix=172.16.1.10 | awk ' + /_uuid/ { uuid = $3 } + /tracked_port/ && length($3) != 36 { print uuid; exit } + ') +test -n "$nat_ar" +wait_row_count sb:Advertised_Route_Status 0 advertised_route=$nat_ar \ + chassis_name=hv1 # Start the listener and install the LB route. be_pid_file=$(mktemp be0_http.XXX.pid) NETNS_DAEMONIZE([be0_ns], [[$PYTHON $srcdir/test-l7.py http]], [$be_pid_file]) wait_row_count Service_Monitor 1 logical_port=be0 status=online +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar \ + advertised_route_uuid=$lb_ar chassis_name=hv1 \ + desired_status=advertised operational_status=installed # Both routes are installed at different priorities. AT_CHECK([ ip route list vrf ovnvrf1342 | grep -c "blackhole 172.16.1.10" @@ -23446,6 +23548,12 @@ AT_CHECK([ # Disable the LB route. The NAT route remains. check ovn-sbctl set Advertised_Route $lb_ar external_ids:enabled=false +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar \ + advertised_route_uuid=$lb_ar chassis_name=hv1 \ + desired_status=withdrawn operational_status=withdrawn +check_column 'administrative-policy=disabled' \ + Advertised_Route_Status withdrawal_reasons \ + advertised_route=$lb_ar chassis_name=hv1 AT_CHECK([ ip route list vrf ovnvrf1342 | grep -c "blackhole 172.16.1.10" ], [0], [1 @@ -23453,10 +23561,73 @@ AT_CHECK([ # Re-enable the LB route. check ovn-sbctl remove Advertised_Route $lb_ar external_ids enabled +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar \ + advertised_route_uuid=$lb_ar chassis_name=hv1 \ + desired_status=advertised operational_status=installed + +# Delete the local status row and wait for the controller to recreate it. +old_status=$(ovn-sbctl --bare --columns=_uuid \ + find Advertised_Route_Status advertised_route=$lb_ar chassis_name=hv1) +check ovn-sbctl destroy Advertised_Route_Status $old_status +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar \ + advertised_route_uuid=$lb_ar chassis_name=hv1 \ + desired_status=advertised operational_status=installed +repaired_status=$(ovn-sbctl --bare --columns=_uuid \ + find Advertised_Route_Status advertised_route=$lb_ar chassis_name=hv1) +AT_CHECK([test "$old_status" != "$repaired_status"]) + +# Corrupt the status and wait for the controller to restore it. +check ovn-sbctl set Advertised_Route_Status $repaired_status \ + desired_status=withdrawn operational_status=unknown \ + withdrawal_reasons:unexpected=forced-corruption error=forced-corruption +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar \ + advertised_route_uuid=$lb_ar chassis_name=hv1 \ + desired_status=advertised operational_status=installed +check_column '' Advertised_Route_Status withdrawal_reasons \ + advertised_route=$lb_ar chassis_name=hv1 +OVS_WAIT_UNTIL([ + test X"`ovn-sbctl --bare --columns=error \ + find Advertised_Route_Status advertised_route=$lb_ar \ + chassis_name=hv1`" = X +]) + +# The controller removes an unexpected row bearing its chassis name. +hv1_chassis=$(fetch_column Chassis _uuid name=hv1) +unexpected_status=$(ovn-sbctl create Advertised_Route_Status \ + advertised_route_uuid=$hv1_chassis chassis=$hv1_chassis \ + chassis_name=hv1 desired_status=advertised operational_status=unknown) +OVS_WAIT_UNTIL([ + ! ovn-sbctl get Advertised_Route_Status $unexpected_status _uuid \ + 2>/dev/null +]) + +# Table 252 (RT_TABLE_COMPAT) is rejected by route exchange. Check the +# reported error, then restore the VRF table. +check ovn-nbctl --wait=hv set Logical_Router lr-origin \ + options:dynamic-routing-vrf-id=252 +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar \ + chassis_name=hv1 desired_status=advertised operational_status=unknown +OVS_WAIT_UNTIL([ + ovn-sbctl --bare --columns=error find Advertised_Route_Status \ + advertised_route=$lb_ar chassis_name=hv1 | + grep -q 'invalid route table ID' +]) + +check ovn-nbctl --wait=hv set Logical_Router lr-origin \ + options:dynamic-routing-vrf-id=1342 +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar \ + chassis_name=hv1 desired_status=advertised operational_status=installed +OVS_WAIT_UNTIL([ + test X"`ovn-sbctl --bare --columns=error find Advertised_Route_Status \ + advertised_route=$lb_ar chassis_name=hv1`" = X +]) # Stop the listener and withdraw the LB route. kill `cat $be_pid_file` wait_row_count Service_Monitor 1 logical_port=be0 status=offline +wait_row_count sb:Advertised_Route_Status 1 advertised_route=$lb_ar \ + advertised_route_uuid=$lb_ar chassis_name=hv1 \ + desired_status=withdrawn operational_status=withdrawn # The NAT route remains installed. AT_CHECK([ ip route list vrf ovnvrf1342 | grep -c "blackhole 172.16.1.10" @@ -23515,6 +23686,8 @@ exact_ar=$(ovn-sbctl --bare --columns=_uuid find Advertised_Route \ test -n "$exact_ar" AT_CHECK([! ovn-sbctl --bare get Advertised_Route $exact_ar external_ids | \ grep -Eq 'source|distributed-lb|health-checks']) +wait_row_count sb:Advertised_Route_Status 0 advertised_route=$exact_ar \ + chassis_name=hv1 AT_CHECK([ ip route list vrf ovnvrf1342 | grep -c "blackhole 172.16.1.20" ], [0], [1 @@ -23630,6 +23803,12 @@ check ovn-sbctl set Port_Binding be0 chassis=$hv2 OVS_WAIT_UNTIL([ ! ip route list vrf ovnvrf1342 | grep -q "blackhole 172.16.1.10"]) +# With the backend remote and local-only enabled, hv1 must not +# publish an Advertised_Route_Status row for the VIP: the route is +# not selected for this chassis, so no status is recorded. +wait_row_count sb:Advertised_Route_Status 0 \ + chassis_name=hv1 + # Return the backend to hv1. check ovn-sbctl clear Port_Binding be0 chassis check ovn-sbctl destroy Chassis $hv2 @@ -23642,6 +23821,11 @@ wait_row_count Port_Binding 1 logical_port=be0 chassis=$hv1_uuid OVS_WAIT_UNTIL([ ip route list vrf ovnvrf1342 | grep -q "blackhole 172.16.1.10"]) +# With the backend local again, hv1 publishes an Advertised_Route_Status +# row with desired_status=advertised. +wait_row_count sb:Advertised_Route_Status 1 \ + chassis_name=hv1 + # Disable local-only. Even with the backend on a remote chassis the # route stays because local-only no longer restricts installation. check ovn-nbctl --wait=hv set Logical_Router_Port lr-origin-share \ diff --git a/utilities/ovn-sbctl.8.xml b/utilities/ovn-sbctl.8.xml index 18c4a5d36..12bc58dd1 100644 --- a/utilities/ovn-sbctl.8.xml +++ b/utilities/ovn-sbctl.8.xml @@ -322,8 +322,9 @@ <dt>[<code>--if-exists</code>] <var>chassis-del <var>chassis</var></var></dt> <dd> <p> - Deletes <var>chassis</var> and its <var>encaps</var> and - <var>gateway_ports</var>. + Deletes <var>chassis</var>, its <var>encaps</var> and + <var>gateway_ports</var>, and its + <code>Advertised_Route_Status</code> records. </p> <p> diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c index c06bc435c..d7fbc765e 100644 --- a/utilities/ovn-sbctl.c +++ b/utilities/ovn-sbctl.c @@ -360,6 +360,9 @@ pre_get_info(struct ctl_context *ctx) ovsdb_idl_add_column(ctx->idl, &sbrec_chassis_private_col_name); + ovsdb_idl_add_column(ctx->idl, + &sbrec_advertised_route_status_col_chassis_name); + ovsdb_idl_add_column(ctx->idl, &sbrec_encap_col_type); ovsdb_idl_add_column(ctx->idl, &sbrec_encap_col_ip); @@ -510,6 +513,20 @@ cmd_chassis_del(struct ctl_context *ctx) free(sbctl_ch_priv); } + if (sbrec_server_has_advertised_route_status_table(ctx->idl)) { + const struct sbrec_advertised_route_status_table + *status_table = + sbrec_advertised_route_status_table_get(ctx->idl); + const struct sbrec_advertised_route_status *status; + SBREC_ADVERTISED_ROUTE_STATUS_TABLE_FOR_EACH_SAFE ( + status, status_table) { + if (!strcmp(status->chassis_name, + sbctl_ch->ch_cfg->name)) { + sbrec_advertised_route_status_delete(status); + } + } + } + sbrec_chassis_delete(sbctl_ch->ch_cfg); } shash_find_and_delete(&sbctl_ctx->chassis, ctx->argv[1]); -- 2.53.0 _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
