Improve openvswitch and linuxbridge agents' parsing of mappings
Created generic quantum.utils.parse_mappings function that detects duplicate key or value strings, and changed openvswitch and linuxbridge agents to use this to parse their mappings from physical networks to bridges or interfaces. Fixes bug 1067669. Also fixed some typos in comments. Change-Id: I342eaeeb6ff4c6e25d57d631f250faac082011b8
This commit is contained in:
parent
92ee709521
commit
5109f13abf
@ -134,3 +134,34 @@ def subprocess_popen(args, stdin=None, stdout=None, stderr=None, shell=False,
|
||||
return subprocess.Popen(args, shell=shell, stdin=stdin, stdout=stdout,
|
||||
stderr=stderr, preexec_fn=_subprocess_setup,
|
||||
env=env)
|
||||
|
||||
|
||||
def parse_mappings(mapping_list, unique_values=True):
|
||||
"""Parse a list of of mapping strings into a dictionary.
|
||||
|
||||
:param mapping_list: a list of strings of the form '<key>:<value>'
|
||||
:param unique_values: values must be unique if True
|
||||
:returns: a dict mapping keys to values
|
||||
"""
|
||||
mappings = {}
|
||||
for mapping in mapping_list:
|
||||
mapping = mapping.strip()
|
||||
if not mapping:
|
||||
continue
|
||||
split_result = mapping.split(':')
|
||||
if len(split_result) != 2:
|
||||
raise ValueError(_("Invalid mapping: '%s'") % mapping)
|
||||
key = split_result[0].strip()
|
||||
if not key:
|
||||
raise ValueError(_("Missing key in mapping: '%s'") % mapping)
|
||||
value = split_result[1].strip()
|
||||
if not value:
|
||||
raise ValueError(_("Missing value in mapping: '%s'") % mapping)
|
||||
if key in mappings:
|
||||
raise ValueError(_("Key %s in mapping: '%s' not unique") %
|
||||
(key, mapping))
|
||||
if unique_values and value in mappings.itervalues():
|
||||
raise ValueError(_("Value %s in mapping: '%s' not unique") %
|
||||
(value, mapping))
|
||||
mappings[key] = value
|
||||
return mappings
|
||||
|
@ -35,6 +35,7 @@ from quantum.agent import rpc as agent_rpc
|
||||
from quantum.common import config as logging_config
|
||||
from quantum.common import constants
|
||||
from quantum.common import topics
|
||||
from quantum.common import utils as q_utils
|
||||
from quantum.openstack.common import cfg
|
||||
from quantum.openstack.common import context
|
||||
from quantum.openstack.common import log as logging
|
||||
@ -594,27 +595,24 @@ def main():
|
||||
# (TODO) gary - swap with common logging
|
||||
logging_config.setup_logging(cfg.CONF)
|
||||
|
||||
interface_mappings = {}
|
||||
for mapping in cfg.CONF.LINUX_BRIDGE.physical_interface_mappings:
|
||||
try:
|
||||
physical_network, physical_interface = mapping.split(':')
|
||||
interface_mappings[physical_network] = physical_interface
|
||||
LOG.debug("physical network %s mapped to physical interface %s" %
|
||||
(physical_network, physical_interface))
|
||||
except ValueError as ex:
|
||||
LOG.error("Invalid physical interface mapping: %s - %s. "
|
||||
"Agent terminated!" %
|
||||
(mapping, ex))
|
||||
interface_mappings = q_utils.parse_mappings(
|
||||
cfg.CONF.LINUX_BRIDGE.physical_interface_mappings)
|
||||
except ValueError as e:
|
||||
LOG.error(_("Parsing physical_interface_mappings failed: %s."
|
||||
" Agent terminated!"), e)
|
||||
sys.exit(1)
|
||||
LOG.info(_("Interface mappings: %s") % interface_mappings)
|
||||
|
||||
polling_interval = cfg.CONF.AGENT.polling_interval
|
||||
root_helper = cfg.CONF.AGENT.root_helper
|
||||
plugin = LinuxBridgeQuantumAgentRPC(interface_mappings,
|
||||
polling_interval,
|
||||
root_helper)
|
||||
LOG.info("Agent initialized successfully, now running... ")
|
||||
LOG.info(_("Agent initialized successfully, now running... "))
|
||||
plugin.daemon_loop()
|
||||
sys.exit(0)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
main()
|
||||
|
@ -32,6 +32,7 @@ from quantum.agent import rpc as agent_rpc
|
||||
from quantum.common import config as logging_config
|
||||
from quantum.common import constants as q_const
|
||||
from quantum.common import topics
|
||||
from quantum.common import utils as q_utils
|
||||
from quantum.openstack.common import cfg
|
||||
from quantum.openstack.common import context
|
||||
from quantum.openstack.common import log as logging
|
||||
@ -140,7 +141,7 @@ class OVSQuantumAgent(object):
|
||||
:param integ_br: name of the integration bridge.
|
||||
:param tun_br: name of the tunnel bridge.
|
||||
:param local_ip: local IP address of this hypervisor.
|
||||
:param bridge_mappings: mappings from phyiscal interface to bridge.
|
||||
:param bridge_mappings: mappings from physical network name to bridge.
|
||||
:param root_helper: utility to use when running shell cmds.
|
||||
:param polling_interval: interval (secs) to poll DB.
|
||||
:param enable_tunneling: if True enable GRE networks.
|
||||
@ -459,7 +460,7 @@ class OVSQuantumAgent(object):
|
||||
def setup_physical_bridges(self, bridge_mappings):
|
||||
'''Setup the physical network bridges.
|
||||
|
||||
Creates phyiscal network bridges and links them to the
|
||||
Creates physical network bridges and links them to the
|
||||
integration bridge using veths.
|
||||
|
||||
:param bridge_mappings: map physical network names to bridge names.'''
|
||||
@ -641,33 +642,17 @@ class OVSQuantumAgent(object):
|
||||
self.rpc_loop()
|
||||
|
||||
|
||||
def parse_bridge_mappings(bridge_mapping_list):
|
||||
"""Parse a list of physical network to bridge mappings.
|
||||
|
||||
:param bridge_mapping_list: a list of strings of the form
|
||||
'<physical network>:<bridge>'
|
||||
:returns: a dict mapping physical networks to bridges
|
||||
"""
|
||||
bridge_mappings = {}
|
||||
for mapping in bridge_mapping_list:
|
||||
mapping = mapping.strip()
|
||||
if not mapping:
|
||||
continue
|
||||
split_result = [x.strip() for x in mapping.split(':', 1) if x.strip()]
|
||||
if len(split_result) != 2:
|
||||
raise ValueError('Invalid bridge mapping: %s.' % mapping)
|
||||
physical_network, bridge = split_result
|
||||
bridge_mappings[physical_network] = bridge
|
||||
return bridge_mappings
|
||||
|
||||
|
||||
def create_agent_config_map(config):
|
||||
"""Create a map of agent config parameters.
|
||||
|
||||
:param config: an instance of cfg.CONF
|
||||
:returns: a map of agent configuration parameters
|
||||
"""
|
||||
bridge_mappings = parse_bridge_mappings(config.OVS.bridge_mappings)
|
||||
try:
|
||||
bridge_mappings = q_utils.parse_mappings(config.OVS.bridge_mappings)
|
||||
except ValueError as e:
|
||||
raise ValueError(_("Parsing bridge_mappings failed: %s.") % e)
|
||||
|
||||
kwargs = dict(
|
||||
integ_br=config.OVS.integration_bridge,
|
||||
tun_br=config.OVS.tunnel_bridge,
|
||||
|
@ -22,30 +22,6 @@ from quantum.plugins.openvswitch.agent import ovs_quantum_agent
|
||||
from quantum.plugins.openvswitch.common import config
|
||||
|
||||
|
||||
class TestParseBridgeMappings(unittest.TestCase):
|
||||
|
||||
def parse(self, bridge_mapping_list):
|
||||
return ovs_quantum_agent.parse_bridge_mappings(bridge_mapping_list)
|
||||
|
||||
def test_parse_bridge_mappings_fails_for_missing_separator(self):
|
||||
with self.assertRaises(ValueError):
|
||||
self.parse(['net'])
|
||||
|
||||
def test_parse_bridge_mappings_fails_for_missing_value(self):
|
||||
with self.assertRaises(ValueError):
|
||||
self.parse(['net:'])
|
||||
|
||||
def test_parse_bridge_mappings_succeeds_for_one_mapping(self):
|
||||
self.assertEqual(self.parse(['net:br']), {'net': 'br'})
|
||||
|
||||
def test_parse_bridge_mappings_succeeds_for_n_mappings(self):
|
||||
self.assertEqual(self.parse(['net:br', 'net1:br1']),
|
||||
{'net': 'br', 'net1': 'br1'})
|
||||
|
||||
def test_parse_bridge_mappings_succeeds_for_no_mappings(self):
|
||||
self.assertEqual(self.parse(['']), {})
|
||||
|
||||
|
||||
class CreateAgentConfigMap(unittest.TestCase):
|
||||
|
||||
def test_create_agent_config_map_succeeds(self):
|
||||
|
60
quantum/tests/unit/test_common_utils.py
Normal file
60
quantum/tests/unit/test_common_utils.py
Normal file
@ -0,0 +1,60 @@
|
||||
# Copyright (c) 2012 OpenStack, LLC.
|
||||
#
|
||||
# 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 unittest2 as unittest
|
||||
|
||||
from quantum.common import utils
|
||||
|
||||
|
||||
class TestParseMappings(unittest.TestCase):
|
||||
def parse(self, mapping_list, unique_values=True):
|
||||
return utils.parse_mappings(mapping_list, unique_values)
|
||||
|
||||
def test_parse_mappings_fails_for_missing_separator(self):
|
||||
with self.assertRaises(ValueError):
|
||||
self.parse(['key'])
|
||||
|
||||
def test_parse_mappings_fails_for_missing_key(self):
|
||||
with self.assertRaises(ValueError):
|
||||
self.parse([':val'])
|
||||
|
||||
def test_parse_mappings_fails_for_missing_value(self):
|
||||
with self.assertRaises(ValueError):
|
||||
self.parse(['key:'])
|
||||
|
||||
def test_parse_mappings_fails_for_extra_separator(self):
|
||||
with self.assertRaises(ValueError):
|
||||
self.parse(['key:val:junk'])
|
||||
|
||||
def test_parse_mappings_fails_for_duplicate_key(self):
|
||||
with self.assertRaises(ValueError):
|
||||
self.parse(['key:val1', 'key:val2'])
|
||||
|
||||
def test_parse_mappings_fails_for_duplicate_value(self):
|
||||
with self.assertRaises(ValueError):
|
||||
self.parse(['key1:val', 'key2:val'])
|
||||
|
||||
def test_parse_mappings_succeeds_for_one_mapping(self):
|
||||
self.assertEqual(self.parse(['key:val']), {'key': 'val'})
|
||||
|
||||
def test_parse_mappings_succeeds_for_n_mappings(self):
|
||||
self.assertEqual(self.parse(['key1:val1', 'key2:val2']),
|
||||
{'key1': 'val1', 'key2': 'val2'})
|
||||
|
||||
def test_parse_mappings_succeeds_for_duplicate_value(self):
|
||||
self.assertEqual(self.parse(['key1:val', 'key2:val'], False),
|
||||
{'key1': 'val', 'key2': 'val'})
|
||||
|
||||
def test_parse_mappings_succeeds_for_no_mappings(self):
|
||||
self.assertEqual(self.parse(['']), {})
|
Loading…
Reference in New Issue
Block a user