Fix: obtain the correct external gateway NAT IP address

1. Add verbose message in stateful SNAT rules installation phase
2. Refactor openstack networking util

Change-Id: Ia74a529657bf0e7b34957053e1411786ba3fe0d5
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingArpHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingArpHandler.java
index b6bde75..04e6b62 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingArpHandler.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingArpHandler.java
@@ -96,9 +96,9 @@
 import static org.onosproject.openstacknetworking.impl.OsgiPropertyConstants.ARP_MODE_DEFAULT;
 import static org.onosproject.openstacknetworking.impl.OsgiPropertyConstants.GATEWAY_MAC_DEFAULT;
 import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.associatedFloatingIp;
+import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.externalGatewayIp;
 import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.externalPeerRouterForNetwork;
 import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.floatingIpByInstancePort;
-import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.getExternalIp;
 import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.getGwByComputeDevId;
 import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.getGwByInstancePort;
 import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.getPropertyValue;
@@ -539,7 +539,7 @@
 
     private void setFakeGatewayArpRuleByRouter(Router router, boolean install) {
         if (ARP_BROADCAST_MODE.equals(getArpMode())) {
-            IpAddress externalIp = getExternalIp(router, osNetworkService);
+            IpAddress externalIp = externalGatewayIp(router, osNetworkService);
 
             if (externalIp == null) {
                 log.debug("External IP is not found");
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingSnatHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingSnatHandler.java
index d835cca..4a8dc7c 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingSnatHandler.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingSnatHandler.java
@@ -116,10 +116,10 @@
 import static org.onosproject.openstacknetworking.api.OpenstackNetwork.Type.VLAN;
 import static org.onosproject.openstacknetworking.impl.OsgiPropertyConstants.USE_STATEFUL_SNAT;
 import static org.onosproject.openstacknetworking.impl.OsgiPropertyConstants.USE_STATEFUL_SNAT_DEFAULT;
-import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.externalIpFromSubnet;
+import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.externalGatewayIpSnatEnabled;
 import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.externalPeerRouterFromSubnet;
-import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.getExternalIp;
 import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.getPropertyValueAsBoolean;
+import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.getRouterFromSubnet;
 import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.tunnelPortNumByNetType;
 import static org.onosproject.openstacknetworking.util.RulePopulatorUtil.CT_NAT_SRC_FLAG;
 import static org.onosproject.openstacknetworking.util.RulePopulatorUtil.buildExtension;
@@ -298,8 +298,17 @@
 
         IpAddress srcIp = IpAddress.valueOf(iPacket.getSourceAddress());
         Subnet srcSubnet = getSourceSubnet(srcInstPort, srcIp);
+
+        Router osRouter = getRouterFromSubnet(srcSubnet, osRouterService);
+
+        if (osRouter == null || osRouter.getExternalGatewayInfo() == null) {
+            // this router does not have external connectivity
+            log.warn("No router is associated with the given subnet {}", srcSubnet);
+            return;
+        }
+
         IpAddress externalGatewayIp =
-                externalIpFromSubnet(srcSubnet, osRouterService, osNetworkService);
+                externalGatewayIpSnatEnabled(osRouter, osNetworkAdminService);
 
         if (externalGatewayIp == null) {
             return;
@@ -804,36 +813,38 @@
         Type netType = osNetworkAdminService.networkType(osSubnet.getNetworkId());
 
         if (netType == FLAT) {
+            log.warn("FLAT typed network does not need SNAT rules");
             return;
         }
 
         Optional<Router> osRouter = osRouterService.routers().stream()
-                .filter(router -> osRouterService.routerInterfaces(routerIface.getId()) != null)
+                .filter(router -> routerIface.getId().equals(router.getId()))
                 .findAny();
 
         if (!osRouter.isPresent()) {
-            log.error("Cannot find a router for router interface {} ", routerIface);
+            log.warn("Cannot find a router attached with the given router interface {} ", routerIface);
             return;
         }
 
-        IpAddress natAddress = getExternalIp(osRouter.get(), osNetworkService);
+        IpAddress natAddress = externalGatewayIpSnatEnabled(osRouter.get(), osNetworkAdminService);
         if (natAddress == null) {
+            log.warn("NAT address is not found");
             return;
         }
 
         IpAddress extRouterAddress = getGatewayIpAddress(osRouter.get());
         if (extRouterAddress == null) {
+            log.warn("External router address is not found");
             return;
         }
 
         ExternalPeerRouter externalPeerRouter =
                 osNetworkService.externalPeerRouter(extRouterAddress);
         if (externalPeerRouter == null) {
+            log.warn("External peer router not found");
             return;
         }
 
-        String netId = osNetworkAdminService.subnet(routerIface.getSubnetId()).getNetworkId();
-
         Map<OpenstackNode, PortRange> gwPortRangeMap = getAssignedPortsForGateway(
                 ImmutableList.copyOf(osNodeService.nodes(GATEWAY)));
 
@@ -865,7 +876,7 @@
             return;
         }
 
-        IpAddress natAddress = getExternalIp(osRouter, osNetworkAdminService);
+        IpAddress natAddress = externalGatewayIpSnatEnabled(osRouter, osNetworkAdminService);
         if (natAddress == null) {
             return;
         }
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingSnatIcmpHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingSnatIcmpHandler.java
index 5e89e4b..6b9adac 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingSnatIcmpHandler.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackRoutingSnatIcmpHandler.java
@@ -85,9 +85,10 @@
 import static org.onosproject.openstacknetworking.api.Constants.OPENSTACK_NETWORKING_APP_ID;
 import static org.onosproject.openstacknetworking.api.Constants.PRIORITY_INTERNAL_ROUTING_RULE;
 import static org.onosproject.openstacknetworking.impl.OsgiPropertyConstants.USE_STATEFUL_SNAT;
-import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.externalIpFromSubnet;
+import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.externalGatewayIp;
 import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.externalPeerRouterFromSubnet;
 import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.getPropertyValueAsBoolean;
+import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.getRouterFromSubnet;
 import static org.onosproject.openstacknode.api.OpenstackNode.NodeType.GATEWAY;
 import static org.slf4j.LoggerFactory.getLogger;
 
@@ -347,9 +348,17 @@
                 // this is a request to an external network
                 log.trace("Icmp request to external {} from {}", dstIp, srcIp);
 
-                IpAddress externalIp = externalIpFromSubnet(srcSubnet,
-                                            osRouterService, osNetworkService);
-                if (externalIp == null) {
+                Router osRouter = getRouterFromSubnet(srcSubnet, osRouterService);
+
+                if (osRouter == null || osRouter.getExternalGatewayInfo() == null) {
+                    // this router does not have external connectivity
+                    log.warn("No router is associated with the given subnet {}", srcSubnet);
+                    return false;
+                }
+
+                IpAddress externalGatewayIp = externalGatewayIp(osRouter, osNetworkService);
+
+                if (externalGatewayIp == null) {
                     log.warn(ERR_REQ + "failed to get external ip");
                     return false;
                 }
@@ -362,12 +371,11 @@
                     return false;
                 }
 
-                String icmpInfoKey = icmpInfoKey(icmp,
-                        externalIp.toString(),
+                String icmpInfoKey = icmpInfoKey(icmp, externalGatewayIp.toString(),
                         IPv4.fromIPv4Address(ipPacket.getDestinationAddress()));
                 log.trace("Created icmpInfo key is {}", icmpInfoKey);
 
-                sendRequestForExternal(ipPacket, srcDevice, externalIp, externalPeerRouter);
+                sendRequestForExternal(ipPacket, srcDevice, externalGatewayIp, externalPeerRouter);
 
                 try {
                     icmpInfoMap.compute(icmpInfoKey, (id, existing) -> {
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingIcmpHandler.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingIcmpHandler.java
index b75e5fa..914a910 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingIcmpHandler.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackSwitchingIcmpHandler.java
@@ -72,7 +72,7 @@
 import static org.onosproject.openstacknetworking.api.Constants.PRIORITY_ICMP_RULE;
 import static org.onosproject.openstacknetworking.api.Constants.ROUTING_TABLE;
 import static org.onosproject.openstacknetworking.impl.OsgiPropertyConstants.USE_STATEFUL_SNAT;
-import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.getExternalIp;
+import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.externalGatewayIp;
 import static org.onosproject.openstacknetworking.util.OpenstackNetworkingUtil.getPropertyValueAsBoolean;
 import static org.onosproject.openstacknetworking.util.RulePopulatorUtil.NXM_NX_IP_TTL;
 import static org.onosproject.openstacknetworking.util.RulePopulatorUtil.NXM_OF_ICMP_TYPE;
@@ -193,7 +193,7 @@
                                              Router osRouter,
                                              boolean install) {
 
-        IpAddress natAddress = getExternalIp(osRouter, osNetworkService);
+        IpAddress natAddress = externalGatewayIp(osRouter, osNetworkService);
         if (natAddress == null) {
             return;
         }
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/util/OpenstackNetworkingUtil.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/util/OpenstackNetworkingUtil.java
index 0a35e34..ff53f1b 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/util/OpenstackNetworkingUtil.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/util/OpenstackNetworkingUtil.java
@@ -1167,64 +1167,27 @@
     }
 
     /**
-     * Returns the external ip address with specified router information.
-     *
-     * @param srcSubnet source subnet
-     * @param osRouterService openstack router service
-     * @param osNetworkService openstack network service
-     * @return external ip address
-     */
-    public static IpAddress externalIpFromSubnet(Subnet srcSubnet,
-                                                 OpenstackRouterService
-                                                         osRouterService,
-                                                 OpenstackNetworkService
-                                                         osNetworkService) {
-
-        Router osRouter = getRouterFromSubnet(srcSubnet, osRouterService);
-
-        if (osRouter.getExternalGatewayInfo() == null) {
-            // this router does not have external connectivity
-            log.trace("router({}) has no external gateway",
-                    osRouter.getName());
-            return null;
-        }
-
-        return getExternalIp(osRouter, osNetworkService);
-    }
-
-    /**
-     * Returns the external ip address with specified router information.
+     * Returns the external gateway IP address with specified router information.
      *
      * @param router openstack router
      * @param osNetworkService openstack network service
-     * @return external ip address
+     * @return external IP address
      */
-    public static IpAddress getExternalIp(Router router,
-                                          OpenstackNetworkService osNetworkService) {
-        if (router == null) {
-            return null;
-        }
+    public static IpAddress externalGatewayIp(Router router,
+                                              OpenstackNetworkService osNetworkService) {
+        return externalGatewayIpBase(router, false, osNetworkService);
+    }
 
-        ExternalGateway externalGateway = router.getExternalGatewayInfo();
-        if (externalGateway == null || !externalGateway.isEnableSnat()) {
-            log.trace("Failed to get externalIp for router {} because " +
-                            "externalGateway is null or SNAT is disabled",
-                    router.getId());
-            return null;
-        }
-
-        // TODO fix openstack4j for ExternalGateway provides external fixed IP list
-        Port exGatewayPort = osNetworkService.ports(externalGateway.getNetworkId())
-                .stream()
-                .filter(port -> Objects.equals(port.getDeviceId(), router.getId()))
-                .findAny().orElse(null);
-
-        if (exGatewayPort == null) {
-            return null;
-        }
-
-        return IpAddress.valueOf(exGatewayPort.getFixedIps().stream()
-                .findAny().get().getIpAddress());
+    /**
+     * Returns the external gateway IP address (SNAT enabled) with specified router information.
+     *
+     * @param router openstack router
+     * @param osNetworkService openstack network service
+     * @return external IP address
+     */
+    public static IpAddress externalGatewayIpSnatEnabled(Router router,
+                                                         OpenstackNetworkService osNetworkService) {
+        return externalGatewayIpBase(router, true, osNetworkService);
     }
 
     /**
@@ -1407,8 +1370,15 @@
         return null;
     }
 
-    private static Router getRouterFromSubnet(Subnet subnet,
-                                              OpenstackRouterService osRouterService) {
+    /**
+     * Return the router associated with the given subnet.
+     *
+     * @param subnet openstack subnet
+     * @param osRouterService openstack router service
+     * @return router
+     */
+    public static Router getRouterFromSubnet(Subnet subnet,
+                                             OpenstackRouterService osRouterService) {
         RouterInterface osRouterIface = osRouterService.routerInterfaces().stream()
                 .filter(i -> Objects.equals(i.getSubnetId(), subnet.getId()))
                 .findAny().orElse(null);
@@ -1611,6 +1581,49 @@
         return gw;
     }
 
+    /**
+     * Returns the external gateway IP address with specified router information.
+     *
+     * @param router openstack router
+     * @param snatOnly true for only query SNAT enabled case, false otherwise
+     * @param osNetworkService openstack network service
+     * @return external IP address
+     */
+    private static IpAddress externalGatewayIpBase(Router router, boolean snatOnly,
+                                                   OpenstackNetworkService osNetworkService) {
+        if (router == null) {
+            return null;
+        }
+
+        ExternalGateway externalGateway = router.getExternalGatewayInfo();
+        if (externalGateway == null) {
+            log.info("Failed to get external IP for router {} because no " +
+                            "external gateway is associated with the router",
+                    router.getId());
+            return null;
+        }
+
+        if (snatOnly) {
+            if (!externalGateway.isEnableSnat()) {
+                log.warn("The given router {} SNAT is configured as false", router.getId());
+                return null;
+            }
+        }
+
+        // TODO fix openstack4j for ExternalGateway provides external fixed IP list
+        Port exGatewayPort = osNetworkService.ports(externalGateway.getNetworkId())
+                .stream()
+                .filter(port -> Objects.equals(port.getDeviceId(), router.getId()))
+                .findAny().orElse(null);
+
+        if (exGatewayPort == null) {
+            return null;
+        }
+
+        return IpAddress.valueOf(exGatewayPort.getFixedIps().stream()
+                .findAny().get().getIpAddress());
+    }
+
     private static void print(String format, Object... args) {
         System.out.println(String.format(format, args));
     }