Refactor channel and mastership handling in P4Runtime
This (big) change aims at solving the issue observed with mastership flapping
and device connection/disconnection with P4Runtime.
Channel handling is now based on the underlying gRPC channel state. Before,
channel events (open/close/error) were generated as a consequence of P4Runtime
StreamChannel events, making device availability dependent on mastership. Now
Stream Channel events only affect mastership (MASTER/STANDBY or NONE when the
SteamChannel RPC is not active).
Mastership handling has been refactored to generate P4Runtime election IDs that
are compatible with the mastership preference decided by the MastershipService.
GeneralDeviceProvider has been re-implemented to support in-order
device event processing and to reduce implementation complexity. Stats polling
has been moved to a separate component, and netcfg handling updated to only
depend on BasicDeviceConfig, augmented with a pipeconf field, and re-using the
managementAddress field to set the gRPC server endpoints (e.g.
grpc://myswitch.local:50051). Before it was depending on 3 different config
classes, making hard to detect changes.
Finally, this change affects some core interfaces:
- Adds a method to DeviceProvider and DeviceHandshaker to check for device
availability, making the meaning of availability device-specific. This is needed
in cases where the device manager needs to change the availability state of a
device (as in change #20842)
- Support device providers not capable of reconciling mastership role responses
with requests (like P4Runtime).
- Clarify the meaning of "connection" in the DeviceConnect behavior.
- Allows driver-based providers to check devices for reachability and
availability without probing the device via the network.
Change-Id: I7ff30d29f5d02ad938e3171536e54ae2916629a2
diff --git a/core/api/src/main/java/org/onosproject/net/driver/DeviceConnect.java b/core/api/src/main/java/org/onosproject/net/driver/DeviceConnect.java
index f30386d..433be71 100644
--- a/core/api/src/main/java/org/onosproject/net/driver/DeviceConnect.java
+++ b/core/api/src/main/java/org/onosproject/net/driver/DeviceConnect.java
@@ -21,7 +21,9 @@
/**
* Abstraction of handler behaviour used to set-up and tear-down connections
- * with a device.
+ * with a device. A connection is intended as the presence of state (e.g. a
+ * transport session) required to carry messages between this node and the
+ * device.
*/
@Beta
public interface DeviceConnect extends HandlerBehaviour {
@@ -29,17 +31,33 @@
/**
* Connects to the device, for example by opening the transport session that
* will be later used to send control messages. Returns true if the
- * connection was initiated successfully, false otherwise.
+ * connection was initiated successfully, false otherwise. The
+ * implementation might require probing the device over the network to
+ * initiate the connection.
* <p>
- * Calling multiple times this method while a connection to the device is
- * open should result in a no-op.
+ * When calling this method while a connection to the device already exists,
+ * the behavior is not defined. For example, some implementations might
+ * require to first call {@link #disconnect()}, while other might behave as
+ * a no-op.
*
- * @return CompletableFuture with true if the operation was successful
+ * @return CompletableFuture eventually true if a connection was created
+ * successfully, false otherwise
+ * @throws IllegalStateException if a connection already exists and the
+ * implementation requires to call {@link
+ * #disconnect()} first.
*/
- CompletableFuture<Boolean> connect();
+ CompletableFuture<Boolean> connect() throws IllegalStateException;
/**
- * Returns true if a connection to the device is open, false otherwise.
+ * Returns true if a connection to the device exists, false otherwise. This
+ * method is NOT expected to send any message over the network to check for
+ * device reachability, but rather it should only give an indication if any
+ * internal connection state exists for the device. As such, it should NOT
+ * block execution.
+ * <p>
+ * In general, when called after {@link #connect()} it should always return
+ * true, while it is expected to always return false after calling {@link
+ * #disconnect()} or if {@link #connect()} was never called.
*
* @return true if the connection is open, false otherwise
*/
@@ -47,14 +65,10 @@
/**
* Disconnects from the device, for example closing the transport session
- * previously opened. Returns true if the disconnection procedure was
- * successful, false otherwise.
+ * previously opened.
* <p>
* Calling multiple times this method while a connection to the device is
- * closed should result in a no-op.
- *
- * @return CompletableFuture with true if the operation was successful
+ * already closed should result in a no-op.
*/
- CompletableFuture<Boolean> disconnect();
-
+ void disconnect();
}