Moving LLDP provider configuration processing off the event thread.
Change-Id: Ief44d354081107c037870d9a7ede60e8a6447113
diff --git a/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LldpLinkProvider.java b/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LldpLinkProvider.java
index 2bdaeab..7f67d8f 100644
--- a/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LldpLinkProvider.java
+++ b/providers/lldp/src/main/java/org/onosproject/provider/lldp/impl/LldpLinkProvider.java
@@ -784,44 +784,46 @@
@Override
public void event(NetworkConfigEvent event) {
- if (event.configClass() == LinkDiscoveryFromDevice.class &&
- CONFIG_CHANGED.contains(event.type())) {
+ executor.submit(() -> {
+ if (event.configClass() == LinkDiscoveryFromDevice.class &&
+ CONFIG_CHANGED.contains(event.type())) {
- if (event.subject() instanceof DeviceId) {
- final DeviceId did = (DeviceId) event.subject();
- Device device = deviceService.getDevice(did);
- updateDevice(device).ifPresent(ld -> updatePorts(ld, did));
- }
-
- } else if (event.configClass() == LinkDiscoveryFromPort.class &&
- CONFIG_CHANGED.contains(event.type())) {
-
- if (event.subject() instanceof ConnectPoint) {
- ConnectPoint cp = (ConnectPoint) event.subject();
- if (cp.elementId() instanceof DeviceId) {
- final DeviceId did = (DeviceId) cp.elementId();
+ if (event.subject() instanceof DeviceId) {
+ final DeviceId did = (DeviceId) event.subject();
Device device = deviceService.getDevice(did);
- Port port = deviceService.getPort(did, cp.port());
- updateDevice(device).ifPresent(ld -> updatePort(ld, port));
+ updateDevice(device).ifPresent(ld -> updatePorts(ld, did));
}
+
+ } else if (event.configClass() == LinkDiscoveryFromPort.class &&
+ CONFIG_CHANGED.contains(event.type())) {
+
+ if (event.subject() instanceof ConnectPoint) {
+ ConnectPoint cp = (ConnectPoint) event.subject();
+ if (cp.elementId() instanceof DeviceId) {
+ final DeviceId did = (DeviceId) cp.elementId();
+ Device device = deviceService.getDevice(did);
+ Port port = deviceService.getPort(did, cp.port());
+ updateDevice(device).ifPresent(ld -> updatePort(ld, port));
+ }
+ }
+
+ } else if (event.configClass() == FingerprintProbeFromDevice.class &&
+ CONFIG_CHANGED.contains(event.type())) {
+
+ if (event.subject() instanceof DeviceId) {
+ final DeviceId did = (DeviceId) event.subject();
+ Device device = deviceService.getDevice(did);
+ updateDevice(device);
+ }
+
+ } else if (event.configClass().equals(SuppressionConfig.class) &&
+ (event.type() == NetworkConfigEvent.Type.CONFIG_ADDED ||
+ event.type() == NetworkConfigEvent.Type.CONFIG_UPDATED)) {
+ SuppressionConfig cfg = cfgRegistry.getConfig(appId, SuppressionConfig.class);
+ reconfigureSuppressionRules(cfg);
+ log.trace("Network config reconfigured");
}
-
- } else if (event.configClass() == FingerprintProbeFromDevice.class &&
- CONFIG_CHANGED.contains(event.type())) {
-
- if (event.subject() instanceof DeviceId) {
- final DeviceId did = (DeviceId) event.subject();
- Device device = deviceService.getDevice(did);
- updateDevice(device);
- }
-
- } else if (event.configClass().equals(SuppressionConfig.class) &&
- (event.type() == NetworkConfigEvent.Type.CONFIG_ADDED ||
- event.type() == NetworkConfigEvent.Type.CONFIG_UPDATED)) {
- SuppressionConfig cfg = cfgRegistry.getConfig(appId, SuppressionConfig.class);
- reconfigureSuppressionRules(cfg);
- log.trace("Network config reconfigured");
- }
+ });
}
}
}
diff --git a/providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LldpLinkProviderTest.java b/providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LldpLinkProviderTest.java
index 8b47f44..a1cbee5 100644
--- a/providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LldpLinkProviderTest.java
+++ b/providers/lldp/src/test/java/org/onosproject/provider/lldp/impl/LldpLinkProviderTest.java
@@ -93,6 +93,7 @@
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
+import static org.onlab.junit.TestTools.assertAfter;
import static org.onosproject.provider.lldp.impl.LldpLinkProvider.DEFAULT_RULES;
@@ -101,6 +102,7 @@
private static final DeviceId DID1 = DeviceId.deviceId("of:0000000000000001");
private static final DeviceId DID2 = DeviceId.deviceId("of:0000000000000002");
private static final DeviceId DID3 = DeviceId.deviceId("of:0000000000000003");
+ private static final int EVENT_MS = 500;
private static Port pd1;
private static Port pd2;
@@ -138,7 +140,7 @@
cfg = new TestSuppressionConfig();
coreService = createMock(CoreService.class);
expect(coreService.registerApplication(appId.name()))
- .andReturn(appId).anyTimes();
+ .andReturn(appId).anyTimes();
replay(coreService);
provider.cfgService = new ComponentConfigAdapter();
@@ -199,8 +201,8 @@
// update device in stub DeviceService with suppression config
deviceService.putDevice(device(DID3, DefaultAnnotations.builder()
- .set(LldpLinkProvider.NO_LLDP, "true")
- .build()));
+ .set(LldpLinkProvider.NO_LLDP, "true")
+ .build()));
deviceListener.event(deviceEvent(DeviceEvent.Type.DEVICE_UPDATED, DID3));
// discovery on device is expected to be gone or stopped
@@ -222,12 +224,13 @@
DID3,
LinkDiscoveryFromDevice.class));
- // discovery helper for device is expected to be gone or stopped
- LinkDiscovery linkDiscovery = provider.discoverers.get(DID3);
- if (linkDiscovery != null) {
- assertTrue("Discovery expected to be stopped", linkDiscovery.isStopped());
- }
-
+ assertAfter(EVENT_MS, () -> {
+ // discovery helper for device is expected to be gone or stopped
+ LinkDiscovery linkDiscovery = provider.discoverers.get(DID3);
+ if (linkDiscovery != null) {
+ assertTrue("Discovery expected to be stopped", linkDiscovery.isStopped());
+ }
+ });
}
@Test
@@ -258,7 +261,7 @@
assertTrue("Port is not gone.", vanishedPort(3L));
assertFalse("Port was not removed from discoverer",
- provider.discoverers.get(DID1).containsPort(3L));
+ provider.discoverers.get(DID1).containsPort(3L));
}
/**
@@ -271,8 +274,8 @@
// add device in stub DeviceService with suppression configured
deviceService.putDevice(device(DID3, DefaultAnnotations.builder()
- .set(LldpLinkProvider.NO_LLDP, "true")
- .build()));
+ .set(LldpLinkProvider.NO_LLDP, "true")
+ .build()));
deviceListener.event(deviceEvent(DeviceEvent.Type.DEVICE_ADDED, DID3));
// non-suppressed port added to suppressed device
@@ -395,9 +398,9 @@
// suppressed port added to non-suppressed device
final long portno3 = 3L;
final Port port3 = port(DID3, portno3, true,
- DefaultAnnotations.builder()
- .set(LldpLinkProvider.NO_LLDP, "true")
- .build());
+ DefaultAnnotations.builder()
+ .set(LldpLinkProvider.NO_LLDP, "true")
+ .build());
deviceService.putPorts(DID3, port3);
deviceListener.event(portEvent(DeviceEvent.Type.PORT_ADDED, DID3, port3));
@@ -486,27 +489,27 @@
private DefaultDevice device(DeviceId did) {
return new DefaultDevice(ProviderId.NONE, did, Device.Type.SWITCH,
- "TESTMF", "TESTHW", "TESTSW", "TESTSN", new ChassisId());
+ "TESTMF", "TESTHW", "TESTSW", "TESTSN", new ChassisId());
}
private DefaultDevice device(DeviceId did, Device.Type type) {
return new DefaultDevice(ProviderId.NONE, did, type,
- "TESTMF", "TESTHW", "TESTSW", "TESTSN", new ChassisId());
+ "TESTMF", "TESTHW", "TESTSW", "TESTSN", new ChassisId());
}
private DefaultDevice device(DeviceId did, Annotations annotations) {
return new DefaultDevice(ProviderId.NONE, did, Device.Type.SWITCH,
- "TESTMF", "TESTHW", "TESTSW", "TESTSN", new ChassisId(), annotations);
+ "TESTMF", "TESTHW", "TESTSW", "TESTSN", new ChassisId(), annotations);
}
- @SuppressWarnings(value = { "unused" })
+ @SuppressWarnings(value = {"unused"})
private DeviceEvent portEvent(DeviceEvent.Type type, DeviceId did, PortNumber port) {
- return new DeviceEvent(type, deviceService.getDevice(did),
- deviceService.getPort(did, port));
+ return new DeviceEvent(type, deviceService.getDevice(did),
+ deviceService.getPort(did, port));
}
private DeviceEvent portEvent(DeviceEvent.Type type, DeviceId did, Port port) {
- return new DeviceEvent(type, deviceService.getDevice(did), port);
+ return new DeviceEvent(type, deviceService.getDevice(did), port);
}
private Port port(DeviceId did, long port, boolean enabled) {
@@ -578,8 +581,10 @@
configEvent(NetworkConfigEvent.Type.CONFIG_UPDATED);
- assertTrue(provider.rules().getSuppressedDeviceType().contains(deviceType1));
- assertTrue(provider.rules().getSuppressedDeviceType().contains(deviceType2));
+ assertAfter(EVENT_MS, () -> {
+ assertTrue(provider.rules().getSuppressedDeviceType().contains(deviceType1));
+ assertTrue(provider.rules().getSuppressedDeviceType().contains(deviceType2));
+ });
}
@Test
@@ -594,9 +599,11 @@
configEvent(NetworkConfigEvent.Type.CONFIG_ADDED);
- assertTrue(provider.rules().getSuppressedAnnotation().containsKey(key1));
- assertEquals(value1, provider.rules().getSuppressedAnnotation().get(key1));
- assertFalse(provider.rules().getSuppressedAnnotation().containsKey(key2));
+ assertAfter(EVENT_MS, () -> {
+ assertTrue(provider.rules().getSuppressedAnnotation().containsKey(key1));
+ assertEquals(value1, provider.rules().getSuppressedAnnotation().get(key1));
+ assertFalse(provider.rules().getSuppressedAnnotation().containsKey(key2));
+ });
}
@Test
@@ -610,25 +617,29 @@
configEvent(NetworkConfigEvent.Type.CONFIG_ADDED);
- assertTrue(provider.rules().getSuppressedAnnotation().containsKey(key1));
- assertEquals(value1, provider.rules().getSuppressedAnnotation().get(key1));
- assertFalse(provider.rules().getSuppressedAnnotation().containsKey(key2));
+ assertAfter(EVENT_MS, () -> {
+ assertTrue(provider.rules().getSuppressedAnnotation().containsKey(key1));
+ assertEquals(value1, provider.rules().getSuppressedAnnotation().get(key1));
+ assertFalse(provider.rules().getSuppressedAnnotation().containsKey(key2));
+ });
annotation.put(key2, value2);
cfg.annotation(annotation);
configEvent(NetworkConfigEvent.Type.CONFIG_UPDATED);
- assertTrue(provider.rules().getSuppressedAnnotation().containsKey(key1));
- assertEquals(value1, provider.rules().getSuppressedAnnotation().get(key1));
- assertTrue(provider.rules().getSuppressedAnnotation().containsKey(key2));
- assertEquals(value2, provider.rules().getSuppressedAnnotation().get(key2));
+ assertAfter(EVENT_MS, () -> {
+ assertTrue(provider.rules().getSuppressedAnnotation().containsKey(key1));
+ assertEquals(value1, provider.rules().getSuppressedAnnotation().get(key1));
+ assertTrue(provider.rules().getSuppressedAnnotation().containsKey(key2));
+ assertEquals(value2, provider.rules().getSuppressedAnnotation().get(key2));
+ });
}
private void configEvent(NetworkConfigEvent.Type evType) {
configListener.event(new NetworkConfigEvent(evType,
- appId,
- SuppressionConfig.class));
+ appId,
+ SuppressionConfig.class));
}
private class TestPacketContext implements PacketContext {
@@ -706,6 +717,7 @@
private final Map<DeviceId, Device> devices = new HashMap<>();
private final ArrayListMultimap<DeviceId, Port> ports =
ArrayListMultimap.create();
+
public TestDeviceService() {
Device d1 = new DefaultDevice(ProviderId.NONE, DID1, Device.Type.SWITCH,
"TESTMF", "TESTHW", "TESTSW", "TESTSN", new ChassisId());
@@ -727,7 +739,7 @@
devices.put(deviceId, device);
}
- private void putPorts(DeviceId did, Port...ports) {
+ private void putPorts(DeviceId did, Port... ports) {
this.ports.putAll(did, Lists.newArrayList(ports));
}
@@ -831,7 +843,7 @@
}
private final class TestNetworkConfigRegistry
- extends NetworkConfigRegistryAdapter {
+ extends NetworkConfigRegistryAdapter {
@SuppressWarnings("unchecked")
@Override
public <S, C extends Config<S>> C getConfig(S subj, Class<C> configClass) {
@@ -896,8 +908,8 @@
final IpAddress addr = IpAddress.valueOf(0);
final Partition p = new DefaultPartition(PartitionId.from(1), Sets.newHashSet(nid));
return new ClusterMetadata("test-cluster",
- Sets.newHashSet(new DefaultControllerNode(nid, addr)),
- Sets.newHashSet(p));
+ Sets.newHashSet(new DefaultControllerNode(nid, addr)),
+ Sets.newHashSet(p));
}
@Override