@@ -35,6 +35,44 @@ 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_inited(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);
+}
+
void
chassis_register_ovs_idl(struct ovsdb_idl *ovs_idl)
{
@@ -103,16 +141,20 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
const struct ovsrec_bridge *br_int,
const struct sset *transport_zones)
{
- const struct sbrec_chassis *chassis_rec
- = chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id);
+ const struct ovsrec_open_vswitch *cfg;
+ const char *encap_type, *encap_ip;
+
+ const struct sbrec_chassis *chassis_rec = NULL;
+
+ if (chassis_info_id_inited(&chassis_state)) {
+ chassis_rec = chassis_lookup_by_name(sbrec_chassis_by_name,
+ chassis_info_id(&chassis_state));
+ }
+
if (!ovnsb_idl_txn) {
return chassis_rec;
}
- const struct ovsrec_open_vswitch *cfg;
- const char *encap_type, *encap_ip;
- static bool inited = false;
-
cfg = ovsrec_open_vswitch_table_first(ovs_table);
if (!cfg) {
VLOG_INFO("No Open_vSwitch row defined.");
@@ -251,10 +293,8 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
if (same) {
/* Nothing changed. */
- inited = true;
- ds_destroy(&iface_types);
- return chassis_rec;
- } else if (!inited) {
+ goto inited;
+ } else if (!chassis_info_id_inited(&chassis_state)) {
struct ds cur_encaps = DS_EMPTY_INITIALIZER;
for (int i = 0; i < chassis_rec->n_encaps; i++) {
ds_put_format(&cur_encaps, "%s,",
@@ -281,7 +321,6 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
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);
- sbrec_chassis_set_name(chassis_rec, chassis_id);
sbrec_chassis_set_hostname(chassis_rec, hostname);
sbrec_chassis_set_external_ids(chassis_rec, &ext_ids);
smap_destroy(&ext_ids);
@@ -315,7 +354,13 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn,
free(tokstr);
free(encaps);
- inited = true;
+inited:
+ /* Store the name of the chassis for further lookups. */
+ if (!chassis_rec->name || strcmp(chassis_id, chassis_rec->name)) {
+ sbrec_chassis_set_name(chassis_rec, chassis_id);
+ chassis_info_set_id(&chassis_state, chassis_id);
+ }
+
return chassis_rec;
}
@@ -336,3 +381,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_inited(&chassis_state)) {
+ return chassis_info_id(&chassis_state);
+ }
+
+ return NULL;
+}
@@ -36,5 +36,6 @@ const struct sbrec_chassis *chassis_run(
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 */
@@ -2062,7 +2062,7 @@ main(int argc, char *argv[])
const struct ovsrec_bridge *br_int = get_br_int(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)
@@ -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])
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 (shortly after start up) we store the chassis-id. 2. for subsequent chassis_run calls we use 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. Reported-at: https://bugzilla.redhat.com/1708146 Reported-by: Haidong Li <haili@redhat.com> Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- ovn/controller/chassis.c | 82 +++++++++++++++++++++++++++++++++------ ovn/controller/chassis.h | 1 ovn/controller/ovn-controller.c | 2 - tests/ovn-controller.at | 9 ++++ 4 files changed, 81 insertions(+), 13 deletions(-)