Some improvements on cordvtn

- Removed unnecessary CordVtnNodeConfig class
- Don't allow local and host management IP range overlapping
- Check node init state saved in the store instead of really check when a
  VM is detected or vanished since it's too slow

Change-Id: I076780bdc3946b2000176cb05805003ba7c8724d
diff --git a/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtn.java b/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtn.java
index 39a5e69..4267838 100644
--- a/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtn.java
+++ b/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtn.java
@@ -70,9 +70,11 @@
 
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.concurrent.ExecutorService;
 import java.util.stream.Collectors;
+import java.util.stream.StreamSupport;
 
 import static com.google.common.base.Preconditions.checkNotNull;
 import static java.util.concurrent.Executors.newSingleThreadScheduledExecutor;
@@ -266,7 +268,7 @@
             }
         }
 
-        Set<IpAddress> ip = Sets.newHashSet(vPort.fixedIps().values());
+        Set<IpAddress> fixedIp = Sets.newHashSet(vPort.fixedIps().values());
         DefaultAnnotations.Builder annotations = DefaultAnnotations.builder()
                 .set(SERVICE_ID, vPort.networkId())
                 .set(OPENSTACK_VM_ID, vPort.deviceId())
@@ -283,7 +285,7 @@
                 mac,
                 VlanId.NONE,
                 new HostLocation(connectPoint, System.currentTimeMillis()),
-                ip,
+                fixedIp,
                 annotations.build());
 
         hostProvider.hostDetected(hostId, hostDesc, false);
@@ -357,6 +359,30 @@
     }
 
     /**
+     * Returns public ip addresses of vSGs running inside a give vSG host.
+     *
+     * @param vSgHost vSG host
+     * @return map of ip and mac address, or empty map
+     */
+    private Map<IpAddress, MacAddress> getSubscriberGateways(Host vSgHost) {
+        String vPortId = vSgHost.annotations().value(OPENSTACK_PORT_ID);
+        String serviceVlan = vSgHost.annotations().value(S_TAG);
+
+        OpenstackPort vPort = openstackService.port(vPortId);
+        if (vPort == null) {
+            log.warn("Failed to get OpenStack port {} for VM {}", vPortId, vSgHost.id());
+            return Maps.newHashMap();
+        }
+
+        if (!serviceVlan.equals(getServiceVlan(vPort))) {
+            log.error("Host({}) s-tag does not match with vPort s-tag", vSgHost.id());
+            return Maps.newHashMap();
+        }
+
+        return vPort.allowedAddressPairs();
+    }
+
+    /**
      * Returns CordService by service ID.
      *
      * @param serviceId service id
@@ -453,6 +479,16 @@
     }
 
     /**
+     * Returns service ID of this host.
+     *
+     * @param host host
+     * @return service id, or null if not found
+     */
+    private String getServiceId(Host host) {
+        return host.annotations().value(SERVICE_ID);
+    }
+
+    /**
      * Returns hosts associated with a given OpenStack network.
      *
      * @param vNet openstack network
@@ -461,40 +497,10 @@
     private Set<Host> getHostsWithOpenstackNetwork(OpenstackNetwork vNet) {
         checkNotNull(vNet);
 
-        Set<Host> hosts = openstackService.ports(vNet.id()).stream()
-                .filter(port -> port.deviceOwner().contains("compute"))
-                .map(port -> hostService.getHostsByMac(port.macAddress())
-                        .stream()
-                        .findFirst()
-                        .orElse(null))
+        String vNetId = vNet.id();
+        return StreamSupport.stream(hostService.getHosts().spliterator(), false)
+                .filter(host -> Objects.equals(vNetId, getServiceId(host)))
                 .collect(Collectors.toSet());
-
-        hosts.remove(null);
-        return hosts;
-    }
-
-    /**
-     * Returns public ip addresses of vSGs running inside a give vSG host.
-     *
-     * @param vSgHost vSG host
-     * @return map of ip and mac address, or empty map
-     */
-    private Map<IpAddress, MacAddress> getSubscriberGateways(Host vSgHost) {
-        String vPortId = vSgHost.annotations().value(OPENSTACK_PORT_ID);
-        String serviceVlan = vSgHost.annotations().value(S_TAG);
-
-        OpenstackPort vPort = openstackService.port(vPortId);
-        if (vPort == null) {
-            log.warn("Failed to get OpenStack port {} for VM {}", vPortId, vSgHost.id());
-            return Maps.newHashMap();
-        }
-
-        if (!serviceVlan.equals(getServiceVlan(vPort))) {
-            log.error("Host({}) s-tag does not match with vPort s-tag", vSgHost.id());
-            return Maps.newHashMap();
-        }
-
-        return vPort.allowedAddressPairs();
     }
 
     /**
diff --git a/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnConfig.java b/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnConfig.java
index ec83432..a44ec21 100644
--- a/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnConfig.java
+++ b/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnConfig.java
@@ -18,6 +18,7 @@
 import com.fasterxml.jackson.databind.JsonNode;
 import com.google.common.collect.Maps;
 import com.google.common.collect.Sets;
+import org.onlab.packet.Ip4Address;
 import org.onlab.packet.IpAddress;
 import org.onlab.packet.MacAddress;
 import org.onlab.packet.TpPort;
@@ -29,7 +30,6 @@
 import java.util.Map;
 import java.util.Set;
 
-import static com.google.common.base.Preconditions.checkNotNull;
 import static org.slf4j.LoggerFactory.getLogger;
 
 /**
@@ -45,11 +45,12 @@
     public static final String GATEWAY_MAC = "gatewayMac";
     public static final String LOCAL_MANAGEMENT_IP = "localManagementIp";
     public static final String OVSDB_PORT = "ovsdbPort";
+
     public static final String SSH_PORT = "sshPort";
     public static final String SSH_USER = "sshUser";
     public static final String SSH_KEY_FILE = "sshKeyFile";
-    public static final String CORDVTN_NODES = "nodes";
 
+    public static final String CORDVTN_NODES = "nodes";
     public static final String HOSTNAME = "hostname";
     public static final String HOST_MANAGEMENT_IP = "hostManagementIp";
     public static final String DATA_PLANE_IP = "dataPlaneIp";
@@ -61,31 +62,70 @@
      *
      * @return set of CordVtnNodeConfig or null
      */
-    public Set<CordVtnNodeConfig> cordVtnNodes() {
-        Set<CordVtnNodeConfig> nodes = Sets.newHashSet();
+    public Set<CordVtnNode> cordVtnNodes() {
 
+        Set<CordVtnNode> nodes = Sets.newHashSet();
         JsonNode jsonNodes = object.get(CORDVTN_NODES);
         if (jsonNodes == null) {
+            log.debug("No CORD VTN nodes found");
             return null;
         }
 
-        jsonNodes.forEach(jsonNode -> {
+        for (JsonNode jsonNode : jsonNodes) {
             try {
-                nodes.add(new CordVtnNodeConfig(
-                        jsonNode.path(HOSTNAME).asText(),
-                        NetworkAddress.valueOf(jsonNode.path(HOST_MANAGEMENT_IP).asText()),
-                        NetworkAddress.valueOf(jsonNode.path(DATA_PLANE_IP).asText()),
-                        jsonNode.path(DATA_PLANE_INTF).asText(),
-                        DeviceId.deviceId(jsonNode.path(BRIDGE_ID).asText())));
+                NetworkAddress hostMgmt = NetworkAddress.valueOf(getConfig(jsonNode, HOST_MANAGEMENT_IP));
+                NetworkAddress localMgmt = NetworkAddress.valueOf(getConfig(object, LOCAL_MANAGEMENT_IP));
+                if (hostMgmt.prefix().contains(localMgmt.prefix()) ||
+                        localMgmt.prefix().contains(hostMgmt.prefix())) {
+                    log.error("hostMamt and localMgmt cannot be overlapped, skip this node");
+                    continue;
+                }
+
+                Ip4Address hostMgmtIp = hostMgmt.ip().getIp4Address();
+                SshAccessInfo sshInfo = new SshAccessInfo(
+                        hostMgmtIp,
+                        TpPort.tpPort(Integer.parseInt(getConfig(object, SSH_PORT))),
+                        getConfig(object, SSH_USER), getConfig(object, SSH_KEY_FILE));
+
+                String hostname = getConfig(jsonNode, HOSTNAME);
+                CordVtnNode newNode = new CordVtnNode(
+                        hostname, hostMgmt, localMgmt,
+                        NetworkAddress.valueOf(getConfig(jsonNode, DATA_PLANE_IP)),
+                        TpPort.tpPort(Integer.parseInt(getConfig(object, OVSDB_PORT))),
+                        sshInfo,
+                        DeviceId.deviceId(getConfig(jsonNode, BRIDGE_ID)),
+                        getConfig(jsonNode, DATA_PLANE_INTF));
+
+                log.info("Successfully read {} from the config", hostname);
+                nodes.add(newNode);
             } catch (IllegalArgumentException | NullPointerException e) {
-                log.error("Failed to read {}", e.toString());
+                log.error("{}", e.toString());
             }
-        });
+        }
 
         return nodes;
     }
 
     /**
+     * Returns value of a given path. If the path is missing, show log and return
+     * null.
+     *
+     * @param path path
+     * @return value or null
+     */
+    private String getConfig(JsonNode jsonNode, String path) {
+        jsonNode = jsonNode.path(path);
+
+        if (jsonNode.isMissingNode()) {
+            log.error("{} is not configured", path);
+            return null;
+        } else {
+            log.debug("{} : {}", path, jsonNode.asText());
+            return jsonNode.asText();
+        }
+    }
+
+    /**
      * Returns private network gateway MAC address.
      *
      * @return mac address, or null
@@ -112,7 +152,7 @@
     public Map<IpAddress, MacAddress> publicGateways() {
         JsonNode jsonNodes = object.get(PUBLIC_GATEWAYS);
         if (jsonNodes == null) {
-            return null;
+            return Maps.newHashMap();
         }
 
         Map<IpAddress, MacAddress> publicGateways = Maps.newHashMap();
@@ -128,155 +168,5 @@
 
         return publicGateways;
     }
-
-    /**
-     * Returns local management network address.
-     *
-     * @return network address
-     */
-    public NetworkAddress localMgmtIp() {
-        JsonNode jsonNode = object.get(LOCAL_MANAGEMENT_IP);
-        if (jsonNode == null) {
-            return null;
-        }
-
-        try {
-            return NetworkAddress.valueOf(jsonNode.asText());
-        } catch (IllegalArgumentException e) {
-            log.error("Wrong address format {}", jsonNode.asText());
-            return null;
-        }
-    }
-
-    /**
-     * Returns the port number used for OVSDB connection.
-     *
-     * @return port number, or null
-     */
-    public TpPort ovsdbPort() {
-        JsonNode jsonNode = object.get(OVSDB_PORT);
-        if (jsonNode == null) {
-            return null;
-        }
-
-        try {
-            return TpPort.tpPort(jsonNode.asInt());
-        } catch (IllegalArgumentException e) {
-            log.error("Wrong TCP port format {}", jsonNode.asText());
-            return null;
-        }
-    }
-
-    /**
-     * Returns the port number used for SSH connection.
-     *
-     * @return port number, or null
-     */
-    public TpPort sshPort() {
-        JsonNode jsonNode = object.get(SSH_PORT);
-        if (jsonNode == null) {
-            return null;
-        }
-
-        try {
-            return TpPort.tpPort(jsonNode.asInt());
-        } catch (IllegalArgumentException e) {
-            log.error("Wrong TCP port format {}", jsonNode.asText());
-            return null;
-        }
-    }
-
-    /**
-     * Returns the user name for SSH connection.
-     *
-     * @return user name, or null
-     */
-    public String sshUser() {
-        JsonNode jsonNode = object.get(SSH_USER);
-        if (jsonNode == null) {
-            return null;
-        }
-
-        return jsonNode.asText();
-    }
-
-    /**
-     * Returns the private key file for SSH connection.
-     *
-     * @return file path, or null
-     */
-    public String sshKeyFile() {
-        JsonNode jsonNode = object.get(SSH_KEY_FILE);
-        if (jsonNode == null) {
-            return null;
-        }
-
-        return jsonNode.asText();
-    }
-
-    /**
-     * Configuration for CordVtn node.
-     */
-    public static class CordVtnNodeConfig {
-
-        private final String hostname;
-        private final NetworkAddress hostMgmtIp;
-        private final NetworkAddress dpIp;
-        private final String dpIntf;
-        private final DeviceId bridgeId;
-
-        public CordVtnNodeConfig(String hostname, NetworkAddress hostMgmtIp, NetworkAddress dpIp,
-                                 String dpIntf, DeviceId bridgeId) {
-            this.hostname = checkNotNull(hostname);
-            this.hostMgmtIp = checkNotNull(hostMgmtIp);
-            this.dpIp = checkNotNull(dpIp);
-            this.dpIntf = checkNotNull(dpIntf);
-            this.bridgeId = checkNotNull(bridgeId);
-        }
-
-        /**
-         * Returns hostname of the node.
-         *
-         * @return hostname
-         */
-        public String hostname() {
-            return this.hostname;
-        }
-
-        /**
-         * Returns the host management network address of the node.
-         *
-         * @return management network address
-         */
-        public NetworkAddress hostMgmtIp() {
-            return this.hostMgmtIp;
-        }
-
-        /**
-         * Returns the data plane network address.
-         *
-         * @return network address
-         */
-        public NetworkAddress dpIp() {
-            return this.dpIp;
-        }
-
-        /**
-         * Returns the data plane interface name.
-         *
-         * @return interface name
-         */
-        public String dpIntf() {
-            return this.dpIntf;
-        }
-
-        /**
-         * Returns integration bridge id of the node.
-         *
-         * @return device id
-         */
-        public DeviceId bridgeId() {
-            return this.bridgeId;
-        }
-    }
 }
+
diff --git a/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnNodeManager.java b/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnNodeManager.java
index 9dd8c24..954e55b 100644
--- a/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnNodeManager.java
+++ b/apps/cordvtn/src/main/java/org/onosproject/cordvtn/CordVtnNodeManager.java
@@ -24,7 +24,6 @@
 import org.apache.felix.scr.annotations.ReferenceCardinality;
 import org.apache.felix.scr.annotations.Service;
 import org.onlab.packet.IpAddress;
-import org.onlab.packet.TpPort;
 import org.onlab.util.ItemNotFoundException;
 import org.onlab.util.KryoNamespace;
 import org.onosproject.cluster.ClusterService;
@@ -64,6 +63,7 @@
 import org.onosproject.store.service.ConsistentMap;
 import org.onosproject.store.service.Serializer;
 import org.onosproject.store.service.StorageService;
+import org.onosproject.store.service.Versioned;
 import org.slf4j.Logger;
 
 import java.util.ArrayList;
@@ -300,6 +300,22 @@
     }
 
     /**
+     * Returns if current node state saved in nodeStore is COMPLETE or not.
+     *
+     * @param node cordvtn node
+     * @return true if it's complete state, otherwise false
+     */
+    private boolean isNodeStateComplete(CordVtnNode node) {
+        checkNotNull(node);
+
+        // 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);
+        return state != null && state.value().equals(NodeState.COMPLETE);
+    }
+
+    /**
      * Returns detailed node initialization state.
      *
      * @param node cordvtn node
@@ -771,7 +787,7 @@
             log.debug("Port {} is added to {}", portName, node.hostname());
 
             if (portName.startsWith(VPORT_PREFIX)) {
-                if (isNodeInitComplete(node)) {
+                if (isNodeStateComplete(node)) {
                     cordVtnService.addServiceVm(node, getConnectPoint(port));
                 } else {
                     log.debug("VM is detected on incomplete node, ignore it.", portName);
@@ -799,7 +815,7 @@
             log.debug("Port {} is removed from {}", portName, node.hostname());
 
             if (portName.startsWith(VPORT_PREFIX)) {
-                if (isNodeInitComplete(node)) {
+                if (isNodeStateComplete(node)) {
                     cordVtnService.removeServiceVm(getConnectPoint(port));
                 } else {
                     log.debug("VM is vanished from incomplete node, ignore it.", portName);
@@ -847,36 +863,12 @@
      */
     private void readConfiguration() {
         CordVtnConfig config = configRegistry.getConfig(appId, CordVtnConfig.class);
-
         if (config == null) {
             log.debug("No configuration found");
             return;
         }
 
-        NetworkAddress localMgmtIp = config.localMgmtIp();
-        TpPort ovsdbPort = config.ovsdbPort();
-        TpPort sshPort = config.sshPort();
-        String sshUser = config.sshUser();
-        String sshKeyFile = config.sshKeyFile();
-
-        config.cordVtnNodes().forEach(node -> {
-            log.debug("Read node {}", node.hostname());
-            CordVtnNode cordVtnNode = new CordVtnNode(
-                    node.hostname(),
-                    node.hostMgmtIp(),
-                    localMgmtIp,
-                    node.dpIp(),
-                    ovsdbPort,
-                    new SshAccessInfo(node.hostMgmtIp().ip().getIp4Address(),
-                                      sshPort,
-                                      sshUser,
-                                      sshKeyFile),
-                    node.bridgeId(),
-                    node.dpIntf());
-
-            addNode(cordVtnNode);
-        });
-
+        config.cordVtnNodes().forEach(this::addNode);
         // TODO remove nodes if needed
     }
 
diff --git a/apps/cordvtn/src/main/java/org/onosproject/cordvtn/SshAccessInfo.java b/apps/cordvtn/src/main/java/org/onosproject/cordvtn/SshAccessInfo.java
index 094e792..6f9494d 100644
--- a/apps/cordvtn/src/main/java/org/onosproject/cordvtn/SshAccessInfo.java
+++ b/apps/cordvtn/src/main/java/org/onosproject/cordvtn/SshAccessInfo.java
@@ -21,6 +21,8 @@
 
 import java.util.Objects;
 
+import static com.google.common.base.Preconditions.checkNotNull;
+
 /**
  * Representation of SSH access information.
  */
@@ -40,10 +42,10 @@
      * @param privateKey path of ssh private key
      */
     public SshAccessInfo(Ip4Address remoteIp, TpPort port, String user, String privateKey) {
-        this.remoteIp = remoteIp;
-        this.port = port;
-        this.user = user;
-        this.privateKey = privateKey;
+        this.remoteIp = checkNotNull(remoteIp);
+        this.port = checkNotNull(port);
+        this.user = checkNotNull(user);
+        this.privateKey = checkNotNull(privateKey);
     }
 
     /**
diff --git a/apps/cordvtn/src/main/java/org/onosproject/cordvtn/rest/NeutronMl2NetworksWebResource.java b/apps/cordvtn/src/main/java/org/onosproject/cordvtn/rest/NeutronMl2NetworksWebResource.java
index 16bbc61..407926b 100644
--- a/apps/cordvtn/src/main/java/org/onosproject/cordvtn/rest/NeutronMl2NetworksWebResource.java
+++ b/apps/cordvtn/src/main/java/org/onosproject/cordvtn/rest/NeutronMl2NetworksWebResource.java
@@ -43,7 +43,7 @@
     @Consumes(MediaType.APPLICATION_JSON)
     @Produces(MediaType.APPLICATION_JSON)
     public Response createNetwork(InputStream input) {
-        log.debug(String.format(NETWORKS_MESSAGE, "create"));
+        log.trace(String.format(NETWORKS_MESSAGE, "create"));
         return Response.status(Response.Status.OK).build();
     }
 
@@ -52,7 +52,7 @@
     @Consumes(MediaType.APPLICATION_JSON)
     @Produces(MediaType.APPLICATION_JSON)
     public Response updateNetwork(@PathParam("id") String id, InputStream input) {
-        log.debug(String.format(NETWORKS_MESSAGE, "update"));
+        log.trace(String.format(NETWORKS_MESSAGE, "update"));
         return Response.status(Response.Status.OK).build();
     }
 
@@ -60,7 +60,7 @@
     @Path("{id}")
     @Produces(MediaType.APPLICATION_JSON)
     public Response deleteNetwork(@PathParam("id") String id) {
-        log.debug(String.format(NETWORKS_MESSAGE, "delete"));
+        log.trace(String.format(NETWORKS_MESSAGE, "delete"));
         return Response.status(Response.Status.OK).build();
     }
 }
diff --git a/apps/cordvtn/src/main/java/org/onosproject/cordvtn/rest/NeutronMl2PortsWebResource.java b/apps/cordvtn/src/main/java/org/onosproject/cordvtn/rest/NeutronMl2PortsWebResource.java
index 21bbda4..7c7c356 100644
--- a/apps/cordvtn/src/main/java/org/onosproject/cordvtn/rest/NeutronMl2PortsWebResource.java
+++ b/apps/cordvtn/src/main/java/org/onosproject/cordvtn/rest/NeutronMl2PortsWebResource.java
@@ -63,7 +63,7 @@
     @Consumes(MediaType.APPLICATION_JSON)
     @Produces(MediaType.APPLICATION_JSON)
     public Response createPorts(InputStream input) {
-        log.debug(String.format(PORTS_MESSAGE, "create"));
+        log.trace(String.format(PORTS_MESSAGE, "create"));
         return Response.status(Response.Status.OK).build();
     }
 
@@ -110,7 +110,7 @@
     @DELETE
     @Produces(MediaType.APPLICATION_JSON)
     public Response deletePorts(@PathParam("id") String id) {
-        log.debug(String.format(PORTS_MESSAGE, "delete"));
+        log.trace(String.format(PORTS_MESSAGE, "delete"));
         return Response.status(Response.Status.OK).build();
     }
 }
diff --git a/apps/cordvtn/src/main/java/org/onosproject/cordvtn/rest/NeutronMl2SubnetsWebResource.java b/apps/cordvtn/src/main/java/org/onosproject/cordvtn/rest/NeutronMl2SubnetsWebResource.java
index 188ec9d..2c2a905 100644
--- a/apps/cordvtn/src/main/java/org/onosproject/cordvtn/rest/NeutronMl2SubnetsWebResource.java
+++ b/apps/cordvtn/src/main/java/org/onosproject/cordvtn/rest/NeutronMl2SubnetsWebResource.java
@@ -43,7 +43,7 @@
     @Consumes(MediaType.APPLICATION_JSON)
     @Produces(MediaType.APPLICATION_JSON)
     public Response createSubnet(InputStream input) {
-        log.debug(String.format(SUBNETS_MESSAGE, "create"));
+        log.trace(String.format(SUBNETS_MESSAGE, "create"));
         return Response.status(Response.Status.OK).build();
     }
 
@@ -53,7 +53,7 @@
     @Produces(MediaType.APPLICATION_JSON)
     @Consumes(MediaType.APPLICATION_JSON)
     public Response updateSubnet(@PathParam("id") String id, InputStream input) {
-        log.debug(String.format(SUBNETS_MESSAGE, "update"));
+        log.trace(String.format(SUBNETS_MESSAGE, "update"));
         return Response.status(Response.Status.OK).build();
 
     }
@@ -62,7 +62,7 @@
     @Path("{id}")
     @Produces(MediaType.APPLICATION_JSON)
     public Response deleteSubnet(@PathParam("id") String id) {
-        log.debug(String.format(SUBNETS_MESSAGE, "delete"));
+        log.trace(String.format(SUBNETS_MESSAGE, "delete"));
         return Response.status(Response.Status.OK).build();
     }
 }