-
Notifications
You must be signed in to change notification settings - Fork 65
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
Extend scope to enable common pluggable runtime extensions. #16
Conversation
6f528ce
to
5665bda
Compare
I'm not sure I like the idea of making this much more complex. That said, I keep wanting use NRI to do things and it doesn't have the information available to do what I need. |
c2cefdd
to
1a7172f
Compare
There are undeniable pros with a hook-like setup, for instance it does not matter how much you leak memory for processing a single request because the leaks do not cumulate. Then there are cons, for instance how much overhead you incur for processing a single request, whether your request processing is typically stateful (IOW if the result of processing a request has effects of subsequent requests or not) and so on. I guess whether the pros outweigh the cons with a hook-like approach or a daemon-like one depends on the collective nature of things you try to accomplish with NRI.
If it is possible to share what you'd like to accomplish using NRI it would be interesting to hear it. We could check if our proposed changes could make those possible and if not what it would take to make them possible. |
d987dd6
to
5b3288e
Compare
bdcfe7b
to
a1dbeb4
Compare
ece02b4
to
b04840e
Compare
Note: with respect to heading to an rpc (ttrpc/grpc) see cni v2 discussion migrating to rpc containernetworking/cni#785 |
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Run all tests in CI. Use dedicated targets in Makefile to run tests and collect test coverage metrics. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Update README.md providing a brief description of the revised NRI goals, background, protocol, etc. Link to the original description as README-v0.1.0.md. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Add a plugin to emulate the v0.1.0 NRI interface. This plugin registers itself for RunPodSandbox, StopPodSandbox, StartContainer and StopContainer events, then uses the original NRI Client API to load and execute any configured v0.1.0 plugins. The request data for the original NRI Client API is constructed from data provided by the new NRI API with some of the bits (containerd.Task) faked. The emulation is hopefully accurate enough to keep any existing plugins functional. Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
fece943
to
2b09b4a
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
some nit comments on the readme files..
looking very good just a couple nits on the readme.. another manual run against the now merged containerd/containerd main build with your new NRI service plugin and good to go I think. |
Move project status widgets and the draft status notice back to README.md. Co-authored-by: Mike Brown <brownwm@us.ibm.com> Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
Signed-off-by: Krisztian Litkey <krisztian.litkey@intel.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Thanks a ton this looks great.
const ( | ||
// SELinuxRelabel is a Mount pseudo-option to request relabeling. | ||
SELinuxRelabel = "relabel" | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this constant not consumed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm... no. Support for relabeling an injected mount (which was requested using that pseudo mount option) was dropped at one point, as it was unclear if it is needed. But I forgot to remove the related const. I'll file a PR for removing it.
Background
This PR aims to extend NRI beyond its current v1 capabilities. The primary goal is to allow NRI to act as a common infrastructure for plugging in extensions to runtimes. Among other things, these runtime extensions could then implement vendor- and domain-specific custom logic for improved container configuration.
The PR is both related to this KubeCon talk, and to this NRI issue.
Description
Note: For a more detailed description please refer to the README.
This PR
Using the plugin- and runtime-integration APIs allows one to hook plugins into the lifecycle of containers managed by a runtime. These plugins can then adjust a limited set of container parameters in connection with some of the container's lifecycle state transitions.
The proposal changes the v1 hook-like incarnation of plugins to more daemon-like entities, where a single instance of a plugin is responsible for handling the full stream of events intended for that plugin. It also defines a ttRPC-based plugin protocol which allows plugins to subscribe to lifecycle events of pods and containers and to make changes to containers during a container's creation, update and stop events. Adjustment is possible to the following container parameters during a container's creation:
Other than during creation, a container's assigned resources can be updated by plugin.
Related PRs
There are two more PRs closely related to this proposal:
Additionally there is a work-in-progress implementation of a more complex, custom resource assignment algorithm which exercises most of the capabilities offered by this proposal.