[OVN] Only account for bound ports in metadata agent
Due to bug #1922934 there might be situations with stale ports in the OVN database. When instances request metadata, the agent will try to fetch all ports for that particular network with the same IP address and there should be only one. However, when there are stale ports in OVN database but not in Neutron, those ports may have the same IP address and the result is that metadata won't work. As the stale ports will never be bound to any hypervisor, this patch is accounting only for bound ports so that those ports don't interfere until they eventually get fixed by the maintenance task. Related-Bug: #1922934 Signed-off-by: Daniel Alvarez Sanchez <dalvarez@redhat.com> Change-Id: I708904b982d243359f2eeda809beae0321f1a7db
This commit is contained in:
parent
d5b5f1ac9c
commit
32e938e698
|
@ -222,7 +222,8 @@ class MetadataAgent(object):
|
||||||
self._load_config()
|
self._load_config()
|
||||||
|
|
||||||
# Launch the server that will act as a proxy between the VM's and Nova.
|
# Launch the server that will act as a proxy between the VM's and Nova.
|
||||||
proxy = metadata_server.UnixDomainMetadataProxy(self.conf)
|
proxy = metadata_server.UnixDomainMetadataProxy(self.conf,
|
||||||
|
self.chassis)
|
||||||
proxy.run()
|
proxy.run()
|
||||||
|
|
||||||
tables = ('Encap', 'Port_Binding', 'Datapath_Binding', 'SB_Global',
|
tables = ('Encap', 'Port_Binding', 'Datapath_Binding', 'SB_Global',
|
||||||
|
|
|
@ -43,8 +43,9 @@ MODE_MAP = {
|
||||||
|
|
||||||
class MetadataProxyHandler(object):
|
class MetadataProxyHandler(object):
|
||||||
|
|
||||||
def __init__(self, conf):
|
def __init__(self, conf, chassis):
|
||||||
self.conf = conf
|
self.conf = conf
|
||||||
|
self.chassis = chassis
|
||||||
self.subscribe()
|
self.subscribe()
|
||||||
|
|
||||||
def subscribe(self):
|
def subscribe(self):
|
||||||
|
@ -56,7 +57,8 @@ class MetadataProxyHandler(object):
|
||||||
# We need to open a connection to OVN SouthBound database for
|
# We need to open a connection to OVN SouthBound database for
|
||||||
# each worker so that we can process the metadata requests.
|
# each worker so that we can process the metadata requests.
|
||||||
self.sb_idl = ovsdb.MetadataAgentOvnSbIdl(
|
self.sb_idl = ovsdb.MetadataAgentOvnSbIdl(
|
||||||
tables=('Port_Binding', 'Datapath_Binding')).start()
|
tables=('Port_Binding', 'Datapath_Binding', 'Chassis'),
|
||||||
|
chassis=self.chassis).start()
|
||||||
|
|
||||||
@webob.dec.wsgify(RequestClass=webob.Request)
|
@webob.dec.wsgify(RequestClass=webob.Request)
|
||||||
def __call__(self, req):
|
def __call__(self, req):
|
||||||
|
@ -173,8 +175,9 @@ class MetadataProxyHandler(object):
|
||||||
|
|
||||||
class UnixDomainMetadataProxy(object):
|
class UnixDomainMetadataProxy(object):
|
||||||
|
|
||||||
def __init__(self, conf):
|
def __init__(self, conf, chassis):
|
||||||
self.conf = conf
|
self.conf = conf
|
||||||
|
self.chassis = chassis
|
||||||
agent_utils.ensure_directory_exists_without_file(
|
agent_utils.ensure_directory_exists_without_file(
|
||||||
cfg.CONF.metadata_proxy_socket)
|
cfg.CONF.metadata_proxy_socket)
|
||||||
|
|
||||||
|
@ -203,7 +206,7 @@ class UnixDomainMetadataProxy(object):
|
||||||
md_workers = self.conf.metadata_workers
|
md_workers = self.conf.metadata_workers
|
||||||
if md_workers is None:
|
if md_workers is None:
|
||||||
md_workers = 2
|
md_workers = 2
|
||||||
self.server.start(MetadataProxyHandler(self.conf),
|
self.server.start(MetadataProxyHandler(self.conf, self.chassis),
|
||||||
self.conf.metadata_proxy_socket,
|
self.conf.metadata_proxy_socket,
|
||||||
workers=md_workers,
|
workers=md_workers,
|
||||||
backlog=self.conf.metadata_backlog,
|
backlog=self.conf.metadata_backlog,
|
||||||
|
|
|
@ -851,15 +851,24 @@ class OvsdbSbOvnIdl(sb_impl_idl.OvnSbApiIdlImpl, Backend):
|
||||||
rows = self.db_list_rows('Port_Binding').execute(check_error=True)
|
rows = self.db_list_rows('Port_Binding').execute(check_error=True)
|
||||||
# TODO(twilson) It would be useful to have a db_find that takes a
|
# TODO(twilson) It would be useful to have a db_find that takes a
|
||||||
# comparison function
|
# comparison function
|
||||||
# TODO(dalvarez): Remove the comparison to r.datapath.uuid in Y cycle
|
|
||||||
# when we are sure that all namespaces will be created with the
|
def check_net_and_ip(port):
|
||||||
# Neutron network UUID and not anymore with the OVN datapath UUID.
|
# If the port is not bound to any chassis it is not relevant
|
||||||
return [r for r in rows
|
if not port.chassis:
|
||||||
if (r.mac and (
|
return False
|
||||||
str(r.datapath.uuid) == network or
|
|
||||||
utils.get_network_name_from_datapath(
|
# TODO(dalvarez): Remove the comparison to port.datapath.uuid in Y
|
||||||
r.datapath) == network)) and
|
# cycle when we are sure that all namespaces will be created with
|
||||||
ip_address in r.mac[0].split(' ')]
|
# the Neutron network UUID and not anymore with the OVN datapath
|
||||||
|
# UUID.
|
||||||
|
is_in_network = lambda port: (
|
||||||
|
str(port.datapath.uuid) == network or
|
||||||
|
utils.get_network_name_from_datapath(port.datapath) == network)
|
||||||
|
|
||||||
|
return port.mac and is_in_network(port) and (
|
||||||
|
ip_address in port.mac[0].split(' '))
|
||||||
|
|
||||||
|
return [r for r in rows if check_net_and_ip(r)]
|
||||||
|
|
||||||
def set_port_cidrs(self, name, cidrs):
|
def set_port_cidrs(self, name, cidrs):
|
||||||
# TODO(twilson) add if_exists to db commands
|
# TODO(twilson) add if_exists to db commands
|
||||||
|
|
|
@ -27,7 +27,8 @@ from neutron.conf.agent.metadata import config as meta_conf
|
||||||
from neutron.conf.agent.ovn.metadata import config as ovn_meta_conf
|
from neutron.conf.agent.ovn.metadata import config as ovn_meta_conf
|
||||||
from neutron.tests import base
|
from neutron.tests import base
|
||||||
|
|
||||||
OvnPortInfo = collections.namedtuple('OvnPortInfo', 'external_ids')
|
OvnPortInfo = collections.namedtuple(
|
||||||
|
'OvnPortInfo', ['external_ids', 'chassis'])
|
||||||
|
|
||||||
|
|
||||||
class ConfFixture(config_fixture.Config):
|
class ConfFixture(config_fixture.Config):
|
||||||
|
@ -54,7 +55,7 @@ class TestMetadataProxyHandler(base.BaseTestCase):
|
||||||
self.useFixture(self.fake_conf_fixture)
|
self.useFixture(self.fake_conf_fixture)
|
||||||
self.log_p = mock.patch.object(agent, 'LOG')
|
self.log_p = mock.patch.object(agent, 'LOG')
|
||||||
self.log = self.log_p.start()
|
self.log = self.log_p.start()
|
||||||
self.handler = agent.MetadataProxyHandler(self.fake_conf)
|
self.handler = agent.MetadataProxyHandler(self.fake_conf, 'chassis1')
|
||||||
self.handler.sb_idl = mock.Mock()
|
self.handler.sb_idl = mock.Mock()
|
||||||
|
|
||||||
def test_call(self):
|
def test_call(self):
|
||||||
|
@ -113,7 +114,8 @@ class TestMetadataProxyHandler(base.BaseTestCase):
|
||||||
|
|
||||||
ovn_port = OvnPortInfo(
|
ovn_port = OvnPortInfo(
|
||||||
external_ids={'neutron:device_id': 'device_id',
|
external_ids={'neutron:device_id': 'device_id',
|
||||||
'neutron:project_id': 'project_id'})
|
'neutron:project_id': 'project_id'},
|
||||||
|
chassis=['chassis1'])
|
||||||
ports = [[ovn_port]]
|
ports = [[ovn_port]]
|
||||||
|
|
||||||
self.assertEqual(
|
self.assertEqual(
|
||||||
|
@ -220,14 +222,14 @@ class TestUnixDomainMetadataProxy(base.BaseTestCase):
|
||||||
|
|
||||||
@mock.patch.object(fileutils, 'ensure_tree')
|
@mock.patch.object(fileutils, 'ensure_tree')
|
||||||
def test_init_doesnot_exists(self, ensure_dir):
|
def test_init_doesnot_exists(self, ensure_dir):
|
||||||
agent.UnixDomainMetadataProxy(mock.Mock())
|
agent.UnixDomainMetadataProxy(mock.Mock(), 'chassis1')
|
||||||
ensure_dir.assert_called_once_with('/the', mode=0o755)
|
ensure_dir.assert_called_once_with('/the', mode=0o755)
|
||||||
|
|
||||||
def test_init_exists(self):
|
def test_init_exists(self):
|
||||||
with mock.patch('os.path.isdir') as isdir:
|
with mock.patch('os.path.isdir') as isdir:
|
||||||
with mock.patch('os.unlink') as unlink:
|
with mock.patch('os.unlink') as unlink:
|
||||||
isdir.return_value = True
|
isdir.return_value = True
|
||||||
agent.UnixDomainMetadataProxy(mock.Mock())
|
agent.UnixDomainMetadataProxy(mock.Mock(), 'chassis1')
|
||||||
unlink.assert_called_once_with('/the/path')
|
unlink.assert_called_once_with('/the/path')
|
||||||
|
|
||||||
def test_init_exists_unlink_no_file(self):
|
def test_init_exists_unlink_no_file(self):
|
||||||
|
@ -238,7 +240,7 @@ class TestUnixDomainMetadataProxy(base.BaseTestCase):
|
||||||
exists.return_value = False
|
exists.return_value = False
|
||||||
unlink.side_effect = OSError
|
unlink.side_effect = OSError
|
||||||
|
|
||||||
agent.UnixDomainMetadataProxy(mock.Mock())
|
agent.UnixDomainMetadataProxy(mock.Mock(), 'chassis1')
|
||||||
unlink.assert_called_once_with('/the/path')
|
unlink.assert_called_once_with('/the/path')
|
||||||
|
|
||||||
def test_init_exists_unlink_fails_file_still_exists(self):
|
def test_init_exists_unlink_fails_file_still_exists(self):
|
||||||
|
@ -250,14 +252,14 @@ class TestUnixDomainMetadataProxy(base.BaseTestCase):
|
||||||
unlink.side_effect = OSError
|
unlink.side_effect = OSError
|
||||||
|
|
||||||
with testtools.ExpectedException(OSError):
|
with testtools.ExpectedException(OSError):
|
||||||
agent.UnixDomainMetadataProxy(mock.Mock())
|
agent.UnixDomainMetadataProxy(mock.Mock(), 'chassis1')
|
||||||
unlink.assert_called_once_with('/the/path')
|
unlink.assert_called_once_with('/the/path')
|
||||||
|
|
||||||
@mock.patch.object(agent, 'MetadataProxyHandler')
|
@mock.patch.object(agent, 'MetadataProxyHandler')
|
||||||
@mock.patch.object(agent_utils, 'UnixDomainWSGIServer')
|
@mock.patch.object(agent_utils, 'UnixDomainWSGIServer')
|
||||||
@mock.patch.object(fileutils, 'ensure_tree')
|
@mock.patch.object(fileutils, 'ensure_tree')
|
||||||
def test_run(self, ensure_dir, server, handler):
|
def test_run(self, ensure_dir, server, handler):
|
||||||
p = agent.UnixDomainMetadataProxy(self.cfg.CONF)
|
p = agent.UnixDomainMetadataProxy(self.cfg.CONF, 'chassis1')
|
||||||
p.run()
|
p.run()
|
||||||
|
|
||||||
ensure_dir.assert_called_once_with('/the', mode=0o755)
|
ensure_dir.assert_called_once_with('/the', mode=0o755)
|
||||||
|
|
Loading…
Reference in New Issue