diff --git a/akanda/rug/api/neutron.py b/akanda/rug/api/neutron.py index 96839ac7..8d87e37c 100644 --- a/akanda/rug/api/neutron.py +++ b/akanda/rug/api/neutron.py @@ -104,16 +104,18 @@ class RouterGatewayMissing(Exception): class MissingIPAllocation(Exception): - def __init__(self, port_id, missing): + def __init__(self, port_id, missing=None): self.port_id = port_id self.missing = missing - msg = 'Port %s missing an expected ' % port_id - ip_msg = ' and '.join( - ('IPv%s address from one of %s' % - (mv, missing_subnets)) - for mv, missing_subnets in missing - ) - super(MissingIPAllocation, self).__init__(msg + ip_msg) + msg = 'Port %s missing expected IPs ' % port_id + if missing: + ip_msg = ' and '.join( + ('IPv%s address from one of %s' % + (mv, missing_subnets)) + for mv, missing_subnets in missing + ) + msg = msg + ip_msg + super(MissingIPAllocation, self).__init__(msg) class DictModelBase(object): @@ -754,8 +756,8 @@ class Neutron(object): time.sleep(self.conf.retry_delay) raise RouterGatewayMissing() - def _ensure_local_port(self, network_id, subnet_id, - network_type, ip_address): + def _ensure_local_port(self, network_id, subnet_id, prefix, + network_type): driver = importutils.import_object(self.conf.interface_driver, self.conf) @@ -783,7 +785,6 @@ class Neutron(object): 'name': name, 'device_id': host_id, 'fixed_ips': [{ - 'ip_address': ip_address.split('/')[0], 'subnet_id': subnet_id }], 'binding:host_id': socket.gethostname() @@ -811,24 +812,29 @@ class Neutron(object): # add sleep to ensure that port is setup before use time.sleep(1) - driver.init_l3(driver.get_device_name(port), [ip_address]) - return port + try: + fixed_ip = [fip for fip in port.fixed_ips + if fip.subnet_id == subnet_id][0] + except IndexError: + raise MissingIPAllocation(port.id) + + ip_cidr = '%s/%s' % (fixed_ip.ip_address, prefix.split('/')[1]) + driver.init_l3(driver.get_device_name(port), [ip_cidr]) + return ip_cidr def ensure_local_external_port(self): return self._ensure_local_port( self.conf.external_network_id, self.conf.external_subnet_id, - 'external', - get_local_external_ip(self.conf) - ) + self.conf.external_prefix, + 'external') def ensure_local_service_port(self): return self._ensure_local_port( self.conf.management_network_id, self.conf.management_subnet_id, - 'service', - get_local_service_ip(self.conf) - ) + self.conf.management_prefix, + 'service') def purge_management_interface(self): driver = importutils.import_object( @@ -862,17 +868,3 @@ class Neutron(object): def clear_device_id(self, port): self.api_client.update_port(port.id, {'port': {'device_id': ''}}) - - -def get_local_service_ip(conf): - mgt_net = netaddr.IPNetwork(conf.management_prefix) - rug_ip = '%s/%s' % (netaddr.IPAddress(mgt_net.first + 1), - mgt_net.prefixlen) - return rug_ip - - -def get_local_external_ip(conf): - external_net = netaddr.IPNetwork(conf.external_prefix) - external_ip = '%s/%s' % (netaddr.IPAddress(external_net.first + 1), - external_net.prefixlen) - return external_ip diff --git a/akanda/rug/main.py b/akanda/rug/main.py index 5761aecb..5dd25bcd 100644 --- a/akanda/rug/main.py +++ b/akanda/rug/main.py @@ -120,7 +120,7 @@ def main(argv=sys.argv[1:]): # neutron.purge_management_interface() # bring the mgt tap interface up - neutron.ensure_local_service_port() + mgt_ip_address = neutron.ensure_local_service_port().split('/')[0] # bring the external port if cfg.CONF.plug_external_port: @@ -160,7 +160,6 @@ def main(argv=sys.argv[1:]): else: coordinator_proc = None - mgt_ip_address = neutron_api.get_local_service_ip(cfg.CONF).split('/')[0] metadata_proc = multiprocessing.Process( target=metadata.serve, args=(mgt_ip_address,), diff --git a/akanda/rug/test/unit/api/test_neutron_wrapper.py b/akanda/rug/test/unit/api/test_neutron_wrapper.py index a43ca368..69dc89d3 100644 --- a/akanda/rug/test/unit/api/test_neutron_wrapper.py +++ b/akanda/rug/test/unit/api/test_neutron_wrapper.py @@ -20,9 +20,11 @@ import copy import mock import netaddr -from akanda.rug.test.unit import base +from akanda.rug.test.unit import base, fakes from akanda.rug.api import neutron +from oslo_config import cfg + class TestuNeutronModels(base.RugTestBase): def test_router(self): @@ -556,3 +558,148 @@ class TestExternalPort(base.RugTestBase): self.assertEqual(6, e.missing[1][0]) else: self.fail('Should have seen MissingIPAllocation') + + +class TestLocalServicePorts(base.RugTestBase): + def setUp(self): + super(TestLocalServicePorts, self).setUp() + self.config(external_network_id='fake_extnet_network_id') + self.config(external_subnet_id='fake_extnet_subnet_id') + self.config(external_prefix='172.16.77.0/24') + self.config(management_network_id='fake_mgtnet_network_id') + self.config(management_subnet_id='fake_mgtnet_subnet_id') + self.config(management_prefix='172.16.77.0/24') + self.config(management_prefix='fdca:3ba5:a17a:acda::/64') + self.neutron_wrapper = neutron.Neutron(cfg.CONF) + self.fake_interface_driver = mock.Mock( + plug=mock.Mock(), + init_l3=mock.Mock(), + get_device_name=mock.Mock()) + + def test_ensure_local_external_port(self): + with mock.patch.object(self.neutron_wrapper, + '_ensure_local_port') as ep: + self.neutron_wrapper.ensure_local_external_port() + ep.assert_called_with( + 'fake_extnet_network_id', + 'fake_extnet_subnet_id', + '172.16.77.0/24', + 'external', + ) + + def test_ensure_local_service_port(self): + with mock.patch.object(self.neutron_wrapper, + '_ensure_local_port') as ep: + self.neutron_wrapper.ensure_local_service_port() + ep.assert_called_with( + 'fake_mgtnet_network_id', + 'fake_mgtnet_subnet_id', + 'fdca:3ba5:a17a:acda::/64', + 'service', + ) + + @mock.patch('akanda.rug.api.neutron.ip_lib') + @mock.patch('akanda.rug.api.neutron.uuid') + @mock.patch('akanda.rug.api.neutron.importutils') + def test__ensure_local_port_neutron_port_exists(self, fake_import, + fake_uuid, fake_ip_lib): + fake_ip_lib.device_exists.return_value = True + fake_uuid.uuid5.return_value = 'fake_host_id' + fake_import.import_object.return_value = self.fake_interface_driver + + fake_port = fakes.fake_port() + fake_port_dict = { + 'ports': [fake_port._neutron_port_dict], + } + fake_client = mock.Mock( + list_ports=mock.Mock(return_value=fake_port_dict) + ) + self.neutron_wrapper.api_client = fake_client + self.fake_interface_driver.get_device_name.return_value = 'fake_dev' + + self.neutron_wrapper._ensure_local_port( + 'fake_network_id', + 'fake_subnet_id', + 'fdca:3ba5:a17a:acda:f816:3eff:fe2b::1/64', + 'service') + + exp_query = { + 'network_id': 'fake_network_id', + 'device_owner': 'network:akanda', + 'name': 'AKANDA:RUG:SERVICE', + 'device_id': 'fake_host_id' + } + fake_client.list_ports.assert_called_with(**exp_query) + self.fake_interface_driver.init_l3.assert_called_with( + 'fake_dev', ['fdca:3ba5:a17a:acda:f816:3eff:fe2b:ced0/64'] + ) + + @mock.patch('akanda.rug.api.neutron.socket') + @mock.patch('akanda.rug.api.neutron.ip_lib') + @mock.patch('akanda.rug.api.neutron.uuid') + @mock.patch('akanda.rug.api.neutron.importutils') + def test__ensure_local_port_no_neutron_port(self, fake_import, fake_uuid, + fake_ip_lib, fake_socket): + fake_socket.gethostname.return_value = 'foo_hostname' + fake_ip_lib.device_exists.return_value = True + fake_uuid.uuid5.return_value = 'fake_host_id' + fake_import.import_object.return_value = self.fake_interface_driver + + fake_created_port = {'port': fakes.fake_port().to_dict()} + fake_client = mock.Mock( + list_ports=mock.Mock(return_value={'ports': []}), + create_port=mock.Mock(return_value=fake_created_port)) + self.neutron_wrapper.api_client = fake_client + self.fake_interface_driver.get_device_name.return_value = 'fake_dev' + + self.neutron_wrapper._ensure_local_port( + 'fake_network_id', + 'fake_subnet_id', + 'fdca:3ba5:a17a:acda:f816:3eff:fe2b::1/64', + 'service') + + exp_port_create_dict = {'port': { + 'admin_state_up': True, + 'binding:host_id': 'foo_hostname', + 'device_id': 'fake_host_id', + 'device_owner': 'network:router_interface', + 'fixed_ips': [{'subnet_id': 'fake_subnet_id'}], + 'name': 'AKANDA:RUG:SERVICE', + 'network_id': 'fake_network_id' + }} + fake_client.create_port.assert_called_with(exp_port_create_dict) + self.fake_interface_driver.init_l3.assert_called_with( + 'fake_dev', ['fdca:3ba5:a17a:acda:f816:3eff:fe2b:ced0/64'] + ) + + @mock.patch('time.sleep') + @mock.patch('akanda.rug.api.neutron.ip_lib') + @mock.patch('akanda.rug.api.neutron.uuid') + @mock.patch('akanda.rug.api.neutron.importutils') + def test__ensure_local_port_plug(self, fake_import, + fake_uuid, fake_ip_lib, fake_sleep): + fake_ip_lib.device_exists.return_value = False + fake_uuid.uuid5.return_value = 'fake_host_id' + fake_import.import_object.return_value = self.fake_interface_driver + + fake_port = fakes.fake_port() + fake_port_dict = { + 'ports': [fake_port._neutron_port_dict], + } + fake_client = mock.Mock( + list_ports=mock.Mock(return_value=fake_port_dict) + ) + self.neutron_wrapper.api_client = fake_client + self.fake_interface_driver.get_device_name.return_value = 'fake_dev' + + self.neutron_wrapper._ensure_local_port( + 'fake_network_id', + 'fake_subnet_id', + 'fdca:3ba5:a17a:acda:f816:3eff:fe2b::1/64', + 'service') + + self.fake_interface_driver.plug.assert_called_with( + 'fake_network_id', + fake_port.id, + 'fake_dev', + fake_port.mac_address) diff --git a/akanda/rug/test/unit/fakes.py b/akanda/rug/test/unit/fakes.py index e9827433..acd183f6 100644 --- a/akanda/rug/test/unit/fakes.py +++ b/akanda/rug/test/unit/fakes.py @@ -70,6 +70,42 @@ def fake_loadbalancer(): return neutron.LoadBalancer.from_dict(lb_dict) +def fake_port(): + port_dict = { + u'admin_state_up': True, + u'allowed_address_pairs': [], + u'binding:host_id': u'trusty', + u'binding:profile': {}, + u'binding:vif_details': { + u'ovs_hybrid_plug': True, u'port_filter': True + }, + u'binding:vif_type': u'ovs', + u'binding:vnic_type': u'normal', + u'device_id': u'fake_device_id', + u'device_owner': u'network:astara', + u'dns_assignment': [{ + u'fqdn': u'foo.openstacklocal.', + u'hostname': u'host-fdca-3ba5-a17a-acda-f816-3eff-fe2b-ced0', + u'ip_address': u'fdca:3ba5:a17a:acda:f816:3eff:fe2b:ced0' + }], + u'dns_name': u'', + u'extra_dhcp_opts': [], + u'fixed_ips': [{ + u'ip_address': u'fdca:3ba5:a17a:acda:f816:3eff:fe2b:ced0', + u'subnet_id': 'fake_subnet_id', + }], + u'id': u'fake_port_id', + u'mac_address': u'fa:16:3e:2b:ce:d0', + u'name': u'ASTARA:RUG:SERVICE', + u'network_id': u'fake_network_id', + u'port_security_enabled': False, + u'security_groups': [], + u'status': u'ACTIVE', + u'tenant_id': u'fake_tenant_id' + } + return neutron.Port.from_dict(port_dict) + + def fake_router(): router_gateway_port = { 'id': 'ext', diff --git a/akanda/rug/test/unit/test_main.py b/akanda/rug/test/unit/test_main.py index a65c517d..454f5006 100644 --- a/akanda/rug/test/unit/test_main.py +++ b/akanda/rug/test/unit/test_main.py @@ -14,11 +14,7 @@ # License for the specific language governing permissions and limitations # under the License. -import sys -import socket - import mock -import testtools from akanda.rug import main from akanda.rug import notifications as ak_notifications @@ -93,37 +89,3 @@ class TestMainPippo(base.RugTestBase): main.main(argv=self.argv) self.assertEqual(len(notifications.Publisher.mock_calls), 2) self.assertEqual(len(notifications.NoopPublisher.mock_calls), 0) - - -@mock.patch('akanda.rug.api.neutron.importutils') -@mock.patch('akanda.rug.api.neutron.AkandaExtClientWrapper') -@mock.patch('akanda.rug.main.multiprocessing') -@mock.patch('akanda.rug.main.notifications') -@mock.patch('akanda.rug.main.scheduler') -@mock.patch('akanda.rug.main.populate') -@mock.patch('akanda.rug.main.health') -@mock.patch('akanda.rug.main.shuffle_notifications') -@mock.patch('akanda.rug.api.neutron.get_local_service_ip') -class TestMainExtPortBinding(base.RugTestBase): - - @testtools.skipIf( - sys.platform != 'linux2', - 'unsupported platform' - ) - def test_ensure_local_port_host_binding( - self, get_local_service_ip, shuffle_notifications, health, - populate, scheduler, notifications, multiprocessing, - akanda_wrapper, importutils): - - self.test_config.config(plug_external_port=False) - - def side_effect(**kwarg): - return {'ports': {}} - - akanda_wrapper.return_value.list_ports.side_effect = side_effect - - main.main(argv=self.argv) - args, kwargs = akanda_wrapper.return_value.create_port.call_args - port = args[0]['port'] - self.assertIn('binding:host_id', port) - self.assertEqual(port['binding:host_id'], socket.gethostname()) diff --git a/releasenotes/notes/dynamic-mgt-port-86d4b9f780fa3d78.yaml b/releasenotes/notes/dynamic-mgt-port-86d4b9f780fa3d78.yaml new file mode 100644 index 00000000..77545ce8 --- /dev/null +++ b/releasenotes/notes/dynamic-mgt-port-86d4b9f780fa3d78.yaml @@ -0,0 +1,3 @@ +--- +fixes: + - Bug `1524068 `_ Local management port addresses are now allocated from the management subnet rather than using a hard-coded address, fixing Neutron port address conflicts when clustering astara-orchestrators.