[Pluggable IPAM] delete subnet in ml2 plugin does not comply with pluggable ipam (deletes ip allocations directly from db)

[Pavel Bondar]

In ml2 plugin delete_subnet workflow ip allocations get deleted [1] directly from database (IPAllocation model).
It conflicts with expected workflow for pluggable ipam because ipam driver in this case is not aware of ip deallocation event.

There is a call to update_port [2] but looks like it never worked correctly because by the time this code is called ip address is already removed from database by [1]. So calling update_port in this case does not trigger correct deallocate event to ipam driver.

For reference ipam driver this issue is not visible, because right after deleting ips directly from db ipam.delete_subnet is called. Deleting ipam subnet cleans up all undeleted ip allocations by FK. And this is the reason why it was not seen until now.

But issue can affect third-party ipam provides. Here is the scenario where it was found (Infoblox IPAM driver):
There is a subnet shared between OpenStack and other applications.
On attempt to delete such subnet ipam provider skips subnet deletion, since it is used by other applications,
but all ports allocated by neutron for this subnet are expected to be deallocated at this point.
Observed that dhcp port is left in allocated state on ipam provider side, because ipam driver did not receive deallocate ip address call.

Proper way to deallocate ip before subnet deletion is to use update_port for each ip deallocation instead of deleting ip allocations directly from database.

[1] https://github.com/openstack/neutron/blob/98c93a050b56bb5425d0b18456a20cf35a4ff449/neutron/plugins/ml2/plugin.py#L960
[2] https://github.com/openstack/neutron/blob/98c93a050b56bb5425d0b18456a20cf35a4ff449/neutron/plugins/ml2/plugin.py#L1020

[Akihiro Motoki (amotoki)]

Mark this high since this needs to be fixed to make the IPAM driver mechanism fully functional.

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/300984

[Pavel Bondar (pasha117)]

Deeper investigation shows that similar issue exists for delete_subnet [1] and delete_network [2] in db_base_plugin_v2:

[1] https://github.com/openstack/neutron/blob/98c93a050b56bb5425d0b18456a20cf35a4ff449/neutron/db/db_base_plugin_v2.py#L856
[2] https://github.com/openstack/neutron/blob/98c93a050b56bb5425d0b18456a20cf35a4ff449/neutron/db/db_base_plugin_v2.py#L377

[Pavel Bondar (pasha117)]

Uploaded initial version of fix for review:
https://review.openstack.org/#/c/300984/1
But it still has jenkins failures, investigating with it.

[Pavel Bondar (pasha117)]

Found root cause of jenkins failures.

Auto-allocated ips (from auto addressed subnet) can not be deleted via update_port workflow. Currently if ips from auto-allocated subnet is not present in fixed_ips list on update_port, auto-allocated ips are not deleted [1].
It works this way because auto-addressed ip allocation is not expected to be deallocated by user via update_port (only on delete_port)

Don't have good fix for this case. Possible ways I see from now are not ideal:

A. Limit scope of current fix to make sure that not-auto allocated ips (like user allocation and dhcp port ips) are removed via update_port workflow (so ipam driver is aware of them), but auto-allocated ips (for auto addressed subnet) are removed on subnet deletion by FK constraint.
This way dhcp port ip will be correctly deallocated via ipam driver for both auto-addressed and normal subnets. But user allocations from auto-addressed subnets (where ip addresses are calculated based on mac) will be hidden from ipam driver and will be removed by some kind of FK on ipam provider side by deletion of ipam subnet.

This way Infoblox use case is addressed (dhcp port that left in allocated state).

B. update_port and [1] may be reworked to deallocate auto_addressed port if update_port is called from delete_subnet.
In this case additional option has to be passed throught update_port->update_port_with_ips->_update_ips_for_port->_get_changed_ips_for_port to alternate behavior of [1].

This way ipam driver will be aware of all deallocation (even slaac), but it involves touching a lot of code and altering update_port method, which is API exposed.

Please share your thoughts about possible solutions.

[1] https://github.com/openstack/neutron/blob/98c93a050b56bb5425d0b18456a20cf35a4ff449/neutron/db/ipam_backend_mixin.py#L407

[John Belamaric (jbelamaric)]

Pavel, can you remind me - is the external IPAM driver made aware upon the allocation of the auto-addressed IPs? If so, then we need to make sure it is informed when they are removed, so we would need to go with B. I could also see implementing A and then creating a separate bug for dealing with auto-addresses.

If external IPAM is not aware of auto-addresses, I think we can go with A above. Management or tracking of auto-addresses via external IPAM would be an RFE then.

[Pavel Bondar (pasha117)]

John, currently external IPAM is aware of auto-addressed allocations.

[Pavel Bondar (pasha117)]

So it looks like we need to go with approach B.

I would prefere to fix it in two step, because we have two almost independent things to do:
- make sure ip allocations are not deleted directly from db, but via update_port;
- tune update_port to be able to remove auto-allocated ips from port;

So the first patch will use approach A, and the second patch on top of the first one will use approach B.

Another reason for doing it this way is that I am not yet sure that changing update_port interface (adding new optional parameter)
is save to do because update_port is part of plugin public interface.

So in the worst case at least first patch will be merged and most common use case will be fixed.

[Miguel Lavalle (minsel)]

Proposed fix:

https://review.openstack.org/#/c/300984/

[Pavel Bondar (pasha117)]

Ip from auto-addressed subnets can not be deleted durig update_port workflow because of [1].
Even if this area was significantly reworked during liberty, the same behavior is preserved at least from kilo [2].
So if ip address is not present in updated 'fixed_ips', ip address is not removed for auto-addressed subnet case.

One way to allow removing ip for auto-addressed subnets is to add additional parameter to update_port, that changes behavior and allows to remove ips for auto-addressed subnets.

Is it ok to fix it that way?

[1] https://github.com/openstack/neutron/blob/f494de47fcef7776f7d29d5ceb2cc4db96bd1efd/neutron/db/ipam_backend_mixin.py#L435
[2] https://github.com/openstack/neutron/blob/stable/kilo/neutron/db/db_base_plugin_v2.py#L444

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: master
Review: https://review.openstack.org/323345

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/300984
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=3a2e41bf703d6721fcfe4d72f9ad5c0565fef42d
Submitter: Jenkins
Branch: master

commit 3a2e41bf703d6721fcfe4d72f9ad5c0565fef42d
Author: Pavel Bondar <email address hidden>
Date: Thu Mar 31 13:20:39 2016 +0300

    Update ml2 delete_subnet to deallocate via ipam

    Previosly during delete_subnet in ml2 plugin ip allocation were removed
    directly from database. This way ipam driver was unaware of this
    deallocation.

    Updated workflow to skip removing ip allocations directly from database.
    Now ips are deallocated during update_port workflow (L1008).

    This resolves issue with dhcp ports left in allocated state on ipam
    provides side. Now they are correctly deallocated via update_port.

    But this patch has next limitation: currently SLAAC allocations can
    not be delete via update_port workflow, so ipam driver is still unaware
    of such deallocations.
    This part of issue is expected to be fixed as separate patch.

    Subnet_in_use check was reworked. Previously it assumed that
    auto-allocated ip are already deleted by the time of this check, but
    with new workflow auto allocated ips are deleted later (on update_port).
    So now this check verifies if there are any user allocated ips instead
    of checking all allocations.

    Partial-Bug: #1564335
    Change-Id: I08d66da8cb57ed88e11ec2b18c8345edfce37d37

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/mitaka
Review: https://review.openstack.org/330374

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/330374
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=71686cdaedab8ea176a98fd760afd3c9af3f6d9d
Submitter: Jenkins
Branch: stable/mitaka

commit 71686cdaedab8ea176a98fd760afd3c9af3f6d9d
Author: Pavel Bondar <email address hidden>
Date: Thu Mar 31 13:20:39 2016 +0300

    Update ml2 delete_subnet to deallocate via ipam

    Previosly during delete_subnet in ml2 plugin ip allocation were removed
    directly from database. This way ipam driver was unaware of this
    deallocation.

    Updated workflow to skip removing ip allocations directly from database.
    Now ips are deallocated during update_port workflow (L1008).

    This resolves issue with dhcp ports left in allocated state on ipam
    provides side. Now they are correctly deallocated via update_port.

    But this patch has next limitation: currently SLAAC allocations can
    not be delete via update_port workflow, so ipam driver is still unaware
    of such deallocations.
    This part of issue is expected to be fixed as separate patch.

    Subnet_in_use check was reworked. Previously it assumed that
    auto-allocated ip are already deleted by the time of this check, but
    with new workflow auto allocated ips are deleted later (on update_port).
    So now this check verifies if there are any user allocated ips instead
    of checking all allocations.

    Partial-Bug: #1564335
    Change-Id: I08d66da8cb57ed88e11ec2b18c8345edfce37d37
    (cherry picked from commit 3a2e41bf703d6721fcfe4d72f9ad5c0565fef42d)

[Miguel Lavalle (minsel)]

Waiting for this fix to merge in master: https://review.openstack.org/#/c/323345

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/liberty
Review: https://review.openstack.org/345512

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/323345
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=dc19411ebf2cab7075dd5abe809797fb7253757c
Submitter: Jenkins
Branch: master

commit dc19411ebf2cab7075dd5abe809797fb7253757c
Author: Pavel Bondar <email address hidden>
Date: Tue May 31 15:12:00 2016 +0300

    Allow auto-addressed ips deletion on port update

    By default ips for auto-addressed subnets can not be removed from port
    using port_update workflow. But during subnet_delete it has to be done
    via update_port to make sure that ipam drivers received appropriate call
    to deallocate ip addresses prior to subnet deletion.

    'fixed_ips' property is tweeked to allow deletion ips from
    auto-addressed subnet. 'delete_subnet' boolean is added to mark subnet
    that is going to be deleted. This flag is analysed in
    _get_changed_ips_for_port to skip re-adding slaac subnets for the port.

    Closes-Bug: #1564335
    Change-Id: Iec171efe8b64f8a6dc6cb003b97c11667c5e0048

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/345512
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=8d753d71145c38cee042db587605787b0b2dc2ea
Submitter: Jenkins
Branch: stable/liberty

commit 8d753d71145c38cee042db587605787b0b2dc2ea
Author: Pavel Bondar <email address hidden>
Date: Thu Mar 31 13:20:39 2016 +0300

    Update ml2 delete_subnet to deallocate via ipam

    Previosly during delete_subnet in ml2 plugin ip allocation were removed
    directly from database. This way ipam driver was unaware of this
    deallocation.

    Updated workflow to skip removing ip allocations directly from database.
    Now ips are deallocated during update_port workflow (L1008).

    This resolves issue with dhcp ports left in allocated state on ipam
    provides side. Now they are correctly deallocated via update_port.

    But this patch has next limitation: currently SLAAC allocations can
    not be delete via update_port workflow, so ipam driver is still unaware
    of such deallocations.
    This part of issue is expected to be fixed as separate patch.

    Subnet_in_use check was reworked. Previously it assumed that
    auto-allocated ip are already deleted by the time of this check, but
    with new workflow auto allocated ips are deleted later (on update_port).
    So now this check verifies if there are any user allocated ips instead
    of checking all allocations.

    Partial-Bug: #1564335
    Change-Id: I08d66da8cb57ed88e11ec2b18c8345edfce37d37
    (cherry picked from commit 3a2e41bf703d6721fcfe4d72f9ad5c0565fef42d)
    (cherry picked from commit 71686cdaedab8ea176a98fd760afd3c9af3f6d9d)

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/mitaka
Review: https://review.openstack.org/349555

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/349555
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=cf3009a984b01eb90046c8c7513e531b7ab63762
Submitter: Jenkins
Branch: stable/mitaka

commit cf3009a984b01eb90046c8c7513e531b7ab63762
Author: Pavel Bondar <email address hidden>
Date: Tue May 31 15:12:00 2016 +0300

    Allow auto-addressed ips deletion on port update

    By default ips for auto-addressed subnets can not be removed from port
    using port_update workflow. But during subnet_delete it has to be done
    via update_port to make sure that ipam drivers received appropriate call
    to deallocate ip addresses prior to subnet deletion.

    'fixed_ips' property is tweeked to allow deletion ips from
    auto-addressed subnet. 'delete_subnet' boolean is added to mark subnet
    that is going to be deleted. This flag is analysed in
    _get_changed_ips_for_port to skip re-adding slaac subnets for the port.

    Manually resolved conflicts:
        neutron/tests/unit/plugins/ml2/test_plugin.py

    Closes-Bug: #1564335
    Change-Id: Iec171efe8b64f8a6dc6cb003b97c11667c5e0048
    Cherry-picked-From: dc19411ebf2cab7075dd5abe809797fb7253757c

[OpenStack Infra (hudson-openstack)]

Fix proposed to branch: stable/liberty
Review: https://review.openstack.org/362196

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 9.0.0.0b3 development milestone.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/362196
Committed: https://git.openstack.org/cgit/openstack/neutron/commit/?id=e0698725ce441c81575359d9f502d47e3e08efd4
Submitter: Jenkins
Branch: stable/liberty

commit e0698725ce441c81575359d9f502d47e3e08efd4
Author: Pavel Bondar <email address hidden>
Date: Tue May 31 15:12:00 2016 +0300

    Allow auto-addressed ips deletion on port update

    By default ips for auto-addressed subnets can not be removed from port
    using port_update workflow. But during subnet_delete it has to be done
    via update_port to make sure that ipam drivers received appropriate call
    to deallocate ip addresses prior to subnet deletion.

    'fixed_ips' property is tweeked to allow deletion ips from
    auto-addressed subnet. 'delete_subnet' boolean is added to mark subnet
    that is going to be deleted. This flag is analysed in
    _get_changed_ips_for_port to skip re-adding slaac subnets for the port.

    Manually resolved conflicts:
        neutron/tests/unit/plugins/ml2/test_plugin.py

    Closes-Bug: #1564335
    Change-Id: Iec171efe8b64f8a6dc6cb003b97c11667c5e0048
    Cherry-picked-From: dc19411ebf2cab7075dd5abe809797fb7253757c
    (cherry picked from commit cf3009a984b01eb90046c8c7513e531b7ab63762)

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 7.2.0 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 8.3.0 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 7.2.0 release.

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron 8.3.0 release.