x
/
group-based-policy
Code Issues Proposed changes

Fix router interface port status 

The router interface port status shows as down after connecting the
router to a subnet on a tenant network. This patch fixes the behavior
by calling update_port to bind the port, which changes its status to ACTIVE.

Change-Id: I4bee2cca0eb5b5d40fb802c31307c2608e5b8d8f
Closes-Bug: 1709453
(cherry picked from commit 632d82eef9)
(cherry picked from commit cb0497c537)
(cherry picked from commit f74281b3e9)
This commit is contained in:
Thomas Bachman 2017-08-07 18:40:27 +00:00
parent 0319b1555f
commit 496f33f785
5 changed files with 151 additions and 8 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
gbpservice/neutron/plugins/ml2plus/drivers/apic_aim/mechanism_driver.py
View File

@ -84,6 +84,8 @@ AGENT_TYPE_DVS = 'DVS agent'
VIF_TYPE_DVS = 'dvs'
PROMISCUOUS_TYPES = [n_constants.DEVICE_OWNER_DHCP,
n_constants.DEVICE_OWNER_LOADBALANCER]
VIF_TYPE_FABRIC = 'fabric'
FABRIC_HOST_ID = 'fabric'
NO_ADDR_SCOPE = object()
@ -1191,6 +1193,14 @@ class ApicMechanismDriver(api_plus.MechanismDriver,
vnic_type)
return
if port['binding:host_id'].startswith(FABRIC_HOST_ID):
for segment in context.segments_to_bind:
context.set_binding(segment[api.ID],
VIF_TYPE_FABRIC,
{portbindings.CAP_PORT_FILTER: False},
status=n_constants.PORT_STATUS_ACTIVE)
return
is_vm_port = port['device_owner'].startswith('compute:')
if (is_vm_port and self.gbp_driver and not

13
gbpservice/neutron/services/apic_aim/l3_plugin.py
View File

@ -21,6 +21,7 @@ from neutron.db import db_base_plugin_v2
from neutron.db import extraroute_db
from neutron.db import l3_gwmode_db
from neutron.extensions import l3
from neutron.extensions import portbindings
from neutron.plugins.common import constants
from neutron_lib import exceptions
from oslo_log import log as logging
@ -31,6 +32,8 @@ from gbpservice.neutron import extensions as extensions_pkg
from gbpservice.neutron.extensions import cisco_apic_l3 as l3_ext
from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import (
extension_db as extn_db)
from gbpservice.neutron.plugins.ml2plus.drivers.apic_aim import (
mechanism_driver as md)
LOG = logging.getLogger(__name__)
@ -157,6 +160,16 @@ class ApicL3Plugin(common_db_mixin.CommonDbMixin,
info = super(ApicL3Plugin, self).add_router_interface(
context, router_id, interface_info)
del context.override_network_routing_topology_validation
# REVISIT(tbachman): This update port triggers port
# binding, which means that port-binding happens inside
# of a transaction, which shouldn't happen. This isn't
# an issue with the AIM MD, but should be fixed at some
# point (e.g. move port-binding outside, possibly queued
# to be handled outside of the transaction, with some
# sort of cleanup if it fails).
self._core_plugin.update_port(context, info['port_id'],
{'port': {portbindings.HOST_ID:
md.FABRIC_HOST_ID}})
return info
def _add_interface_by_subnet(self, context, router, subnet_id, owner):

11
gbpservice/neutron/tests/unit/plugins/ml2plus/test_apic_aim.py
View File

@ -3179,6 +3179,17 @@ class TestPortBinding(ApicAimTestCase):
self.assertEqual('opflex', p2_ctx.top_bound_segment['network_type'])
self.assertEqual('vlan', p2_ctx.bottom_bound_segment['network_type'])
def test_bind_fabric_router_port(self):
net = self._make_network(self.fmt, 'net1', True)
self._make_subnet(self.fmt, net, '10.0.1.1', '10.0.1.0/24')
port = self._make_port(self.fmt, net['network']['id'])['port']
port_id = port['id']
port = self._bind_port_to_host(port_id, 'fabric')['port']
self.assertEqual('fabric', port['binding:vif_type'])
self.assertEqual({'port_filter': False},
port['binding:vif_details'])
self.assertEqual(n_constants.PORT_STATUS_ACTIVE, port['status'])
# TODO(rkukura): Add tests for opflex, local and unsupported
# network_type values.

95
gbpservice/neutron/tests/unit/plugins/ml2plus/test_l3_apic_aim.py Normal file
View File

@ -0,0 +1,95 @@
# Copyright (c) 2017 Cisco Systems
# All Rights Reserved.
#
# Licensed under the Apache License, Version 2.0 (the "License"); you may
# not use this file except in compliance with the License. You may obtain
# a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
# WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
# License for the specific language governing permissions and limitations
# under the License.
import mock
from neutron import context
from gbpservice.neutron.services.apic_aim import l3_plugin
from gbpservice.neutron.tests.unit.services.grouppolicy import (
test_aim_mapping_driver)
TENANT = 'tenant1'
ROUTER = 'router1'
NETWORK = 'network1'
CIDR = '10.0.0.0/16'
class TestCiscoApicAimL3Plugin(test_aim_mapping_driver.AIMBaseTestCase):
'''Test class for the Cisco APIC AIM L3 Plugin
This is a set of tests specific to the Cisco APIC AIM
L3 plugin. It currently derives from the AIMBaseTestCase
class so that it can inherit test infrastructure from those classes.
'''
def setUp(self):
super(TestCiscoApicAimL3Plugin, self).setUp()
# Set up L2 objects for L3 test
attr = {'tenant_id': TENANT}
resp = self._create_network(self.fmt, NETWORK, True, **attr)
self.network = self.deserialize(self.fmt, resp)['network']
attr.update({'network_id': self.network['id']})
resp = self._create_subnet(self.fmt, self.network['id'], CIDR, **attr)
self.subnet = self.deserialize(self.fmt, resp)['subnet']
resp = self._create_port(self.fmt, self.network['id'],
tenant_id=TENANT)
self.port = self.deserialize(self.fmt, resp)['port']
self.interface_info = {'subnet': {'subnet_id': self.subnet['id']},
'port': {'port_id': self.port['id']}}
self.context = context.get_admin_context()
self.context.tenant_id = TENANT
self.plugin = l3_plugin.ApicL3Plugin()
def _test_add_router_interface(self, interface_info):
attr = {'router': {'tenant_id': TENANT,
'admin_state_up': True,
'name': ROUTER}}
router = self.plugin.create_router(self.context, attr)
with mock.patch('neutron.callbacks.registry.notify'):
info = self.plugin.add_router_interface(self.context,
router['id'],
interface_info)
self.assertEqual(info['id'], router['id'])
self.assertEqual(info['tenant_id'], TENANT)
if interface_info.get('port_id'):
self.assertEqual(info['port_id'], self.port['id'])
else:
self.assertNotEqual(info['port_id'], self.port['id'])
self.assertEqual(info['subnet_id'], self.subnet['id'])
self.assertEqual(info['network_id'], self.network['id'])
def _test_remove_router_interface(self, interface_info):
with mock.patch('neutron.db.l3_db.'
'L3_NAT_db_mixin.remove_router_interface') as if_mock:
self.plugin.remove_router_interface(self.context, ROUTER,
interface_info)
self.assertEqual(1, if_mock.call_count)
def test_add_router_interface_subnet(self):
self._test_add_router_interface(self.interface_info['subnet'])
def test_add_router_interface_port(self):
self._test_add_router_interface(self.interface_info['port'])
def test_remove_router_interface_subnet(self):
self._test_remove_router_interface(self.interface_info['subnet'])
def test_remove_router_interface_port(self):
self._test_remove_router_interface(self.interface_info['port'])

30
gbpservice/neutron/tests/unit/services/grouppolicy/test_aim_mapping_driver.py
View File

@ -3763,9 +3763,17 @@ class NotificationTest(AIMBaseTestCase):
for sg_id in sg_ids:
self.new_delete_request(
'security-groups', sg_id).get_response(self.ext_api)
notifier.assert_has_calls(expected_calls(), any_order=False)
# Remove any port update calls for port-binding
new_mock = mock.MagicMock()
call_list = []
for call in notifier.mock_calls:
if call[1][-1] != 'port.update.end':
call_list.append(call)
new_mock.mock_calls = call_list
new_mock.assert_has_calls(expected_calls(), any_order=False)
# test that no notifications have been left out
self.assertEqual({}, self.notification_queue)
return new_mock
def _disable_checks(self, no_batch_event, with_batch_event):
# this is a temporarty workaround to avoid having to repeatedly
@ -3798,8 +3806,8 @@ class NotificationTest(AIMBaseTestCase):
def test_dhcp_notifier(self):
with mock.patch.object(dhcp_rpc_agent_api.DhcpAgentNotifyAPI,
'notify') as dhcp_notifier_no_batch:
self._test_notifier(dhcp_notifier_no_batch,
self._expected_dhcp_agent_call_list, False)
no_batch = self._test_notifier(dhcp_notifier_no_batch,
self._expected_dhcp_agent_call_list, False)
self.assertEqual(0, self.queue_notification_call_count)
self.assertEqual(0, self.max_notification_queue_length)
@ -3808,8 +3816,8 @@ class NotificationTest(AIMBaseTestCase):
with mock.patch.object(dhcp_rpc_agent_api.DhcpAgentNotifyAPI,
'notify') as dhcp_notifier_with_batch:
self._test_notifier(dhcp_notifier_with_batch,
self._expected_dhcp_agent_call_list, True)
batch = self._test_notifier(dhcp_notifier_with_batch,
self._expected_dhcp_agent_call_list, True)
self.assertLess(0, self.queue_notification_call_count)
self.assertLess(0, self.max_notification_queue_length)
@ -3818,8 +3826,7 @@ class NotificationTest(AIMBaseTestCase):
# of notifications should be sent
self.assertEqual(4, self.post_notifications_from_queue_call_count)
self._test_notifications(dhcp_notifier_no_batch.call_args_list,
dhcp_notifier_with_batch.call_args_list)
self._test_notifications(no_batch.call_args_list, batch.call_args_list)
def test_nova_notifier(self):
with mock.patch.object(nova.Notifier,
@ -3859,8 +3866,15 @@ class NotificationTest(AIMBaseTestCase):
'send_network_change') as nova_notifier:
self.create_policy_target_group(name="ptg1",
expected_res_status=500)
# Remove any port updates, as those don't count
args_list = []
new_dhcp = mock.MagicMock()
for call_args in dhcp_notifier.call_args_list:
if call_args[0][-1] != 'port.update.end':
args_list.append(call_args)
new_dhcp.call_args_list = args_list
# test that notifier was not called
self.assertEqual([], dhcp_notifier.call_args_list)
self.assertEqual([], new_dhcp.call_args_list)
self.assertEqual([], nova_notifier.call_args_list)
# test that notification queue has been flushed
self.assertEqual({}, self.notification_queue)