# Akri: Modularize `DiscoveryHandlers` as Pods ###### tags: `Proposals` ## Background Currently in Akri, protocol discovery is all implemented in the Akri Agent, which as explained in the [protocol extension proposal](https://github.com/deislabs/akri/blob/main/docs/proposals/simple-protocol-extension.md#background) makes the Agent bigger than needed (if someone is not using all the protocols), creates a larger attack surface, and requires `DiscoveryHandlers` (`DHs`) to be written in rust. Akri should be deployable with only the desired discovery protocols, and discovery protocols should be able to be added without changing Akri's core components (the Agent, Controller, and Configuration CRD). This would make extending Akri easier. This could be done by implementing idea 3 of the [protocol extension proposal](https://github.com/deislabs/akri/blob/main/docs/proposals/simple-protocol-extension.md#background), wherein protocols are implemented as pods and exposed over a gRPC/REST interface. The `DiscoveryHandler`'s `discover` method could be exposed over this interface. Additionally, the Configuration CRD would need to be modified so protocols are generic rather than explicitly defined in the CRD. Also, the Agent should still be able to be compiled with `DiscoveryHandlers` in case users want the smaller footprint of one Agent pod rather than Agent pod + 1 pod for each `DiscoveryHandler`. ## Alternatives Akri could stick with its current monolith structure of the Agent or implement one of the other ideas listed in the [protocol extension proposal](https://github.com/deislabs/akri/blob/main/docs/proposals/simple-protocol-extension.md#background), such as exposing the Agent as a library. > *Note*: The current implementation choice of protocols as Pods does not necessarily exclude later implementation of another protocol extension design. For example, Akri could support protocols as Pods and Agent as a library. ## Benefits of protocols as Pods - Smaller Agent attack surface - Agent does not have `NetworkPolicies` or volumes specific to a `DiscoveryHandler` - Agent has less dependencies and is easier to build locally - Developers can create `DiscoveryHandler` Pods and use them right away, without having to fork Akri or contribute them back. - Implementation of a `DiscoveryHandler` could be as simple as using a well-defined SDK (via OpenAPI or Protos) to implement the serving or receiving -- see push vs pull -- end of discovery. Currently, extending Akri requires changes to the Akri Controller, Agent, Configuration CRD, etc, and, therefore, understanding of the Akri code base. - `DiscoveryHandlers` could be written in any language - `DiscoveryHandlers` could exist outside the cluster as some external service ## Drawbacks - Larger Agent footprint (Agent Pod + 1 Pod per `DiscoveryHandler`) - Increased complexity: additional documentation, handling errors from 3rd party `DHs` - Harder Configuration validation as a result of making protocol definitions in the Configuration CRD generic ## Agent and `DiscoveryHandler` communication When designing the interaction between the Agent and the `DH` pods, a couple considerations come to mind. 1. Push vs Pull Agent: Should the Agent (1) periodically call `discover` on services exposed by `DiscoveryHandler` Pods or (2) have an endpoint for receiving discovery results that any `DiscoveryHandler` could contact? 2. Does a `DH` need to register with the Agent or should the Agent expose a single endpoint for receiving devices from any `DH`? These considerations led to the following three implementation options, with the third option being ultimately selected. ### Agent exposes a single `DiscoveryResults` service Instead of requesting the list of visible devices, the Agent could be randomly informed of devices by `DiscoveryHandlers`. This could be done by exposing a single discovery results endpoint. The Agent could have the simple design of one endpoint/interface for receiving discovered devices, and a new Configuration would initiate the acceptance of more device types. The Agent would be responsible for distinguishing when to reject devices without an associated Configuration. **How would the Configuration details, such as filters and other protocol specific information be passed to the `DH`?** The Agent could expose a `Register` endpoint to pass Configuration details to `DHs`, but how would it inform the `DHs` of the updated Configuration? The `DHs` would also need to expose a service for receiving Configuration details, requiring the Agent to also implement a client. ```sequence Title: Devices Pushed to Agent participant Operator participant Agent participant DH DH->Agent:Register Operator->etcd:Apply Configuration Note over etcd:Config A etcd-->Agent:Detect New Configuration Agent->DH: New Configuration Note over DH:Config A DH-->Agent:Discovered devices DH-->Agent:Discovered devices ``` Drawbacks - Complexity of sending updated Configuration information. - Does the agent assume the devices are still visible unless told otherwise? What if the DH Pod dies? How would the Agent distinguish between this and no device visibility changes? The Device Plugin Manager handles this by the Device Plugin's `ListAndWatch` function being a constant stream. If the stream drops, the Device Plugin Manager assumes that the device is gone. The Agent could do the same but maintaining a connection with each `DH` seems unneccessarily complex compared to the alternatives. - Agent could still be receiving results from `DHs` whose associated Configuration has been deleted. Unneccessary traffic to filter? - Fairly big architecture change. Benefits - This reduces the size and complexity of the Agent and provides more flexibilty. For example, with this design, one Configuration could have multiple `DiscoveryHanlder` Pods, without the Agent having to handle calling both. - A single `DH` Pod oftentimes will be associated with multiple Configurations (say discovering both udev cameras and microphones), so it may be unjustifiably complex for the Agent to call the same `DH` Pod multiple times for each Configuration/device type on a periodic basis. ### `DiscoveryHandlers` expose `Discovery` service In this scenario, the Agent would poll `DiscoveryHandlers` for a list of discovered devices. This fits in well with Akri's current implementation of the Agent having a [periodic discovery loop](https://sourcegraph.com/github.com/deislabs/akri/-/blob/agent/src/util/config_action.rs#L321:35) for each Configuration, that calls `discover` on the associated embedded `DiscoveryHandler` at regular intervals. This function instead would simply ping discovery endpoint(s) specified in a Configuration periodically. **How would the Configuration details, such as filters and other protocol specific information be passed to the `DH`?** Configuration details could be passed with the `discover` request. ```sequence Title: Agent Pulls Devices from Specified Endpoints participant Operator participant Agent participant DH Operator->etcd:Apply Configuration Note over etcd:Config A etcd-->Agent:Detect New Configuration Note left of Agent: get DH endpoints \n from config Agent-->DH:Discover Note right of Agent: Loop Note over DH:Config A DH-->Agent:Discovered Devices ``` Drawbacks - Requires operator to know the endpoints the `DH` Pods are exposing their discovery services on. - Continuous polling could be expensive, and what if there are multiple `DiscoveryHandler` Pods per Configuration? Polling each of these would only increase the cost and complexity. - Have to spawn a "thread" (not an OS thread) per Configuration for periodic discovery. - If the `DiscoveryHandlers` are not yet deployed or erroring, the Agent would be doing wasted work. (This could be resolved by have `DiscoveryHandlers` first register with a `register` endpoint exposed by the Agent as in next section). Benefits - Fits in well with current architecture - Agent only calls endpoints specified in the Configuration which can presumably be trusted, and Configuration nwould be the source of truth. - Could be a phased in approach, where some `DH`s, such as Zeroconf, are remoted with while others are still conditionally compiled in. ### Dual: Agent exposes `Register` service; `DiscoveryHandlers` expose `Discover` service Instead of only the `DHs` exposing `Discover` services as mentioned in the previous section, the Agent could also expose a `Register` service, that any `DiscoveryHandler` could call. The Agent would decide whether or not to accept a `DH` by checking for a Configuration with the same protocol name. Then the Agent can call `discover` on the `DH`. Since the `DH` would need to implement both a client stub for the `Register` service and its own Discovery service, this bi-directional interaction favors using gRPC and defining the interface with protobuf (like Device Plugin). This is similar to the design of the Kubernetes Device Plugin Manager. The Agent's function is analogous to the Device Plugin Manager. Just as the Device Plugin Manager periodically receives a list of a devices' health from Device Plugins, the Agent periodically receives a list of discovered devices from `DHs`. Instead of the Agent polling the `DH` with `Discover` calls like in the previous section, in this case, the `DiscoverResponse` would be a streamed response, allowing the `DH` to determine when to send an updated list of devices. **How would the Configuration details, such as filters and other protocol specific information be passed to the `DH`?** Configuration details would be sent with the initial `discover` request. If a Configuration was modified, the current stream would be dropped, and a new discover call would be made with the updated Configuration. ```sequence Title: Agent Pulls Devices from Registered DHs participant Operator participant Agent participant DH DH->Agent:Register Operator->etcd:Apply Configuration Note over etcd:Config A etcd-->Agent:Detect New Configuration Agent-->DH:Discover Note over DH:Config A Note left of DH: Stream DH-->Agent:Discovered Devices ``` > **Note**: the sequence diagrams appear to be rendering weirdly. An online version of the diagram can be [found here](https://sequencediagram.org/index.html#initialData=C4S2BsFMAIEEHNIDtjQCIgM4GMD2A3SAJwE8AoAQ22FyOgHkAHYimossxio0bELlNABEAahEAqcQHopsANZEQAHSRxEKIdAqY1yVAGIA7AA4AYqYDCATk7de-CoNETpUjDgLESACUcATKCJNbXRvaH1YAAYrACYAUWMyNG8AWgA+BD0ALgAlSHgsYGIOMiYWNnTIYGw-LNhGRnASaAtcJAAzEHgAVyJWEDaOJFwi6E86Kpqs1o6uuA5MlAAeFJTJ2rQqyGpoADlIAHcWts6evtBB8FxcRmgQDtwyReBVtOSs9zxCdipQfFYYMkyMNRuNQtMTnNYElUulnh8sF9iJA-OhIPgQNhIJgyH5IL8QP9RkDkH4yEA). Drawbacks - Have to spawn a "thread" (not an OS thread) per Configuration for receiving discover results. - Bi-directional communication and streamed response could be more complex for implementing `DH`s. - Complexity of hosting Register service in Agent. When would a service be de-registered? When it drops its end of the stream and does not respond to a new discover call? Benefits - Modeled after Device Plugin -- tried and true design? - `DHs` decide the rate at which to report discovered devices, which is likely protocol specific. - Using protobuf to simplify developer experience. Also, easy versioning with protobuf. - Fits in well with current design. - Could be a phased in approach, where some `DH`s, such as Zeroconf, are remoted with while others are still conditionally compiled in. **The fact that this fits in reasonably with the current design and is modeled after a working solution (Device Plugin) make this the preferred approach.** ## gRPC vs REST Both gRPC and REST permit running handlers remotely. Furthermore, gRPC services can be exposed as REST. Building off of [Daz's summary](https://github.com/deislabs/akri/issues/198#issuecomment-758845076) in the this document's associated feature request: gRPC Pros: - Supports bidirectional|streaming. gRPC clients and servers can stream events to one another. - gRPC provides mechanisms that expose services as REST/HTTP for non-gRPC uses - gRPC Is probably better if RPC better matches the interaction pattern - protobuf simplifies implementation and is supported in most languages REST Pros: - REST is more common => more developer tools - Can easily curl to debug - Protocol implementations are clients of Akri (supports [pull strategy](./#pull-agent-calls-discover-on-preconfigured-discoveryhandler-endpoints)); if not, would we need to add e.g. WebSockets for callbacks? - OpenAPI describes sufficient typing to offset any benefits of protobufs - REST Is probably better if the Akri discovery host is more of a set of resources with state Kubernetes' components frequently use gRPC service-service but the API Server (clients-service) is almost exclusively REST. Since both the Agent and the `DHs` expose services, gRPC is preferred. Plus, some components of the Device Plugin API proto file are directly added to Akri's discovery services, see the [discovery proto file](#discovery-proto-file). Using gRPC makes it easier to transfer associated updates from the Device Plugin API. ## Configuration re-design The Configuration CRD must be modified so `DiscoveryHandlers` are genericly rather than explicitly defined in the CRD. Currently, when a `DH` is added, its specific filtering options and other settings are defined in the Configuration CRD. For example, the ONVIF `DH` is defined in the [Configuration CRD](https://sourcegraph.com/github.com/deislabs/akri/-/blob/deployment/helm/crds/akri-configuration-crd.yaml#L30) as follows: ```yaml= protocol: # {{ProtocolHandler}} type: object properties: onvif: # {{OnvifDiscoveryHandler}} type: object properties: ipAddresses: # {{FilterList}} type: object properties: action: type: string enum: - Include - Exclude items: type: array items: type: string # explicitly defined `DH`s for udev, opcua, etc # ... ``` And the associated `OnvifDiscoveryHandlerConfig` is: ```rust= pub struct OnvifDiscoveryHandlerConfig { #[serde(default, skip_serializing_if = "Option::is_none")] pub ip_addresses: Option<FilterList>, #[serde(default, skip_serializing_if = "Option::is_none")] pub mac_addresses: Option<FilterList>, #[serde(default, skip_serializing_if = "Option::is_none")] pub scopes: Option<FilterList>, #[serde(default = "default_discovery_timeout_seconds")] pub discovery_timeout_seconds: i32, } ``` The Configuration CRD and `DH` fields are too tightly coupled. Instead in a Configuration, a protocol (`DH`) could be represented by a name and `discoveryDetails` HashMap with string keys and values. This map would be passed to `DHs` along with discover requests. The protocol section of Configuration may look like the following: ```yaml= protocol: type: object properties: name: type: string discoveryDetails: # map<string, string> additionalProperties: type: string type: object ``` For existing protocols, the key `protocolHandler` could be added to the `discoveryDetails` map, and value would be serialized yaml of the `ProtocolHandler`. For example, for ONVIF: > **Note**: `|+` makes the all subfields into one string ```yaml= protocol: name: onvif discoveryDetails: protocolHandler: |+ onvif: ipAddresses: action: Exclude items: - 10.0.0.1 - 10.0.0.2 macAddresses: action: Exclude items: [] scopes: action: Include items: - onvif://www.onvif.org/name/GreatONVIFCamera - onvif://www.onvif.org/name/AwesomeONVIFCamera discoveryTimeoutSeconds: 2 ``` Then, the ONVIF `DH` can be grabbed by getting the value with the key `protocolHandler` and deserializing it. A simple unit test of this: ```rust= let json = r#"{"protocol":{"name":"onvif", "discoveryDetails":{"protocolHandler":"{\"onvif\":{}}"}}}"#; let deserialized: Configuration = serde_json::from_str(json).unwrap(); match serde_json::from_str(&deserialized.protocol.discovery_details.get("protocolHandler").unwrap()).unwrap() { ProtocolHandler::onvif(_) => {} _ => panic!("protocol should be Onvif"), } ``` > **Note**: `ProtocolHandler` is an enum that matches to our currently supported embedded `DH`s. ### Configuration validation Figuring out how to pass generic `DH` information in the Configuration in a way that it can be validated is tricky. Akri could provide a Webhook for validating Configurations -- a [Webhook](https://github.com/deislabs/akri/pull/206) is currently in progress for the current Configuration CRD. For every new supported protocol, the Webhook could be extended to check that the format of the contents of the `discovery_details` map is as expected by its protocol. All protocols in the Akri repo would have this validation. Alternatively, the Webhook could expose an endpoint for receiving details on what schema to accept from `DHs`. Then, the Webhook code would not need to be extended for each protocol. It should not be assumed that all `DH` implementors will extend the Webhook or that all Akri users will used the Webhook, so another method of determining whether Configurations are valid is needed. A status field could be added to Configurations. Then,`DH` Pods can return an error status if it receives an invalid `discovery_details` map. The Agent would then update the Configuration with that status. This would at least be a transparent way of informing an Operator that they have applied an invalid Configuration. ## Discovery proto file For discovery, as discussed in the "Dual: Agent ..." section, two services need to be created: a Registration service exposed by each Agent and a Discovery service exposed by each `DiscoveryHandler` Pod. These services are defined in the following proto file: ```go syntax = "proto3"; package discovery; // Registration is the service advertised by the Akri Agent. // Any `DiscoveryHandler` can register with Akri under a specified // protocol. service Registration { rpc Register(RegisterRequest) returns (Empty) {} } message RegisterRequest { // Name of the protocol that the client uses for discovery string protocol = 1; // Name of the endpoint that is hosting the discovery service // for the protocol string endpoint = 2; // Specifies whether this device can only be ever seen by this node (e.g. a local USB device) // rather than being visible to multiple nodes (e.g. an IP camera) bool is_local = 3; } message Empty { } service Discovery { rpc Discover (DiscoverRequest) returns (stream DiscoverResponse); } message DiscoverRequest { // Map containing all the details (such as filtering options) // the discovery handler needs to find a set of devices. map<string, string> discovery_details = 1; } message DiscoverResponse { // List of discovered devices repeated Device devices = 1; } message Device { // Identifier for this device string id = 1; // Properties that identify the device. These are stored in the device's instance // and set as environment variables in the device's broker Pods. May be information // about where to find the device such as an RTSP URL or a device node (e.g. `/dev/video1`) map<string, string> properties = 2; // Optionally specify mounts for Pods that request this device as a resource repeated Mount mounts = 3; // Optionally specify device information to be mounted for Pods that request this device as a resource repeated DeviceSpec device_specs = 4; } // From Device Plugin API // Mount specifies a host volume to mount into a container. // where device library or tools are installed on host and container message Mount { // Path of the mount within the container. string container_path = 1; // Path of the mount on the host. string host_path = 2; // If set, the mount is read-only. bool read_only = 3; } // From Device Plugin API // DeviceSpec specifies a host device to mount into a container. message DeviceSpec { // Path of the device within the container. string container_path = 1; // Path of the device on the host. string host_path = 2; // Cgroups permissions of the device, candidates are one or more of // * r - allows container to read from the specified device. // * w - allows container to write to the specified device. // * m - allows container to create device files that do not yet exist. string permissions = 3; } ``` The `Mount` and `DeviceSpec` messages are taken directly from the [Device Plugin API](https://github.com/kubernetes/kubernetes/blob/09f4baed35865d410febb3220811ca5c2fe1cf42/staging/src/k8s.io/kubelet/pkg/apis/deviceplugin/v1beta1/api.proto#L191). This gives `DHs` the ability to tell the Agent (so it can tell kubelet) what needs to be mounted in Pods that request a device as a resource. For example, for devices discovered via `udev`, currently, the device path (e.g. `/dev/video1`) is [specified as a volume mount](https://sourcegraph.com/github.com/deislabs/akri/-/blob/agent/src/util/device_plugin_service.rs#L458). This is protocol specific and therefore should be deligated to `DHs`. A `DH` specifies whether the devices it discovers are local or network/remote in a `RegisterRequest`. Note that `is_local` is replacing `is_shared` due to the confusion around the term. ## Other Considerations ### Handling disappearance of `DH` Pods While this architecture provides a `Register` function, should a `DeRegister` endpoint also be provided? What happens if a `DH` goes offline? There doesn't seem to be a case where it makes sense for a `DH` to de-register itself, since the Agent can detect whether a `DH` is down via a failed `Discover` call and then remove the `DH` from the list of registered `DHs`. Furthermore, the deletion of a Configuration that uses that `DH`, stops agent from pinging that `DH` for devices. ### Continuing to support all-in-one Agent Akri should continue to provide an Agent with all `DHs` included in one Pod, so as to provide a low footprint solution. Akri will create containers for each supported `DH`, an Agent without `DHs`, and an Agent with all `DHs` included. If someone wants an Agent with only some `DHs` included, they can conditionally compile the Agent to include their required `DHs`. Ideally, each `DH`/protocol will have its own repo, and then the Agent can include those modules or the `DHs` can be put into their own Pod. The goal is to have 100% shared code between the Podded `DH` and embedded `DH`. This necessitates that the design has one interface that is used for both embedded `DHs` and podded `DHs`.