Check network presence 10/85710/3
authorRobert Varga <robert.varga@pantheon.tech>
Fri, 8 Nov 2019 11:49:43 +0000 (12:49 +0100)
committerStephen Kitt <skitt@redhat.com>
Wed, 13 Nov 2019 16:45:13 +0000 (16:45 +0000)
In case we end up with an UUID for which we cannot find a network,
we end up with this splat:

java.lang.NullPointerException: null
        at org.opendaylight.netvirt.neutronvpn.NeutronvpnUtils.getIsExternal(NeutronvpnUtils.java:988) ~[?:?]
        at org.opendaylight.netvirt.neutronvpn.NeutronvpnUtils.getVpnForRouter(NeutronvpnUtils.java:291) ~[?:?]
        at org.opendaylight.netvirt.neutronvpn.NeutronvpnManager.createL3InternalVpn(NeutronvpnManager.java:1203) ~[?:?]
        at org.opendaylight.netvirt.neutronvpn.NeutronRouterChangeListener.add(NeutronRouterChangeListener.java:84) ~[?:?]
        at org.opendaylight.netvirt.neutronvpn.NeutronRouterChangeListener.add(NeutronRouterChangeListener.java:36) ~[?:?]
        at org.opendaylight.genius.datastoreutils.AsyncDataTreeChangeListenerBase$DataTreeChangeHandler.run(AsyncDataTreeChangeListenerBase.java:171) ~[274:org.opendaylight.genius.mdsalutil-api:0.7.1]
        at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149) ~[?:?]
        at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624) ~[?:?]
        at java.lang.Thread.run(Thread.java:748) [?:?]

This adds appropriate annotations so that callers of getNetwork()
are flagged when they do not handle nulls. Fixes the reported offender
by moving the checking code into a utility class.

JIRA: NETVIRT-1636
Change-Id: I8872699a1bbf56a55977a9f1e8a0d97b1ff45f94
Signed-off-by: Robert Varga <robert.varga@pantheon.tech>
(cherry picked from commit f82e3f86775b7310369aca1ee2b00222615333d2)

neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java

index f22f6a65bc67917aa0f2450cd96c04691e3a5340..21c1df342bfeb1e9f00879ed7e86ffaee88a24c2 100644 (file)
@@ -295,12 +295,8 @@ public class NeutronvpnUtils {
                     continue;
                 }
                 // Skip router vpnId fetching from internet BGP-VPN
-                if (vpnMap.getNetworkIds() != null && !vpnMap.getNetworkIds().isEmpty()) {
-                    // We only need to check the first network; if it’s not an external network there’s no
-                    // need to check the rest of the VPN’s network list
-                    if (getIsExternal(getNeutronNetwork(vpnMap.getNetworkIds().iterator().next()))) {
-                        continue;
-                    }
+                if (hasExternalNetwork(vpnMap.getNetworkIds())) {
+                    continue;
                 }
                 // FIXME: NETVIRT-1503: this check can be replaced by a ReadOnlyTransaction.exists()
                 if (routerIdsList.stream().anyMatch(routerIds -> routerId.equals(routerIds.getRouterId()))) {
@@ -320,6 +316,26 @@ public class NeutronvpnUtils {
         return null;
     }
 
+    // We only need to check the first network; if it’s not an external network there’s no
+    // need to check the rest of the VPN’s network list. Note that some UUIDs may point to unknown networks, in which
+    // case we check more  and assume false.
+    private boolean hasExternalNetwork(List<Uuid> uuids) {
+        if (uuids != null) {
+            for (Uuid uuid : uuids) {
+                final Network network = getNeutronNetwork(uuid);
+                if (network != null) {
+                    if (Boolean.TRUE.equals(getIsExternal(network))) {
+                        return true;
+                    }
+                } else {
+                    LOG.debug("hasExternalNetwork: cannot find network for {}", uuid);
+                }
+            }
+        }
+        return false;
+    }
+
+
     @Nullable
     protected List<Uuid> getRouterIdListforVpn(Uuid vpnId) {
         Optional<VpnMap> optionalVpnMap = read(LogicalDatastoreType.CONFIGURATION, vpnMapIdentifier(vpnId));
@@ -400,33 +416,24 @@ public class NeutronvpnUtils {
         return NEUTRON_ROUTERS_IID.child(Router.class, new RouterKey(routerId));
     }
 
-    protected Network getNeutronNetwork(Uuid networkId) {
-        Network network = null;
-        network = networkMap.get(networkId);
+    protected @Nullable Network getNeutronNetwork(Uuid networkId) {
+        Network network = networkMap.get(networkId);
         if (network != null) {
             return network;
         }
         LOG.debug("getNeutronNetwork for {}", networkId.getValue());
         InstanceIdentifier<Network> inst = NEUTRON_NETWORKS_IID.child(Network.class, new NetworkKey(networkId));
-        Optional<Network> net = read(LogicalDatastoreType.CONFIGURATION, inst);
-        if (net.isPresent()) {
-            network = net.get();
-        }
-        return network;
+        return read(LogicalDatastoreType.CONFIGURATION, inst).orNull();
     }
 
-    protected Port getNeutronPort(Uuid portId) {
+    protected @Nullable Port getNeutronPort(Uuid portId) {
         Port prt = portMap.get(portId);
         if (prt != null) {
             return prt;
         }
         LOG.debug("getNeutronPort for {}", portId.getValue());
         InstanceIdentifier<Port> inst = NEUTRON_PORTS_IID.child(Port.class, new PortKey(portId));
-        Optional<Port> port = read(LogicalDatastoreType.CONFIGURATION, inst);
-        if (port.isPresent()) {
-            prt = port.get();
-        }
-        return prt;
+        return read(LogicalDatastoreType.CONFIGURATION, inst).orNull();
     }
 
     public PortIdToSubport getPortIdToSubport(Uuid portId) {