ONOS-5784: Empty latitude/longitude in ONOS Web UI
- (part one)
- Enhanced HostManager to react to NetworkConfigEvents such that
it applies annotations to relevant host instances from the
config data.
- A little refactoring in DeviceManager.
- Updated topoModel.js to use updated field names latOrY/longOrX.
Change-Id: I06536a6b2279291ffe638549a80b56a9fe94f48a
(cherry picked from commit 78193fd06661a7fa524c8b3a8fa7ddb69d7437c9)
diff --git a/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java b/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
index a5cb416..cc80502 100644
--- a/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
+++ b/core/net/src/main/java/org/onosproject/net/device/impl/DeviceManager.java
@@ -15,20 +15,10 @@
*/
package org.onosproject.net.device.impl;
-import java.util.Collection;
-import java.util.HashSet;
-import java.util.List;
-import java.util.Map;
-import java.util.Objects;
-import java.util.Optional;
-import java.util.Set;
-import java.util.concurrent.CompletableFuture;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.concurrent.CopyOnWriteArrayList;
-import java.util.concurrent.ExecutionException;
-import java.util.concurrent.ScheduledExecutorService;
-import java.util.concurrent.TimeUnit;
-import java.util.stream.Collectors;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Maps;
+import com.google.common.collect.Multimap;
+import com.google.common.util.concurrent.Futures;
import org.apache.felix.scr.annotations.Activate;
import org.apache.felix.scr.annotations.Component;
import org.apache.felix.scr.annotations.Deactivate;
@@ -78,10 +68,20 @@
import org.onosproject.net.provider.ProviderId;
import org.slf4j.Logger;
-import com.google.common.collect.ImmutableList;
-import com.google.common.collect.Maps;
-import com.google.common.collect.Multimap;
-import com.google.common.util.concurrent.Futures;
+import java.util.Collection;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Optional;
+import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.CopyOnWriteArrayList;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.ScheduledExecutorService;
+import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
import static com.google.common.base.Preconditions.checkNotNull;
import static com.google.common.collect.Multimaps.newListMultimap;
@@ -146,8 +146,8 @@
* Index to look up PortConfigOperator from Config each PortConfigOperator uses.
*/
private final Multimap<Class<? extends Config<ConnectPoint>>, PortConfigOperator> portOpsIndex
- = synchronizedListMultimap(
- newListMultimap(new ConcurrentHashMap<>(), CopyOnWriteArrayList::new));
+ = synchronizedListMultimap(
+ newListMultimap(new ConcurrentHashMap<>(), CopyOnWriteArrayList::new));
// not part of portOps. must be executed at the end
private PortAnnotationOperator portAnnotationOp;
@@ -164,6 +164,7 @@
dateTime = now;
}
}
+
private final Map<DeviceId, LocalStatus> deviceLocalStatus =
Maps.newConcurrentMap();
@@ -173,7 +174,7 @@
portOpsIndex.put(PortAnnotationConfig.class, portAnnotationOp);
backgroundService = newSingleThreadScheduledExecutor(
- groupedThreads("onos/device", "manager-background", log));
+ groupedThreads("onos/device", "manager-background", log));
localNodeId = clusterService.getLocalNode().id();
store.setDelegate(delegate);
@@ -447,10 +448,10 @@
// Generate updated description and establish my Role
deviceDescription = BasicDeviceOperator.combine(cfg, deviceDescription);
Futures.getUnchecked(mastershipService.requestRoleFor(deviceId)
- .thenAccept(role -> {
- log.info("Local role is {} for {}", role, deviceId);
- applyRole(deviceId, role);
- }));
+ .thenAccept(role -> {
+ log.info("Local role is {} for {}", role, deviceId);
+ applyRole(deviceId, role);
+ }));
DeviceEvent event = store.createOrUpdateDevice(provider().id(), deviceId,
deviceDescription);
@@ -573,11 +574,11 @@
return;
}
if ((Device.Type.ROADM.equals(device.type())) ||
- (Device.Type.OTN.equals(device.type()))) {
+ (Device.Type.OTN.equals(device.type()))) {
// FIXME This is ignoring all other info in portDescription given as input??
PortDescription storedPortDesc = store.getPortDescription(provider().id(),
- deviceId,
- portDescription.portNumber());
+ deviceId,
+ portDescription.portNumber());
portDescription = ensurePortEnabledState(storedPortDesc,
portDescription.isEnabled());
}
@@ -811,9 +812,9 @@
// device is not connected to this node
if (mastershipService.getLocalRole(did) == NONE) {
log.debug("Node was instructed to be {} role for {}, "
- + "but this node cannot reach the device "
- + "and role is already None. Ignoring request.",
- myNextRole, did);
+ + "but this node cannot reach the device "
+ + "and role is already None. Ignoring request.",
+ myNextRole, did);
} else if (myNextRole != NONE) {
log.warn("Node was instructed to be {} role for {}, "
+ "but this node cannot reach the device. "
@@ -891,7 +892,7 @@
return (event.type() == NetworkConfigEvent.Type.CONFIG_ADDED
|| event.type() == NetworkConfigEvent.Type.CONFIG_UPDATED)
&& (event.configClass().equals(BasicDeviceConfig.class)
- || portOpsIndex.containsKey(event.configClass()));
+ || portOpsIndex.containsKey(event.configClass()));
}
@Override
@@ -900,29 +901,33 @@
if (event.configClass().equals(BasicDeviceConfig.class)) {
log.debug("Detected device network config event {}", event.type());
DeviceId did = (DeviceId) event.subject();
- BasicDeviceConfig cfg = networkConfigService.getConfig(did, BasicDeviceConfig.class);
+ DeviceProvider dp = getProvider(did);
+ BasicDeviceConfig cfg =
+ networkConfigService.getConfig(did, BasicDeviceConfig.class);
if (!isAllowed(cfg)) {
kickOutBadDevice(did);
} else {
Device dev = getDevice(did);
- DeviceDescription desc = (dev == null) ? null : BasicDeviceOperator.descriptionOf(dev);
+ DeviceDescription desc =
+ (dev == null) ? null : BasicDeviceOperator.descriptionOf(dev);
desc = BasicDeviceOperator.combine(cfg, desc);
- if (desc != null && getProvider(did) != null) {
- de = store.createOrUpdateDevice(getProvider(did).id(), did, desc);
+ if (desc != null && dp != null) {
+ de = store.createOrUpdateDevice(dp.id(), did, desc);
}
}
}
if (portOpsIndex.containsKey(event.configClass())) {
ConnectPoint cpt = (ConnectPoint) event.subject();
DeviceId did = cpt.deviceId();
+ DeviceProvider dp = getProvider(did);
// Note: assuming PortOperator can modify existing port,
// but cannot add new port purely from Config.
- de = Optional.ofNullable(getProvider(did))
+ de = Optional.ofNullable(dp)
.map(provider -> store.getPortDescription(provider.id(), did, cpt.port()))
.map(desc -> applyAllPortOps(cpt, desc))
- .map(desc -> store.updatePortStatus(getProvider(did).id(), did, desc))
+ .map(desc -> store.updatePortStatus(dp.id(), did, desc))
.orElse(null);
}
@@ -943,7 +948,7 @@
@Override
@SafeVarargs
public final void registerPortConfigOperator(PortConfigOperator portOp,
- Class<? extends Config<ConnectPoint>>...configs) {
+ Class<? extends Config<ConnectPoint>>... configs) {
checkNotNull(portOp);
portOp.bindService(networkConfigService);
@@ -959,25 +964,25 @@
// TODO: Should we be applying to all existing Ports?
Tools.stream(store.getAvailableDevices())
- .map(Device::id)
- .filter(mastershipService::isLocalMaster)
- // for each locally managed Device, update all port descriptions
- .map(did -> {
- ProviderId pid = Optional.ofNullable(getProvider(did))
- .map(Provider::id)
- .orElse(null);
- if (pid == null) {
- log.warn("Provider not found for {}", did);
- return ImmutableList.<DeviceEvent>of();
- }
- List<PortDescription> pds
- = store.getPortDescriptions(pid, did)
- .map(pdesc -> applyAllPortOps(did, pdesc))
- .collect(Collectors.toList());
- return store.updatePorts(pid, did, pds);
+ .map(Device::id)
+ .filter(mastershipService::isLocalMaster)
+ // for each locally managed Device, update all port descriptions
+ .map(did -> {
+ ProviderId pid = Optional.ofNullable(getProvider(did))
+ .map(Provider::id)
+ .orElse(null);
+ if (pid == null) {
+ log.warn("Provider not found for {}", did);
+ return ImmutableList.<DeviceEvent>of();
+ }
+ List<PortDescription> pds
+ = store.getPortDescriptions(pid, did)
+ .map(pdesc -> applyAllPortOps(did, pdesc))
+ .collect(Collectors.toList());
+ return store.updatePorts(pid, did, pds);
})
- // ..and port port update event if necessary
- .forEach(evts -> evts.forEach(this::post));
+ // ..and port port update event if necessary
+ .forEach(evts -> evts.forEach(this::post));
}
@Override
@@ -1009,8 +1014,8 @@
/**
* Merges the appropriate PortConfig with the description.
*
- * @param cpt ConnectPoint where the port is attached
- * @param desc {@link PortDescription}
+ * @param cpt ConnectPoint where the port is attached
+ * @param desc {@link PortDescription}
* @return merged {@link PortDescription}
*/
private PortDescription applyAllPortOps(ConnectPoint cpt, PortDescription desc) {
diff --git a/core/net/src/main/java/org/onosproject/net/host/impl/BasicHostOperator.java b/core/net/src/main/java/org/onosproject/net/host/impl/BasicHostOperator.java
index 97b87f0..dd56217 100644
--- a/core/net/src/main/java/org/onosproject/net/host/impl/BasicHostOperator.java
+++ b/core/net/src/main/java/org/onosproject/net/host/impl/BasicHostOperator.java
@@ -18,6 +18,7 @@
import org.onlab.packet.IpAddress;
import org.onosproject.net.ConnectPoint;
import org.onosproject.net.DefaultAnnotations;
+import org.onosproject.net.Host;
import org.onosproject.net.HostLocation;
import org.onosproject.net.SparseAnnotations;
import org.onosproject.net.config.basics.BasicHostConfig;
@@ -27,6 +28,8 @@
import java.util.Set;
+import static com.google.common.base.Preconditions.checkNotNull;
+
/**
* Implementations of merge policies for various sources of host configuration
* information. This includes applications, providers, and network configurations.
@@ -46,7 +49,7 @@
*/
public static HostDescription combine(BasicHostConfig cfg,
HostDescription descr) {
- if (cfg == null) {
+ if (cfg == null || descr == null) {
return descr;
}
@@ -64,8 +67,8 @@
SparseAnnotations sa = combine(cfg, descr.annotations());
return new DefaultHostDescription(descr.hwAddress(), descr.vlan(),
- location, ipAddresses,
- descr.configured(), sa);
+ location, ipAddresses,
+ descr.configured(), sa);
}
/**
@@ -82,4 +85,17 @@
return DefaultAnnotations.union(an, builder.build());
}
+
+ /**
+ * Returns a description of the given host.
+ *
+ * @param host the host
+ * @return a description of the host
+ */
+ public static HostDescription descriptionOf(Host host) {
+ checkNotNull(host, "Must supply a non-null Host");
+ return new DefaultHostDescription(host.mac(), host.vlan(), host.location(),
+ host.ipAddresses(), host.configured(),
+ (SparseAnnotations) host.annotations());
+ }
}
diff --git a/core/net/src/main/java/org/onosproject/net/host/impl/HostManager.java b/core/net/src/main/java/org/onosproject/net/host/impl/HostManager.java
index 8a3cf7d..06f3120 100644
--- a/core/net/src/main/java/org/onosproject/net/host/impl/HostManager.java
+++ b/core/net/src/main/java/org/onosproject/net/host/impl/HostManager.java
@@ -30,17 +30,16 @@
import org.onlab.util.Tools;
import org.onosproject.cfg.ComponentConfigService;
import org.onosproject.incubator.net.intf.InterfaceService;
-import org.onosproject.net.edge.EdgePortService;
-import org.onosproject.net.provider.AbstractListenerProviderRegistry;
-import org.onosproject.net.config.NetworkConfigEvent;
-import org.onosproject.net.config.NetworkConfigListener;
-import org.onosproject.net.config.NetworkConfigService;
-import org.onosproject.net.config.basics.BasicHostConfig;
import org.onosproject.net.ConnectPoint;
import org.onosproject.net.DeviceId;
import org.onosproject.net.Host;
import org.onosproject.net.HostId;
+import org.onosproject.net.config.NetworkConfigEvent;
+import org.onosproject.net.config.NetworkConfigListener;
+import org.onosproject.net.config.NetworkConfigService;
+import org.onosproject.net.config.basics.BasicHostConfig;
import org.onosproject.net.device.DeviceService;
+import org.onosproject.net.edge.EdgePortService;
import org.onosproject.net.host.HostAdminService;
import org.onosproject.net.host.HostDescription;
import org.onosproject.net.host.HostEvent;
@@ -52,6 +51,7 @@
import org.onosproject.net.host.HostStore;
import org.onosproject.net.host.HostStoreDelegate;
import org.onosproject.net.packet.PacketService;
+import org.onosproject.net.provider.AbstractListenerProviderRegistry;
import org.onosproject.net.provider.AbstractProviderService;
import org.osgi.service.component.ComponentContext;
import org.slf4j.Logger;
@@ -64,8 +64,9 @@
import static org.onlab.packet.IPv6.getLinkLocalAddress;
import static org.onosproject.net.link.ProbedLinkProvider.DEFAULT_MAC;
import static org.onosproject.security.AppGuard.checkPermission;
+import static org.onosproject.security.AppPermission.Type.HOST_EVENT;
+import static org.onosproject.security.AppPermission.Type.HOST_READ;
import static org.slf4j.LoggerFactory.getLogger;
-import static org.onosproject.security.AppPermission.Type.*;
/**
* Provides basic implementation of the host SB & NB APIs.
@@ -149,7 +150,7 @@
}
@Modified
- public void modified(ComponentContext context) {
+ public void modified(ComponentContext context) {
boolean oldValue = monitorHosts;
readComponentConfiguration(context);
if (probeRate > 0) {
@@ -183,7 +184,7 @@
} else {
monitorHosts = flag;
log.info("Configured. monitorHosts {}",
- monitorHosts ? "enabled" : "disabled");
+ monitorHosts ? "enabled" : "disabled");
}
Long longValue = Tools.getLongProperty(properties, "probeRate");
@@ -217,24 +218,22 @@
/**
* Starts monitoring the hosts by IP Address.
- *
*/
private void startMonitoring() {
store.getHosts().forEach(host -> {
- host.ipAddresses().forEach(ip -> {
- monitor.addMonitoringFor(ip);
+ host.ipAddresses().forEach(ip -> {
+ monitor.addMonitoringFor(ip);
});
});
}
/**
* Stops monitoring the hosts by IP Address.
- *
*/
private void stopMonitoring() {
store.getHosts().forEach(host -> {
- host.ipAddresses().forEach(ip -> {
- monitor.stopMonitoring(ip);
+ host.ipAddresses().forEach(ip -> {
+ monitor.stopMonitoring(ip);
});
});
}
@@ -447,27 +446,49 @@
// links that the config does not allow
private class InternalNetworkConfigListener implements NetworkConfigListener {
@Override
+ public boolean isRelevant(NetworkConfigEvent event) {
+ return (event.type() == NetworkConfigEvent.Type.CONFIG_ADDED
+ || event.type() == NetworkConfigEvent.Type.CONFIG_UPDATED)
+ && (event.configClass().equals(BasicHostConfig.class));
+ }
+
+ @Override
public void event(NetworkConfigEvent event) {
- if ((event.type() == NetworkConfigEvent.Type.CONFIG_ADDED ||
- event.type() == NetworkConfigEvent.Type.CONFIG_UPDATED) &&
- event.configClass().equals(BasicHostConfig.class)) {
- log.debug("Detected host network config event {}", event.type());
- kickOutBadHost(((HostId) event.subject()));
+ log.debug("Detected host network config event {}", event.type());
+ HostEvent he = null;
+
+ HostId hostId = (HostId) event.subject();
+ BasicHostConfig cfg =
+ networkConfigService.getConfig(hostId, BasicHostConfig.class);
+
+ if (!isAllowed(cfg)) {
+ kickOutBadHost(hostId);
+ } else {
+ Host host = getHost(hostId);
+ HostDescription desc =
+ (host == null) ? null : BasicHostOperator.descriptionOf(host);
+ desc = BasicHostOperator.combine(cfg, desc);
+ if (desc != null) {
+ he = store.createOrUpdateHost(host.providerId(), hostId, desc, false);
+ }
+ }
+
+ if (he != null) {
+ post(he);
}
}
}
- // checks if the specified host is allowed by the BasicHostConfig
- // and if not, removes it
+ // by default allowed, otherwise check flag
+ private boolean isAllowed(BasicHostConfig cfg) {
+ return (cfg == null || cfg.isAllowed());
+ }
+
+ // removes the specified host, if it exists
private void kickOutBadHost(HostId hostId) {
- BasicHostConfig cfg = networkConfigService.getConfig(hostId, BasicHostConfig.class);
- if (cfg != null && !cfg.isAllowed()) {
- Host badHost = getHost(hostId);
- if (badHost != null) {
- removeHost(hostId);
- } else {
- log.info("Failed removal: Host {} does not exist", hostId);
- }
+ Host badHost = getHost(hostId);
+ if (badHost != null) {
+ removeHost(hostId);
}
}
}