Merge "Improve openvswitch and linuxbridge agents' parsing of mappings"

quantum/common/utils.py

@@ -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

quantum/plugins/linuxbridge/agent/linuxbridge_quantum_agent.py

@@ -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 = {}
+    try:
-    for mapping in cfg.CONF.LINUX_BRIDGE.physical_interface_mappings:
+        interface_mappings = q_utils.parse_mappings(
-        try:
+            cfg.CONF.LINUX_BRIDGE.physical_interface_mappings)
-            physical_network, physical_interface = mapping.split(':')
+    except ValueError as e:
-            interface_mappings[physical_network] = physical_interface
+        LOG.error(_("Parsing physical_interface_mappings failed: %s."
-            LOG.debug("physical network %s mapped to physical interface %s" %
+                    " Agent terminated!"), e)
-                      (physical_network, physical_interface))
+        sys.exit(1)
-        except ValueError as ex:
+    LOG.info(_("Interface mappings: %s") % interface_mappings)
-            LOG.error("Invalid physical interface mapping: %s - %s. "
-                      "Agent terminated!" %
-                      (mapping, ex))
-            sys.exit(1)
 
     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()

quantum/plugins/openvswitch/agent/ovs_quantum_agent.py

@@ -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,

quantum/tests/unit/openvswitch/test_ovs_quantum_agent.py

@@ -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):

quantum/tests/unit/test_common_utils.py

@@ -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(['']), {})