Fix to avoid duplication of key and flow when host event occurs.

- modify key of the security group maps. - search for a subnet interface to the host. Change-Id: I502a35735b423fb6c27bab6b83b642a5f916e37c

Fix to avoid duplication of key and flow when host event occurs.
- modify key of the security group maps. - search for a subnet interface to the host. Change-Id: I502a35735b423fb6c27bab6b83b642a5f916e37c
jskim · Gerrit Code Review
Commit 8f27c4e095af31c195a8c345c09e193f20e02551 8f27c4e0 1 parent 63298808
Showing 3 changed files with 30 additions and 14 deletions
apps/openstacknetworking/routing/src/main/java/org/onosproject/openstacknetworking/routing/OpenstackRoutingManager.java
apps/openstacknetworking/switching/src/main/java/org/onosproject/openstacknetworking/switching/OpenstackSecurityGroupManager.java
apps/openstacknetworking/switching/src/main/java/org/onosproject/openstacknetworking/switching/OpenstackSwitchingHostManager.java
--- a/apps/openstacknetworking/routing/src/main/java/org/onosproject/openstacknetworking/routing/OpenstackRoutingManager.java
View file @8f27c4e
+++ b/apps/openstacknetworking/routing/src/main/java/org/onosproject/openstacknetworking/routing/OpenstackRoutingManager.java
View file @8f27c4e
@@ -276,6 +276,12 @@ public class OpenstackRoutingManager extends AbstractVmHandler implements Openst
                 .map(OpenstackSubnet::id)
                 .collect(Collectors.toSet());
+        if (host.isPresent()) {
+            if (!routableSubNetIds.contains(host.get().annotations().value(SUBNET_ID))) {
+                // subnet of host is not connected to this router, do nothing.
+                return;
+            }
+        }
         Set<Host> hosts = host.isPresent() ? ImmutableSet.of(host.get()) :
                 Tools.stream(hostService.getHosts())
@@ -307,11 +313,12 @@ public class OpenstackRoutingManager extends AbstractVmHandler implements Openst
                 r.id().equals(routerId)).iterator().next();
     }
-    private Optional<OpenstackPort> routerIfacePort(String osNetId) {
+    private Optional<OpenstackPort> routerIfacePort(String osNetId, String osSubNetId) {
         // FIXME router interface is subnet specific, not network
         return openstackService.ports().stream()
                 .filter(p -> p.deviceOwner().equals(DEVICE_OWNER_ROUTER_INTERFACE) &&
-                        p.networkId().equals(osNetId))
+                        p.networkId().equals(osNetId) &&
+                        p.fixedIps().containsKey(osSubNetId))
                 .findAny();
     }
@@ -538,7 +545,8 @@ public class OpenstackRoutingManager extends AbstractVmHandler implements Openst
     @Override
     protected void hostDetected(Host host) {
         String osNetId = host.annotations().value(NETWORK_ID);
-        Optional<OpenstackPort> routerIface = routerIfacePort(osNetId);
+        String osSubNetId = host.annotations().value(SUBNET_ID);
+        Optional<OpenstackPort> routerIface = routerIfacePort(osNetId, osSubNetId);
         if (!routerIface.isPresent()) {
             return;
         }
@@ -550,7 +558,8 @@ public class OpenstackRoutingManager extends AbstractVmHandler implements Openst
     @Override
     protected void hostRemoved(Host host) {
         String osNetId = host.annotations().value(NETWORK_ID);
-        Optional<OpenstackPort> routerIface = routerIfacePort(osNetId);
+        String osSubNetId = host.annotations().value(SUBNET_ID);
+        Optional<OpenstackPort> routerIface = routerIfacePort(osNetId, osSubNetId);
         if (!routerIface.isPresent()) {
             return;
         }
--- a/apps/openstacknetworking/switching/src/main/java/org/onosproject/openstacknetworking/switching/OpenstackSecurityGroupManager.java
View file @8f27c4e
+++ b/apps/openstacknetworking/switching/src/main/java/org/onosproject/openstacknetworking/switching/OpenstackSecurityGroupManager.java
View file @8f27c4e
@@ -33,6 +33,7 @@ import org.onlab.util.Tools;
 import org.onosproject.core.ApplicationId;
 import org.onosproject.net.DeviceId;
 import org.onosproject.net.Host;
+import org.onosproject.net.HostId;
 import org.onosproject.net.flow.DefaultTrafficSelector;
 import org.onosproject.net.flow.DefaultTrafficTreatment;
 import org.onosproject.net.flow.TrafficSelector;
@@ -78,7 +79,9 @@ public class OpenstackSecurityGroupManager extends AbstractVmHandler
     private static final String PROTO_UDP = "UDP";
     private static final String ETHTYPE_IPV4 = "IPV4";
-    private final Map<Host, Set<SecurityGroupRule>> securityGroupRuleMap = Maps.newConcurrentMap();
+    private final Map<HostId, Set<SecurityGroupRule>> securityGroupRuleMap = Maps.newConcurrentMap();
+    private final Map<HostId, Host> hostInfoMap = Maps.newHashMap();
+
     private ApplicationId appId;
     @Activate
@@ -118,9 +121,9 @@ public class OpenstackSecurityGroupManager extends AbstractVmHandler
      */
     private void populateSecurityGroupRules(String tenantId, boolean install) {
         securityGroupRuleMap.entrySet().stream()
-                .filter(entry -> getTenantId(entry.getKey()).equals(tenantId))
+                .filter(entry -> getTenantId(hostInfoMap.get(entry.getKey())).equals(tenantId))
                 .forEach(entry -> {
-                    Host local = entry.getKey();
+                    Host local = hostInfoMap.get(entry.getKey());
                     entry.getValue().forEach(sgRule -> {
                         setSecurityGroupRule(local.location().deviceId(),
                                 sgRule.rule(),
@@ -259,7 +262,8 @@ public class OpenstackSecurityGroupManager extends AbstractVmHandler
                 log.warn("Failed to get security group {}", sgId);
             }
         });
-        securityGroupRuleMap.put(host, rules);
+        hostInfoMap.put(host.id(), host);
+        securityGroupRuleMap.put(host.id(), rules);
     }
     /**
@@ -293,11 +297,11 @@ public class OpenstackSecurityGroupManager extends AbstractVmHandler
     private Set<IpPrefix> getRemoteIps(String tenantId, String sgId) {
         Set<IpPrefix> remoteIps = Sets.newHashSet();
         securityGroupRuleMap.entrySet().stream()
-                .filter(entry -> Objects.equals(getTenantId(entry.getKey()), tenantId))
+                .filter(entry -> Objects.equals(getTenantId(hostInfoMap.get(entry.getKey())), tenantId))
                 .forEach(entry -> {
                     if (entry.getValue().stream()
                             .anyMatch(rule -> rule.rule().secuityGroupId().equals(sgId))) {
-                        remoteIps.add(IpPrefix.valueOf(getIp(entry.getKey()), 32));
+                        remoteIps.add(IpPrefix.valueOf(getIp(hostInfoMap.get(entry.getKey())), 32));
                     }
                 });
         return remoteIps;
@@ -310,7 +314,8 @@ public class OpenstackSecurityGroupManager extends AbstractVmHandler
         if (isHostAdded) {
             updateSecurityGroupRulesMap(host);
         } else {
-            securityGroupRuleMap.remove(host);
+            securityGroupRuleMap.remove(host.id());
+            hostInfoMap.remove(host.id());
         }
         Tools.stream(hostService.getHosts())
--- a/apps/openstacknetworking/switching/src/main/java/org/onosproject/openstacknetworking/switching/OpenstackSwitchingHostManager.java
View file @8f27c4e
+++ b/apps/openstacknetworking/switching/src/main/java/org/onosproject/openstacknetworking/switching/OpenstackSwitchingHostManager.java
View file @8f27c4e
@@ -161,8 +161,11 @@ public final class OpenstackSwitchingHostManager extends AbstractProvider
             return;
         }
+        Map.Entry<String, Ip4Address> fixedIp = osPort.fixedIps().entrySet().stream().findFirst().get();
+
         OpenstackSubnet openstackSubnet = openstackService.subnets().stream()
-                .filter(n -> n.networkId().equals(osPort.networkId()))
+                .filter(n -> n.networkId().equals(osPort.networkId()) &&
+                n.id().equals(fixedIp.getKey()))
                 .findFirst().orElse(null);
         if (openstackSubnet == null) {
             log.warn("Failed to find subnet for {}", osPort);
@@ -171,8 +174,7 @@ public final class OpenstackSwitchingHostManager extends AbstractProvider
         registerDhcpInfo(osPort, openstackSubnet);
         ConnectPoint connectPoint = new ConnectPoint(port.element().id(), port.number());
-        // TODO remove gateway IP from host annotation
+
-        Map.Entry<String, Ip4Address> fixedIp = osPort.fixedIps().entrySet().stream().findFirst().get();
         // Added CREATE_TIME intentionally to trigger HOST_UPDATED event for the
         // existing instances.