This reverts commit 7031ffc6711951df4d484522e2ba0e8467ec10df. Advertised_Route_Status has no current consumer. ovn-heater measurements also confirmed that publishing status rows adds Southbound database work. Remove the table while its consumers and scalability requirements are reconsidered.
Signed-off-by: Dmitrii Shcherbakov <[email protected]> --- NEWS | 3 - 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 | 6 - tests/system-ovn.at | 184 --------------------- utilities/ovn-sbctl.8.xml | 5 +- utilities/ovn-sbctl.c | 17 -- 19 files changed, 20 insertions(+), 845 deletions(-) diff --git a/NEWS b/NEWS index 3f16ccdfb..c007767d0 100644 --- a/NEWS +++ b/NEWS @@ -1,8 +1,5 @@ Post v26.03.0 ------------- - - New Southbound Advertised_Route_Status table has been added. This is used - in order for ovn-controller instances to know the state of advertised - routes amongst each other. - Logical_Router_Port: Added a new "options:dhcpv4_primary_ip" key that selects which of the port's IPv4 addresses ovn-northd uses as the relay agent address (giaddr) in relayed DHCP packets. When diff --git a/controller/chassis.c b/controller/chassis.c index 07d375a81..0e3bd37d8 100644 --- a/controller/chassis.c +++ b/controller/chassis.c @@ -1181,26 +1181,14 @@ 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_advertised_route_status_table *status_table) + const struct sbrec_chassis_private *chassis_private_rec) { - 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) && !has_route_status) { + !is_chassis_idx_stored(ovs_table)) { return true; } + const char *chassis_name = get_ovs_chassis_id(ovs_table); if (ovs_idl_txn) { ovsdb_idl_txn_add_comment( ovs_idl_txn, @@ -1219,15 +1207,6 @@ 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 3be39c957..45dd9537a 100644 --- a/controller/chassis.h +++ b/controller/chassis.h @@ -29,7 +29,6 @@ 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; @@ -51,8 +50,7 @@ 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_advertised_route_status_table *); + const struct sbrec_chassis_private *); 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 38c1acfe8..e7fe5dbc3 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -175,7 +175,6 @@ 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. */ @@ -222,7 +221,6 @@ 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, @@ -243,8 +241,7 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, * * Monitor ECMP_Nexthop for local datapaths. * - * Monitor Advertised/Learned_Route for local datapaths and - * Advertised_Route_Status for the local chassis. + * Monitor Advertised/Learned_Route for local datapaths. * * 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 @@ -264,7 +261,6 @@ 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); @@ -301,7 +297,6 @@ 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; } @@ -370,16 +365,6 @@ 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; @@ -449,7 +434,6 @@ 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), }; @@ -473,7 +457,6 @@ 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; @@ -785,8 +768,7 @@ 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, chassis_id, - NULL, NULL, NULL, true); + update_sb_monitors(ovnsb_idl, NULL, NULL, NULL, NULL, true); if (sb_cond_seqno) { *sb_cond_seqno = next_cond_seqno; } @@ -5295,12 +5277,6 @@ 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; }; @@ -5357,11 +5333,9 @@ 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); @@ -5370,7 +5344,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; } @@ -5388,8 +5362,6 @@ 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; @@ -5408,7 +5380,6 @@ 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 @@ -5743,113 +5714,10 @@ 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), @@ -5859,11 +5727,6 @@ 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 = @@ -5903,106 +5766,6 @@ 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; @@ -7223,7 +6986,6 @@ 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) @@ -7369,8 +7131,6 @@ 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, @@ -7702,16 +7462,6 @@ 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); @@ -7984,8 +7734,7 @@ 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, NULL, false); + update_sb_monitors(ovnsb_idl_loop.idl, NULL, NULL, NULL, NULL, false); stopwatch_create(CONTROLLER_LOOP_STOPWATCH_NAME, SW_MS); stopwatch_create(OFCTRL_PUT_STOPWATCH_NAME, SW_MS); @@ -8320,7 +8069,6 @@ 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. @@ -8529,7 +8277,7 @@ main(int argc, char *argv[]) ovnsb_expected_cond_seqno; ovnsb_expected_cond_seqno = update_sb_monitors( - ovnsb_idl_loop.idl, chassis, chassis_id, + ovnsb_idl_loop.idl, chassis, &runtime_data->local_lports, &runtime_data->lbinding_data.bindings, &runtime_data->local_datapaths, @@ -8803,19 +8551,11 @@ 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, - status_table) && done; + chassis, chassis_private) && 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 24d5b5875..027375071 100644 --- a/controller/route-exchange.c +++ b/controller/route-exchange.c @@ -321,19 +321,13 @@ route_exchange_run(const struct route_exchange_ctx_in *r_ctx_in, int error; CLEAR_ROUTE_EXCHANGE_NL_STATUS(); - struct advertise_datapath_entry *ad; + const 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; } @@ -346,8 +340,6 @@ 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; } @@ -371,9 +363,6 @@ 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; } @@ -394,9 +383,6 @@ 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; } @@ -406,7 +392,6 @@ 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); @@ -440,27 +425,6 @@ 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 7ecc66671..a1ef4a359 100644 --- a/controller/route-exchange.h +++ b/controller/route-exchange.h @@ -26,9 +26,8 @@ struct route_exchange_ctx_in { struct ovsdb_idl_index *sbrec_learned_route_by_datapath; const struct sbrec_chassis *chassis; - /* Contains struct advertise_datapath_entry. route_exchange_run() records - * the reconciliation result in each entry. */ - struct hmap *announce_routes; + /* Contains struct advertise_datapath_entry */ + const struct hmap *announce_routes; }; struct route_exchange_ctx_out { diff --git a/controller/route.c b/controller/route.c index 00fd9a105..bfa27f4c7 100644 --- a/controller/route.c +++ b/controller/route.c @@ -637,58 +637,6 @@ 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) @@ -847,25 +795,14 @@ 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 cd078eb96..f629945a8 100644 --- a/controller/route.h +++ b/controller/route.h @@ -69,21 +69,6 @@ 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 { @@ -132,11 +117,6 @@ 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 dbb9b7a26..499cf9edd 100644 --- a/northd/ovn-northd.c +++ b/northd/ovn-northd.c @@ -132,10 +132,6 @@ 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; @@ -226,14 +222,6 @@ 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, @@ -953,11 +941,6 @@ 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 3587024bf..6b703f328 100644 --- a/ovn-sb.ovsschema +++ b/ovn-sb.ovsschema @@ -1,7 +1,7 @@ { "name": "OVN_Southbound", - "version": "21.12.0", - "cksum": "1561088612 38543", + "version": "21.11.0", + "cksum": "4289271680 36997", "tables": { "SB_Global": { "columns": { @@ -682,36 +682,6 @@ "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 d5c5210dc..037954800 100644 --- a/ovn-sb.xml +++ b/ovn-sb.xml @@ -5538,98 +5538,6 @@ 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 d4c1d3710..8084357b5 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_DATA) +check-system-dpdk: all 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_DATA) +check-kernel: all 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_DATA) +check-system-userspace: all 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 8af3be8d9..e17ebea76 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -3006,65 +3006,6 @@ 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 ea70293ce..81f8c151f 100644 --- a/tests/ovn-inc-proc-graph-dump.at +++ b/tests/ovn-inc-proc-graph-dump.at @@ -463,7 +463,6 @@ 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"]]; @@ -472,7 +471,6 @@ 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 8c8d7852e..85f001003 100644 --- a/tests/ovn-northd.at +++ b/tests/ovn-northd.at @@ -15867,17 +15867,6 @@ 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 2ce682870..c789d27d8 100644 --- a/tests/ovn-sbctl.at +++ b/tests/ovn-sbctl.at @@ -96,11 +96,6 @@ 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 @@ -115,7 +110,6 @@ 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]) diff --git a/tests/system-ovn.at b/tests/system-ovn.at index d4f2ce69e..dc1bb1d74 100644 --- a/tests/system-ovn.at +++ b/tests/system-ovn.at @@ -22725,47 +22725,6 @@ 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([ @@ -22781,7 +22740,6 @@ 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"]) @@ -22798,9 +22756,6 @@ 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"]) @@ -22818,9 +22773,6 @@ 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 @@ -22831,7 +22783,6 @@ 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) @@ -22867,23 +22818,12 @@ 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"]) @@ -22947,9 +22887,6 @@ 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 @@ -22983,9 +22920,6 @@ 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 @@ -23010,12 +22944,6 @@ 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"' \ @@ -23025,9 +22953,6 @@ 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"]) @@ -23047,21 +22972,10 @@ 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]) @@ -23518,28 +23432,12 @@ 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" @@ -23548,12 +23446,6 @@ 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 @@ -23561,73 +23453,10 @@ 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" @@ -23686,8 +23515,6 @@ 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 @@ -23803,12 +23630,6 @@ 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 @@ -23820,11 +23641,6 @@ 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 12bc58dd1..18c4a5d36 100644 --- a/utilities/ovn-sbctl.8.xml +++ b/utilities/ovn-sbctl.8.xml @@ -322,9 +322,8 @@ <dt>[<code>--if-exists</code>] <var>chassis-del <var>chassis</var></var></dt> <dd> <p> - Deletes <var>chassis</var>, its <var>encaps</var> and - <var>gateway_ports</var>, and its - <code>Advertised_Route_Status</code> records. + Deletes <var>chassis</var> and its <var>encaps</var> and + <var>gateway_ports</var>. </p> <p> diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c index d7fbc765e..c06bc435c 100644 --- a/utilities/ovn-sbctl.c +++ b/utilities/ovn-sbctl.c @@ -360,9 +360,6 @@ 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); @@ -513,20 +510,6 @@ 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
