Bug 2707: Pipeline flows are not programmed because of failed onNodeUpdated call 70/15370/1
authorSam Hague <shague@redhat.com>
Mon, 16 Feb 2015 16:43:24 +0000 (11:43 -0500)
committerSam Hague <shague@redhat.com>
Mon, 16 Feb 2015 16:43:24 +0000 (11:43 -0500)
Cause: we used the onNodeUdpated() OpendaylightInventoryListener interface to receive updates when openflow nodes were updated. If we thought it was the first time and the openflow node was just added we would then program the pipeline flows. ovsdb checked the FlowCapableNode in the datastore for the isPreset flag. It looks like there might have been some race condition where another writer like InventoryManager was updated the FlowCapablNode such that when ovsdb would do the above check isPresent would return true and ignore the update - and not program the pipeline flows.

Solution: replace the deprecated call with an mdsal flowCapablenode DataChangeListener. This ensures ovsdb will get the node when created.

Also added a couple log.info's to track when the nodes are added to help with future debugging.

Change-Id: Ib6867927cb1ce61670d7566ea40b88966243bdca
Signed-off-by: Sam Hague <shague@redhat.com>
openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/FlowCapableNodeDataChangeListener.java [new file with mode: 0644]
openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/MdsalConsumerImpl.java
openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/PipelineOrchestrator.java
openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/PipelineOrchestratorImpl.java
openstack/net-virt/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/SouthboundHandler.java

diff --git a/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/FlowCapableNodeDataChangeListener.java b/openstack/net-virt-providers/src/main/java/org/opendaylight/ovsdb/openstack/netvirt/providers/openflow13/FlowCapableNodeDataChangeListener.java
new file mode 100644 (file)
index 0000000..7a22f13
--- /dev/null
@@ -0,0 +1,68 @@
+/*
+ * Copyright (C) 2015 Red Hat, Inc.
+ *
+ * This program and the accompanying materials are made available under the
+ * terms of the Eclipse Public License v1.0 which accompanies this distribution,
+ * and is available at http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Authors : Sam Hague
+ */
+package org.opendaylight.ovsdb.openstack.netvirt.providers.openflow13;
+
+import java.util.Map;
+import org.opendaylight.controller.md.sal.binding.api.DataBroker;
+import org.opendaylight.controller.md.sal.binding.api.DataChangeListener;
+import org.opendaylight.controller.md.sal.common.api.data.AsyncDataBroker;
+import org.opendaylight.controller.md.sal.common.api.data.AsyncDataChangeEvent;
+import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
+import org.opendaylight.controller.sal.utils.ServiceHelper;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.FlowCapableNode;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.Nodes;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node;
+import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.NodeKey;
+import org.opendaylight.yangtools.concepts.ListenerRegistration;
+import org.opendaylight.yangtools.yang.binding.DataObject;
+import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
+
+public class FlowCapableNodeDataChangeListener implements DataChangeListener, AutoCloseable {
+    private static final Logger LOG = LoggerFactory.getLogger(FlowCapableNodeDataChangeListener.class);
+    private ListenerRegistration<DataChangeListener> registration;
+
+    public static final InstanceIdentifier<FlowCapableNode> createFlowCapableNodePath () {
+        return InstanceIdentifier.builder(Nodes.class)
+                .child(Node.class)
+                .augmentation(FlowCapableNode.class)
+                .build();
+    }
+
+    public FlowCapableNodeDataChangeListener (DataBroker dataBroker) {
+        LOG.info("Registering FlowCapableNodeChangeListener");
+        registration = dataBroker.registerDataChangeListener(LogicalDatastoreType.CONFIGURATION,
+                createFlowCapableNodePath(), this, AsyncDataBroker.DataChangeScope.ONE);
+    }
+
+    @Override
+    public void close () throws Exception {
+        registration.close();
+    }
+
+    @Override
+    public void onDataChanged (AsyncDataChangeEvent<InstanceIdentifier<?>,
+            DataObject> changes) {
+
+        LOG.debug("onDataChanged: {}", changes);
+        for( Map.Entry<InstanceIdentifier<?>, DataObject> created : changes.getCreatedData().entrySet()) {
+            InstanceIdentifier<?> iiD = created.getKey();
+            LOG.debug(">>>>> created iiD: {} - first: {} - NodeKey: {}",
+                    iiD,
+                    iiD.firstIdentifierOf(Node.class),
+                    iiD.firstKeyOf(Node.class, NodeKey.class).getId().getValue());
+
+            PipelineOrchestrator pipelineOrchestrator =
+                    (PipelineOrchestrator) ServiceHelper.getGlobalInstance(PipelineOrchestrator.class, this);
+            pipelineOrchestrator.enqueue(iiD.firstKeyOf(Node.class, NodeKey.class).getId().getValue());
+        }
+    }
+}
index 1ce9aa3516944e8621a12bba83ac2cbfab0f7b09..1f16f1569718ef77a66586c160fec0f084c8ef8b 100644 (file)
@@ -10,8 +10,6 @@
 
 package org.opendaylight.ovsdb.openstack.netvirt.providers.openflow13;
 
-import java.util.Collection;
-
 import org.apache.felix.dm.Component;
 import org.opendaylight.controller.md.sal.binding.api.DataBroker;
 import org.opendaylight.controller.sal.binding.api.BindingAwareBroker;
@@ -20,7 +18,6 @@ import org.opendaylight.controller.sal.binding.api.BindingAwareBroker.ProviderCo
 import org.opendaylight.controller.sal.binding.api.BindingAwareConsumer;
 import org.opendaylight.controller.sal.binding.api.BindingAwareProvider;
 import org.opendaylight.controller.sal.binding.api.NotificationProviderService;
-import org.opendaylight.yangtools.yang.binding.RpcService;
 import org.osgi.framework.BundleContext;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -31,6 +28,7 @@ public class MdsalConsumerImpl implements BindingAwareConsumer, MdsalConsumer, B
     private ConsumerContext consumerContext = null;
     private DataBroker dataBroker;
     private NotificationProviderService notificationService;
+    private FlowCapableNodeDataChangeListener flowCapableNodeChangeListener;
 
     static final Logger logger = LoggerFactory.getLogger(MdsalConsumerImpl.class);
 
@@ -61,6 +59,7 @@ public class MdsalConsumerImpl implements BindingAwareConsumer, MdsalConsumer, B
         this.consumerContext = session;
         dataBroker = session.getSALService(DataBroker.class);
         logger.info("OVSDB Neutron Session Initialized with CONSUMER CONTEXT {}", session.toString());
+        flowCapableNodeChangeListener = new FlowCapableNodeDataChangeListener(dataBroker);
     }
 
     @Override
index b2d97dd7dd2c336141076e8bd0307c39142cddf9..d30047ad9f0324d34217a96cec4ce3bd45084c8a 100644 (file)
@@ -17,4 +17,5 @@ package org.opendaylight.ovsdb.openstack.netvirt.providers.openflow13;
 public interface PipelineOrchestrator {
     public Service getNextServiceInPipeline(Service service);
     AbstractServiceInstance getServiceInstance(Service service);
+    public void enqueue(String nodeId);
 }
index 8c84f1acdba8fb5526c35613c4571c3d430427a6..6004b06fe68f251fe454381cd10a90fa91c3eae0 100644 (file)
 
 package org.opendaylight.ovsdb.openstack.netvirt.providers.openflow13;
 
+import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
 import java.util.List;
 import java.util.Map;
 import java.util.concurrent.BlockingQueue;
 import java.util.concurrent.ExecutorService;
 import java.util.concurrent.Executors;
 import java.util.concurrent.LinkedBlockingQueue;
-
-import org.opendaylight.controller.md.sal.binding.api.BindingTransactionChain;
-import org.opendaylight.controller.md.sal.common.api.data.AsyncTransaction;
-import org.opendaylight.controller.md.sal.common.api.data.LogicalDatastoreType;
-import org.opendaylight.controller.md.sal.common.api.data.TransactionChain;
-import org.opendaylight.controller.md.sal.common.api.data.TransactionChainListener;
-import org.opendaylight.controller.sal.binding.api.NotificationProviderService;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.flow.inventory.rev130819.FlowCapableNode;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeConnectorRemoved;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeConnectorUpdated;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeRef;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeRemoved;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.NodeUpdated;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.OpendaylightInventoryListener;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.Node;
-import org.opendaylight.yang.gen.v1.urn.opendaylight.inventory.rev130819.nodes.NodeKey;
-import org.opendaylight.yangtools.yang.binding.DataObject;
-import org.opendaylight.yangtools.yang.binding.InstanceIdentifier;
-import org.opendaylight.yangtools.yang.binding.InstanceIdentifier.InstanceIdentifierBuilder;
 import org.osgi.framework.ServiceReference;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-import com.google.common.base.Optional;
-import com.google.common.collect.Lists;
-import com.google.common.collect.Maps;
-import com.google.common.util.concurrent.CheckedFuture;
-import com.google.common.util.concurrent.FutureCallback;
-import com.google.common.util.concurrent.Futures;
-
-public class PipelineOrchestratorImpl implements PipelineOrchestrator, OpendaylightInventoryListener, TransactionChainListener {
+public class PipelineOrchestratorImpl implements PipelineOrchestrator {
 
     private static final Logger logger = LoggerFactory.getLogger(PipelineOrchestratorImpl.class);
     private List<Service> staticPipeline = Lists.newArrayList(
@@ -63,7 +39,6 @@ public class PipelineOrchestratorImpl implements PipelineOrchestrator, Opendayli
                                                                 Service.L2_FORWARDING
                                                               );
     Map<Service, AbstractServiceInstance> serviceRegistry = Maps.newConcurrentMap();
-    private volatile MdsalConsumer mdsalConsumer;
     private volatile BlockingQueue<String> queue;
     private ExecutorService eventHandler;
     public PipelineOrchestratorImpl() {
@@ -93,11 +68,8 @@ public class PipelineOrchestratorImpl implements PipelineOrchestrator, Opendayli
     public void init() {
         eventHandler = Executors.newSingleThreadExecutor();
         this.queue = new LinkedBlockingQueue<String>();
-        NotificationProviderService notificationService = mdsalConsumer.getNotificationService();
-        if (notificationService != null) {
-            notificationService.registerNotificationListener(this);
-        }
     }
+
     public void start() {
         eventHandler.submit(new Runnable()  {
             @Override
@@ -111,6 +83,7 @@ public class PipelineOrchestratorImpl implements PipelineOrchestrator, Opendayli
                          * causes programming issues. Hence delaying the programming by a second to
                          * avoid the clash. This hack/workaround should be removed once Bug 1997 is resolved.
                          */
+                        logger.info(">>>>> dequeue: {}", nodeId);
                         Thread.sleep(1000);
                         for (Service service : staticPipeline) {
                             AbstractServiceInstance serviceInstance = getServiceInstance(service);
@@ -134,76 +107,13 @@ public class PipelineOrchestratorImpl implements PipelineOrchestrator, Opendayli
         eventHandler.shutdownNow();
     }
 
-    void enqueue(String nodeId) {
+    @Override
+    public void enqueue(String nodeId) {
+        logger.info(">>>>> enqueue: {}", nodeId);
         try {
             queue.put(new String(nodeId));
         } catch (InterruptedException e) {
             logger.warn("Failed to enqueue operation {}", nodeId, e);
         }
     }
-
-
-    /* TODO ADSAL: replace onNodeUpdated with dataChangeListener: https://bugs.opendaylight.org/show_bug.cgi?id=2707 */
-    /**
-     * Process the Node update notification. Check for Openflow node and make sure if the bridge is part of the Pipeline before
-     * programming the Pipeline specific flows.
-     */
-    @Override
-    public void onNodeUpdated(NodeUpdated nodeUpdated) {
-        NodeRef ref = nodeUpdated.getNodeRef();
-        InstanceIdentifier<Node> identifier = (InstanceIdentifier<Node>) ref.getValue();
-        logger.debug("Node Update received for : "+identifier.toString());
-        final NodeKey key = identifier.firstKeyOf(Node.class, NodeKey.class);
-        final String nodeId = key.getId().getValue();
-        if (key != null && key.getId().getValue().contains("openflow")) {
-            InstanceIdentifierBuilder<Node> builder = ((InstanceIdentifier<Node>) ref.getValue()).builder();
-            InstanceIdentifierBuilder<FlowCapableNode> augmentation = builder.augmentation(FlowCapableNode.class);
-            final InstanceIdentifier<FlowCapableNode> path = augmentation.build();
-            BindingTransactionChain txChain = mdsalConsumer.getDataBroker().createTransactionChain(this);
-            CheckedFuture readFuture = txChain.newReadWriteTransaction().read(LogicalDatastoreType.OPERATIONAL, path);
-            Futures.addCallback(readFuture, new FutureCallback<Optional<? extends DataObject>>() {
-                @Override
-                public void onSuccess(Optional<? extends DataObject> optional) {
-                    if (!optional.isPresent()) {
-                        enqueue(nodeId);
-                    }
-                }
-
-                @Override
-                public void onFailure(Throwable throwable) {
-                    logger.debug(String.format("Can't retrieve node data for node %s. Writing node data with table0.", nodeId));
-                    enqueue(nodeId);
-                }
-            });
-        }
-    }
-
-    @Override
-    public void onTransactionChainFailed(final TransactionChain<?, ?> chain, final AsyncTransaction<?, ?> transaction,
-            final Throwable cause) {
-        logger.error("Failed to export Flow Capable Inventory, Transaction {} failed.",transaction.getIdentifier(),cause);
-    }
-
-    @Override
-    public void onTransactionChainSuccessful(final TransactionChain<?, ?> chain) {
-    }
-
-    @Override
-    public void onNodeConnectorRemoved(NodeConnectorRemoved arg0) {
-        // TODO Auto-generated method stub
-
-    }
-
-    @Override
-    public void onNodeConnectorUpdated(NodeConnectorUpdated arg0) {
-        // TODO Auto-generated method stub
-
-    }
-
-    @Override
-    public void onNodeRemoved(NodeRemoved arg0) {
-        // TODO Auto-generated method stub
-
-    }
-
 }
index 07380b8574fb8dc05a29d9b78f3e0fa8f632d927..93ae59f8f3081d3a2e35d41499f0b6559a4b08c5 100644 (file)
@@ -65,6 +65,7 @@ public class SouthboundHandler extends AbstractHandler implements OvsdbInventory
 
     @Override
     public void nodeAdded(Node node, InetAddress address, int port) {
+        logger.info("nodeAdded: {}", node);
         this.enqueueEvent(new SouthboundEvent(node, Action.ADD));
     }