containerd / nri

Node Resource Interface
Apache License 2.0
220 stars 58 forks source link

Support filters for NRI plugins #72

Open hormes opened 4 months ago

hormes commented 4 months ago

From https://github.com/containerd/nri/blob/main/pkg/api/api.proto#L29

// Runtime service is the public API runtimes expose for NRI plugins.
// On this interface RPC requests are initiated by the plugin. This
// only covers plugin registration and unsolicited container updates.
// The rest of the API is defined by the Plugin service.
service Runtime {
    // RegisterPlugin registers the plugin with the runtime.
    rpc RegisterPlugin(RegisterPluginRequest) returns (Empty);
    // UpdateContainers requests unsolicited updates to a set of containers.
    rpc UpdateContainers(UpdateContainersRequest) returns (UpdateContainersResponse);
}

message RegisterPluginRequest {
    // Name of the plugin to register.
    string plugin_name = 1;
    // Plugin invocation index. Plugins are called in ascending index order.
    string plugin_idx = 2;
}

Here, is it possible to support registration plugins to only handle some Pods or containers? Similar to kubernetes webhook, filter is a very common pattern. For example, we want to set some environment variables for the GPU Pod, but we do not want the GPU Pod hook server to affect the CPU Pod.

klihub commented 4 months ago

No, in NRI there is no server-/runtime-side filtering based on message content (for instance, some property of a pod or container). Events are only filtered based on whether a plugin has subscribed for them or not. With NRI you need to implement any filtering criteria/logic you might have in the event handler itself to decide whether you inject an environment variable or not.

hormes commented 4 months ago

This scenario exists. Not all hooks need to process all containers. From a stability perspective, we hope to minimize the impact of different plugins. Imagine what we will do if we cannot support this dispatch in NRI. We need to implement a server externally to register all hook points, and implement a plugin registration management mechanism again to bypass the plugin management logic of NRI. In terms of operation and maintenance, the life cycle of this new process is the same as containerd, and it needs to provide consistent stability guarantee. Within the enterprise, from the perspective of operation and maintenance costs, the most likely approach is to fork containerd and insert the code into the current NRI code. This is actually not what everyone wants to see, right?

klihub commented 4 months ago

This scenario exists. Not all hooks need to process all containers. From a stability perspective, we hope to minimize the impact of different plugins. Imagine what we will do if we cannot support this dispatch in NRI. We need to implement a server externally to register all hook points, and implement a plugin registration management mechanism again to bypass the plugin management logic of NRI.

What is your main concern about NRI plugins short circuiting themselves to ignore containers which they are not interested in ? Is it extra IPC ?

IIUC, you'd like to reduce overall IPC (have NRI runtime make fewer useless calls to NRI plugins). And you'd like to trade this reduction for extra complexity and extra CPU usage on the runtime side. And would like to introduce some filtering language, which then would need to be pretty complex to be descriptive enough to allow filtering to be offloaded to the NRI runtime...

If my assumptions above are in the ballpark, then wouldn't an alternative approach be to update the NRI protocol so, that it would allow the plugin to indicate that it is not interested in a pod or container, in response to Synchronize(), RunPodSandbox() and CreateContainer() NRI events ?

With such a setup, NRI on the runtime side could cache this information per plugin and filter out subsequent events for such pods and containers. Granted, the first event would still be sent to plugins, and plugins would still decide themselves whether to ignore a pod or container. But we would not need to introduce some rather complex data-as-interpreted-language contraption to NRI/the runtimes to describe all possible combinations of all filtering criteria to satisfy the needs of all possible plugins.

hormes commented 3 months ago

The biggest goal is to improve stability. NRI plugin exceptions will not affect containers that it does not interested.

In enterprise application scenarios, there may even be multiple nri plugins, and their ops responsibilities may be distributed among different teams.

hormes commented 3 months ago

From a performance perspective, I think this reduces the container runtime overhead rather than increasing it, because the rpc calls have changed from full to partial, and the increased filter overhead will not become a bottleneck.

mikebrow commented 1 month ago

FYI/for conversation hook injector's schema https://github.com/containers/podman/blob/v3.4.7/pkg/hooks/docs/oci-hooks.5.md