Also btw the CI test failure is my own fault :) I forgot to ignore the new warning when cleaning up hv1. I should have ended the test with:
OVN_CLEANUP([hv2], [hv1 /No encap type and\/or ip on chassis 'hv1'. Deleting chassis/d]) I'll wait to post a v2 until I get other feedback. On 10/28/26 8:48 PM, Rosemarie O'Riorden wrote: > If anyone disagrees with this approach and thinks I should do it the > other way of changing the schema to allow 0 encaps then let me know. > > On 10/28/26 8:46 PM, Rosemarie O'Riorden wrote: >> This change makes it so that if a chassis has no encaps configured, the >> entire Chassis record is deleted. >> >> If a chassis hv1 has encap A, and then encap A is removed from hv1, >> chassis hv2 should be able to use encap A. But this is not possible if >> the Encap record is still present in the sb db. >> >> The reasoning for deleting the entire Chassis record rather than just >> its Encap is that a chassis is already considered misconfigured if it >> has no encaps set. We could allow encap-less chassis, but we already >> have the behavior established of deleting chassis with no valid encaps >> (if there are duplicates) in chassis_update(), and allowing 0 encaps >> would also require a schema change and checking for n_encaps>0 in many >> places. >> >> So the easiest solution is just to remove the chassis record altogether. >> When encaps are readded, the chassis record will be recreated. >> >> Reported-at: https://redhat.atlassian.net/browse/FDP-1945 >> Signed-off-by: Rosemarie O'Riorden <[email protected]> >> --- >> controller/chassis.c | 46 +++++++++++++++++++++++++++++++------ >> tests/ovn-controller.at | 51 +++++++++++++++++++++++++++++++++++++++++ >> 2 files changed, 90 insertions(+), 7 deletions(-) >> >> diff --git a/controller/chassis.c b/controller/chassis.c >> index b82683064..ed3d0e878 100644 >> --- a/controller/chassis.c >> +++ b/controller/chassis.c >> @@ -82,6 +82,12 @@ enum chassis_update_status { >> CHASSIS_NEED_DELETE >> }; >> >> +enum chassis_parse_status { >> + CHASSIS_PARSED, >> + CHASSIS_NO_CFG, >> + CHASSIS_DELETE_NO_ENCAPS, >> +}; >> + >> static void >> ovs_chassis_cfg_init(struct ovs_chassis_cfg *cfg) >> { >> @@ -303,9 +309,12 @@ chassis_parse_ovs_iface_types(char **iface_types, >> size_t n_iface_types, >> } >> >> /* >> - * Parse the 'ovs_table' entry and populate 'ovs_cfg'. >> + * Parse the 'ovs_table' entry and populate 'ovs_cfg'. If successful, >> + * returns CHASSIS_PARSED. >> + * Otherwise, returns CHASSIS_NO_CFG if 'ovs_table' is NULL, or >> + * CHASSIS_DELETE_NO_ENCAPS if encap type or ip is NULL. >> */ >> -static bool >> +static enum chassis_parse_status >> chassis_parse_ovs_config(const struct ovsrec_open_vswitch_table *ovs_table, >> const struct ovsrec_bridge *br_int, >> struct ovs_chassis_cfg *ovs_cfg) >> @@ -316,7 +325,7 @@ chassis_parse_ovs_config(const struct >> ovsrec_open_vswitch_table *ovs_table, >> >> if (!cfg) { >> VLOG_INFO("No Open_vSwitch row defined."); >> - return false; >> + return CHASSIS_NO_CFG; >> } >> >> const char *chassis_id = get_ovs_chassis_id(ovs_table); >> @@ -328,8 +337,9 @@ chassis_parse_ovs_config(const struct >> ovsrec_open_vswitch_table *ovs_table, >> get_chassis_external_id_value(&cfg->external_ids, chassis_id, >> "ovn-encap-ip", NULL); >> if (!encap_type || !encap_ips) { >> - VLOG_INFO_RL(&rl, "Need to specify an encap type and ip"); >> - return false; >> + VLOG_WARN_RL(&rl, "No encap type and/or ip on chassis '%s'. >> Deleting " >> + "chassis.", chassis_id); >> + return CHASSIS_DELETE_NO_ENCAPS; >> } >> >> ovs_cfg->hostname = get_hostname(&cfg->external_ids, chassis_id); >> @@ -386,7 +396,7 @@ chassis_parse_ovs_config(const struct >> ovsrec_open_vswitch_table *ovs_table, >> ovs_cfg->ct_label_flush = >> ovs_feature_is_supported(OVS_CT_LABEL_FLUSH_SUPPORT); >> >> - return true; >> + return CHASSIS_PARSED; >> } >> >> static void >> @@ -958,7 +968,29 @@ chassis_run(struct ovsdb_idl_txn *ovnsb_idl_txn, >> >> /* 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)) { >> + enum chassis_parse_status parse_status = >> + chassis_parse_ovs_config(ovs_table, br_int, &ovs_cfg); >> + >> + if (parse_status == CHASSIS_NO_CFG) { >> + ovs_chassis_cfg_destroy(&ovs_cfg); >> + return NULL; >> + } >> + >> + /* If no encaps are configured, delete the Chassis record. */ >> + if (parse_status == CHASSIS_DELETE_NO_ENCAPS) { >> + const struct sbrec_chassis *chassis_rec = >> + chassis_lookup_by_name(sbrec_chassis_by_name, chassis_id); >> + if (chassis_rec && ovnsb_idl_txn) { >> + sbrec_chassis_delete(chassis_rec); >> + } >> + *chassis_private = >> + chassis_private_lookup_by_name(sbrec_chassis_private_by_name, >> + chassis_id); >> + if (*chassis_private && ovnsb_idl_txn) { >> + sbrec_chassis_private_delete(*chassis_private); >> + } >> + >> + ovs_chassis_cfg_destroy(&ovs_cfg); >> return NULL; >> } >> >> diff --git a/tests/ovn-controller.at b/tests/ovn-controller.at >> index d600ba2cd..4d1860136 100644 >> --- a/tests/ovn-controller.at >> +++ b/tests/ovn-controller.at >> @@ -4520,3 +4520,54 @@ done >> OVN_CLEANUP([gw1],[gw3],[hv2]) >> AT_CLEANUP >> ]) >> + >> +OVN_FOR_EACH_NORTHD([ >> +AT_SETUP([ovn-controller - remove encap from chassis add to other chassis]) >> +AT_KEYWORDS([ovn]) >> +ovn_start >> +net_add n1 >> + >> +sim_add hv1 >> +as hv1 check ovs-vsctl add-br br-phys >> +ovn_attach n1 br-phys 192.168.0.11 >> + >> +sim_add hv2 >> +as hv2 check ovs-vsctl add-br br-phys >> +ovn_attach n1 br-phys 192.168.0.12 >> + >> +as hv2 >> +OVS_WAIT_FOR_OUTPUT([ovs-vsctl find Interface type=geneve | grep -c _uuid], >> [0], [1 >> +]) >> + >> +as hv1 >> +OVS_WAIT_FOR_OUTPUT([ovs-vsctl find Interface type=geneve | grep -c _uuid], >> [0], [1 >> +]) >> +wait_row_count Encap 4 >> +wait_row_count Chassis 2 >> + >> +# Remove hv1's encap. >> +as hv1 check ovs-vsctl remove Open_vSwitch . external-ids >> ovn-encap-ip="192.168.0.11" >> +check ovn-nbctl --wait=sb sync >> + >> +# Make sure that hv1's chassis rec was deleted from sb. And of course with >> +# that its encaps too. >> +OVS_WAIT_UNTIL([test $(ovn-sbctl --bare find Encap chassis_name=hv1 >> ip="192.168.0.11" | wc -l) -eq 0]) >> +wait_row_count Encap 0 chassis_name="hv1" ip="192.168.0.11" >> +wait_row_count Chassis 0 name="hv1" >> +OVS_WAIT_UNTIL([grep -q "No encap type and/or ip on chassis 'hv1'. Deleting >> chassis." hv1/ovn-controller.log >> +]) >> + >> +# Assign hv1's old encap ip to hv2. Verify it worked. >> +as hv2 check ovs-vsctl set Open_vSwitch . >> external-ids:ovn-encap-ip="192.168.0.11" >> +wait_row_count Encap 0 chassis_name="hv2" ip="192.168.0.12" >> +wait_row_count Encap 2 chassis_name="hv2" ip="192.168.0.11" >> + >> +# Now give hv1 a new different encap ip. Verify the chassis is back. >> +as hv1 check ovs-vsctl set Open_vSwitch . >> external-ids:ovn-encap-ip="192.168.0.13" >> +check ovn-nbctl --wait=hv sync >> +wait_row_count Encap 2 chassis_name="hv1" ip="192.168.0.13" >> +wait_row_count Chassis 1 name="hv1" >> + >> +OVN_CLEANUP([hv1],[hv2]) >> +AT_CLEANUP >> +]) > _______________________________________________ dev mailing list [email protected] https://mail.openvswitch.org/mailman/listinfo/ovs-dev
