CORD-562 Don't allow nodes with duplicate hostname

And update existing node if newly added node has the same hostname with existing node. Change-Id: Ifebbf4129df4f742e16b9a25be619dd90e0745ff

CORD-562 Don't allow nodes with duplicate hostname
And update existing node if newly added node has the same hostname with existing node. Change-Id: Ifebbf4129df4f742e16b9a25be619dd90e0745ff
Hyunsun Moon · Gerrit Code Review
Commit af520d3c22ac6b5b8b67eae99081c2240da84ffa af520d3c 1 parent 95c3ee58
Showing 4 changed files with 88 additions and 25 deletions
apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnConfig.java
apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnNode.java
apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnNodeManager.java
apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnNodeState.java
--- a/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnConfig.java
View file @af520d3
+++ b/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnConfig.java
View file @af520d3
@@ -60,7 +60,7 @@ public class CordVtnConfig extends Config<ApplicationId> {
     /**
      * Returns the set of nodes read from network config.
      *
-     * @return set of CordVtnNodeConfig or null
+     * @return set of CordVtnNodeConfig or empty set
      */
     public Set<CordVtnNode> cordVtnNodes() {
@@ -68,7 +68,7 @@ public class CordVtnConfig extends Config<ApplicationId> {
         JsonNode jsonNodes = object.get(CORDVTN_NODES);
         if (jsonNodes == null) {
             log.debug("No CORD VTN nodes found");
-            return null;
+            return nodes;
         }
         for (JsonNode jsonNode : jsonNodes) {
@@ -94,7 +94,8 @@ public class CordVtnConfig extends Config<ApplicationId> {
                         TpPort.tpPort(Integer.parseInt(getConfig(object, OVSDB_PORT))),
                         sshInfo,
                         DeviceId.deviceId(getConfig(jsonNode, BRIDGE_ID)),
-                        getConfig(jsonNode, DATA_PLANE_INTF));
+                        getConfig(jsonNode, DATA_PLANE_INTF),
+                        CordVtnNodeState.noState());
                 log.info("Successfully read {} from the config", hostname);
                 nodes.add(newNode);
--- a/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnNode.java
View file @af520d3
+++ b/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnNode.java
View file @af520d3
 /*
- * Copyright 2014-2015 Open Networking Laboratory
+ * Copyright 2015-2016 Open Networking Laboratory
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -37,6 +37,7 @@ public final class CordVtnNode {
     private final SshAccessInfo sshInfo;
     private final DeviceId bridgeId;
     private final String dpIntf;
+    private final CordVtnNodeState state;
     public static final Comparator<CordVtnNode> CORDVTN_NODE_COMPARATOR =
             (node1, node2) -> node1.hostname().compareTo(node2.hostname());
@@ -52,10 +53,11 @@ public final class CordVtnNode {
      * @param sshInfo SSH access information
      * @param bridgeId integration bridge identifier
      * @param dpIntf data plane interface name
+     * @param state cordvtn node state
      */
     public CordVtnNode(String hostname, NetworkAddress hostMgmtIp, NetworkAddress localMgmtIp,
                        NetworkAddress dpIp, TpPort ovsdbPort, SshAccessInfo sshInfo,
-                       DeviceId bridgeId, String dpIntf) {
+                       DeviceId bridgeId, String dpIntf, CordVtnNodeState state) {
         this.hostname = checkNotNull(hostname, "hostname cannot be null");
         this.hostMgmtIp = checkNotNull(hostMgmtIp, "hostMgmtIp cannot be null");
         this.localMgmtIp = checkNotNull(localMgmtIp, "localMgmtIp cannot be null");
@@ -64,6 +66,23 @@ public final class CordVtnNode {
         this.sshInfo = checkNotNull(sshInfo, "sshInfo cannot be null");
         this.bridgeId = checkNotNull(bridgeId, "bridgeId cannot be null");
         this.dpIntf = checkNotNull(dpIntf, "dpIntf cannot be null");
+        this.state = state;
+    }
+
+    /**
+     * Returns cordvtn node with new state.
+     *
+     * @param node cordvtn node
+     * @param state cordvtn node init state
+     * @return cordvtn node
+     */
+    public static CordVtnNode getUpdatedNode(CordVtnNode node, CordVtnNodeState state) {
+        return new CordVtnNode(node.hostname,
+                               node.hostMgmtIp, node.localMgmtIp, node.dpIp,
+                               node.ovsdbPort,
+                               node.sshInfo,
+                               node.bridgeId,
+                               node.dpIntf, state);
     }
     /**
@@ -147,6 +166,15 @@ public final class CordVtnNode {
         return this.dpIntf;
     }
+    /**
+     * Returns the state of the node.
+     *
+     * @return state
+     */
+    public CordVtnNodeState state() {
+        return this.state;
+    }
+
     @Override
     public boolean equals(Object obj) {
         if (this == obj) {
@@ -180,6 +208,7 @@ public final class CordVtnNode {
                 .add("sshInfo", sshInfo)
                 .add("bridgeId", bridgeId)
                 .add("dpIntf", dpIntf)
+                .add("state", state)
                 .toString();
     }
 }
--- a/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnNodeManager.java
View file @af520d3
+++ b/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnNodeManager.java
View file @af520d3
@@ -72,6 +72,7 @@ import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
+import java.util.stream.Collectors;
 import static com.google.common.base.Preconditions.checkNotNull;
 import static java.util.concurrent.Executors.newSingleThreadScheduledExecutor;
@@ -165,11 +166,11 @@ public class CordVtnNodeManager {
     private final OvsdbHandler ovsdbHandler = new OvsdbHandler();
     private final BridgeHandler bridgeHandler = new BridgeHandler();
-    private ConsistentMap<CordVtnNode, NodeState> nodeStore;
+    private ConsistentMap<String, CordVtnNode> nodeStore;
     private CordVtnRuleInstaller ruleInstaller;
     private ApplicationId appId;
-    private enum NodeState {
+    private enum NodeState implements CordVtnNodeState {
         INIT {
             @Override
@@ -197,7 +198,6 @@ public class CordVtnNodeManager {
             public void process(CordVtnNodeManager nodeManager, CordVtnNode node) {
                 nodeManager.setIpAddress(node);
             }
-
         },
         COMPLETE {
             @Override
@@ -217,7 +217,7 @@ public class CordVtnNodeManager {
     @Activate
     protected void active() {
         appId = coreService.getAppId(CordVtnService.CORDVTN_APP_ID);
-        nodeStore = storageService.<CordVtnNode, NodeState>consistentMapBuilder()
+        nodeStore = storageService.<String, CordVtnNode>consistentMapBuilder()
                 .withSerializer(Serializer.using(NODE_SERIALIZER.build()))
                 .withName("cordvtn-nodestore")
                 .withApplicationId(appId)
@@ -252,7 +252,8 @@ public class CordVtnNodeManager {
     public void addNode(CordVtnNode node) {
         checkNotNull(node);
-        nodeStore.putIfAbsent(node, getNodeState(node));
+        // allow update node attributes
+        nodeStore.put(node.hostname(), CordVtnNode.getUpdatedNode(node, getNodeState(node)));
         initNode(node);
     }
@@ -268,7 +269,7 @@ public class CordVtnNodeManager {
             disconnectOvsdb(node);
         }
-        nodeStore.remove(node);
+        nodeStore.remove(node.hostname());
     }
     /**
@@ -279,12 +280,13 @@ public class CordVtnNodeManager {
     public void initNode(CordVtnNode node) {
         checkNotNull(node);
-        if (!nodeStore.containsKey(node)) {
+        if (!nodeStore.containsKey(node.hostname())) {
             log.warn("Node {} does not exist, add node first", node.hostname());
             return;
         }
         NodeState state = getNodeState(node);
+        log.debug("Init node: {} state: {}", node.hostname(), state.toString());
         state.process(this, node);
     }
@@ -296,7 +298,7 @@ public class CordVtnNodeManager {
      */
     public boolean isNodeInitComplete(CordVtnNode node) {
         checkNotNull(node);
-        return nodeStore.containsKey(node) && getNodeState(node).equals(NodeState.COMPLETE);
+        return nodeStore.containsKey(node.hostname()) && getNodeState(node).equals(NodeState.COMPLETE);
     }
     /**
@@ -311,8 +313,9 @@ public class CordVtnNodeManager {
         // the state saved in nodeStore can be wrong if IP address settings are changed
         // after the node init has been completed since there's no way to detect it
         // getNodeState and checkNodeInitState always return correct answer but can be slow
-        Versioned<NodeState> state = nodeStore.get(node);
+        Versioned<CordVtnNode> versionedNode = nodeStore.get(node.hostname());
-        return state != null && state.value().equals(NodeState.COMPLETE);
+        CordVtnNodeState state = versionedNode.value().state();
+        return state != null && state.equals(NodeState.COMPLETE);
     }
     /**
@@ -324,7 +327,7 @@ public class CordVtnNodeManager {
     public String checkNodeInitState(CordVtnNode node) {
         checkNotNull(node);
-        if (!nodeStore.containsKey(node)) {
+        if (!nodeStore.containsKey(node.hostname())) {
             log.warn("Node {} does not exist, add node first", node.hostname());
             return null;
         }
@@ -337,13 +340,13 @@ public class CordVtnNodeManager {
         Set<IpAddress> intBrIps = RemoteIpCommandUtil.getCurrentIps(session, DEFAULT_BRIDGE);
         String result = String.format(
-                "Integration bridge created/connected : %s (%s)%n" +
+                "br-int created and connected : %s (%s)%n" +
                         "VXLAN interface created : %s%n" +
                         "Data plane interface added : %s (%s)%n" +
                         "IP flushed from %s : %s%n" +
                         "Data plane IP added to br-int : %s (%s)%n" +
                         "Local management IP added to br-int : %s (%s)",
-                isBrIntCreated(node) ? OK : NO, DEFAULT_BRIDGE,
+                isBrIntCreated(node) ? OK : NO, node.intBrId(),
                 isTunnelIntfCreated(node) ? OK : NO,
                 isDataPlaneIntfAdded(node) ? OK : NO, node.dpIntf(),
                 node.dpIntf(),
@@ -371,9 +374,9 @@ public class CordVtnNodeManager {
      * @return list of nodes
      */
     public List<CordVtnNode> getNodes() {
-        List<CordVtnNode> nodes = new ArrayList<>();
+        return nodeStore.values().stream()
-        nodes.addAll(nodeStore.keySet());
+                .map(Versioned::value)
-        return nodes;
+                .collect(Collectors.toList());
     }
     /**
@@ -410,8 +413,7 @@ public class CordVtnNodeManager {
         checkNotNull(node);
         log.debug("Changed {} state: {}", node.hostname(), newState.toString());
-
+        nodeStore.put(node.hostname(), CordVtnNode.getUpdatedNode(node, newState));
-        nodeStore.put(node, newState);
         newState.process(this, node);
     }
@@ -497,7 +499,7 @@ public class CordVtnNodeManager {
     private void connectOvsdb(CordVtnNode node) {
         checkNotNull(node);
-        if (!nodeStore.containsKey(node)) {
+        if (!nodeStore.containsKey(node.hostname())) {
             log.warn("Node {} does not exist", node.hostname());
             return;
         }
@@ -515,7 +517,7 @@ public class CordVtnNodeManager {
     private void disconnectOvsdb(CordVtnNode node) {
         checkNotNull(node);
-        if (!nodeStore.containsKey(node)) {
+        if (!nodeStore.containsKey(node.hostname())) {
             log.warn("Node {} does not exist", node.hostname());
             return;
         }
@@ -747,6 +749,7 @@ public class CordVtnNodeManager {
         @Override
         public void disconnected(Device device) {
             if (!deviceService.isAvailable(device.id())) {
+                log.debug("Device {} is disconnected", device.id());
                 adminService.removeDevice(device.id());
             }
         }
--- a/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnNodeState.java 0 → 100644
View file @af520d3
+++ b/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnNodeState.java 0 → 100644
View file @af520d3
+/*
+ * Copyright 2016 Open Networking Laboratory
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.onosproject.cordvtn;
+
+/**
+ * Entity that defines possible init state of the cordvtn node.
+ */
+public interface CordVtnNodeState {
+    /**
+     * Returns null for no state.
+     *
+     * @return null
+     */
+    static CordVtnNodeState noState() {
+        return null;
+    }
+}