Neutron port removal fixed

	* L3 context update is applied only for persistent ports
	  within subnet to avoid recreation of previously removed
	  ones
	* Router interface port is now already removed in onDelete
	* vif-type is considered if port update

Change-Id: I4d1331845e19313f50a563206f6c4efe67e1fca6
Signed-off-by: Vladimir Lavor <vlavor@cisco.com>

diff --git a/neutron-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/mapper/EndpointRegistrator.java b/neutron-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/mapper/EndpointRegistrator.java
index 8bdfa6bf33194ba3a322dc5f9ecac1484d65618d..e1e7a4b7bc6f40657524301c23ef14cc24598b23 100644 (file)
--- a/neutron-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/mapper/EndpointRegistrator.java
+++ b/neutron-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/mapper/EndpointRegistrator.java
@@ -8,12 +8,12 @@
 
 package org.opendaylight.groupbasedpolicy.neutron.mapper;
 
+import javax.annotation.Nullable;
 import java.util.ArrayList;
 import java.util.List;
 import java.util.concurrent.ExecutionException;
-
-import javax.annotation.Nullable;
-
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableList;
 import org.opendaylight.groupbasedpolicy.neutron.mapper.util.MappingUtils;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IpAddress;
 import org.opendaylight.yang.gen.v1.urn.ietf.params.xml.ns.yang.ietf.inet.types.rev130715.IpPrefix;
@@ -22,15 +22,9 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.base_endpo
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.base_endpoint.rev160427.RegisterEndpointInputBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.base_endpoint.rev160427.UnregisterEndpointInput;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.base_endpoint.rev160427.UnregisterEndpointInputBuilder;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.base_endpoint.rev160427.common.endpoint.fields.NetworkContainmentBuilder;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.base_endpoint.rev160427.common.endpoint.fields.network.containment.containment.NetworkDomainContainmentBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.base_endpoint.rev160427.register.endpoint.input.AddressEndpointReg;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.base_endpoint.rev160427.register.endpoint.input.AddressEndpointRegBuilder;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.base_endpoint.rev160427.register.endpoint.input.ContainmentEndpointReg;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.base_endpoint.rev160427.register.endpoint.input.ContainmentEndpointRegBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.base_endpoint.rev160427.unregister.endpoint.input.AddressEndpointUnreg;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.base_endpoint.rev160427.unregister.endpoint.input.AddressEndpointUnregBuilder;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.common.rev140421.ContextId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.common.rev140421.L3ContextId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.common.rev140421.NetworkDomainId;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.common.rev140421.TenantId;
@@ -40,14 +34,10 @@ import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.endpoint.r
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.endpoint.rev140421.endpoint.fields.L3AddressBuilder;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.endpoint.rev140421.endpoint.l3.prefix.fields.EndpointL3Gateways;
 import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.endpoint.rev140421.endpoint.l3.prefix.fields.EndpointL3GatewaysBuilder;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.groupbasedpolicy.forwarding.l2_l3.rev160427.IpPrefixType;
 import org.opendaylight.yangtools.yang.common.RpcResult;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.base.Preconditions;
-import com.google.common.collect.ImmutableList;
-
 public class EndpointRegistrator {
 
     private static final Logger LOG = LoggerFactory.getLogger(EndpointRegistrator.class);

diff --git a/neutron-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/mapper/NeutronMapper.java b/neutron-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/mapper/NeutronMapper.java
index ac008035ce48a012999f3cff3287d6641af6422a..f5f1939b3f1bdaa92e2506a03af862e938f421fb 100644 (file)
--- a/neutron-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/mapper/NeutronMapper.java
+++ b/neutron-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/mapper/NeutronMapper.java
@@ -7,12 +7,12 @@
  */
 package org.opendaylight.groupbasedpolicy.neutron.mapper;
 
-import java.io.IOException;
 import java.util.ArrayList;
 import java.util.Collection;
 import java.util.List;
 import java.util.Set;
 
+import javax.annotation.Nonnull;
 import javax.annotation.Nullable;
 
 import org.opendaylight.controller.md.sal.binding.api.ClusteredDataTreeChangeListener;
@@ -67,7 +67,7 @@ import com.google.common.collect.PeekingIterator;
 
 public class NeutronMapper implements ClusteredDataTreeChangeListener<Neutron>, AutoCloseable {
 
-    public static final String EXC_MSG_UNKNOWN_MODIFICATION_TYPE_WITHIN_DATA = "Unknown modification type within data ";
+    private static final String EXC_MSG_UNKNOWN_MODIFICATION_TYPE_WITHIN_DATA = "Unknown modification type within data ";
 
     private final static SecurityRuleBuilder EIG_INGRESS_IPV4_SEC_RULE_BUILDER = new SecurityRuleBuilder()
         .setUuid(new Uuid("0a629f80-2408-11e6-b67b-9e71128cae77"))
@@ -120,7 +120,7 @@ public class NeutronMapper implements ClusteredDataTreeChangeListener<Neutron>,
     }
 
     @Override
-    public void onDataTreeChanged(Collection<DataTreeModification<Neutron>> changes) {
+    public void onDataTreeChanged(@Nonnull Collection<DataTreeModification<Neutron>> changes) {
         for (DataTreeModification<Neutron> change : changes) {
             DataObjectModification<Neutron> neutronModif = change.getRootNode();
             resolveAndSetNeutron(neutronModif);
@@ -164,24 +164,19 @@ public class NeutronMapper implements ClusteredDataTreeChangeListener<Neutron>,
     private <T extends DataObject> void onDataObjectModification(List<DataObjectModification<T>> dataModifs,
             NeutronAware<T> neutronAware) {
         for (DataObjectModification<T> dataModif : dataModifs) {
-            switch (dataModif.getModificationType()) {
-                case DELETE:
-                    neutronAware.onDeleted(dataModif.getDataBefore(), neutronBefore, neutronAfter);
-                    break;
-                case SUBTREE_MODIFIED:
-                    neutronAware.onUpdated(dataModif.getDataBefore(), dataModif.getDataAfter(), neutronBefore,
-                            neutronAfter);
-                    break;
-                case WRITE:
-                    if (dataModif.getDataBefore() == null) {
-                        neutronAware.onCreated(dataModif.getDataAfter(), neutronAfter);
-                    } else {
-                        neutronAware.onUpdated(dataModif.getDataBefore(), dataModif.getDataAfter(), neutronBefore,
-                                neutronAfter);
-                    }
-                    break;
-                default:
-                    throw new IllegalStateException(EXC_MSG_UNKNOWN_MODIFICATION_TYPE_WITHIN_DATA + dataModif);
+            final T dataBefore = dataModif.getDataBefore();
+            final T dataAfter = dataModif.getDataAfter();
+            if (dataBefore == null && dataAfter != null) {
+                neutronAware.onCreated(dataAfter, neutronAfter);
+            }
+            else if (dataBefore != null && dataAfter != null) {
+                neutronAware.onUpdated(dataBefore, dataAfter, neutronBefore, neutronAfter);
+            }
+            else if (dataBefore != null) {
+                neutronAware.onDeleted(dataBefore, neutronBefore, neutronAfter);
+            }
+            else {
+                throw new IllegalStateException(EXC_MSG_UNKNOWN_MODIFICATION_TYPE_WITHIN_DATA + dataModif);
             }
         }
     }

diff --git a/neutron-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/mapper/mapping/NeutronPortAware.java b/neutron-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/mapper/mapping/NeutronPortAware.java
index e240de078eb5452bf67f5f1faed56e8033529f03..07a0129a81c9dcd1d3042f2fa58794e87cd08972 100644 (file)
--- a/neutron-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/mapper/mapping/NeutronPortAware.java
+++ b/neutron-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/mapper/mapping/NeutronPortAware.java
@@ -15,6 +15,7 @@ import java.util.Set;
 
 import javax.annotation.Nullable;
 
+import com.sun.jndi.cosnaming.IiopUrl;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.md.sal.binding.api.ReadWriteTransaction;
 import org.opendaylight.controller.md.sal.binding.api.WriteTransaction;
@@ -544,15 +545,15 @@ public class NeutronPortAware implements NeutronAware<Port> {
     @Override
     public void onUpdated(Port oldPort, Port newPort, Neutron oldNeutron, Neutron newNeutron) {
         LOG.trace("updated port - OLD: {}\nNEW: {}", oldPort, newPort);
-        onDeleted(oldPort, oldNeutron, false);
+        onDeleted(oldPort, oldNeutron, newNeutron, false);
         onCreated(newPort, newNeutron, false);
     }
 
     @Override public void onDeleted(Port deletedItem, Neutron oldNeutron, Neutron newNeutron) {
-        onDeleted(deletedItem, oldNeutron, true);
+        onDeleted(deletedItem, oldNeutron, newNeutron, true);
     }
 
-    public void onDeleted(Port port, Neutron oldNeutron, boolean removeBaseEpMapping) {
+    public void onDeleted(Port port, Neutron oldNeutron, Neutron newNeutron, boolean removeBaseEpMapping) {
         LOG.trace("deleted port - {}", port);
         if (PortUtils.isRouterInterfacePort(port)) {
             LOG.trace("Port is router interface port: {}", port.getUuid().getValue());
@@ -566,7 +567,7 @@ public class NeutronPortAware implements NeutronAware<Port> {
             FixedIps portIpWithSubnet = potentialPortIpWithSubnet.get();
             L3ContextId l3Context = new L3ContextId(port.getNetworkId().getValue());
             // change L3Context for all EPs with same subnet as router port
-            changeL3ContextForEpsInSubnet(portIpWithSubnet.getSubnetId(), oldNeutron);
+            changeL3ContextForEpsInSubnet(portIpWithSubnet.getSubnetId(), newNeutron);
             // set L3Context as parent for bridge domain which is parent of subnet
             TenantId tenantId = new TenantId(port.getTenantId().getValue());
             Optional<Subnet> potentialRouterPortSubnet = SubnetUtils.findSubnet(portIpWithSubnet.getSubnetId(),
@@ -588,12 +589,8 @@ public class NeutronPortAware implements NeutronAware<Port> {
             NetworkDomain subnet = NeutronSubnetAware.createSubnet(routerPortSubnet, null);
             rwTx.put(LogicalDatastoreType.CONFIGURATION, L2L3IidFactory.subnetIid(tenantId, subnet.getNetworkDomainId()),
                     subnet);
-            UniqueId portId = new UniqueId(port.getUuid().getValue());
-            PortByBaseEndpointKey portByBaseEndpointKey = new PortByBaseEndpointKey(port.getMacAddress().getValue(),
-                    MacAddressType.class, new ContextId(port.getNetworkId().getValue()), MappingUtils.L2_BRDIGE_DOMAIN);
-            if (removeBaseEpMapping) {
-                removeBaseEndpointMappings(portByBaseEndpointKey, portId, rwTx);
-            }
+            unregisterEndpointAndRemoveMapping(createUnregisterEndpointInput(port, oldNeutron), port, rwTx);
+            unregisterEndpointAndRemoveMapping(createUnregisterBaseEndpointInput(port, oldNeutron), port, rwTx, removeBaseEpMapping);
             DataStoreHelper.submitToDs(rwTx);
         } else if (PortUtils.isDhcpPort(port)) {
             LOG.trace("Port is DHCP port: {}", port.getUuid().getValue());

diff --git a/neutron-vpp-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/vpp/mapper/processors/PortHandler.java b/neutron-vpp-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/vpp/mapper/processors/PortHandler.java
index 3c25a8c913e0b9b56c8bd3a157bc08105b0b2501..32fe340a71bdd7dc7942d1067bdecf7c73ab7035 100644 (file)
--- a/neutron-vpp-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/vpp/mapper/processors/PortHandler.java
+++ b/neutron-vpp-mapper/src/main/java/org/opendaylight/groupbasedpolicy/neutron/vpp/mapper/processors/PortHandler.java
@@ -66,13 +66,14 @@ import com.google.common.base.Optional;
 \r
 public class PortHandler implements TransactionChainListener {\r
 \r
-    private static final Logger LOG = LoggerFactory.getLogger(MappingProvider.class);\r
+    private static final Logger LOG = LoggerFactory.getLogger(PortHandler.class);\r
 \r
     private static final String COMPUTE_OWNER = "compute";\r
     private static final String DHCP_OWNER = "dhcp";\r
     private static final String ROUTER_OWNER = "network:router_interface";\r
     private static final String[] SUPPORTED_DEVICE_OWNERS = {COMPUTE_OWNER, DHCP_OWNER, ROUTER_OWNER};\r
     private static final String VHOST_USER = "vhostuser";\r
+    private static final String UNBOUND = "unbound";\r
     private static final String VPP_INTERFACE_NAME_PREFIX = "neutron_port_";\r
     private static final String TAP_PORT_NAME_PREFIX = "tap";\r
     private static final String RT_PORT_NAME_PREFIX = "qr-";\r
@@ -158,18 +159,38 @@ public class PortHandler implements TransactionChainListener {
         processCreated(delta);\r
     }\r
 \r
-    private boolean isUpdateNeeded(Port oldPort, Port newPort) {\r
+    private boolean isUpdateNeeded(final Port oldPort, final Port newPort) {\r
         //TODO fix this to better support update of ports for VPP\r
-        PortBindingExtension oldPortAugmentation = oldPort.getAugmentation(PortBindingExtension.class);\r
-        PortBindingExtension newPortAugmentation = newPort.getAugmentation(PortBindingExtension.class);\r
-\r
-        List<VifDetails> vifDetails = oldPortAugmentation.getVifDetails();\r
+        final PortBindingExtension oldPortAugmentation = oldPort.getAugmentation(PortBindingExtension.class);\r
+        final PortBindingExtension newPortAugmentation = newPort.getAugmentation(PortBindingExtension.class);\r
 \r
         if (newPortAugmentation == null) {\r
             LOG.trace("Port {} is no longer a vhost type port, updating port...");\r
             return true;\r
         }\r
 \r
+        final String oldDeviceOwner = oldPort.getDeviceOwner();\r
+        final String oldVifType = oldPortAugmentation.getVifType();\r
+        final String newDeviceOwner = newPort.getDeviceOwner();\r
+        final String newVifType = newPortAugmentation.getVifType();\r
+\r
+        // TODO potential bug here\r
+        // Temporary change for Openstack Mitaka: If old neutron-binding:vif-type is vhost, new one is unbound and\r
+        // device owner is ROUTER_OWNER, skip update. Openstack (or ml2) sometimes sends router update messages in\r
+        // incorrect order which causes unwanted port removal\r
+        if (oldVifType.equals(VHOST_USER) && newVifType.equals(UNBOUND) && oldDeviceOwner != null &&\r
+                ROUTER_OWNER.equals(oldDeviceOwner) && ROUTER_OWNER.equals(newDeviceOwner)) {\r
+            LOG.warn("Port vif-type was updated from vhost to unbound. This update is currently disabled and will be skipped");\r
+            return false;\r
+        }\r
+\r
+        if (newVifType != null && !newVifType.equals(oldVifType)) {\r
+            LOG.trace("Vif type changed, old: {} new {}", oldVifType, newVifType);\r
+            return true;\r
+        }\r
+\r
+        final List<VifDetails> vifDetails = oldPortAugmentation.getVifDetails();\r
+\r
         if (!oldPortAugmentation.getHostId().equals(newPortAugmentation.getHostId()) ||\r
             nullToEmpty(vifDetails).size() != nullToEmpty(newPortAugmentation.getVifDetails()).size()) {\r
             return true;\r