We don't have asserts anywhere else. I don't think it makes sense to only
put a couple in.

On Sun, Jan 11, 2015 at 8:29 AM, Sunil Kumar <su...@embrane.com> wrote:

>  Actually, I just noticed that a patch which has been pending review
> since Sep is not merged, and that actually fixes the root cause.
>
> https://review.openstack.org/#/c/119269/
>
> Having said that, I think the code can definitely use some touching. If
> router_ids is not supposed to be None, it should be asserted (enforcing a
> contract with the plugin APIs!) instead of letting python crib about it.
> Also, the code in plugins/ml2/plugin.py should not even be call
> l3plugin.notify_routers_updated if the router_ids set is empty. I think
> assert also belongs there.
>
> -Sunil
>     ------------------------------
> *From:* Salvatore Orlando [sorla...@nicira.com]
> *Sent:* Sunday, January 11, 2015 7:25 AM
> *To:* OpenStack Development Mailing List (not for usage questions)
> *Subject:* Re: [openstack-dev] [Neutron][L3][Devstack] Bug during delete
> floating IPs?
>
>   I think Kevin is right - the actual root cause lies in plugins for
> which disassociate_floating_ips returns None rather than an (empty)
> iterable.
>
>  A quick check revealed that at least:
> neutron.plugins.embrane.base_plugin.EmbranePlugin
>
> neutron.plugions.cisco.service_plugins.cisco_router_plugin.CiscoRouterPlugin
>
>  can return None in some circumstances.
> I reckon the plugins should be fixed.
> While Sunil's patch fixes the issue it does not make a lot of sense since
> it call notify_routers_updated with None for routers_id.
> I agree with his intention of making the ml2 plugin more robust, but in my
> opinion when router_ids is None or empty the notification method should
> probably not be called at all.
>
>  Salvatore
>
> On 11 January 2015 at 12:02, Kevin Benton <blak...@gmail.com> wrote:
>
>> That only seems to bury the root cause because router_ids shouldn't be
>> none in this case with the reference L3 plugin.
>>
>>  The ML2 plugin calls disassociate_floatingips with do_notify set to
>> False so it should always be at least an empty set coming back.
>>
>>  Which L3 plugin were you using? Perhaps the implementation of
>> disassociate_floatingips was incorrect.
>>
>>  On Sat, Jan 10, 2015 at 11:42 PM, Sunil Kumar <su...@embrane.com> wrote:
>>
>>>   This trivial patch fixes the tracebacks:
>>>
>>> $ cat disassociate_floating_ips.patch
>>> --- neutron/db/l3_db.py.orig    2015-01-10 22:20:30.101506298 -0800
>>> +++ neutron/db/l3_db.py 2015-01-10 22:24:18.111479818 -0800
>>> @@ -1257,4 +1257,4 @@
>>>
>>>      def notify_routers_updated(self, context, router_ids):
>>>          super(L3_NAT_db_mixin, self).notify_routers_updated(
>>> -            context, list(router_ids), 'disassociate_floatingips', {})
>>> +            context, list(router_ids) if router_ids else None,
>>> 'disassociate_floatingips', {})
>>>
>>>
>>> -Sunil
>>>     ------------------------------
>>> *From:* Sunil Kumar [su...@embrane.com]
>>> *Sent:* Saturday, January 10, 2015 7:07 PM
>>> *To:* OpenStack Development Mailing List (not for usage questions)
>>> *Subject:* [openstack-dev] [Neutron][L3][Devstack] Bug during delete
>>> floating IPs?
>>>
>>>    Not sure if its something seen by others. I hit this when I run
>>> tempest.scenario.test_network_basic_ops.TestNetworkBasicOps against master:
>>>
>>> 2015-01-10 17:45:13.227 5350 DEBUG neutron.plugins.ml2.plugin
>>> [req-2ab4b380-cf3a-4663-90c3-a05ef5f4da0f None] Deleting port
>>> e5deb014-0063-4d55-8ee3-5ba3524fee14 delete_port
>>> /opt/stack/new/neutron/neutron/plugins/ml2/plugin.py:995
>>> 2015-01-10 17:45:13.228 5350 DEBUG neutron.openstack.common.lockutils
>>> [req-2ab4b380-cf3a-4663-90c3-a05ef5f4da0f ] Created new semaphore
>>> "db-access" internal_lock
>>> /opt/stack/new/neutron/neutron/openstack/common/lockutils.py:206
>>> 2015-01-10 17:45:13.228 5350 DEBUG neutron.openstack.common.lockutils
>>> [req-2ab4b380-cf3a-4663-90c3-a05ef5f4da0f ] Acquired semaphore "db-access"
>>> lock /opt/stack/new/neutron/neutron/openstack/common/lockutils.py:229
>>> 2015-01-10 17:45:13.252 5350 DEBUG neutron.plugins.ml2.plugin
>>> [req-2ab4b380-cf3a-4663-90c3-a05ef5f4da0f None] Calling delete_port for
>>> e5deb014-0063-4d55-8ee3-5ba3524fee14 owned by network:floatingip
>>> delete_port /opt/stack/new/neutron/neutron/plugins/ml2/plugin.py:1043
>>> 2015-01-10 17:45:13.254 5350 DEBUG neutron.openstack.common.lockutils
>>> [req-2ab4b380-cf3a-4663-90c3-a05ef5f4da0f ] Releasing semaphore "db-access"
>>> lock /opt/stack/new/neutron/neutron/openstack/common/lockutils.py:238
>>> 2015-01-10 17:45:13.282 5350 ERROR neutron.api.v2.resource
>>> [req-2ab4b380-cf3a-4663-90c3-a05ef5f4da0f None] delete failed
>>> 2015-01-10 17:45:13.282 5350 TRACE neutron.api.v2.resource Traceback
>>> (most recent call last):
>>> 2015-01-10 17:45:13.282 5350 TRACE neutron.api.v2.resource   File
>>> "/opt/stack/new/neutron/neutron/api/v2/resource.py", line 83, in resource
>>> 2015-01-10 17:45:13.282 5350 TRACE neutron.api.v2.resource     result =
>>> method(request=request, **args)
>>> 2015-01-10 17:45:13.282 5350 TRACE neutron.api.v2.resource   File
>>> "/opt/stack/new/neutron/neutron/api/v2/base.py", line 479, in delete
>>> 2015-01-10 17:45:13.282 5350 TRACE neutron.api.v2.resource
>>> obj_deleter(request.context, id, **kwargs)
>>> 2015-01-10 17:45:13.282 5350 TRACE neutron.api.v2.resource   File
>>> "/opt/stack/new/neutron/neutron/db/l3_dvr_db.py", line 198, in
>>> delete_floatingip
>>> 2015-01-10 17:45:13.282 5350 TRACE neutron.api.v2.resource
>>> self).delete_floatingip(context, id)
>>> 2015-01-10 17:45:13.282 5350 TRACE neutron.api.v2.resource   File
>>> "/opt/stack/new/neutron/neutron/db/l3_db.py", line 1237, in
>>> delete_floatingip
>>> 2015-01-10 17:45:13.282 5350 TRACE neutron.api.v2.resource     router_id
>>> = self._delete_floatingip(context, id)
>>> 2015-01-10 17:45:13.282 5350 TRACE neutron.api.v2.resource   File
>>> "/opt/stack/new/neutron/neutron/db/l3_db.py", line 902, in
>>> _delete_floatingip
>>> 2015-01-10 17:45:13.282 5350 TRACE neutron.api.v2.resource
>>> l3_port_check=False)
>>> 2015-01-10 17:45:13.282 5350 TRACE neutron.api.v2.resource   File
>>> "/opt/stack/new/neutron/neutron/plugins/ml2/plugin.py", line 1050, in
>>> delete_port
>>> 2015-01-10 17:45:13.282 5350 TRACE neutron.api.v2.resource
>>> l3plugin.notify_routers_updated(context, router_ids)
>>> 2015-01-10 17:45:13.282 5350 TRACE neutron.api.v2.resource   File
>>> "/opt/stack/new/neutron/neutron/db/l3_db.py", line 1260, in
>>> notify_routers_updated
>>> 2015-01-10 17:45:13.282 5350 TRACE neutron.api.v2.resource     context,
>>> list(router_ids), 'disassociate_floatingips', {})
>>> 2015-01-10 17:45:13.282 5350 TRACE neutron.api.v2.resource TypeError:
>>> 'NoneType' object is not iterable
>>>
>>> Looks like the code is assuming that router_ids can never be None, which
>>> clearly is the case here. Is that a bug?
>>>
>>> Looking elsewhere in the l3_db.py,
>>> L3RpcNotifierMixin.notify_routers_updated() does make a check for
>>> router_ids (which means that that function does expect it to be empty some
>>> times), but the list() is killing it before it reaches that.
>>>
>>> This backtrace repeats itself many many times in the neutron logs.
>>>
>>> Thanks for your help.
>>> -Sunil
>>>
>>>
>>> __________________________________________________________________________
>>> OpenStack Development Mailing List (not for usage questions)
>>> Unsubscribe:
>>> openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
>>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>>
>>>
>>
>>
>>  --
>>  Kevin Benton
>>
>> __________________________________________________________________________
>> OpenStack Development Mailing List (not for usage questions)
>> Unsubscribe:
>> openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
>> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>>
>>
>
> __________________________________________________________________________
> OpenStack Development Mailing List (not for usage questions)
> Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
> http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev
>
>


-- 
Kevin Benton
__________________________________________________________________________
OpenStack Development Mailing List (not for usage questions)
Unsubscribe: openstack-dev-requ...@lists.openstack.org?subject:unsubscribe
http://lists.openstack.org/cgi-bin/mailman/listinfo/openstack-dev

Reply via email to