Dynamically allocate service port addresses
We currently hard-code the address for management and external ports to the first address on the subnet, which breaks clustering astara-orchestrators when two nodes attempt to create neutron ports with the same addresses. This updates usage to instead rely on a Neutron-assigned address to bring up locally. Note this is a partial fix: we'll need corresponding changes that allow us to push in this address to appliances for metadata access, which is hard-coded there as well to the same address. Change-Id: I88fa97bae84ca245afa5ad0da4ac3c0bc1c441ff Partial-bug: #1524068
This commit is contained in:
Adam Gandelman
parent
de6ade7bd5
commit
eee3aed511
@ -108,16 +108,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):
|
||||
@ -765,8 +767,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)
|
||||
|
||||
@ -818,7 +820,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()
|
||||
@ -846,24 +847,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(
|
||||
@ -904,17 +910,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
|
||||
|
||||
@ -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,),
|
||||
|
||||
@ -20,7 +20,9 @@ import copy
|
||||
import mock
|
||||
import netaddr
|
||||
|
||||
from astara.test.unit import base
|
||||
from oslo_config import cfg
|
||||
|
||||
from astara.test.unit import base, fakes
|
||||
from astara.api import neutron
|
||||
|
||||
|
||||
@ -559,3 +561,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('astara.api.neutron.ip_lib')
|
||||
@mock.patch('astara.api.neutron.uuid')
|
||||
@mock.patch('astara.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:astara',
|
||||
'name': 'ASTARA: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('astara.api.neutron.socket')
|
||||
@mock.patch('astara.api.neutron.ip_lib')
|
||||
@mock.patch('astara.api.neutron.uuid')
|
||||
@mock.patch('astara.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': 'ASTARA: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('astara.api.neutron.ip_lib')
|
||||
@mock.patch('astara.api.neutron.uuid')
|
||||
@mock.patch('astara.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)
|
||||
|
||||
@ -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',
|
||||
|
||||
@ -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 astara import main
|
||||
from astara 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('astara.api.neutron.importutils')
|
||||
@mock.patch('astara.api.neutron.AstaraExtClientWrapper')
|
||||
@mock.patch('astara.main.multiprocessing')
|
||||
@mock.patch('astara.main.notifications')
|
||||
@mock.patch('astara.main.scheduler')
|
||||
@mock.patch('astara.main.populate')
|
||||
@mock.patch('astara.main.health')
|
||||
@mock.patch('astara.main.shuffle_notifications')
|
||||
@mock.patch('astara.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,
|
||||
astara_wrapper, importutils):
|
||||
|
||||
self.test_config.config(plug_external_port=False)
|
||||
|
||||
def side_effect(**kwarg):
|
||||
return {'ports': {}}
|
||||
|
||||
astara_wrapper.return_value.list_ports.side_effect = side_effect
|
||||
|
||||
main.main(argv=self.argv)
|
||||
args, kwargs = astara_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())
|
||||
|
||||
@ -0,0 +1,3 @@
|
||||
---
|
||||
fixes:
|
||||
- Bug `1524068 <https://bugs.launchpad.net/astara/+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.
|
||||
Loading…
Reference in New Issue
Block a user