On Fri, Jun 7, 2019 at 10:42 PM Ben Pfaff <b...@ovn.org> wrote: > > On Wed, May 22, 2019 at 01:26:03PM +0200, Dumitru Ceara wrote: > > 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 > > Thanks for working on this. > > This is a large patch that incorporates both a bug fix and refactoring. > I would prefer to see the refactoring broken out into a separate patch. > Preferably, it would go after the bug fix so that the bug fix could be > backported by itself (if necessary), but if the refactoring is essential > to the bug fix then the refactoring could go first. > > Would you mind breaking the patch apart?
I'll try to split this in two patches. Thanks, Dumitru > > Thanks, > > Ben. _______________________________________________ dev mailing list d...@openvswitch.org https://mail.openvswitch.org/mailman/listinfo/ovs-dev
