Fix linuxbridge RPC message format

The linuxbridge, openvswitch, and hyperv plugins all use the same
basic RPC interface between their plugins and L2 agents. But the
attributes describing a virtual network passed from the plugin to the
agent over this interface differed for historical reasons. The
openvswitch and hyperv plugins each pass network_type,
physical_network, and segmentation_id attributes, whereas the
linuxbridge plugin previously passed vlan_id and physical_network
attributes, using special vlan_id values to indicate flat or local
network types.

This patch changes the linuxbridge plugin to pass network_type and
segmentation_id attributes instead of the vlan_id attribute, bringing
its message formats into sync with the other plugins. RPC
compatibility is required for blueprint modular-l2 so that the ml2
plugin can work with all three existing types of L2 agent. This RPC
message format change is also required for blueprint
vxlan-linuxbridge.

Unlike the vxlan-linuxbridge patch on which it is based (see
https://review.openstack.org/#/c/26516/), this patch does not bump the
linuxbridge RPC version number, as the ml2 plugin will require all
three L2 agents to use the same RPC version. Instead, the updated
linuxbridge agent maintains compatibility with old linuxbridge plugins
by accepting either the old or new attributes. There is also a
configuration option, currently turned on by default, to enable the
updated linuxbridge plugin to pass the vlan_id attribute expected by
old linuxbridge agents along with the new attributes. These message
format compatibility mechanisms are intended to aid during upgrades,
and can eventually be removed.

Change-Id: I7cc1c9f96b09db6bab2c7d9f2b30b79fa4dab919

etc/quantum/plugins/linuxbridge/linuxbridge_conf.ini

@@ -58,6 +58,12 @@ reconnect_interval = 2
 # Agent's polling interval in seconds
 polling_interval = 2
 
+# (BoolOpt) Enable server RPC compatibility with old (pre-havana)
+# agents.
+#
+# Default: rpc_support_old_agents = True
+# Example: rpc_support_old_agents = False
+
 [SECURITYGROUP]
 # Firewall driver for realizing quantum security group function
 firewall_driver = quantum.agent.linux.iptables_firewall.IptablesFirewallDriver

quantum/plugins/linuxbridge/agent/linuxbridge_quantum_agent.py

@@ -253,21 +253,26 @@ class LinuxBridgeManager:
         return bridge_name
 
     def ensure_physical_in_bridge(self, network_id,
+                                  network_type,
                                   physical_network,
-                                  vlan_id):
+                                  segmentation_id):
         physical_interface = self.interface_mappings.get(physical_network)
         if not physical_interface:
             LOG.error(_("No mapping for physical network %s"),
                       physical_network)
             return
-        if int(vlan_id) == lconst.FLAT_VLAN_ID:
+        if network_type == lconst.TYPE_FLAT:
             return self.ensure_flat_bridge(network_id, physical_interface)
-        else:
+        elif network_type == lconst.TYPE_VLAN:
             return self.ensure_vlan_bridge(network_id, physical_interface,
-                                           vlan_id)
+                                           segmentation_id)
+        else:
+            LOG.error(_("Unknown network_type %(network_type)s for network "
+                        "%(network_id)s."), {network_type: network_type,
+                                             network_id: network_id})
 
-    def add_tap_interface(self, network_id, physical_network, vlan_id,
+    def add_tap_interface(self, network_id, network_type, physical_network,
-                          tap_device_name):
+                          segmentation_id, tap_device_name):
         """Add tap interface.
 
         If a VIF has been plugged into a network, this function will
@@ -279,11 +284,12 @@ class LinuxBridgeManager:
             return False
 
         bridge_name = self.get_bridge_name(network_id)
-        if int(vlan_id) == lconst.LOCAL_VLAN_ID:
+        if network_type == lconst.TYPE_LOCAL:
             self.ensure_local_bridge(network_id)
         elif not self.ensure_physical_in_bridge(network_id,
+                                                network_type,
                                                 physical_network,
-                                                vlan_id):
+                                                segmentation_id):
             return False
 
         # Check if device needs to be added to bridge
@@ -305,11 +311,11 @@ class LinuxBridgeManager:
             LOG.debug(msg)
         return True
 
-    def add_interface(self, network_id, physical_network, vlan_id,
+    def add_interface(self, network_id, network_type, physical_network,
-                      port_id):
+                      segmentation_id, port_id):
         tap_device_name = self.get_tap_device_name(port_id)
-        return self.add_tap_interface(network_id,
+        return self.add_tap_interface(network_id, network_type,
-                                      physical_network, vlan_id,
+                                      physical_network, segmentation_id,
                                       tap_device_name)
 
     def delete_vlan_bridge(self, bridge_name):
@@ -433,12 +439,20 @@ class LinuxBridgeRpcCallbacks(sg_rpc.SecurityGroupAgentRpcCallbackMixin):
             self.sg_agent.refresh_firewall()
 
         if port['admin_state_up']:
+            network_type = kwargs.get('network_type')
+            if network_type:
+                segmentation_id = kwargs.get('segmentation_id')
+            else:
+                # compatibility with pre-Havana RPC vlan_id encoding
                 vlan_id = kwargs.get('vlan_id')
+                (network_type,
+                 segmentation_id) = lconst.interpret_vlan_id(vlan_id)
             physical_network = kwargs.get('physical_network')
             # create the networking for the port
             self.agent.br_mgr.add_interface(port['network_id'],
+                                            network_type,
                                             physical_network,
-                                            vlan_id,
+                                            segmentation_id,
                                             port['id'])
             # update plugin about port status
             self.agent.plugin_rpc.update_device_up(self.context,
@@ -569,9 +583,18 @@ class LinuxBridgeQuantumAgentRPC(sg_rpc.SecurityGroupAgentRpcMixin):
                          {'device': device, 'details': details})
                 if details['admin_state_up']:
                     # create the networking for the port
+                    network_type = details.get('network_type')
+                    if network_type:
+                        segmentation_id = details.get('segmentation_id')
+                    else:
+                        # compatibility with pre-Havana RPC vlan_id encoding
+                        vlan_id = details.get('vlan_id')
+                        (network_type,
+                         segmentation_id) = lconst.interpret_vlan_id(vlan_id)
                     self.br_mgr.add_interface(details['network_id'],
+                                              network_type,
                                               details['physical_network'],
-                                              details['vlan_id'],
+                                              segmentation_id,
                                               details['port_id'])
                 else:
                     self.remove_port_binding(details['network_id'],

quantum/plugins/linuxbridge/common/config.py

@@ -46,6 +46,9 @@ agent_opts = [
     cfg.IntOpt('polling_interval', default=2,
                help=_("The number of seconds the agent will wait between "
                       "polling for local device changes.")),
+    #TODO(rkukura): Change default to False before havana rc1
+    cfg.BoolOpt('rpc_support_old_agents', default=True,
+                help=_("Enable server RPC compatibility with old agents")),
 ]
 
 

quantum/plugins/linuxbridge/common/constants.py

@@ -25,3 +25,16 @@ TYPE_FLAT = 'flat'
 TYPE_VLAN = 'vlan'
 TYPE_LOCAL = 'local'
 TYPE_NONE = 'none'
+
+
+# TODO(rkukura): Eventually remove this function, which provides
+# temporary backward compatibility with pre-Havana RPC and DB vlan_id
+# encoding.
+def interpret_vlan_id(vlan_id):
+    """Return (network_type, segmentation_id) tuple for encoded vlan_id."""
+    if vlan_id == LOCAL_VLAN_ID:
+        return (TYPE_LOCAL, None)
+    elif vlan_id == FLAT_VLAN_ID:
+        return (TYPE_FLAT, None)
+    else:
+        return (TYPE_VLAN, vlan_id)

quantum/plugins/linuxbridge/lb_quantum_plugin.py

@@ -87,12 +87,17 @@ class LinuxBridgeRpcCallbacks(dhcp_rpc_base.DhcpRpcCallbackMixin,
         if port:
             binding = db.get_network_binding(db_api.get_session(),
                                              port['network_id'])
+            (network_type,
+             segmentation_id) = constants.interpret_vlan_id(binding.vlan_id)
             entry = {'device': device,
+                     'network_type': network_type,
                      'physical_network': binding.physical_network,
-                     'vlan_id': binding.vlan_id,
+                     'segmentation_id': segmentation_id,
                      'network_id': port['network_id'],
                      'port_id': port['id'],
                      'admin_state_up': port['admin_state_up']}
+            if cfg.CONF.AGENT.rpc_support_old_agents:
+                entry['vlan_id'] = binding.vlan_id
             new_status = (q_const.PORT_STATUS_ACTIVE if port['admin_state_up']
                           else q_const.PORT_STATUS_DOWN)
             if port['status'] != new_status:
@@ -166,11 +171,15 @@ class AgentNotifierApi(proxy.RpcProxy,
                          topic=self.topic_network_delete)
 
     def port_update(self, context, port, physical_network, vlan_id):
-        self.fanout_cast(context,
+        network_type, segmentation_id = constants.interpret_vlan_id(vlan_id)
-                         self.make_msg('port_update',
+        kwargs = {'port': port,
-                                       port=port,
+                  'network_type': network_type,
-                                       physical_network=physical_network,
+                  'physical_network': physical_network,
-                                       vlan_id=vlan_id),
+                  'segmentation_id': segmentation_id}
+        if cfg.CONF.AGENT.rpc_support_old_agents:
+            kwargs['vlan_id'] = vlan_id
+        msg = self.make_msg('port_update', **kwargs)
+        self.fanout_cast(context, msg,
                          topic=self.topic_port_update)
 
 

quantum/tests/unit/linuxbridge/test_defaults.py

@@ -24,6 +24,8 @@ class ConfigurationTest(base.BaseTestCase):
     def test_defaults(self):
         self.assertEqual(2,
                          cfg.CONF.AGENT.polling_interval)
+        self.assertEqual(True,
+                         cfg.CONF.AGENT.rpc_support_old_agents)
         self.assertEqual('sudo',
                          cfg.CONF.AGENT.root_helper)
         self.assertEqual('local',

quantum/tests/unit/linuxbridge/test_lb_quantum_agent.py

@@ -41,6 +41,7 @@ class TestLinuxBridge(base.BaseTestCase):
 
     def test_ensure_physical_in_bridge_invalid(self):
         result = self.linux_bridge.ensure_physical_in_bridge('network_id',
+                                                             lconst.TYPE_VLAN,
                                                              'physnetx',
                                                              7)
         self.assertFalse(result)
@@ -49,14 +50,14 @@ class TestLinuxBridge(base.BaseTestCase):
         with mock.patch.object(self.linux_bridge,
                                'ensure_flat_bridge') as flat_bridge_func:
             self.linux_bridge.ensure_physical_in_bridge(
-                'network_id', 'physnet1', lconst.FLAT_VLAN_ID)
+                'network_id', lconst.TYPE_FLAT, 'physnet1', None)
         self.assertTrue(flat_bridge_func.called)
 
     def test_ensure_physical_in_bridge_vlan(self):
         with mock.patch.object(self.linux_bridge,
                                'ensure_vlan_bridge') as vlan_bridge_func:
             self.linux_bridge.ensure_physical_in_bridge(
-                'network_id', 'physnet1', 7)
+                'network_id', lconst.TYPE_VLAN, 'physnet1', 7)
         self.assertTrue(vlan_bridge_func.called)
 
 
@@ -338,16 +339,19 @@ class TestLinuxBridgeManager(base.BaseTestCase):
 
     def test_ensure_physical_in_bridge(self):
         self.assertFalse(
-            self.lbm.ensure_physical_in_bridge("123", "phys", "1")
+            self.lbm.ensure_physical_in_bridge("123", lconst.TYPE_VLAN,
+                                               "phys", "1")
         )
         with mock.patch.object(self.lbm, "ensure_flat_bridge") as flbr_fn:
             self.assertTrue(
-                self.lbm.ensure_physical_in_bridge("123", "physnet1", "-1")
+                self.lbm.ensure_physical_in_bridge("123", lconst.TYPE_FLAT,
+                                                   "physnet1", None)
             )
             self.assertTrue(flbr_fn.called)
         with mock.patch.object(self.lbm, "ensure_vlan_bridge") as vlbr_fn:
             self.assertTrue(
-                self.lbm.ensure_physical_in_bridge("123", "physnet1", "1")
+                self.lbm.ensure_physical_in_bridge("123", lconst.TYPE_VLAN,
+                                                   "physnet1", "1")
             )
             self.assertTrue(vlbr_fn.called)
 
@@ -355,7 +359,8 @@ class TestLinuxBridgeManager(base.BaseTestCase):
         with mock.patch.object(self.lbm, "device_exists") as de_fn:
             de_fn.return_value = False
             self.assertFalse(
-                self.lbm.add_tap_interface("123", "physnet1", "1", "tap1")
+                self.lbm.add_tap_interface("123", lconst.TYPE_VLAN,
+                                           "physnet1", "1", "tap1")
             )
 
             de_fn.return_value = True
@@ -366,25 +371,33 @@ class TestLinuxBridgeManager(base.BaseTestCase):
             ) as (en_fn, exec_fn, get_br):
                 exec_fn.return_value = False
                 get_br.return_value = True
-                self.assertTrue(self.lbm.add_tap_interface("123", "physnet1",
+                self.assertTrue(self.lbm.add_tap_interface("123",
-                                                           "-2", "tap1"))
+                                                           lconst.TYPE_LOCAL,
+                                                           "physnet1", None,
+                                                           "tap1"))
                 en_fn.assert_called_with("123")
 
                 get_br.return_value = False
                 exec_fn.return_value = True
-                self.assertFalse(self.lbm.add_tap_interface("123", "physnet1",
+                self.assertFalse(self.lbm.add_tap_interface("123",
-                                                            "-2", "tap1"))
+                                                            lconst.TYPE_LOCAL,
+                                                            "physnet1", None,
+                                                            "tap1"))
 
             with mock.patch.object(self.lbm,
                                    "ensure_physical_in_bridge") as ens_fn:
                 ens_fn.return_value = False
-                self.assertFalse(self.lbm.add_tap_interface("123", "physnet1",
+                self.assertFalse(self.lbm.add_tap_interface("123",
-                                                            "1", "tap1"))
+                                                            lconst.TYPE_VLAN,
+                                                            "physnet1", "1",
+                                                            "tap1"))
 
     def test_add_interface(self):
         with mock.patch.object(self.lbm, "add_tap_interface") as add_tap:
-            self.lbm.add_interface("123", "physnet-1", "1", "234")
+            self.lbm.add_interface("123", lconst.TYPE_VLAN, "physnet-1",
-            add_tap.assert_called_with("123", "physnet-1", "1", "tap234")
+                                   "1", "234")
+            add_tap.assert_called_with("123", lconst.TYPE_VLAN, "physnet-1",
+                                       "1", "tap234")
 
     def test_delete_vlan_bridge(self):
         with contextlib.nested(
@@ -508,9 +521,47 @@ class TestLinuxBridgeRpcCallbacks(base.BaseTestCase):
             self.lb_rpc.port_update("unused_context", port=port,
                                     vlan_id="1", physical_network="physnet1")
             self.assertFalse(reffw_fn.called)
-            addif_fn.assert_called_with(port["network_id"],
+            addif_fn.assert_called_with(port["network_id"], lconst.TYPE_VLAN,
                                         "physnet1", "1", port["id"])
 
+            self.lb_rpc.port_update("unused_context", port=port,
+                                    network_type=lconst.TYPE_VLAN,
+                                    segmentation_id="2",
+                                    physical_network="physnet1")
+            self.assertFalse(reffw_fn.called)
+            addif_fn.assert_called_with(port["network_id"], lconst.TYPE_VLAN,
+                                        "physnet1", "2", port["id"])
+
+            self.lb_rpc.port_update("unused_context", port=port,
+                                    vlan_id=lconst.FLAT_VLAN_ID,
+                                    physical_network="physnet1")
+            self.assertFalse(reffw_fn.called)
+            addif_fn.assert_called_with(port["network_id"], lconst.TYPE_FLAT,
+                                        "physnet1", None, port["id"])
+
+            self.lb_rpc.port_update("unused_context", port=port,
+                                    network_type=lconst.TYPE_FLAT,
+                                    segmentation_id=None,
+                                    physical_network="physnet1")
+            self.assertFalse(reffw_fn.called)
+            addif_fn.assert_called_with(port["network_id"], lconst.TYPE_FLAT,
+                                        "physnet1", None, port["id"])
+
+            self.lb_rpc.port_update("unused_context", port=port,
+                                    vlan_id=lconst.LOCAL_VLAN_ID,
+                                    physical_network=None)
+            self.assertFalse(reffw_fn.called)
+            addif_fn.assert_called_with(port["network_id"], lconst.TYPE_LOCAL,
+                                        None, None, port["id"])
+
+            self.lb_rpc.port_update("unused_context", port=port,
+                                    network_type=lconst.TYPE_LOCAL,
+                                    segmentation_id=None,
+                                    physical_network=None)
+            self.assertFalse(reffw_fn.called)
+            addif_fn.assert_called_with(port["network_id"], lconst.TYPE_LOCAL,
+                                        None, None, port["id"])
+
             port["admin_state_up"] = False
             port["security_groups"] = True
             getbr_fn.return_value = "br0"

quantum/tests/unit/linuxbridge/test_rpcapi.py

@@ -18,6 +18,7 @@
 Unit Tests for linuxbridge rpc
 """
 
+from oslo.config import cfg
 import stubout
 
 from quantum.agent import rpc as agent_rpc
@@ -29,10 +30,11 @@ from quantum.tests import base
 
 
 class rpcApiTestCase(base.BaseTestCase):
-
+    def _test_lb_api(self, rpcapi, topic, method, rpc_method,
-    def _test_lb_api(self, rpcapi, topic, method, rpc_method, **kwargs):
+                     expected_msg=None, **kwargs):
         ctxt = context.RequestContext('fake_user', 'fake_project')
         expected_retval = 'foo' if method == 'call' else None
+        if not expected_msg:
             expected_msg = rpcapi.make_msg(method, **kwargs)
         expected_msg['version'] = rpcapi.BASE_RPC_API_VERSION
         if rpc_method == 'cast' and method == 'run_instance':
@@ -52,11 +54,11 @@ class rpcApiTestCase(base.BaseTestCase):
 
         retval = getattr(rpcapi, method)(ctxt, **kwargs)
 
-        self.assertEqual(retval, expected_retval)
+        self.assertEqual(expected_retval, retval)
         expected_args = [ctxt, topic, expected_msg]
 
         for arg, expected_arg in zip(self.fake_args, expected_args):
-            self.assertEqual(arg, expected_arg)
+            self.assertEqual(expected_arg, arg)
 
     def test_delete_network(self):
         rpcapi = plb.AgentNotifierApi(topics.AGENT)
@@ -68,12 +70,38 @@ class rpcApiTestCase(base.BaseTestCase):
                           network_id='fake_request_spec')
 
     def test_port_update(self):
+        cfg.CONF.set_override('rpc_support_old_agents', False, 'AGENT')
         rpcapi = plb.AgentNotifierApi(topics.AGENT)
+        expected_msg = rpcapi.make_msg('port_update',
+                                       port='fake_port',
+                                       network_type='vlan',
+                                       physical_network='fake_net',
+                                       segmentation_id='fake_vlan_id')
         self._test_lb_api(rpcapi,
                           topics.get_topic_name(topics.AGENT,
                                                 topics.PORT,
                                                 topics.UPDATE),
                           'port_update', rpc_method='fanout_cast',
+                          expected_msg=expected_msg,
+                          port='fake_port',
+                          physical_network='fake_net',
+                          vlan_id='fake_vlan_id')
+
+    def test_port_update_old_agent(self):
+        cfg.CONF.set_override('rpc_support_old_agents', True, 'AGENT')
+        rpcapi = plb.AgentNotifierApi(topics.AGENT)
+        expected_msg = rpcapi.make_msg('port_update',
+                                       port='fake_port',
+                                       network_type='vlan',
+                                       physical_network='fake_net',
+                                       segmentation_id='fake_vlan_id',
+                                       vlan_id='fake_vlan_id')
+        self._test_lb_api(rpcapi,
+                          topics.get_topic_name(topics.AGENT,
+                                                topics.PORT,
+                                                topics.UPDATE),
+                          'port_update', rpc_method='fanout_cast',
+                          expected_msg=expected_msg,
                           port='fake_port',
                           physical_network='fake_net',
                           vlan_id='fake_vlan_id')