[APIC-mapping] Fix device_id of port when using VLAN network

When VLAN networks are used instead of OpFlex,
additional networks and ports are created that mirror
the usual networks and ports. The device_id of the
VM's port is set to VM's UUID whereas the port that
mirrors had its device_id set to the PT UUID. This
latter value resulted in failure to lookup metadata
information for the VM.

This change ensures that the device_id for both the
VM port and its mirror stay in sync.

Closes-Bug: 1627915

Change-Id: Ibea325fbfa344acd9626d5e651297dd5e24297b6
Signed-off-by: Amit Bose <amitbose@gmail.com>

gbpservice/neutron/services/grouppolicy/drivers/cisco/apic/apic_mapping.py

@@ -776,7 +776,7 @@ class ApicMappingDriver(api.ResourceMappingDriver,
             self._notify_head_chain_ports(pt['policy_target_group_id'])
 
     def process_port_added(self, context):
-        self._disable_port_on_shadow_subnet(context)
+        self._handle_shadow_port_change(context)
 
     def create_policy_action_precommit(self, context):
         pass
@@ -1567,7 +1567,7 @@ class ApicMappingDriver(api.ResourceMappingDriver,
             self._sync_shadow_subnets(context, l2p, subnet, None)
 
     def process_port_changed(self, context):
-        self._disable_port_on_shadow_subnet(context)
+        self._handle_shadow_port_change(context)
         if (context.original_host != context.host or
                 context.original_bottom_bound_segment !=
                 context.bottom_bound_segment):
@@ -3618,15 +3618,35 @@ class ApicMappingDriver(api.ResourceMappingDriver,
                     grp['id'])
                 raise
 
-    def _disable_port_on_shadow_subnet(self, context):
+    def _handle_shadow_port_change(self, context):
-        """Disable certain kinds of ports in shadow-network."""
+        """Special handling for changes to ports in shadow network.
+
+        1. Disable certain kinds of ports like DHCP to avoid interference
+        on the data path.
+        2. Copy device_id from shadow port to corresponding port in
+           L2P's network.
+
+        """
         port = context.current
+        ptg = self._shadow_network_id_to_ptg(context, port['network_id'])
+        admin_ctx = context._plugin_context.elevated()
+        if ptg:
+            # Disable DHCP port
             if (port['device_owner'] == n_constants.DEVICE_OWNER_DHCP and
-                port['admin_state_up'] is True and
+                    port['admin_state_up'] is True):
-                self._shadow_network_id_to_ptg(context, port['network_id'])):
+                self._update_port(admin_ctx, port['id'],
-            self._update_port(context._plugin_context.elevated(),
-                              port['id'],
                                   {'admin_state_up': False})
+            # Copy device_id
+            l2p = self._get_l2_policy(admin_ctx, ptg['l2_policy_id'])
+            l2p_port = self._get_ports(
+                admin_ctx,
+                filters={'mac_address': [port['mac_address']],
+                         'network_id': [l2p['network_id']]})
+            if l2p_port:
+                l2p_port = l2p_port[0]
+                if l2p_port['device_id'] != port['device_id']:
+                    self._update_port(admin_ctx, l2p_port['id'],
+                                      {'device_id': port['device_id']})
 
     def _delete_ptg_shadow_network(self, context, ptg):
         shadow_net = self._get_ptg_shadow_network(context, ptg)
@@ -3672,12 +3692,13 @@ class ApicMappingDriver(api.ResourceMappingDriver,
         # a port originally, then treat that port as the "shadow" port,
         # else create the shadow port in the shadow network. In both cases,
         # associate the shadow port to the PT.
-        context.current['port_attributes'] = {'device_owner': 'apic',
+        context.current['port_attributes'] = {'device_owner':
-                                              'device_id': pt['id']}
+                                              'apic:%s' % pt['id']}
 
         if pt_port_id:
             shadow_port = self._get_port(context._plugin_context, pt_port_id)
             context.current['port_attributes'].update({
+                'device_id': shadow_port['device_id'],
                 'fixed_ips': self._strip_subnet(shadow_port['fixed_ips']),
                 'mac_address': shadow_port['mac_address']})
 
@@ -3721,8 +3742,7 @@ class ApicMappingDriver(api.ResourceMappingDriver,
             l2p = self._get_l2_policy(context._plugin_context,
                                       ptg['l2_policy_id'])
             implicit_ports = self._get_ports(context._plugin_context,
-                filters={'device_owner': ['apic'],
+                filters={'device_owner': ['apic:%s' % pt['id']],
-                         'device_id': [pt['id']],
                          'network_id': [l2p['network_id']]})
             for p in implicit_ports:
                 self._cleanup_port(context._plugin_context, p['id'])

gbpservice/neutron/tests/unit/services/grouppolicy/test_apic_mapping.py

@@ -1229,6 +1229,15 @@ class TestPolicyTargetVlanNetwork(ApicMappingVlanTestCase,
                          len(ports[0]['fixed_ips']))
         self.assertEqual(shadow_port['fixed_ips'][0]['ip_address'],
                          ports[0]['fixed_ips'][0]['ip_address'])
+        self.assertEqual('apic:%s' % pt1['id'], ports[0]['device_owner'])
+        self.assertEqual('', ports[0]['device_id'])
+
+        # update device_id of shadow port
+        shadow_port = self._update('ports', shadow_port['id'],
+                                   {'port': {'device_id': 'comp1'}})['port']
+        l2p_port = self._get_object(
+            'ports', ports[0]['id'], self.api)['port']
+        self.assertEqual('comp1', l2p_port['device_id'])
 
         self.delete_policy_target(pt1['id'])
         self._get_object('ports', pt1['port_id'], self.api,
@@ -1243,7 +1252,7 @@ class TestPolicyTargetVlanNetwork(ApicMappingVlanTestCase,
                                           self.api)
         subnet = self._get_object('subnets', ptg1['subnets'][0], self.api)
 
-        with self.port(subnet=shadow_subnet1) as p:
+        with self.port(subnet=shadow_subnet1, device_id='vm1') as p:
             port1 = p['port']
 
         pt1 = self.create_policy_target(policy_target_group_id=ptg1['id'],
@@ -1258,6 +1267,15 @@ class TestPolicyTargetVlanNetwork(ApicMappingVlanTestCase,
                          len(ports[0]['fixed_ips']))
         self.assertEqual(port1['fixed_ips'][0]['ip_address'],
                          ports[0]['fixed_ips'][0]['ip_address'])
+        self.assertEqual('apic:%s' % pt1['id'], ports[0]['device_owner'])
+        self.assertEqual('vm1', ports[0]['device_id'])
+
+        # update device_id of explicit port
+        port1 = self._update('ports', port1['id'],
+                             {'port': {'device_id': 'comp1'}})['port']
+        l2p_port = self._get_object(
+            'ports', ports[0]['id'], self.api)['port']
+        self.assertEqual('comp1', l2p_port['device_id'])
 
         self.delete_policy_target(pt1['id'])
         self._get_object('ports', pt1['port_id'], self.api,
@@ -1265,6 +1283,25 @@ class TestPolicyTargetVlanNetwork(ApicMappingVlanTestCase,
         self._get_object('ports', ports[0]['id'], self.api,
                          expected_res_status=404)
 
+    def test_dhcp_disable_in_shadow_network(self):
+        ptg1 = self.create_policy_target_group(
+            name="ptg1")['policy_target_group']
+        self.create_policy_target(
+            policy_target_group_id=ptg1['id'])['policy_target']
+
+        subnet = self._get_object('subnets',
+                                  self._get_ptg_shadow_subnet(ptg1),
+                                  self.api)
+        with self.port(subnet=subnet, device_owner='network:dhcp') as p:
+            port1 = p['port']
+            port1 = self._get_object('ports', port1['id'], self.api)['port']
+            self.assertFalse(port1['admin_state_up'])
+
+            port1 = self._update('ports', port1['id'],
+                                 {'port': {'admin_state_up': True}})['port']
+            port1 = self._get_object('ports', port1['id'], self.api)['port']
+            self.assertFalse(port1['admin_state_up'])
+
     def test_explicit_port_wrong_network(self):
         ptg1 = self.create_policy_target_group()['policy_target_group']
         subnet = self._get_object('subnets', ptg1['subnets'][0], self.api)