ML2 plugin deletes port even if associated with multiple subnets on subnet deletion

Bug #1246737 reported by Oleg Bondarev on 2013-10-31
12
This bug affects 2 people
Affects Status Importance Assigned to Milestone
neutron
Medium
Oleg Bondarev
Havana
Undecided
padkrish

Bug Description

On subnet deletion ml2 plugin deletes all the ports associated with this subnet and does not check if a port is associated with other subnets.
Steps to reproduce:
1) create a network with two subnets
2) create dhcp port for the network, port is associated with both subnets
3) delete one of the subnets
4) dhcp port is getting deleted

Though new dhcp port is created shortly I think it's not ok to delete existing dhcp port.

Robert Kukura (rkukura) on 2013-10-31
Changed in neutron:
status: New → Confirmed
tags: added: havana-backport-potential
Robert Kukura (rkukura) wrote :

In NeutronDbPluginV2.delete_subnet(), there is a statement "allocated.delete()" that I had erroneously interpreted as deleting the port at the DB level - probably mislead by the "# remove network owned ports" comment. I therefore changed Ml2Plugin.delete_subnet() to not call the base class and to instead use its own delete_port() method so that MechanismDrivers would be called for the auto-deleted port.

But, as Oleg discovered, the base class's "allocated.delete()" statement is actually just deleting the IPAllocation record, not the port itself. This is in effect removing the subnet's IP from the port's fixed_ips attribute. Therefore, instead of calling delete_port(), the Ml2Plugin.delete_subnet() method should be calling update_port() for each allocation to remove the IP from the port and call the MechanismDrivers.

This review might be relevant to this bug: https://review.openstack.org/#/c/54850/

summary: - ML2 plugin deletes port even if associated with multiple subnetrs on
+ ML2 plugin deletes port even if associated with multiple subnets on
subnet deletion

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

Changed in neutron:
status: Confirmed → In Progress

Reviewed: https://review.openstack.org/54918
Committed: http://github.com/openstack/neutron/commit/0d131ff0e9964cb6a65f64809270f9d597c2d5d1
Submitter: Jenkins
Branch: master

commit 0d131ff0e9964cb6a65f64809270f9d597c2d5d1
Author: Oleg Bondarev <email address hidden>
Date: Fri Nov 1 15:20:27 2013 +0400

    ML2 plugin should not delete ports on subnet deletion

    On subnet deletion ports are deleted asynchronously by dhcp agent
    so plugin doesn't need to delete them itself.
    Instead Ml2Plugin.delete_subnet() method should call update_port()
    for each allocation to remove the IP from the port and call the MechanismDrivers.
    The patch also adds subnets test suite from test_db_plugin to test_ml2_plugin.

    Closes-Bug: #1246737
    Change-Id: I7cf0461e9a3cfec4921e2de41fb1ab3fc119fddc

Changed in neutron:
status: In Progress → Fix Committed

This has been reverted here:

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

Because of a side-effect that might have been introduced by this one.

Please take the time to look at this one again to ensure the right behavior is implemented at subnet deletion

Changed in neutron:
status: Fix Committed → Confirmed
Akihiro Motoki (amotoki) wrote :

If I understand correctly, there is no patch commited in stable/havana.

Akihiro Motoki (amotoki) wrote :

I confirmed the issue still exists in stable/havana.

Alan Pevec (apevec) wrote :

Sorry for the confusion, script set this to commited on stable/havana based on this bug# mentioned in https://github.com/openstack/neutron/commit/35e595f7b3d85eb6053a50ab08044bd89eb554ab

Shiv Haris (shh) wrote :

Oleg, can u please update what more needs to be done here. it says backport to Havana. really? anyone affected by this?

padkrish (kprad1) wrote :

Just for records, this currently seems to be committed. I was slightly misled, thinking it's reverted by https://review.openstack.org/#/c/58827/ !!

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

https://github.com/openstack/neutron/commit/58cc80a2a883a13120eadd487b55b5c3b683b5b6

padkrish (kprad1) wrote :

We had a discussion today in the ML2 IRC. Does it really need to be back ported to havana? If not, this can be closed. I have modified the status to "Opinion"

Andrey Epifanov (aepifanov) wrote :

This bug was fixed with with the following commit:
https://review.openstack.org/#/c/62696/

Changed in neutron:
status: Confirmed → Fix Committed
Akihiro Motoki (amotoki) on 2014-09-09
Changed in neutron:
milestone: none → juno-rc1
Thierry Carrez (ttx) on 2014-10-03
Changed in neutron:
status: Fix Committed → Fix Released
Thierry Carrez (ttx) on 2014-10-16
Changed in neutron:
milestone: juno-rc1 → 2014.2
Itsuro Oda (oda-g) wrote :

icehouse was released with this patch reverted. Should be re-submitted to icehouse too.

tags: added: icehouse-backport-potentioal
Yaguang Tang (heut2008) on 2014-11-25
tags: removed: havana-backport-potential
tags: added: icehouse-backport-potential
removed: icehouse-backport-potentioal
To post a comment you must log in.
This report contains Public information  Edit
Everyone can see this information.

Other bug subscribers