From f82e3f86775b7310369aca1ee2b00222615333d2 Mon Sep 17 00:00:00 2001 From: Robert Varga Date: Fri, 1 Nov 2019 14:17:48 +0100 Subject: [PATCH] Check network presence 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 --- .../netvirt/neutronvpn/NeutronvpnUtils.java | 51 ++++++++++--------- 1 file changed, 28 insertions(+), 23 deletions(-) diff --git a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java index 27806c64fc..f59656cba6 100644 --- a/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java +++ b/neutronvpn/impl/src/main/java/org/opendaylight/netvirt/neutronvpn/NeutronvpnUtils.java @@ -285,12 +285,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()))) { @@ -310,6 +306,25 @@ 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 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 getRouterIdListforVpn(Uuid vpnId) { InstanceIdentifier vpnMapIdentifier = InstanceIdentifier.builder(VpnMaps.class).child(VpnMap.class, @@ -390,28 +405,22 @@ public class NeutronvpnUtils { } public InstanceIdentifier getNeutronRouterIid(Uuid routerId) { - return InstanceIdentifier.create(Neutron.class).child(Routers.class).child(Router - .class, new RouterKey(routerId)); - + return InstanceIdentifier.create(Neutron.class).child(Routers.class) + .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 inst = InstanceIdentifier.create(Neutron.class).child(Networks.class) .child(Network.class, new NetworkKey(networkId)); - Optional 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; @@ -419,11 +428,7 @@ public class NeutronvpnUtils { LOG.debug("getNeutronPort for {}", portId.getValue()); InstanceIdentifier inst = InstanceIdentifier.create(Neutron.class).child(Ports.class).child(Port.class, new PortKey(portId)); - Optional port = read(LogicalDatastoreType.CONFIGURATION, inst); - if (port.isPresent()) { - prt = port.get(); - } - return prt; + return read(LogicalDatastoreType.CONFIGURATION, inst).orNull(); } /** -- 2.36.6