Refactor external peer router store, fix NPE due to MAC is not ready
Change-Id: Id0381d9d1d7e0888dfbf1fc20acdd44d0a303e4c
diff --git a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackNetworkManager.java b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackNetworkManager.java
index d3e36bf..c79f12c 100644
--- a/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackNetworkManager.java
+++ b/apps/openstacknetworking/app/src/main/java/org/onosproject/openstacknetworking/impl/OpenstackNetworkManager.java
@@ -132,6 +132,12 @@
"OpenStack port ID cannot be null";
private static final String ERR_NULL_PORT_NET_ID =
"OpenStack port network ID cannot be null";
+ private static final String ERR_NULL_PEER_ROUTER =
+ "External peer router cannot be null";
+ private static final String ERR_NULL_PEER_ROUTER_IP =
+ "External peer router IP cannot be null";
+ private static final String ERR_NULL_PEER_ROUTER_MAC =
+ "External peer router MAC cannot be null";
private static final String ERR_IN_USE = " still in use";
@@ -159,20 +165,9 @@
private final OpenstackNetworkStoreDelegate
delegate = new InternalNetworkStoreDelegate();
- private ConsistentMap<String, ExternalPeerRouter> externalPeerRouterMap;
private ConsistentMap<String, OpenstackNetwork> augmentedNetworkMap;
private static final KryoNamespace
- SERIALIZER_EXTERNAL_PEER_ROUTER_MAP = KryoNamespace.newBuilder()
- .register(KryoNamespaces.API)
- .register(ExternalPeerRouter.class)
- .register(DefaultExternalPeerRouter.class)
- .register(MacAddress.class)
- .register(IpAddress.class)
- .register(VlanId.class)
- .build();
-
- private static final KryoNamespace
SERIALIZER_AUGMENTED_NETWORK_MAP = KryoNamespace.newBuilder()
.register(KryoNamespaces.API)
.register(OpenstackNetwork.Type.class)
@@ -182,7 +177,6 @@
private ApplicationId appId;
-
@Activate
protected void activate() {
appId = coreService.registerApplication(Constants.OPENSTACK_NETWORKING_APP_ID);
@@ -190,12 +184,6 @@
osNetworkStore.setDelegate(delegate);
log.info("Started");
- externalPeerRouterMap = storageService.<String, ExternalPeerRouter>consistentMapBuilder()
- .withSerializer(Serializer.using(SERIALIZER_EXTERNAL_PEER_ROUTER_MAP))
- .withName("external-routermap")
- .withApplicationId(appId)
- .build();
-
augmentedNetworkMap = storageService.<String, OpenstackNetwork>consistentMapBuilder()
.withSerializer(Serializer.using(SERIALIZER_AUGMENTED_NETWORK_MAP))
.withName("augmented-networkmap")
@@ -340,7 +328,6 @@
public void clear() {
osNetworkStore.clear();
augmentedNetworkMap.clear();
- externalPeerRouterMap.clear();
}
@Override
@@ -481,10 +468,7 @@
@Override
public ExternalPeerRouter externalPeerRouter(IpAddress ipAddress) {
- if (externalPeerRouterMap.containsKey(ipAddress.toString())) {
- return externalPeerRouterMap.get(ipAddress.toString()).value();
- }
- return null;
+ return osNetworkStore.externalPeerRouter(ipAddress.toString());
}
@Override
@@ -495,11 +479,7 @@
return null;
}
- if (externalPeerRouterMap.containsKey(ipAddress.toString())) {
- return externalPeerRouterMap.get(ipAddress.toString()).value();
- } else {
- return null;
- }
+ return externalPeerRouter(ipAddress);
}
@Override
@@ -516,9 +496,11 @@
return;
}
- if (externalPeerRouterMap.containsKey(targetIp.toString()) &&
- !externalPeerRouterMap.get(
- targetIp.toString()).value().macAddress().equals(MacAddress.NONE)) {
+ ExternalPeerRouter peerRouter = osNetworkStore.externalPeerRouter(targetIp.toString());
+
+ // if peer router's MAC address is not NONE, we assume that peer router's
+ // MAC address has been derived
+ if (peerRouter != null && !peerRouter.macAddress().equals(MacAddress.NONE)) {
return;
}
@@ -555,13 +537,12 @@
treatment,
ByteBuffer.wrap(ethRequest.serialize())));
- externalPeerRouterMap.put(targetIp.toString(),
- DefaultExternalPeerRouter.builder()
- .ipAddress(targetIp)
- .macAddress(MacAddress.NONE)
- .vlanId(vlanId)
- .build());
-
+ ExternalPeerRouter derivedRouter = DefaultExternalPeerRouter.builder()
+ .ipAddress(targetIp)
+ .macAddress(MacAddress.NONE)
+ .vlanId(vlanId)
+ .build();
+ osNetworkStore.createExternalPeerRouter(derivedRouter);
log.info("Initializes external peer router map with peer router IP {}",
targetIp.toString());
}
@@ -573,59 +554,45 @@
}
IpAddress targetIp = getExternalPeerRouterIp(externalGateway);
- if (targetIp == null) {
- return;
- }
-
- if (externalPeerRouterMap.containsKey(targetIp.toString())) {
- externalPeerRouterMap.remove(targetIp.toString());
- }
+ deleteExternalPeerRouter(targetIp.toString());
}
@Override
public void deleteExternalPeerRouter(String ipAddress) {
- if (ipAddress == null) {
- return;
- }
-
- if (externalPeerRouterMap.containsKey(ipAddress)) {
- externalPeerRouterMap.remove(ipAddress);
- }
-
+ osNetworkStore.removeExternalPeerRouter(ipAddress);
}
@Override
public void updateExternalPeerRouterMac(IpAddress ipAddress,
MacAddress macAddress) {
- try {
- externalPeerRouterMap.computeIfPresent(ipAddress.toString(), (id, existing) ->
- DefaultExternalPeerRouter.builder()
- .ipAddress(ipAddress)
- .macAddress(macAddress)
- .vlanId(existing.vlanId())
- .build());
-
- log.info("Updated external peer router map {}",
- externalPeerRouterMap.get(ipAddress.toString()).value().toString());
- } catch (Exception e) {
- log.error("Exception occurred because of {}", e);
- }
+ updateExternalPeerRouter(ipAddress, macAddress, null);
}
@Override
public void updateExternalPeerRouter(IpAddress ipAddress,
MacAddress macAddress,
VlanId vlanId) {
- try {
- externalPeerRouterMap.computeIfPresent(ipAddress.toString(), (id, existing) ->
- DefaultExternalPeerRouter.builder()
- .ipAddress(ipAddress)
- .macAddress(macAddress)
- .vlanId(vlanId)
- .build());
+ checkNotNull(ipAddress, ERR_NULL_PEER_ROUTER_IP);
- } catch (Exception e) {
- log.error("Exception occurred because of {}", e);
+ ExternalPeerRouter existingPeerRouter =
+ osNetworkStore.externalPeerRouter(ipAddress.toString());
+
+ if (existingPeerRouter != null) {
+ ExternalPeerRouter.Builder urBuilder = DefaultExternalPeerRouter.builder()
+ .ipAddress(ipAddress);
+
+ if (macAddress == null) {
+ urBuilder.macAddress(existingPeerRouter.macAddress());
+ } else {
+ urBuilder.macAddress(macAddress);
+ }
+
+ if (vlanId == null) {
+ urBuilder.vlanId(existingPeerRouter.vlanId());
+ } else {
+ urBuilder.vlanId(vlanId);
+ }
+ osNetworkStore.updateExternalPeerRouter(urBuilder.build());
}
}
@@ -636,31 +603,25 @@
if (ipAddress == null) {
return null;
}
- if (externalPeerRouterMap.containsKey(ipAddress.toString())) {
- return externalPeerRouterMap.get(ipAddress.toString()).value().macAddress();
- } else {
+
+ ExternalPeerRouter peerRouter =
+ osNetworkStore.externalPeerRouter(ipAddress.toString());
+
+ if (peerRouter == null) {
throw new NoSuchElementException();
+ } else {
+ return peerRouter.macAddress();
}
}
@Override
public void updateExternalPeerRouterVlan(IpAddress ipAddress, VlanId vlanId) {
-
- try {
- externalPeerRouterMap.computeIfPresent(ipAddress.toString(), (id, existing) ->
- DefaultExternalPeerRouter.builder()
- .ipAddress(ipAddress)
- .macAddress(existing.macAddress())
- .vlanId(vlanId).build());
-
- } catch (Exception e) {
- log.error("Exception occurred because of {}", e);
- }
+ updateExternalPeerRouter(ipAddress, null, vlanId);
}
@Override
public Set<ExternalPeerRouter> externalPeerRouters() {
- return ImmutableSet.copyOf(externalPeerRouterMap.asJavaMap().values());
+ return ImmutableSet.copyOf(osNetworkStore.externalPeerRouters());
}
@Override