[neutron-tempest-plugin] delete_router calls remove_router_extra_routes even when extra_routes extension is disabled

[Aditya Vaja]

A recent change [1] to neutron-tempest-plugin is causing a tempest failure when running against networking-bigswitch neutron plugin with L3 functionality provided by networking-bigswitch.

The failure stack trace looks as follows:

17:12:37 tearDownClass (neutron_tempest_plugin.scenario.admin.test_floatingip.FloatingIpTestCasesAdmin)
17:12:37 ----------------------------------------------------------------------------------------------
17:12:37
17:12:37 Captured traceback:
17:12:37 ~~~~~~~~~~~~~~~~~~~
17:12:37 Traceback (most recent call last):
17:12:37 File "tempest/test.py", line 224, in tearDownClass
17:12:37 six.reraise(etype, value, trace)
17:12:37 File "tempest/test.py", line 196, in tearDownClass
17:12:37 teardown()
17:12:37 File "/opt/stack/neutron-tempest-plugin/neutron_tempest_plugin/scenario/admin/test_floatingip.py", line 56, in resource_cleanup
17:12:37 super(FloatingIpTestCasesAdmin, cls).resource_cleanup()
17:12:37 File "/opt/stack/neutron-tempest-plugin/neutron_tempest_plugin/api/base.py", line 149, in resource_cleanup
17:12:37 router)
17:12:37 File "/opt/stack/neutron-tempest-plugin/neutron_tempest_plugin/api/base.py", line 260, in _try_delete_resource
17:12:37 delete_callable(*args, **kwargs)
17:12:37 File "/opt/stack/neutron-tempest-plugin/neutron_tempest_plugin/api/base.py", line 679, in delete_router
17:12:37 client.remove_router_extra_routes(router['id'])
17:12:37 File "/opt/stack/neutron-tempest-plugin/neutron_tempest_plugin/services/network/json/network_client.py", line 452, in remove_router_extra_routes
17:12:37 self.update_router(router_id, routes=None)
17:12:37 File "/opt/stack/neutron-tempest-plugin/neutron_tempest_plugin/services/network/json/network_client.py", line 394, in update_router
17:12:37 return self._update_router(router_id, set_enable_snat=False, **kwargs)
17:12:37 File "/opt/stack/neutron-tempest-plugin/neutron_tempest_plugin/services/network/json/network_client.py", line 382, in _update_router
17:12:37 resp, body = self.put(uri, update_body)
17:12:37 File "tempest/lib/common/rest_client.py", line 344, in put
17:12:37 return self.request('PUT', url, extra_headers, headers, body, chunked)
17:12:37 File "tempest/lib/common/rest_client.py", line 676, in request
17:12:37 self._error_checker(resp, resp_body)
17:12:37 File "tempest/lib/common/rest_client.py", line 787, in _error_checker
17:12:37 raise exceptions.BadRequest(resp_body, resp=resp)
17:12:37 tempest.lib.exceptions.BadRequest: Bad request
17:12:37 Details: {u'type': u'HTTPBadRequest', u'detail': u'', u'message': u"Unrecognized attribute(s) 'routes'"}
17:12:37

Failure is not during actual test, but rather during tearDown of the test.
Further inspection points to this line [2] being added to delete_router method of the base class.
Typically there is an 'if routes in kwargs' or 'if extra_routes extension enabled' sort of check, before calling the extra_route related add/delete methods.

Not sure if this was intentional and extra_routes is now a mandatory extension. Please correct me if I'm wrong and I can update the neutron plugin to handle it correctly.

Steps to Reproduce:
 - install devstack with a third party neutron L3 plugin enabled
 - ensure that this plugin does not support extra_routes extension
 - run tempest with neutron-tempest-plugin enabled and enabling only floating_ip tests (to trigger L3 methods)

Probable change-id that can provide more info about why the remove was added and if it needs change: Ie9903b7078d6fcff307c5d76770d92df1152d473

[1] https://github.com/openstack/neutron-tempest-plugin/commit/b1a3289fe4b98d1270844aa81f60604e64ae50cf
[2] https://github.com/openstack/neutron-tempest-plugin/blob/b1a3289fe4b98d1270844aa81f60604e64ae50cf/neutron_tempest_plugin/api/base.py#L673

[Brian Haley (brian-haley)]

Yes, that call in delete_router() should have been added in a better way. Best solution is probably to add a cleanup call in the test setup that would trigger that, at least that's my initial thought.

[OpenStack Infra (hudson-openstack)]

Reviewed: https://review.openstack.org/620431
Committed: https://git.openstack.org/cgit/openstack/neutron-tempest-plugin/commit/?id=49819a7b034273a0959959f59989d563bc77236d
Submitter: Zuul
Branch: master

commit 49819a7b034273a0959959f59989d563bc77236d
Author: Aditya Vaja <email address hidden>
Date: Mon Nov 26 14:20:10 2018 -0800

    call remove_router_extra_routes only when extra_routes is enabled

     - check if 'routes' is present in the router dict before calling
       cleanup for extra_routes extension

    Closes-Bug: #1805235
    Change-Id: I055b4053c795554deeaf167922de58ff49c96725

[OpenStack Infra (hudson-openstack)]

This issue was fixed in the openstack/neutron-tempest-plugin 0.3.0 release.