Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support filters for NRI plugins #72

Open
hormes opened this issue Feb 22, 2024 · 6 comments
Open

Support filters for NRI plugins #72

hormes opened this issue Feb 22, 2024 · 6 comments

Comments

@hormes
Copy link

hormes commented Feb 22, 2024

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
Copy link
Member

klihub commented Feb 22, 2024

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
Copy link
Author

hormes commented Feb 23, 2024

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
Copy link
Member

klihub commented Feb 29, 2024

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
Copy link
Author

hormes commented Mar 14, 2024

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
Copy link
Author

hormes commented Mar 14, 2024

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
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants