<div dir="ltr"><div>Aloha everybody!</div><div><br></div><div>This is another long email, so here's the summary for people with 5-minute or less attention span:</div><div><br></div><div>The IPAM reference driver is almost ready [1]. Unfortunately letting the driver doing allocation pools required a few major changes, so the latest patchset is rather big. I would like reviewers to advice on how they prefer to split it out in multiple patches.</div><div><br></div><div>I also had to add an additional method to the subnet interface [2]</div><div><br></div><div>However, I am not sure we are doing the right thing wrt the introduction of the driver. I think we might need to think about it more thoroughly, and possibly introduce a concept of "allocator". Otherwise if we switch an existing deployment to the driver, it's very likely things will just break.</div><div><br></div><div>So here's the detailed part.</div><div>There are only a few bits needed to complete the IPAM reference driver:</div><div>- A thorough unit test case for the newly introduced "subnet manager", which provides db apis.</div><div>- Some more unit tests for driver behaviour</div><div>- Logic for handling allocation pools update (which pretty much needs to be copied over from db base plugin with minor changes)</div><div>- Rework the synchronisation between the ipam object and the neutron db object. The current one is very error prone.</div><div><br></div><div>While dealing with management of allocation pools, I found out that it's not convenient for the IPAM driver to rely on Neutron's DB allocation pools. This is a bit of a chicken and egg problem, since:</div><div>A) Neutron needs to call the driver to define allocation pools</div><div>B) The driver needs neutron to define allocation pools before creating availability ranges</div><div>C) Neutron cannot create allocation pools if the driver has not defined them</div><div>I therefore decided, reluctantly, to add more data duplication - introducing IPAM tables for allocation pools and availability ranges. The latter is meant to replace Neutron's one when the IPAM driver is enabled, whereas the former is pure data duplication. There is also an association table between neutron subnets and ipam objects for the same subnet; I decided to do so to not do further duplication.</div><div>I dislike this data separation; on the other hand the only viable alternative would be to give the IPAM driver full control over neutron's database tables for IP Allocation and Subnet allocation pools. While this is feasible, I think it would be even worse to give code which can potentially be 3rd party control over data structures used by the Neutron API.</div><div><br></div><div>As a result, the patch is different and larger. I would like to split it to make it simpler to review, but rather than just doing that of my own accord I would like to ask reviewers how they would prefer to have it split. At the end of the day I'm not the one who has to spend a lot of time reviewing that code.</div><div><br></div><div>Nevertheless, I think I've realised our refactoring approach is kind of flawed, since it might work for greenfields deployments, but I don't think it will work for brownfields deployments. Also, switching between drivers is pretty much impossible (but we were already aware of that and we agreed to address this in the future).</div><div>The decorator approach currently taken in [3] allows to either use the driver or not - which means that if an operator decides to switch to the IPAM driver, then all allocation data for existing subnets would be simply lost, unless we provide a solution for migrating data. This is feasible and rather trivial, but implies an additional management step.</div><div><br></div><div>An alternative solution would be to introduce a concept of "allocator" for subnets. Such information should be stored in the database. It could point to an IPAM driver or to nothing. In the latter case it would simply instruct to use the "usual" baked-in IPAM code. This would allow us to make the driver work on existing deployments, and pave the way for multiple drivers. Indeed in this way, the new IPAM driver will be used for subnets created after enabling it, whereas existing subnets will keep using the existing logic. This should also make safe cases in which operators revert the decision of using the IPAM driver. Additionally, administrative APIs might be provided to migrate existing subnets to/from the driver. However, when adopting solutions like this, it is important to pay extra care in ensuring that the database does not start relying on the current configuration, and therefore we need to find a way to decouple the allocator from the actual IPAM driver, which should represent its realization.</div><div><br></div><div>Any feedback is very appreciated as usual.</div><div><br></div><div>Salvatore</div><div><br></div><div>[1] <a href="https://review.openstack.org/#/c/150485/">https://review.openstack.org/#/c/150485/</a></div><div>[2] <a href="https://review.openstack.org/#/c/150485/6/neutron/ipam/driver.py">https://review.openstack.org/#/c/150485/6/neutron/ipam/driver.py</a></div><div>[3] <a href="https://review.openstack.org/#/c/153236/15/neutron/db/db_base_plugin_v2.py">https://review.openstack.org/#/c/153236/15/neutron/db/db_base_plugin_v2.py</a></div><div><br></div><div><br></div><div><br></div><div><br></div><div><br></div><div> </div><div><br></div><div><br></div></div>