The chassis_run code didn't take into account the scenario when the system-id was changed in the Open_vSwitch table. Due to this the code was trying to insert a new Chassis record in the OVN_Southbound DB with the same Encaps as the previous Chassis record. The transaction used to insert the new records was aborting due to the ["type", "ip"] index constraint violation as we were creating new Encap entries with the same "type" and "ip" as the old ones.
In order to fix this issue the flow is now: 1. the first time ovn-controller initializes the Chassis entry (shortly after start up) we first check if there is a stale Chassis record in the OVN_Southbound DB by checking if any of the old Encap entries associated to the Chassis record match the new tunnel configuration. If found it means that ovn-controller didn't shutdown gracefully last time it was run so it didn't cleanup the Chassis table. Potentially in the meantime the OVS system-id was also changed. We then update the stale entry with the new configuration and store the last configured chassis-id in memory to avoid walking the Chassis table every time. 2. for subsequent chassis_run calls we use the last configured chassis-id stored at the previous step to lookup the old Chassis record. 3. when ovn-controller shuts down gracefully we lookup the Chassis record based on the chassis-id stored in memory at steps 1 and 2 above. This is to avoid failing to cleanup the Chassis record in OVN_Southbound DB if the OVS system-id changes between the last call to chassis_run and chassis_cleanup. With this commit we also: - refactor chassis.c to abstract the string processing and use library data structures (e.g., sset) - rename the get_chassis_id function in ovn-controller.c to get_ovs_chassis_id to avoid confusion with the newly added chassis_get_id function from chassis.c which returns the last successfully configured chassis-id. - add a test case in ovn-controller.at to check that OVS system-id changes are properly propagated to OVN_Southbound DB Reported-at: https://bugzilla.redhat.com/1708146 Reported-by: Haidong Li <ha...@redhat.com> Signed-off-by: Dumitru Ceara <dce...@redhat.com> --- ovn/controller/chassis.c | 660 ++++++++++++++++++++++++++++------------ ovn/controller/chassis.h | 4 +- ovn/controller/ovn-controller.c | 7 +- tests/ovn-controller.at | 9 + 4 files changed, 481 insertions(+), 199 deletions(-) diff --git a/ovn/controller/chassis.c b/ovn/controller/chassis.c index 0f537f1..28ac01d 100644 --- a/ovn/controller/chassis.c +++ b/ovn/controller/chassis.c @@ -35,6 +35,79 @@ VLOG_DEFINE_THIS_MODULE(chassis); #define HOST_NAME_MAX 255 #endif /* HOST_NAME_MAX */ +/* + * Structure to hold chassis specific state (currently just chassis-id) + * to avoid database lookups when changes happen while the controller is + * running. + */ +struct chassis_info { + /* Last ID we initialized the Chassis SB record with. */ + struct ds id; + + /* True if Chassis SB record is initialized, false otherwise. */ + uint32_t id_inited : 1; +}; + +static struct chassis_info chassis_state = { + .id = DS_EMPTY_INITIALIZER, + .id_inited = false, +}; + +static void +chassis_info_set_id(struct chassis_info *info, const char *id) +{ + ds_clear(&info->id); + ds_put_cstr(&info->id, id); + info->id_inited = true; +} + +static bool +chassis_info_id_initialized(const struct chassis_info *info) +{ + return info->id_inited; +} + +static const char * +chassis_info_id(const struct chassis_info *info) +{ + return ds_cstr_ro(&info->id); +} + +/* + * Structure for storing the chassis config parsed from the ovs table. + */ +struct ovs_chassis_cfg { + /* Single string fields parsed from external-ids. */ + const char *hostname; + const char *bridge_mappings; + const char *datapath_type; + const char *encap_csum; + const char *cms_options; + + /* Set of encap types parsed from the 'ovn-encap-type' external-id. */ + struct sset encap_type_set; + /* Set of encap IPs parsed from the 'ovn-encap-type' external-id. */ + struct sset encap_ip_set; + /* Interface type list formatted in the OVN-SB Chassis required format. */ + struct ds iface_types; +}; + +static void +ovs_chassis_cfg_init(struct ovs_chassis_cfg *cfg) +{ + sset_init(&cfg->encap_type_set); + sset_init(&cfg->encap_ip_set); + ds_init(&cfg->iface_types); +} + +static void +ovs_chassis_cfg_destroy(struct ovs_chassis_cfg *cfg) +{ + sset_destroy(&cfg->encap_type_set); + sset_destroy(&cfg->encap_ip_set); + ds_destroy(&cfg->iface_types); +} + void chassis_register_ovs_idl(struct ovsdb_idl *ovs_idl) { @@ -46,20 +119,21 @@ chassis_register_ovs_idl(struct ovsdb_idl *ovs_idl) } static const char * -pop_tunnel_name(uint32_t *type) +get_hostname(const struct smap *ext_ids) { - if (*type & GENEVE) { - *type &= ~GENEVE; - return "geneve"; - } else if (*type & STT) { - *type &= ~STT; - return "stt"; - } else if (*type & VXLAN) { - *type &= ~VXLAN; - return "vxlan"; + const char *hostname = smap_get_def(ext_ids, "hostname", ""); + + if (strlen(hostname) == 0) { + static char hostname_[HOST_NAME_MAX + 1]; + + if (gethostname(hostname_, sizeof(hostname_))) { + hostname_[0] = 0; + } + + return &hostname_[0]; } - OVS_NOT_REACHED(); + return hostname; } static const char * @@ -74,6 +148,23 @@ get_cms_options(const struct smap *ext_ids) return smap_get_def(ext_ids, "ovn-cms-options", ""); } + +static const char * +get_encap_csum(const struct smap *ext_ids) +{ + return smap_get_def(ext_ids, "ovn-encap-csum", "true"); +} + +static const char * +get_datapath_type(const struct ovsrec_bridge *br_int) +{ + if (br_int && br_int->datapath_type) { + return br_int->datapath_type; + } + + return ""; +} + static void update_chassis_transport_zones(const struct sset *transport_zones, const struct sbrec_chassis *chassis_rec) @@ -94,228 +185,394 @@ update_chassis_transport_zones(const struct sset *transport_zones, sset_destroy(&chassis_tzones_set); } -/* Returns this chassis's Chassis record, if it is available. */ -const struct sbrec_chassis * -chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn, - struct ovsdb_idl_index *sbrec_chassis_by_name, - const struct ovsrec_open_vswitch_table *ovs_table, - const char *chassis_id, - const struct ovsrec_bridge *br_int, - const struct sset *transport_zones) +/* + * Parse an ovs 'encap_type' string and stores the resulting types in the + * 'encap_type_set' string set. + */ +static bool +chassis_parse_ovs_encap_type(const char *encap_type, + struct sset *encap_type_set) +{ + sset_from_delimited_string(encap_type_set, encap_type, ","); + + const char *type; + + SSET_FOR_EACH (type, encap_type_set) { + if (!get_tunnel_type(type)) { + VLOG_INFO("Unknown tunnel type: %s", type); + } + } + + return true; +} + +/* + * Parse an ovs 'encap_ip' string and stores the resulting IP representations + * in the 'encap_ip_set' string set. + */ +static bool +chassis_parse_ovs_encap_ip(const char *encap_ip, struct sset *encap_ip_set) { - const struct sbrec_chassis *chassis_rec - = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); - if (!ovnsb_idl_txn) { - return chassis_rec; + sset_from_delimited_string(encap_ip_set, encap_ip, ","); + return true; +} + +/* + * Parse the ovs 'iface_types' and store them in the format required by the + * Chassis record. + */ +static bool +chassis_parse_ovs_iface_types(char **iface_types, size_t n_iface_types, + struct ds *iface_types_str) +{ + for (size_t i = 0; i < n_iface_types; i++) { + ds_put_format(iface_types_str, "%s,", iface_types[i]); } + ds_chomp(iface_types_str, ','); + return true; +} - const struct ovsrec_open_vswitch *cfg; - const char *encap_type, *encap_ip; - static bool inited = false; +/* + * Parse the 'ovs_table' entry and populate 'ovs_cfg'. + */ +static bool +chassis_parse_ovs_config(const struct ovsrec_open_vswitch_table *ovs_table, + const struct ovsrec_bridge *br_int, + struct ovs_chassis_cfg *ovs_cfg) +{ + const struct ovsrec_open_vswitch *cfg = + ovsrec_open_vswitch_table_first(ovs_table); - cfg = ovsrec_open_vswitch_table_first(ovs_table); if (!cfg) { VLOG_INFO("No Open_vSwitch row defined."); - return NULL; + return false; } - encap_type = smap_get(&cfg->external_ids, "ovn-encap-type"); - encap_ip = smap_get(&cfg->external_ids, "ovn-encap-ip"); - if (!encap_type || !encap_ip) { + const char *encap_type = smap_get(&cfg->external_ids, "ovn-encap-type"); + const char *encap_ips = smap_get(&cfg->external_ids, "ovn-encap-ip"); + if (!encap_type || !encap_ips) { VLOG_INFO("Need to specify an encap type and ip"); - return NULL; + return false; } - char *tokstr = xstrdup(encap_type); - char *save_ptr = NULL; - char *token; - uint32_t req_tunnels = 0; - for (token = strtok_r(tokstr, ",", &save_ptr); token != NULL; - token = strtok_r(NULL, ",", &save_ptr)) { - uint32_t type = get_tunnel_type(token); - if (!type) { - VLOG_INFO("Unknown tunnel type: %s", token); - } - req_tunnels |= type; + ovs_cfg->hostname = get_hostname(&cfg->external_ids); + ovs_cfg->bridge_mappings = get_bridge_mappings(&cfg->external_ids); + ovs_cfg->datapath_type = get_datapath_type(br_int); + ovs_cfg->encap_csum = get_encap_csum(&cfg->external_ids); + ovs_cfg->cms_options = get_cms_options(&cfg->external_ids); + + if (!chassis_parse_ovs_encap_type(encap_type, &ovs_cfg->encap_type_set)) { + return false; } - free(tokstr); - - tokstr = xstrdup(encap_ip); - save_ptr = NULL; - uint32_t nencap_ips = 0; - for (token = strtok_r(tokstr, ",", &save_ptr); token != NULL; - token = strtok_r(NULL, ",", &save_ptr)) { - nencap_ips++; + + if (!chassis_parse_ovs_encap_ip(encap_ips, &ovs_cfg->encap_ip_set)) { + sset_destroy(&ovs_cfg->encap_type_set); + return false; } - free(tokstr); - const char *hostname = smap_get_def(&cfg->external_ids, "hostname", ""); - char hostname_[HOST_NAME_MAX + 1]; - if (!hostname[0]) { - if (gethostname(hostname_, sizeof hostname_)) { - hostname_[0] = '\0'; - } - hostname = hostname_; + if (!chassis_parse_ovs_iface_types(cfg->iface_types, + cfg->n_iface_types, + &ovs_cfg->iface_types)) { + sset_destroy(&ovs_cfg->encap_type_set); + sset_destroy(&ovs_cfg->encap_ip_set); + } + + return true; +} + +static void +chassis_build_external_ids(struct smap *ext_ids, const char *bridge_mappings, + const char *datapath_type, const char *cms_options, + const char *iface_types) +{ + smap_replace(ext_ids, "ovn-bridge-mappings", bridge_mappings); + smap_replace(ext_ids, "datapath-type", datapath_type); + smap_replace(ext_ids, "ovn-cms-options", cms_options); + smap_replace(ext_ids, "iface-types", iface_types); +} + +/* + * Returns true if any external-id doesn't match the values in 'chassis-rec'. + */ +static bool +chassis_external_ids_changed(const char *bridge_mappings, + const char *datapath_type, + const char *cms_options, + const struct ds *iface_types, + const struct sbrec_chassis *chassis_rec) +{ + const char *chassis_bridge_mappings = + get_bridge_mappings(&chassis_rec->external_ids); + + if (strcmp(bridge_mappings, chassis_bridge_mappings)) { + return true; } - const char *bridge_mappings = get_bridge_mappings(&cfg->external_ids); - const char *datapath_type = - br_int && br_int->datapath_type ? br_int->datapath_type : ""; - const char *cms_options = get_cms_options(&cfg->external_ids); + const char *chassis_datapath_type = + smap_get_def(&chassis_rec->external_ids, "datapath-type", ""); - struct ds iface_types = DS_EMPTY_INITIALIZER; - ds_put_cstr(&iface_types, ""); - for (int j = 0; j < cfg->n_iface_types; j++) { - ds_put_format(&iface_types, "%s,", cfg->iface_types[j]); + if (strcmp(datapath_type, chassis_datapath_type)) { + return true; + } + + const char *chassis_cms_options = + get_cms_options(&chassis_rec->external_ids); + + if (strcmp(cms_options, chassis_cms_options)) { + return true; } - ds_chomp(&iface_types, ','); - const char *iface_types_str = ds_cstr(&iface_types); - - const char *encap_csum = smap_get_def(&cfg->external_ids, - "ovn-encap-csum", "true"); - int n_encaps = count_1bits(req_tunnels); - if (chassis_rec) { - if (strcmp(hostname, chassis_rec->hostname)) { - sbrec_chassis_set_hostname(chassis_rec, hostname); + + const char *chassis_iface_types = + smap_get_def(&chassis_rec->external_ids, "iface-types", ""); + + if (strcmp(ds_cstr_ro(iface_types), chassis_iface_types)) { + return true; + } + + return false; +} + +/* + * Returns true if the tunnel config obtained by combining 'encap_type_set' + * with 'encap_ip_set' and 'encap_csum' doesn't match the values in + * 'chassis-rec'. + */ +static bool +chassis_tunnels_changed(const struct sset *encap_type_set, + const struct sset *encap_ip_set, + const char *encap_csum, + const struct sbrec_chassis *chassis_rec) +{ + size_t encap_type_count = 0; + + for (int i = 0; i < chassis_rec->n_encaps; i++) { + if (strcmp(chassis_rec->name, chassis_rec->encaps[i]->chassis_name)) { + return true; } - update_chassis_transport_zones(transport_zones, chassis_rec); - - /* Determine new values for Chassis external-ids. */ - const char *chassis_bridge_mappings - = get_bridge_mappings(&chassis_rec->external_ids); - const char *chassis_datapath_type - = smap_get_def(&chassis_rec->external_ids, "datapath-type", ""); - const char *chassis_iface_types - = smap_get_def(&chassis_rec->external_ids, "iface-types", ""); - const char *chassis_cms_options - = get_cms_options(&chassis_rec->external_ids); - - /* If any of the external-ids should change, update them. */ - if (strcmp(bridge_mappings, chassis_bridge_mappings) || - strcmp(datapath_type, chassis_datapath_type) || - strcmp(iface_types_str, chassis_iface_types) || - strcmp(cms_options, chassis_cms_options)) { - struct smap new_ids; - smap_clone(&new_ids, &chassis_rec->external_ids); - smap_replace(&new_ids, "ovn-bridge-mappings", bridge_mappings); - smap_replace(&new_ids, "datapath-type", datapath_type); - smap_replace(&new_ids, "iface-types", iface_types_str); - smap_replace(&new_ids, "ovn-cms-options", cms_options); - sbrec_chassis_verify_external_ids(chassis_rec); - sbrec_chassis_set_external_ids(chassis_rec, &new_ids); - smap_destroy(&new_ids); + if (!sset_contains(encap_type_set, chassis_rec->encaps[i]->type)) { + return true; } + encap_type_count++; - /* Compare desired tunnels against those currently in the database. */ - - /* - * We walk through the types and the IP's rather than check for the - * combination since we create a mesh; if we create specific tunnel- - * type combinations, then we'd need to check for the type-remote-ip - * pair. - */ - uint32_t cur_tunnels = 0; - bool same = true; - for (int i = 0; i < chassis_rec->n_encaps; i++) { - cur_tunnels |= get_tunnel_type(chassis_rec->encaps[i]->type); - - same = same && !strcmp( - smap_get_def(&chassis_rec->encaps[i]->options, "csum", ""), - encap_csum); + if (!sset_contains(encap_ip_set, chassis_rec->encaps[i]->ip)) { + return true; } - same = same && req_tunnels == cur_tunnels; - - same = same && chassis_rec->n_encaps == nencap_ips * n_encaps; - if (same) { - tokstr = xstrdup(encap_ip); - save_ptr = NULL; - bool found = false; - - for (token = strtok_r(tokstr, ",", &save_ptr); token != NULL; - token = strtok_r(NULL, ",", &save_ptr)) { - found = false; - for (int i = 0; i < chassis_rec->n_encaps; i++) { - if (!strcmp(chassis_rec->encaps[i]->ip, token)) { - found = true; - break; - } - } - same = same && found; - if (!same) { - break; - } - } - free(tokstr); + + if (strcmp(smap_get_def(&chassis_rec->encaps[i]->options, "csum", ""), + encap_csum)) { + return true; + } + } + + size_t tunnel_count = + sset_count(encap_type_set) * sset_count(encap_ip_set); + + if (tunnel_count != chassis_rec->n_encaps) { + return true; + } + + if (sset_count(encap_type_set) != encap_type_count) { + return true; + } + + return false; +} + +/* + * Build the new encaps config (full mesh of 'encap_type_set' and + * 'encap_ip_set'). Allocates and stores the new 'n_encap' Encap records in + * 'encaps'. + */ +static struct sbrec_encap ** +chassis_build_encaps(struct ovsdb_idl_txn *ovnsb_idl_txn, + const struct sset *encap_type_set, + const struct sset *encap_ip_set, + const char *chassis_id, + const char *encap_csum, + size_t *n_encap) +{ + size_t tunnel_count = 0; + + struct sbrec_encap **encaps = + xmalloc(sset_count(encap_type_set) * sset_count(encap_ip_set) * + sizeof(*encaps)); + const struct smap options = SMAP_CONST1(&options, "csum", encap_csum); + + const char *encap_ip; + const char *encap_type; + + SSET_FOR_EACH (encap_ip, encap_ip_set) { + SSET_FOR_EACH (encap_type, encap_type_set) { + struct sbrec_encap *encap = sbrec_encap_insert(ovnsb_idl_txn); + + sbrec_encap_set_type(encap, encap_type); + sbrec_encap_set_ip(encap, encap_ip); + sbrec_encap_set_options(encap, &options); + sbrec_encap_set_chassis_name(encap, chassis_id); + + encaps[tunnel_count] = encap; + tunnel_count++; } + } - if (same) { - /* Nothing changed. */ - inited = true; - ds_destroy(&iface_types); - return chassis_rec; - } else if (!inited) { - struct ds cur_encaps = DS_EMPTY_INITIALIZER; - for (int i = 0; i < chassis_rec->n_encaps; i++) { - ds_put_format(&cur_encaps, "%s,", - chassis_rec->encaps[i]->type); + *n_encap = tunnel_count; + return encaps; +} + +/* + * Returns a pointer to a chassis record from 'chassis_table' that + * matches at least one tunnel config. + */ +static const struct sbrec_chassis * +chassis_get_stale_record(const struct sbrec_chassis_table *chassis_table, + const struct ovs_chassis_cfg *ovs_cfg, + const char *chassis_id) +{ + const struct sbrec_chassis *chassis_rec; + + SBREC_CHASSIS_TABLE_FOR_EACH (chassis_rec, chassis_table) { + for (size_t i = 0; i < chassis_rec->n_encaps; i++) { + if (sset_contains(&ovs_cfg->encap_type_set, + chassis_rec->encaps[i]->type) && + sset_contains(&ovs_cfg->encap_ip_set, + chassis_rec->encaps[i]->ip)) { + return chassis_rec; + } + if (strcmp(chassis_rec->name, chassis_id) == 0) { + return chassis_rec; } - ds_chomp(&cur_encaps, ','); - - VLOG_WARN("Chassis config changing on startup, make sure " - "multiple chassis are not configured : %s/%s->%s/%s", - ds_cstr(&cur_encaps), - chassis_rec->encaps[0]->ip, - encap_type, encap_ip); - ds_destroy(&cur_encaps); } } - ovsdb_idl_txn_add_comment(ovnsb_idl_txn, - "ovn-controller: registering chassis '%s'", - chassis_id); + return NULL; +} - if (!chassis_rec) { - struct smap ext_ids = SMAP_INITIALIZER(&ext_ids); - smap_add(&ext_ids, "ovn-bridge-mappings", bridge_mappings); - smap_add(&ext_ids, "datapath-type", datapath_type); - smap_add(&ext_ids, "iface-types", iface_types_str); - chassis_rec = sbrec_chassis_insert(ovnsb_idl_txn); +/* If this is a chassis config update after we initialized the record once + * then we should always be able to find it with the ID we saved in + * chassis_state. + * Otherwise (i.e., first time we create the record) then we check if there's + * a stale record from a previous controller run that didn't end gracefully + * and reuse it. If not then we create a new record. + */ +static const struct sbrec_chassis * +chassis_get_record(struct ovsdb_idl_txn *ovnsb_idl_txn, + struct ovsdb_idl_index *sbrec_chassis_by_name, + const struct sbrec_chassis_table *chassis_table, + const struct ovs_chassis_cfg *ovs_cfg, + const char *chassis_id) +{ + const struct sbrec_chassis *chassis_rec; + + if (chassis_info_id_initialized(&chassis_state)) { + chassis_rec = chassis_lookup_by_name(sbrec_chassis_by_name, + chassis_info_id(&chassis_state)); + if (!chassis_rec) { + VLOG_WARN("Could not find Chassis : stored (%s) ovs (%s)", + chassis_info_id(&chassis_state), chassis_id); + } + } else { + chassis_rec = + chassis_get_stale_record(chassis_table, ovs_cfg, chassis_id); + + if (!chassis_rec && ovnsb_idl_txn) { + chassis_rec = sbrec_chassis_insert(ovnsb_idl_txn); + } + } + return chassis_rec; +} + +/* Update a Chassis record based on the config in the ovs config. */ +static void +chassis_update(const struct sbrec_chassis *chassis_rec, + struct ovsdb_idl_txn *ovnsb_idl_txn, + const struct ovs_chassis_cfg *ovs_cfg, + const char *chassis_id, + const struct sset *transport_zones) +{ + if (strcmp(chassis_id, chassis_rec->name)) { sbrec_chassis_set_name(chassis_rec, chassis_id); - sbrec_chassis_set_hostname(chassis_rec, hostname); + } + + if (strcmp(ovs_cfg->hostname, chassis_rec->hostname)) { + sbrec_chassis_set_hostname(chassis_rec, ovs_cfg->hostname); + } + + if (chassis_external_ids_changed(ovs_cfg->bridge_mappings, + ovs_cfg->datapath_type, + ovs_cfg->cms_options, + &ovs_cfg->iface_types, + chassis_rec)) { + struct smap ext_ids; + + smap_clone(&ext_ids, &chassis_rec->external_ids); + chassis_build_external_ids(&ext_ids, ovs_cfg->bridge_mappings, + ovs_cfg->datapath_type, + ovs_cfg->cms_options, + ds_cstr_ro(&ovs_cfg->iface_types)); + sbrec_chassis_verify_external_ids(chassis_rec); sbrec_chassis_set_external_ids(chassis_rec, &ext_ids); smap_destroy(&ext_ids); } - ds_destroy(&iface_types); - struct sbrec_encap **encaps = - xmalloc((nencap_ips * n_encaps) * sizeof *encaps); - const struct smap options = SMAP_CONST1(&options, "csum", encap_csum); - tokstr = xstrdup(encap_ip); - save_ptr = NULL; - uint32_t save_req_tunnels = req_tunnels; - uint32_t tuncnt = 0; - for (token = strtok_r(tokstr, ",", &save_ptr); token != NULL; - token = strtok_r(NULL, ",", &save_ptr)) { - - req_tunnels = save_req_tunnels; - for (int i = 0; i < n_encaps; i++) { - const char *type = pop_tunnel_name(&req_tunnels); - - encaps[tuncnt] = sbrec_encap_insert(ovnsb_idl_txn); - - sbrec_encap_set_type(encaps[tuncnt], type); - sbrec_encap_set_ip(encaps[tuncnt], token); - sbrec_encap_set_options(encaps[tuncnt], &options); - sbrec_encap_set_chassis_name(encaps[tuncnt], chassis_id); - tuncnt++; - } + update_chassis_transport_zones(transport_zones, chassis_rec); + + /* If any of the encaps should change, update them. */ + bool tunnels_changed = + chassis_tunnels_changed(&ovs_cfg->encap_type_set, + &ovs_cfg->encap_ip_set, ovs_cfg->encap_csum, + chassis_rec); + if (!tunnels_changed) { + return; } - sbrec_chassis_set_encaps(chassis_rec, encaps, tuncnt); - free(tokstr); + + struct sbrec_encap **encaps; + size_t n_encap; + + encaps = + chassis_build_encaps(ovnsb_idl_txn, &ovs_cfg->encap_type_set, + &ovs_cfg->encap_ip_set, chassis_id, + ovs_cfg->encap_csum, &n_encap); + sbrec_chassis_set_encaps(chassis_rec, encaps, n_encap); free(encaps); +} - inited = true; +/* Returns this chassis's Chassis record, if it is available. */ +const struct sbrec_chassis * +chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn, + struct ovsdb_idl_index *sbrec_chassis_by_name, + const struct ovsrec_open_vswitch_table *ovs_table, + const struct sbrec_chassis_table *chassis_table, + const char *chassis_id, + const struct ovsrec_bridge *br_int, + const struct sset *transport_zones) +{ + struct ovs_chassis_cfg ovs_cfg; + + /* Get the chassis config from the ovs table. */ + ovs_chassis_cfg_init(&ovs_cfg); + if (!chassis_parse_ovs_config(ovs_table, br_int, &ovs_cfg)) { + return NULL; + } + + const struct sbrec_chassis *chassis_rec = + chassis_get_record(ovnsb_idl_txn, sbrec_chassis_by_name, + chassis_table, &ovs_cfg, chassis_id); + + /* If we found (or created) a record, update it with the correct config + * and store the current chassis_id for fast lookup in case it gets + * modified in the ovs table. + */ + if (chassis_rec && ovnsb_idl_txn) { + chassis_update(chassis_rec, ovnsb_idl_txn, &ovs_cfg, chassis_id, + transport_zones); + chassis_info_set_id(&chassis_state, chassis_id); + ovsdb_idl_txn_add_comment(ovnsb_idl_txn, + "ovn-controller: registering chassis '%s'", + chassis_id); + } + + ovs_chassis_cfg_destroy(&ovs_cfg); return chassis_rec; } @@ -336,3 +593,16 @@ chassis_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, } return false; } + +/* + * Returns the last initialized chassis-id. + */ +const char * +chassis_get_id(void) +{ + if (chassis_info_id_initialized(&chassis_state)) { + return chassis_info_id(&chassis_state); + } + return NULL; +} + diff --git a/ovn/controller/chassis.h b/ovn/controller/chassis.h index 9847e19..26ba18d 100644 --- a/ovn/controller/chassis.h +++ b/ovn/controller/chassis.h @@ -31,10 +31,12 @@ void chassis_register_ovs_idl(struct ovsdb_idl *); const struct sbrec_chassis *chassis_run( struct ovsdb_idl_txn *ovnsb_idl_txn, struct ovsdb_idl_index *sbrec_chassis_by_name, - const struct ovsrec_open_vswitch_table *, + const struct ovsrec_open_vswitch_table *ovs_table, + const struct sbrec_chassis_table *chassis_table, const char *chassis_id, const struct ovsrec_bridge *br_int, const struct sset *transport_zones); bool chassis_cleanup(struct ovsdb_idl_txn *ovnsb_idl_txn, const struct sbrec_chassis *); +const char *chassis_get_id(void); #endif /* ovn/chassis.h */ diff --git a/ovn/controller/ovn-controller.c b/ovn/controller/ovn-controller.c index 69eeee5..c7df8c0 100644 --- a/ovn/controller/ovn-controller.c +++ b/ovn/controller/ovn-controller.c @@ -269,7 +269,7 @@ get_br_int(struct ovsdb_idl_txn *ovs_idl_txn, } static const char * -get_chassis_id(const struct ovsrec_open_vswitch_table *ovs_table) +get_ovs_chassis_id(const struct ovsrec_open_vswitch_table *ovs_table) { const struct ovsrec_open_vswitch *cfg = ovsrec_open_vswitch_table_first(ovs_table); @@ -698,7 +698,7 @@ main(int argc, char *argv[]) ovsrec_bridge_table_get(ovs_idl_loop.idl), ovsrec_open_vswitch_table_get(ovs_idl_loop.idl)); const char *chassis_id - = get_chassis_id(ovsrec_open_vswitch_table_get( + = get_ovs_chassis_id(ovsrec_open_vswitch_table_get( ovs_idl_loop.idl)); const struct sbrec_chassis *chassis = NULL; @@ -706,6 +706,7 @@ main(int argc, char *argv[]) chassis = chassis_run( ovnsb_idl_txn, sbrec_chassis_by_name, ovsrec_open_vswitch_table_get(ovs_idl_loop.idl), + sbrec_chassis_table_get(ovnsb_idl_loop.idl), chassis_id, br_int, &transport_zones); encaps_run( ovs_idl_txn, @@ -927,7 +928,7 @@ main(int argc, char *argv[]) const struct ovsrec_bridge *br_int = get_br_int(ovs_idl_txn, bridge_table, ovs_table); - const char *chassis_id = get_chassis_id(ovs_table); + const char *chassis_id = chassis_get_id(); const struct sbrec_chassis *chassis = (chassis_id ? chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id) diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index d4bb071..343c2ab 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -187,6 +187,15 @@ OVS_WAIT_UNTIL([ test "${expected_iface_types}" = "${chassis_iface_types}" ]) +# Change the value of external_ids:system-id and make sure it's mirrored +# in the Chassis record in the OVN_Southbound database. +sysid=${sysid}-foo +ovs-vsctl set Open_vSwitch . external-ids:system-id="${sysid}" +OVS_WAIT_UNTIL([ + chassis_id=$(ovn-sbctl get Chassis "${sysid}" name) + test "${sysid}" = "${chassis_id}" +]) + # Gracefully terminate daemons OVN_CLEANUP_SBOX([hv]) OVN_CLEANUP_VSWITCH([main]) -- 1.8.3.1 _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev