Should not allow nodes to have same node name 67/867/2
authorPramila Singh <pramisin@cisco.com>
Tue, 13 Aug 2013 22:30:03 +0000 (15:30 -0700)
committerPramila Singh <pramisin@cisco.com>
Wed, 14 Aug 2013 00:01:00 +0000 (17:01 -0700)
Change-Id: Ia658826c85dfd9a9fa5e4b82731c6e26d52e5b40
Signed-off-by: Pramila Singh <pramisin@cisco.com>
opendaylight/clustering/test/src/main/java/org/opendaylight/controller/clustering/test/internal/LoggingListener.java
opendaylight/configuration/integrationtest/src/test/java/org/opendaylight/controller/configuration/internal/ConfigurationIT.java
opendaylight/forwardingrulesmanager/integrationtest/src/test/java/org/opendaylight/controller/forwardingrulesmanager/internal/ForwardingRulesManagerIT.java
opendaylight/hosttracker/integrationtest/src/test/java/org/opendaylight/controller/hosttracker/internal/HostTrackerIT.java
opendaylight/protocol_plugins/openflow/src/main/java/org/opendaylight/controller/protocol_plugin/openflow/internal/InventoryService.java
opendaylight/switchmanager/implementation/pom.xml
opendaylight/switchmanager/implementation/src/main/java/org/opendaylight/controller/switchmanager/internal/Activator.java
opendaylight/switchmanager/implementation/src/main/java/org/opendaylight/controller/switchmanager/internal/SwitchManager.java
opendaylight/switchmanager/integrationtest/src/test/java/org/opendaylight/controller/switchmanager/internal/SwitchManagerIT.java

index 4f5d432..1aef9ce 100644 (file)
@@ -25,10 +25,10 @@ public class LoggingListener implements IGetUpdates<Integer, StringContainer> {
     }
 
     @Override
-    public void entryUpdated(Integer key, StringContainer new_value,
+    public void entryUpdated(Integer key, StringContainer newValue,
             String containerName, String cacheName, boolean originLocal) {
         logger.debug(" Cache entry with key " + key + " modified to value "
-                + new_value + "  in cache " + cacheName);
+                + newValue + "  in cache " + cacheName);
     }
 
     @Override
index 17b3414..956028b 100644 (file)
@@ -91,10 +91,6 @@ public class ConfigurationIT {
                         "0.4.0-SNAPSHOT"),
                 mavenBundle("org.opendaylight.controller",
                         "containermanager.implementation", "0.4.0-SNAPSHOT"),
-                mavenBundle("org.opendaylight.controller", "switchmanager",
-                        "0.5.0-SNAPSHOT"),
-                mavenBundle("org.opendaylight.controller",
-                        "switchmanager.implementation", "0.4.0-SNAPSHOT"),
                 // needed bundles by configuration
                 mavenBundle("org.opendaylight.controller",
                         "clustering.services", "0.4.0-SNAPSHOT"), // what are
index c497eb2..6e8d9ab 100644 (file)
@@ -111,11 +111,11 @@ public class ForwardingRulesManagerIT {
                         .versionAsInProject(),
                 mavenBundle("org.opendaylight.controller", "switchmanager")
                         .versionAsInProject(),
+                mavenBundle("org.opendaylight.controller", "statisticsmanager").versionAsInProject(),
                 mavenBundle("org.opendaylight.controller",
                         "switchmanager.implementation").versionAsInProject(),
                 mavenBundle("org.opendaylight.controller", "configuration")
                         .versionAsInProject(),
-
                 mavenBundle("org.opendaylight.controller",
                         "configuration.implementation").versionAsInProject(),
                 mavenBundle("org.opendaylight.controller", "hosttracker")
index b5e6629..3734a63 100644 (file)
@@ -99,6 +99,8 @@ public class HostTrackerIT {
 
                 // needed by forwardingrulesmanager
                 mavenBundle("org.opendaylight.controller", "switchmanager").versionAsInProject(),
+                mavenBundle("org.opendaylight.controller", "forwardingrulesmanager").versionAsInProject(),
+                mavenBundle("org.opendaylight.controller", "statisticsmanager").versionAsInProject(),
                 mavenBundle("org.opendaylight.controller", "switchmanager.implementation").versionAsInProject(),
                 mavenBundle("org.opendaylight.controller", "configuration").versionAsInProject(),
                 mavenBundle("org.opendaylight.controller", "configuration.implementation").versionAsInProject(),
index a5a0711..f983e12 100644 (file)
@@ -22,7 +22,6 @@ import org.opendaylight.controller.protocol_plugin.openflow.IInventoryProvider;
 import org.opendaylight.controller.protocol_plugin.openflow.IInventoryShimInternalListener;
 import org.opendaylight.controller.protocol_plugin.openflow.core.IController;
 import org.opendaylight.controller.protocol_plugin.openflow.core.ISwitch;
-import org.opendaylight.controller.sal.connection.IPluginOutConnectionService;
 import org.opendaylight.controller.sal.core.Node;
 import org.opendaylight.controller.sal.core.NodeConnector;
 import org.opendaylight.controller.sal.core.Property;
@@ -45,11 +44,11 @@ public class InventoryService implements IInventoryShimInternalListener,
         IPluginInInventoryService, IInventoryProvider {
     protected static final Logger logger = LoggerFactory
             .getLogger(InventoryService.class);
-    private Set<IPluginOutInventoryService> pluginOutInventoryServices =
+    private final Set<IPluginOutInventoryService> pluginOutInventoryServices =
             new CopyOnWriteArraySet<IPluginOutInventoryService>();
     private IController controller = null;
-    private ConcurrentMap<Node, Map<String, Property>> nodeProps; // properties are maintained in global container only
-    private ConcurrentMap<NodeConnector, Map<String, Property>> nodeConnectorProps; // properties are maintained in global container only
+    private ConcurrentMap<Node, Map<String, Property>> nodeProps;
+    private ConcurrentMap<NodeConnector, Map<String, Property>> nodeConnectorProps;
     private boolean isDefaultContainer = false;
     private String containerName = null;
 
index bcb9e0b..f86f521 100644 (file)
@@ -48,7 +48,9 @@
               org.opendaylight.controller.sal.core,
               org.opendaylight.controller.sal.utils,
               org.opendaylight.controller.sal.packet,
+              org.opendaylight.controller.sal.reader,
               org.opendaylight.controller.sal.inventory,
+              org.opendaylight.controller.statisticsmanager,
               org.slf4j,
               org.apache.felix.dm,
               org.eclipse.osgi.framework.console,
       <artifactId>switchmanager</artifactId>
       <version>0.5.0-SNAPSHOT</version>
     </dependency>
+    <dependency>
+      <groupId>org.opendaylight.controller</groupId>
+      <artifactId>statisticsmanager</artifactId>
+      <version>0.4.0-SNAPSHOT</version>
+    </dependency>
   </dependencies>
 </project>
index b574269..0cb55d6 100644 (file)
@@ -21,6 +21,7 @@ import org.opendaylight.controller.configuration.IConfigurationContainerAware;
 import org.opendaylight.controller.sal.core.ComponentActivatorAbstractBase;
 import org.opendaylight.controller.sal.inventory.IInventoryService;
 import org.opendaylight.controller.sal.inventory.IListenInventoryUpdates;
+import org.opendaylight.controller.statisticsmanager.IStatisticsManager;
 import org.opendaylight.controller.switchmanager.IInventoryListener;
 import org.opendaylight.controller.switchmanager.ISpanAware;
 import org.opendaylight.controller.switchmanager.ISwitchManager;
@@ -106,6 +107,10 @@ public class Activator extends ComponentActivatorAbstractBase {
                     IInventoryService.class).setCallbacks(
                     "setInventoryService", "unsetInventoryService")
                     .setRequired(false));
+            c.add(createContainerServiceDependency(containerName).setService(
+                    IStatisticsManager.class).setCallbacks(
+                    "setStatisticsManager", "unsetStatisticsManager")
+                    .setRequired(false));
             c.add(createContainerServiceDependency(containerName).setService(
                     ISwitchManagerAware.class).setCallbacks(
                     "setSwitchManagerAware", "unsetSwitchManagerAware")
index 4c4adf0..6bd89a8 100644 (file)
@@ -54,6 +54,7 @@ import org.opendaylight.controller.sal.core.Tier;
 import org.opendaylight.controller.sal.core.UpdateType;
 import org.opendaylight.controller.sal.inventory.IInventoryService;
 import org.opendaylight.controller.sal.inventory.IListenInventoryUpdates;
+import org.opendaylight.controller.sal.reader.NodeDescription;
 import org.opendaylight.controller.sal.utils.GlobalConstants;
 import org.opendaylight.controller.sal.utils.HexEncode;
 import org.opendaylight.controller.sal.utils.IObjectReader;
@@ -61,6 +62,7 @@ import org.opendaylight.controller.sal.utils.ObjectReader;
 import org.opendaylight.controller.sal.utils.ObjectWriter;
 import org.opendaylight.controller.sal.utils.Status;
 import org.opendaylight.controller.sal.utils.StatusCode;
+import org.opendaylight.controller.statisticsmanager.IStatisticsManager;
 import org.opendaylight.controller.switchmanager.IInventoryListener;
 import org.opendaylight.controller.switchmanager.ISpanAware;
 import org.opendaylight.controller.switchmanager.ISwitchManager;
@@ -104,6 +106,7 @@ CommandProvider {
     private ConcurrentMap<Node, Map<String, NodeConnector>> nodeConnectorNames;
     private ConcurrentMap<String, Property> controllerProps;
     private IInventoryService inventoryService;
+    private IStatisticsManager statisticsManager;
     private final Set<ISwitchManagerAware> switchManagerAware = Collections
             .synchronizedSet(new HashSet<ISwitchManagerAware>());
     private final Set<IInventoryListener> inventoryListeners = Collections
@@ -698,18 +701,42 @@ CommandProvider {
         }
 
         Map<String, Property> updateProperties = switchConfig.getNodeProperties();
-        String nodeId = switchConfig.getNodeId();
         ForwardingMode mode = (ForwardingMode) updateProperties.get(ForwardingMode.name);
         if (mode != null) {
             if (isDefaultContainer) {
                 if (!mode.isValid()) {
-                    return new Status(StatusCode.BADREQUEST, "Invalid Forwarding Mode Value.");
+                    return new Status(StatusCode.BADREQUEST, "Invalid Forwarding Mode Value");
                 }
             } else {
                 return new Status(StatusCode.NOTACCEPTABLE,
                         "Forwarding Mode modification is allowed only in default container");
             }
         }
+
+        Description description = (Description) switchConfig.getProperty(Description.propertyName);
+        String nodeId = switchConfig.getNodeId();
+        Node node = Node.fromString(nodeId);
+        NodeDescription nodeDesc = (this.statisticsManager == null) ? null : this.statisticsManager
+                .getNodeDescription(node);
+        String advertisedDesc = (nodeDesc == null) ? "" : nodeDesc.getDescription();
+        if (description != null && description.getValue() != null) {
+            if (description.getValue().isEmpty() || description.getValue().equals(advertisedDesc)) {
+                updateProperties.remove(Description.propertyName);
+                switchConfig = new SwitchConfig(nodeId, updateProperties);
+            } else {
+                // check if description is configured or was published by any other node
+                for (Node n : nodeProps.keySet()) {
+                    Description desc = (Description) getNodeProp(n, Description.propertyName);
+                    NodeDescription nDesc = (this.statisticsManager == null) ? null : this.statisticsManager
+                            .getNodeDescription(n);
+                    String advDesc = (nDesc == null) ? "" : nDesc.getDescription();
+                    if ((description.equals(desc) || description.getValue().equals(advDesc)) && !node.equals(n)) {
+                        return new Status(StatusCode.CONFLICT, "Node name already in use");
+                    }
+                }
+            }
+        }
+
         boolean modeChange = false;
         SwitchConfig sc = nodeConfigList.get(nodeId);
         Map<String, Property> prevNodeProperties = new HashMap<String, Property>();
@@ -743,30 +770,27 @@ CommandProvider {
                 }
             }
         }
-        Node node = Node.fromString(nodeId);
         Map<String, Property> propMapCurr = nodeProps.get(node);
         if (propMapCurr == null) {
             return new Status(StatusCode.SUCCESS);
         }
         Map<String, Property> propMap = new HashMap<String, Property>(propMapCurr);
-        if (!prevNodeProperties.isEmpty()) {
-            for (String prop : prevNodeProperties.keySet()) {
-                if (!updateProperties.containsKey(prop)) {
-                    if (prop.equals(Description.propertyName)) {
-                        Map<Node, Map<String, Property>> nodeProp = this.inventoryService.getNodeProps();
-                        if (nodeProp.get(node) != null) {
-                            propMap.put(Description.propertyName, nodeProp.get(node).get(Description.propertyName));
-                            continue;
-                        }
+        for (String prop : prevNodeProperties.keySet()) {
+            if (!updateProperties.containsKey(prop)) {
+                if (prop.equals(Description.propertyName)) {
+                    if (!advertisedDesc.isEmpty()) {
+                        Property desc = new Description(advertisedDesc);
+                        propMap.put(Description.propertyName, desc);
                     }
-                    propMap.remove(prop);
+                    continue;
                 }
+                propMap.remove(prop);
             }
         }
         propMap.putAll(updateProperties);
         if (!nodeProps.replace(node, propMapCurr, propMap)) {
             // TODO rollback using Transactionality
-            return new Status(StatusCode.CONFLICT, "Cluster conflict: Unable to update node configuration.");
+            return new Status(StatusCode.CONFLICT, "Cluster conflict: Unable to update node configuration");
         }
         if (modeChange) {
             notifyModeChange(node, (mode == null) ? false : mode.isProactive());
@@ -887,7 +911,7 @@ CommandProvider {
     }
 
     @Override
-    public void entryUpdated(Long key, String new_value, String cacheName,
+    public void entryUpdated(Long key, String newValue, String cacheName,
             boolean originLocal) {
         saveSwitchConfigInternal();
     }
@@ -1446,9 +1470,13 @@ CommandProvider {
                         }
                     }
                     map.remove(name.getValue());
-                    if (!nodeConnectorNames.replace(node, mapCurr, map)) {
-                        log.warn("Cluster conflict: Unable remove Name property of nodeconnector {}, skip.",
-                                nodeConnector.getID());
+                    if (map.isEmpty()) {
+                        nodeConnectorNames.remove(node);
+                    } else {
+                        if (!nodeConnectorNames.replace(node, mapCurr, map)) {
+                            log.warn("Cluster conflict: Unable remove Name property of nodeconnector {}, skip.",
+                                    nodeConnector.getID());
+                        }
                     }
                 }
 
@@ -1532,6 +1560,16 @@ CommandProvider {
         clearInventories();
     }
 
+    public void setStatisticsManager(IStatisticsManager statisticsManager) {
+        log.trace("Got statistics manager set request {}", statisticsManager);
+        this.statisticsManager = statisticsManager;
+    }
+
+    public void unsetStatisticsManager(IStatisticsManager statisticsManager) {
+        log.trace("Got statistics manager UNset request");
+        this.statisticsManager = null;
+    }
+
     public void setSwitchManagerAware(ISwitchManagerAware service) {
         log.trace("Got inventory service set request {}", service);
         if (this.switchManagerAware != null) {
@@ -2024,12 +2062,6 @@ CommandProvider {
         nodeProps.put(node, propMap);
     }
 
-    private void removeNodeProps(Node node) {
-        if (getUpNodeConnectors(node).size() == 0) {
-            nodeProps.remove(node);
-        }
-    }
-
     @Override
     public Status saveConfiguration() {
         return saveSwitchConfig();
index 39a9cce..54dbbdc 100644 (file)
@@ -110,6 +110,10 @@ public class SwitchManagerIT {
                                 "protocol_plugins.stub").versionAsInProject(),
                 mavenBundle("org.opendaylight.controller", "switchmanager")
                         .versionAsInProject(),
+                mavenBundle("org.opendaylight.controller", "topologymanager").versionAsInProject(),
+                mavenBundle("org.opendaylight.controller", "hosttracker").versionAsInProject(),
+                mavenBundle("org.opendaylight.controller", "forwardingrulesmanager").versionAsInProject(),
+                mavenBundle("org.opendaylight.controller", "statisticsmanager").versionAsInProject(),
                 mavenBundle("org.opendaylight.controller",
                         "switchmanager.implementation").versionAsInProject(),
                 mavenBundle("org.jboss.spec.javax.transaction",