Revert "Removing applyRole to avoid double mastership requests to the provider."
This reverts commit 82e21897f3e735594510709fc9f0bc85f37d8f4c.
Change-Id: Id0a76f915fadcc0f83b588159eef537a5d695c62
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 a5d8abc..2161f7d 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
@@ -519,6 +519,31 @@
super(provider);
}
+ /**
+ * Apply role in reaction to provider event.
+ *
+ * @param deviceId device identifier
+ * @param newRole new role to apply to the device
+ * @return true if the request was sent to provider
+ */
+ private boolean applyRole(DeviceId deviceId, MastershipRole newRole) {
+
+ if (newRole.equals(MastershipRole.NONE)) {
+ //no-op
+ return true;
+ }
+
+ DeviceProvider provider = provider();
+ if (provider == null) {
+ log.warn("Provider for {} was not found. Cannot apply role {}",
+ deviceId, newRole);
+ return false;
+ }
+ provider.roleChanged(deviceId, newRole);
+ // not triggering probe when triggered by provider service event
+ return true;
+ }
+
@Override
public void deviceConnected(DeviceId deviceId,
DeviceDescription deviceDescription) {
@@ -545,6 +570,7 @@
log.info("Local role is {} for {}", role, deviceId);
DeviceEvent event = store.createOrUpdateDevice(provider().id(), deviceId,
deviceDescription);
+ applyRole(deviceId, role);
if (portConfig != null) {
//updating the ports if configration exists
diff --git a/providers/netconf/device/src/main/java/org/onosproject/provider/netconf/device/impl/NetconfDeviceProvider.java b/providers/netconf/device/src/main/java/org/onosproject/provider/netconf/device/impl/NetconfDeviceProvider.java
index 02c629e..8281f4e 100644
--- a/providers/netconf/device/src/main/java/org/onosproject/provider/netconf/device/impl/NetconfDeviceProvider.java
+++ b/providers/netconf/device/src/main/java/org/onosproject/provider/netconf/device/impl/NetconfDeviceProvider.java
@@ -71,7 +71,6 @@
import org.slf4j.Logger;
import java.io.IOException;
-import java.net.InetSocketAddress;
import java.net.Socket;
import java.net.URI;
import java.net.URISyntaxException;
@@ -328,13 +327,9 @@
// method to test reachability.
//test connection to device opening a socket to it.
log.debug("Testing reachability for {}:{}", ip, port);
- Socket socket = new Socket();
- try {
- socket.connect(new InetSocketAddress(ip, port), 1000);
+ try (Socket socket = new Socket(ip, port)) {
log.debug("rechability of {}, {}, {}", deviceId, socket.isConnected(), !socket.isClosed());
- boolean isConnected = socket.isConnected() && !socket.isClosed();
- socket.close();
- return isConnected;
+ return socket.isConnected() && !socket.isClosed();
} catch (IOException e) {
log.info("Device {} is not reachable", deviceId);
log.debug(" error details", e);
@@ -431,10 +426,6 @@
log.trace("{} not my scheme, skipping", deviceId);
return;
}
- if (!isReachable(deviceId)) {
- log.warn("Can't connect to device {}", deviceId);
- return;
- }
DeviceDescription deviceDescription = createDeviceRepresentation(deviceId, config);
log.debug("Connecting NETCONF device {}, on {}:{} with username {}",
deviceId, config.ip(), config.port(), config.username());
@@ -443,6 +434,11 @@
if (deviceService.getDevice(deviceId) == null) {
providerService.deviceConnected(deviceId, deviceDescription);
}
+ try {
+ checkAndUpdateDevice(deviceId, deviceDescription, true);
+ } catch (Exception e) {
+ log.error("Unhandled exception checking {}", deviceId, e);
+ }
}
private void checkAndUpdateDevice(DeviceId deviceId, DeviceDescription deviceDescription, boolean newlyConnected) {
@@ -455,7 +451,7 @@
if (!isReachable && deviceService.isAvailable(deviceId)) {
providerService.deviceDisconnected(deviceId);
return;
- } else if (newlyConnected && mastershipService.isLocalMaster(deviceId)) {
+ } else if (newlyConnected) {
updateDeviceDescription(deviceId, deviceDescription, device);
}
if (isReachable && deviceService.isAvailable(deviceId) &&
@@ -482,16 +478,11 @@
deviceId, new DefaultDeviceDescription(
updatedDeviceDescription, true,
updatedDeviceDescription.annotations()));
- } else if (updatedDeviceDescription == null && deviceDescription != null) {
+ } else if (updatedDeviceDescription == null) {
providerService.deviceConnected(
deviceId, new DefaultDeviceDescription(
deviceDescription, true,
deviceDescription.annotations()));
- } else {
- providerService.deviceConnected(deviceId, new DefaultDeviceDescription(deviceId.uri(),
- device.type(), device.manufacturer(), device.hwVersion(), device.swVersion(),
- device.serialNumber(), device.chassisId(), true,
- (SparseAnnotations) device.annotations()));
}
}
} else {
@@ -549,7 +540,6 @@
.set(IPADDRESS, ipAddress)
.set(PORT, String.valueOf(config.port()))
.set(AnnotationKeys.PROTOCOL, SCHEME_NAME.toUpperCase())
- .set(AnnotationKeys.PROVIDER_MARK_ONLINE, String.valueOf(true))
.build();
return new DefaultDeviceDescription(
deviceId.uri(),
@@ -578,15 +568,8 @@
private void initiateConnection(DeviceId deviceId, MastershipRole newRole) {
try {
if (isReachable(deviceId)) {
- NetconfDevice device = controller.connectDevice(deviceId);
- if (device != null) {
- providerService.receivedRoleReply(deviceId, newRole, MastershipRole.MASTER);
- try {
- checkAndUpdateDevice(deviceId, null, true);
- } catch (Exception e) {
- log.error("Unhandled exception checking {}", deviceId, e);
- }
- }
+ controller.connectDevice(deviceId);
+ providerService.receivedRoleReply(deviceId, newRole, MastershipRole.MASTER);
}
} catch (Exception e) {
if (deviceService.getDevice(deviceId) != null) {
@@ -662,15 +645,12 @@
private class InternalDeviceListener implements DeviceListener {
@Override
public void event(DeviceEvent event) {
- if (deviceService.isAvailable(event.subject().id())) {
- executor.execute(() -> discoverPorts(event.subject().id()));
- }
+ executor.execute(() -> discoverPorts(event.subject().id()));
}
@Override
public boolean isRelevant(DeviceEvent event) {
- if (event.type() != DeviceEvent.Type.DEVICE_ADDED &&
- event.type() != DeviceEvent.Type.DEVICE_AVAILABILITY_CHANGED) {
+ if (event.type() != DeviceEvent.Type.DEVICE_ADDED) {
return false;
}
if (mastershipService.getMasterFor(event.subject().id()) == null) {
diff --git a/providers/netconf/device/src/test/java/org/onosproject/provider/netconf/device/impl/NetconfDeviceProviderTest.java b/providers/netconf/device/src/test/java/org/onosproject/provider/netconf/device/impl/NetconfDeviceProviderTest.java
index 90763ec..53e7838 100644
--- a/providers/netconf/device/src/test/java/org/onosproject/provider/netconf/device/impl/NetconfDeviceProviderTest.java
+++ b/providers/netconf/device/src/test/java/org/onosproject/provider/netconf/device/impl/NetconfDeviceProviderTest.java
@@ -263,11 +263,12 @@
assertNotNull(providerService);
assertTrue("Event should be relevant", provider.cfgListener.isRelevant(deviceAddedEvent));
available = true;
-
- assertFalse("Device should not be reachable" + NETCONF_DEVICE_ID_STRING,
- provider.isReachable(DeviceId.deviceId(NETCONF_DEVICE_ID_STRING)));
provider.cfgListener.event(deviceAddedEvent);
+ deviceAdded.await();
+ assertEquals("Device should be added", 1, deviceStore.getDeviceCount());
+ assertTrue("Device incorrectly added" + NETCONF_DEVICE_ID_STRING,
+ devices.containsKey(DeviceId.deviceId(NETCONF_DEVICE_ID_STRING)));
devices.clear();
}
@@ -340,11 +341,6 @@
DeviceListener listener = null;
@Override
- public boolean isAvailable(DeviceId deviceId) {
- return true;
- }
-
- @Override
public Device getDevice(DeviceId deviceId) {
if (deviceId.toString().equals(NETCONF_DEVICE_ID_STRING)) {
return null;