Message ID | 20200903150357.26775.17767.stgit@dceara.remote.csb |
---|---|
State | Accepted |
Headers | show |
Series | [ovs-dev,v2,1/2] chassis: Fix the way encaps are updated for a chassis record. | expand |
On Thu, Sep 3, 2020 at 8:34 PM Dumitru Ceara <dceara@redhat.com> wrote: > Also: > - Change conditional monitoring for Chassis_Private to use the chassis uuid > instead of chassis name. Using the chassis->name field does not work > because this is the old value of the field and would cause ovsdb-server > to inform ovn-controller that the updated Chassis_Private record was > "deleted" because it doesn't match the monitor condition anymore. > - Allow ovn-sbctl to access Chassis_Private records by name. > > Reported-at: https://bugzilla.redhat.com/1873032 > Reported-by: Ying Xu <yinxu@redhat.com> > CC: Han Zhou <hzhou@ovn.org> > Fixes: 4adc10f58127 ("Avoid nb_cfg update notification flooding") > Signed-off-by: Dumitru Ceara <dceara@redhat.com> > > --- > v2: > - Fix the monitor condition update reported by Numan. > Thanks Dumitru. I applied both the patches to master. Numan > --- > controller/chassis.c | 92 > ++++++++++++++++++++++++++++++++++++++----- > controller/chassis.h | 2 + > controller/ovn-controller.c | 13 ++++-- > tests/ovn-controller.at | 18 ++++++++ > utilities/ovn-sbctl.c | 3 + > 5 files changed, 112 insertions(+), 16 deletions(-) > > diff --git a/controller/chassis.c b/controller/chassis.c > index 97120a9..45e018e 100644 > --- a/controller/chassis.c > +++ b/controller/chassis.c > @@ -635,6 +635,77 @@ chassis_update(const struct sbrec_chassis > *chassis_rec, > return true; > } > > +/* > + * Returns a pointer to a chassis_private record from 'chassis_pvt_table' > that > + * matches the chassis record. > + */ > +static const struct sbrec_chassis_private * > +chassis_private_get_stale_record( > + const struct sbrec_chassis_private_table *chassis_pvt_table, > + const struct sbrec_chassis *chassis) > +{ > + const struct sbrec_chassis_private *chassis_pvt_rec; > + > + SBREC_CHASSIS_PRIVATE_TABLE_FOR_EACH (chassis_pvt_rec, > chassis_pvt_table) { > + if (chassis_pvt_rec->chassis == chassis) { > + return chassis_pvt_rec; > + } > + } > + > + return NULL; > +} > + > +/* If this is a chassis_private 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 created the chassis record or if the > + * system-id changed) 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. > + * > + * Returns the local chassis record. > + */ > +static const struct sbrec_chassis_private * > +chassis_private_get_record( > + struct ovsdb_idl_txn *ovnsb_idl_txn, > + struct ovsdb_idl_index *sbrec_chassis_pvt_by_name, > + const struct sbrec_chassis_private_table *chassis_pvt_table, > + const struct sbrec_chassis *chassis) > +{ > + const struct sbrec_chassis_private *chassis_p = NULL; > + > + if (chassis_info_id_inited(&chassis_state)) { > + chassis_p = > + chassis_private_lookup_by_name(sbrec_chassis_pvt_by_name, > + > chassis_info_id(&chassis_state)); > + } > + > + if (!chassis_p) { > + chassis_p = chassis_private_get_stale_record(chassis_pvt_table, > + chassis); > + } > + > + if (!chassis_p && ovnsb_idl_txn) { > + return sbrec_chassis_private_insert(ovnsb_idl_txn); > + } > + > + return chassis_p; > +} > + > +static void > +chassis_private_update(const struct sbrec_chassis_private *chassis_pvt, > + const struct sbrec_chassis *chassis, > + const char *chassis_id) > +{ > + if (!chassis_pvt->name || strcmp(chassis_pvt->name, chassis_id)) { > + sbrec_chassis_private_set_name(chassis_pvt, chassis_id); > + } > + > + if (chassis_pvt->chassis != chassis) { > + sbrec_chassis_private_set_chassis(chassis_pvt, chassis); > + } > +} > + > /* Returns this chassis's Chassis record, if it is available. */ > const struct sbrec_chassis * > chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > @@ -642,6 +713,7 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > struct ovsdb_idl_index *sbrec_chassis_private_by_name, > const struct ovsrec_open_vswitch_table *ovs_table, > const struct sbrec_chassis_table *chassis_table, > + const struct sbrec_chassis_private_table *chassis_pvt_table, > const char *chassis_id, > const struct ovsrec_bridge *br_int, > const struct sset *transport_zones, > @@ -670,7 +742,6 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > bool updated = chassis_update(chassis_rec, ovnsb_idl_txn, > &ovs_cfg, > chassis_id, transport_zones); > > - chassis_info_set_id(&chassis_state, chassis_id); > if (!existed || updated) { > ovsdb_idl_txn_add_comment(ovnsb_idl_txn, > "ovn-controller: %s chassis '%s'", > @@ -678,17 +749,16 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn, > chassis_id); > } > > - const struct sbrec_chassis_private *chassis_private_rec = > - chassis_private_lookup_by_name(sbrec_chassis_private_by_name, > - chassis_id); > - if (!chassis_private_rec && ovnsb_idl_txn) { > - chassis_private_rec = > sbrec_chassis_private_insert(ovnsb_idl_txn); > - sbrec_chassis_private_set_name(chassis_private_rec, > - chassis_id); > - sbrec_chassis_private_set_chassis(chassis_private_rec, > - chassis_rec); > + *chassis_private = > + chassis_private_get_record(ovnsb_idl_txn, > + sbrec_chassis_private_by_name, > + chassis_pvt_table, chassis_rec); > + > + if (*chassis_private) { > + chassis_private_update(*chassis_private, chassis_rec, > chassis_id); > } > - *chassis_private = chassis_private_rec; > + > + chassis_info_set_id(&chassis_state, chassis_id); > } > > ovs_chassis_cfg_destroy(&ovs_cfg); > diff --git a/controller/chassis.h b/controller/chassis.h > index 81055b4..220f726 100644 > --- a/controller/chassis.h > +++ b/controller/chassis.h > @@ -26,6 +26,7 @@ struct ovsrec_bridge; > struct ovsrec_open_vswitch_table; > struct sbrec_chassis; > struct sbrec_chassis_table; > +struct sbrec_chassis_private_table; > struct sset; > struct eth_addr; > struct smap; > @@ -37,6 +38,7 @@ const struct sbrec_chassis *chassis_run( > struct ovsdb_idl_index *sbrec_chassis_private_by_name, > const struct ovsrec_open_vswitch_table *, > const struct sbrec_chassis_table *, > + const struct sbrec_chassis_private_table *, > const char *chassis_id, const struct ovsrec_bridge *br_int, > const struct sset *transport_zones, > const struct sbrec_chassis_private **chassis_private); > diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c > index 874087c..67e06f1 100644 > --- a/controller/ovn-controller.c > +++ b/controller/ovn-controller.c > @@ -179,7 +179,7 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, > * chassis */ > sbrec_port_binding_add_clause_type(&pb, OVSDB_F_EQ, > "chassisredirect"); > sbrec_port_binding_add_clause_type(&pb, OVSDB_F_EQ, "external"); > - if (chassis) { > + if (chassis && !sbrec_chassis_is_new(chassis)) { > /* This should be mostly redundant with the other clauses for port > * bindings, but it allows us to catch any ports that are > assigned to > * us but should not be. That way, we can clear their chassis > @@ -202,9 +202,9 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, > sbrec_igmp_group_add_clause_chassis(&igmp, OVSDB_F_EQ, > &chassis->header_.uuid); > > - /* Monitors Chassis_Private record for current chassis only */ > - sbrec_chassis_private_add_clause_name(&chprv, OVSDB_F_EQ, > - chassis->name); > + /* Monitors Chassis_Private record for current chassis only. */ > + sbrec_chassis_private_add_clause_chassis(&chprv, OVSDB_F_EQ, > + &chassis->header_.uuid); > } else { > /* During initialization, we monitor all records in > Chassis_Private so > * that we don't try to recreate existing ones. */ > @@ -2425,6 +2425,8 @@ main(int argc, char *argv[]) > ovsrec_open_vswitch_table_get(ovs_idl_loop.idl); > const struct sbrec_chassis_table *chassis_table = > sbrec_chassis_table_get(ovnsb_idl_loop.idl); > + const struct sbrec_chassis_private_table *chassis_pvt_table = > + sbrec_chassis_private_table_get(ovnsb_idl_loop.idl); > const struct ovsrec_bridge *br_int = > process_br_int(ovs_idl_txn, bridge_table, ovs_table); > const char *chassis_id = get_ovs_chassis_id(ovs_table); > @@ -2433,7 +2435,8 @@ main(int argc, char *argv[]) > if (chassis_id) { > chassis = chassis_run(ovnsb_idl_txn, > sbrec_chassis_by_name, > sbrec_chassis_private_by_name, > - ovs_table, chassis_table, > chassis_id, > + ovs_table, chassis_table, > + chassis_pvt_table, chassis_id, > br_int, &transport_zones, > &chassis_private); > } > diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at > index f2faf1f..812946b 100644 > --- a/tests/ovn-controller.at > +++ b/tests/ovn-controller.at > @@ -195,6 +195,15 @@ OVS_WAIT_UNTIL([ > chassis_id=$(ovn-sbctl get Chassis "${sysid}" name) > test "${sysid}" = "${chassis_id}" > ]) > +OVS_WAIT_UNTIL([ > + chassis_id=$(ovn-sbctl get Chassis_Private "${sysid}" name) > + test "${sysid}" = "${chassis_id}" > +]) > + > +# Only one Chassis_Private record should exist. > +OVS_WAIT_UNTIL([ > + test $(ovn-sbctl --columns _uuid list chassis_private | wc -l) -eq 1 > +]) > > # Simulate system-id changing while ovn-controller is disconnected from > the > # SB. > @@ -212,6 +221,15 @@ OVS_WAIT_UNTIL([ > chassis_id=$(ovn-sbctl get Chassis "${sysid}" name) > test "${sysid}" = "${chassis_id}" > ]) > +OVS_WAIT_UNTIL([ > + chassis_id=$(ovn-sbctl get Chassis_Private "${sysid}" name) > + test "${sysid}" = "${chassis_id}" > +]) > + > +# Only one Chassis_Private record should exist. > +OVS_WAIT_UNTIL([ > + test $(ovn-sbctl --columns _uuid list chassis_private | wc -l) -eq 1 > +]) > > # Gracefully terminate daemons > OVN_CLEANUP_SBOX([hv]) > diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c > index 04e082c..d3eec91 100644 > --- a/utilities/ovn-sbctl.c > +++ b/utilities/ovn-sbctl.c > @@ -1391,6 +1391,9 @@ cmd_set_ssl(struct ctl_context *ctx) > static const struct ctl_table_class tables[SBREC_N_TABLES] = { > [SBREC_TABLE_CHASSIS].row_ids[0] = {&sbrec_chassis_col_name, NULL, > NULL}, > > + [SBREC_TABLE_CHASSIS_PRIVATE].row_ids[0] > + = {&sbrec_chassis_private_col_name, NULL, NULL}, > + > [SBREC_TABLE_DATAPATH_BINDING].row_ids > = {{&sbrec_datapath_binding_col_external_ids, "name", NULL}, > {&sbrec_datapath_binding_col_external_ids, "name2", NULL}, > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev > >
On 9/4/20 3:52 PM, Numan Siddique wrote: > > > On Thu, Sep 3, 2020 at 8:34 PM Dumitru Ceara <dceara@redhat.com > <mailto:dceara@redhat.com>> wrote: > > Also: > - Change conditional monitoring for Chassis_Private to use the > chassis uuid > instead of chassis name. Using the chassis->name field does not work > because this is the old value of the field and would cause > ovsdb-server > to inform ovn-controller that the updated Chassis_Private record was > "deleted" because it doesn't match the monitor condition anymore. > - Allow ovn-sbctl to access Chassis_Private records by name. > > Reported-at: https://bugzilla.redhat.com/1873032 > Reported-by: Ying Xu <yinxu@redhat.com <mailto:yinxu@redhat.com>> > CC: Han Zhou <hzhou@ovn.org <mailto:hzhou@ovn.org>> > Fixes: 4adc10f58127 ("Avoid nb_cfg update notification flooding") > Signed-off-by: Dumitru Ceara <dceara@redhat.com > <mailto:dceara@redhat.com>> > > --- > v2: > - Fix the monitor condition update reported by Numan. > > > Thanks Dumitru. I applied both the patches to master. > > Numan Thanks Numan! Patch 1/2 should apply cleanly to branch20.06 too. Would it be possible to backport it there too? For branch20.03 I can send a separate backport patch if you think that's needed. Thanks, Dumitru
On Fri, Sep 4, 2020 at 7:26 PM Dumitru Ceara <dceara@redhat.com> wrote: > On 9/4/20 3:52 PM, Numan Siddique wrote: > > > > > > On Thu, Sep 3, 2020 at 8:34 PM Dumitru Ceara <dceara@redhat.com > > <mailto:dceara@redhat.com>> wrote: > > > > Also: > > - Change conditional monitoring for Chassis_Private to use the > > chassis uuid > > instead of chassis name. Using the chassis->name field does not > work > > because this is the old value of the field and would cause > > ovsdb-server > > to inform ovn-controller that the updated Chassis_Private record > was > > "deleted" because it doesn't match the monitor condition anymore. > > - Allow ovn-sbctl to access Chassis_Private records by name. > > > > Reported-at: https://bugzilla.redhat.com/1873032 > > Reported-by: Ying Xu <yinxu@redhat.com <mailto:yinxu@redhat.com>> > > CC: Han Zhou <hzhou@ovn.org <mailto:hzhou@ovn.org>> > > Fixes: 4adc10f58127 ("Avoid nb_cfg update notification flooding") > > Signed-off-by: Dumitru Ceara <dceara@redhat.com > > <mailto:dceara@redhat.com>> > > > > --- > > v2: > > - Fix the monitor condition update reported by Numan. > > > > > > Thanks Dumitru. I applied both the patches to master. > > > > Numan > > Thanks Numan! > > Patch 1/2 should apply cleanly to branch20.06 too. Would it be possible > to backport it there too? > Ok. I backported patch 1 to branch-20.06. > > For branch20.03 I can send a separate backport patch if you think that's > needed. > Feel free to submit the patch against branch-20.03 if you think it's required. I'm fine backporting it. Thanks Numan > Thanks, > Dumitru > > _______________________________________________ > dev mailing list > dev@openvswitch.org > https://mail.openvswitch.org/mailman/listinfo/ovs-dev >
On 9/4/20 4:36 PM, Numan Siddique wrote: > > > On Fri, Sep 4, 2020 at 7:26 PM Dumitru Ceara <dceara@redhat.com > <mailto:dceara@redhat.com>> wrote: > > On 9/4/20 3:52 PM, Numan Siddique wrote: > > > > > > On Thu, Sep 3, 2020 at 8:34 PM Dumitru Ceara <dceara@redhat.com > <mailto:dceara@redhat.com> > > <mailto:dceara@redhat.com <mailto:dceara@redhat.com>>> wrote: > > > > Also: > > - Change conditional monitoring for Chassis_Private to use the > > chassis uuid > > instead of chassis name. Using the chassis->name field does > not work > > because this is the old value of the field and would cause > > ovsdb-server > > to inform ovn-controller that the updated Chassis_Private > record was > > "deleted" because it doesn't match the monitor condition > anymore. > > - Allow ovn-sbctl to access Chassis_Private records by name. > > > > Reported-at: https://bugzilla.redhat.com/1873032 > > Reported-by: Ying Xu <yinxu@redhat.com > <mailto:yinxu@redhat.com> <mailto:yinxu@redhat.com > <mailto:yinxu@redhat.com>>> > > CC: Han Zhou <hzhou@ovn.org <mailto:hzhou@ovn.org> > <mailto:hzhou@ovn.org <mailto:hzhou@ovn.org>>> > > Fixes: 4adc10f58127 ("Avoid nb_cfg update notification flooding") > > Signed-off-by: Dumitru Ceara <dceara@redhat.com > <mailto:dceara@redhat.com> > > <mailto:dceara@redhat.com <mailto:dceara@redhat.com>>> > > > > --- > > v2: > > - Fix the monitor condition update reported by Numan. > > > > > > Thanks Dumitru. I applied both the patches to master. > > > > Numan > > Thanks Numan! > > Patch 1/2 should apply cleanly to branch20.06 too. Would it be possible > to backport it there too? > > > Ok. I backported patch 1 to branch-20.06. > > > For branch20.03 I can send a separate backport patch if you think that's > needed. > > > Feel free to submit the patch against branch-20.03 if you think it's > required. I'm fine backporting it. > Thanks Numan! I sent a separate backport patch for branch-20.03: http://patchwork.ozlabs.org/project/ovn/patch/1599473445-29748-1-git-send-email-dceara@redhat.com/ Regards, Dumitru
diff --git a/controller/chassis.c b/controller/chassis.c index 97120a9..45e018e 100644 --- a/controller/chassis.c +++ b/controller/chassis.c @@ -635,6 +635,77 @@ chassis_update(const struct sbrec_chassis *chassis_rec, return true; } +/* + * Returns a pointer to a chassis_private record from 'chassis_pvt_table' that + * matches the chassis record. + */ +static const struct sbrec_chassis_private * +chassis_private_get_stale_record( + const struct sbrec_chassis_private_table *chassis_pvt_table, + const struct sbrec_chassis *chassis) +{ + const struct sbrec_chassis_private *chassis_pvt_rec; + + SBREC_CHASSIS_PRIVATE_TABLE_FOR_EACH (chassis_pvt_rec, chassis_pvt_table) { + if (chassis_pvt_rec->chassis == chassis) { + return chassis_pvt_rec; + } + } + + return NULL; +} + +/* If this is a chassis_private 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 created the chassis record or if the + * system-id changed) 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. + * + * Returns the local chassis record. + */ +static const struct sbrec_chassis_private * +chassis_private_get_record( + struct ovsdb_idl_txn *ovnsb_idl_txn, + struct ovsdb_idl_index *sbrec_chassis_pvt_by_name, + const struct sbrec_chassis_private_table *chassis_pvt_table, + const struct sbrec_chassis *chassis) +{ + const struct sbrec_chassis_private *chassis_p = NULL; + + if (chassis_info_id_inited(&chassis_state)) { + chassis_p = + chassis_private_lookup_by_name(sbrec_chassis_pvt_by_name, + chassis_info_id(&chassis_state)); + } + + if (!chassis_p) { + chassis_p = chassis_private_get_stale_record(chassis_pvt_table, + chassis); + } + + if (!chassis_p && ovnsb_idl_txn) { + return sbrec_chassis_private_insert(ovnsb_idl_txn); + } + + return chassis_p; +} + +static void +chassis_private_update(const struct sbrec_chassis_private *chassis_pvt, + const struct sbrec_chassis *chassis, + const char *chassis_id) +{ + if (!chassis_pvt->name || strcmp(chassis_pvt->name, chassis_id)) { + sbrec_chassis_private_set_name(chassis_pvt, chassis_id); + } + + if (chassis_pvt->chassis != chassis) { + sbrec_chassis_private_set_chassis(chassis_pvt, chassis); + } +} + /* Returns this chassis's Chassis record, if it is available. */ const struct sbrec_chassis * chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn, @@ -642,6 +713,7 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn, struct ovsdb_idl_index *sbrec_chassis_private_by_name, const struct ovsrec_open_vswitch_table *ovs_table, const struct sbrec_chassis_table *chassis_table, + const struct sbrec_chassis_private_table *chassis_pvt_table, const char *chassis_id, const struct ovsrec_bridge *br_int, const struct sset *transport_zones, @@ -670,7 +742,6 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn, bool updated = chassis_update(chassis_rec, ovnsb_idl_txn, &ovs_cfg, chassis_id, transport_zones); - chassis_info_set_id(&chassis_state, chassis_id); if (!existed || updated) { ovsdb_idl_txn_add_comment(ovnsb_idl_txn, "ovn-controller: %s chassis '%s'", @@ -678,17 +749,16 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn, chassis_id); } - const struct sbrec_chassis_private *chassis_private_rec = - chassis_private_lookup_by_name(sbrec_chassis_private_by_name, - chassis_id); - if (!chassis_private_rec && ovnsb_idl_txn) { - chassis_private_rec = sbrec_chassis_private_insert(ovnsb_idl_txn); - sbrec_chassis_private_set_name(chassis_private_rec, - chassis_id); - sbrec_chassis_private_set_chassis(chassis_private_rec, - chassis_rec); + *chassis_private = + chassis_private_get_record(ovnsb_idl_txn, + sbrec_chassis_private_by_name, + chassis_pvt_table, chassis_rec); + + if (*chassis_private) { + chassis_private_update(*chassis_private, chassis_rec, chassis_id); } - *chassis_private = chassis_private_rec; + + chassis_info_set_id(&chassis_state, chassis_id); } ovs_chassis_cfg_destroy(&ovs_cfg); diff --git a/controller/chassis.h b/controller/chassis.h index 81055b4..220f726 100644 --- a/controller/chassis.h +++ b/controller/chassis.h @@ -26,6 +26,7 @@ struct ovsrec_bridge; struct ovsrec_open_vswitch_table; struct sbrec_chassis; struct sbrec_chassis_table; +struct sbrec_chassis_private_table; struct sset; struct eth_addr; struct smap; @@ -37,6 +38,7 @@ const struct sbrec_chassis *chassis_run( struct ovsdb_idl_index *sbrec_chassis_private_by_name, const struct ovsrec_open_vswitch_table *, const struct sbrec_chassis_table *, + const struct sbrec_chassis_private_table *, const char *chassis_id, const struct ovsrec_bridge *br_int, const struct sset *transport_zones, const struct sbrec_chassis_private **chassis_private); diff --git a/controller/ovn-controller.c b/controller/ovn-controller.c index 874087c..67e06f1 100644 --- a/controller/ovn-controller.c +++ b/controller/ovn-controller.c @@ -179,7 +179,7 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, * chassis */ sbrec_port_binding_add_clause_type(&pb, OVSDB_F_EQ, "chassisredirect"); sbrec_port_binding_add_clause_type(&pb, OVSDB_F_EQ, "external"); - if (chassis) { + if (chassis && !sbrec_chassis_is_new(chassis)) { /* This should be mostly redundant with the other clauses for port * bindings, but it allows us to catch any ports that are assigned to * us but should not be. That way, we can clear their chassis @@ -202,9 +202,9 @@ update_sb_monitors(struct ovsdb_idl *ovnsb_idl, sbrec_igmp_group_add_clause_chassis(&igmp, OVSDB_F_EQ, &chassis->header_.uuid); - /* Monitors Chassis_Private record for current chassis only */ - sbrec_chassis_private_add_clause_name(&chprv, OVSDB_F_EQ, - chassis->name); + /* Monitors Chassis_Private record for current chassis only. */ + sbrec_chassis_private_add_clause_chassis(&chprv, OVSDB_F_EQ, + &chassis->header_.uuid); } else { /* During initialization, we monitor all records in Chassis_Private so * that we don't try to recreate existing ones. */ @@ -2425,6 +2425,8 @@ main(int argc, char *argv[]) ovsrec_open_vswitch_table_get(ovs_idl_loop.idl); const struct sbrec_chassis_table *chassis_table = sbrec_chassis_table_get(ovnsb_idl_loop.idl); + const struct sbrec_chassis_private_table *chassis_pvt_table = + sbrec_chassis_private_table_get(ovnsb_idl_loop.idl); const struct ovsrec_bridge *br_int = process_br_int(ovs_idl_txn, bridge_table, ovs_table); const char *chassis_id = get_ovs_chassis_id(ovs_table); @@ -2433,7 +2435,8 @@ main(int argc, char *argv[]) if (chassis_id) { chassis = chassis_run(ovnsb_idl_txn, sbrec_chassis_by_name, sbrec_chassis_private_by_name, - ovs_table, chassis_table, chassis_id, + ovs_table, chassis_table, + chassis_pvt_table, chassis_id, br_int, &transport_zones, &chassis_private); } diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at index f2faf1f..812946b 100644 --- a/tests/ovn-controller.at +++ b/tests/ovn-controller.at @@ -195,6 +195,15 @@ OVS_WAIT_UNTIL([ chassis_id=$(ovn-sbctl get Chassis "${sysid}" name) test "${sysid}" = "${chassis_id}" ]) +OVS_WAIT_UNTIL([ + chassis_id=$(ovn-sbctl get Chassis_Private "${sysid}" name) + test "${sysid}" = "${chassis_id}" +]) + +# Only one Chassis_Private record should exist. +OVS_WAIT_UNTIL([ + test $(ovn-sbctl --columns _uuid list chassis_private | wc -l) -eq 1 +]) # Simulate system-id changing while ovn-controller is disconnected from the # SB. @@ -212,6 +221,15 @@ OVS_WAIT_UNTIL([ chassis_id=$(ovn-sbctl get Chassis "${sysid}" name) test "${sysid}" = "${chassis_id}" ]) +OVS_WAIT_UNTIL([ + chassis_id=$(ovn-sbctl get Chassis_Private "${sysid}" name) + test "${sysid}" = "${chassis_id}" +]) + +# Only one Chassis_Private record should exist. +OVS_WAIT_UNTIL([ + test $(ovn-sbctl --columns _uuid list chassis_private | wc -l) -eq 1 +]) # Gracefully terminate daemons OVN_CLEANUP_SBOX([hv]) diff --git a/utilities/ovn-sbctl.c b/utilities/ovn-sbctl.c index 04e082c..d3eec91 100644 --- a/utilities/ovn-sbctl.c +++ b/utilities/ovn-sbctl.c @@ -1391,6 +1391,9 @@ cmd_set_ssl(struct ctl_context *ctx) static const struct ctl_table_class tables[SBREC_N_TABLES] = { [SBREC_TABLE_CHASSIS].row_ids[0] = {&sbrec_chassis_col_name, NULL, NULL}, + [SBREC_TABLE_CHASSIS_PRIVATE].row_ids[0] + = {&sbrec_chassis_private_col_name, NULL, NULL}, + [SBREC_TABLE_DATAPATH_BINDING].row_ids = {{&sbrec_datapath_binding_col_external_ids, "name", NULL}, {&sbrec_datapath_binding_col_external_ids, "name2", NULL},
Also: - Change conditional monitoring for Chassis_Private to use the chassis uuid instead of chassis name. Using the chassis->name field does not work because this is the old value of the field and would cause ovsdb-server to inform ovn-controller that the updated Chassis_Private record was "deleted" because it doesn't match the monitor condition anymore. - Allow ovn-sbctl to access Chassis_Private records by name. Reported-at: https://bugzilla.redhat.com/1873032 Reported-by: Ying Xu <yinxu@redhat.com> CC: Han Zhou <hzhou@ovn.org> Fixes: 4adc10f58127 ("Avoid nb_cfg update notification flooding") Signed-off-by: Dumitru Ceara <dceara@redhat.com> --- v2: - Fix the monitor condition update reported by Numan. --- controller/chassis.c | 92 ++++++++++++++++++++++++++++++++++++++----- controller/chassis.h | 2 + controller/ovn-controller.c | 13 ++++-- tests/ovn-controller.at | 18 ++++++++ utilities/ovn-sbctl.c | 3 + 5 files changed, 112 insertions(+), 16 deletions(-)