containerd / nri

Node Resource Interface
Apache License 2.0
257 stars 65 forks source link

Support hooks modifying exec paramters #44

Open PaulFurtado opened 1 year ago

PaulFurtado commented 1 year ago

We currently wrap the runc binary in order to modify containers and exec commands. It'd be great to move to NRI, but NRI does not appear to support modifying runc exec. Modifying runc exec is useful for many reasons, here are some that we currently use in production:

mikebrow commented 1 year ago

nod no hooks for exec... setting priv/user from env .. interesting (also probes?)

@mrunalp @haircommander note ^

haircommander commented 1 year ago

I wonder how we'd define the API so it'd differentiate exec vs execsync. I imagine all of that is largely unnecessary for probes, though they may want a different set of injections

PaulFurtado commented 1 year ago

setting priv/user from env .. interesting (also probes?)

Yeah, we want containers to run as non-root, but teams often need root to debug, so this allowed us bring back the behavior of docker exec --user feature that kubectl exec was always lacking. We actually block these directives in probes at admission control though since we tie the ability to set these back to specific user groups and probes would slip past that.

We are a less traditional kubernetes user since the vast majority of our workloads are stateful databases which have much more demanding debugging requirements from the database teams, so it's great having the containers fully locked down by default, but then letting database admins escalate via kubectl exec when necessary.

I wonder how we'd define the API so it'd differentiate exec vs execsync.

In terms of parameters, is there any difference between the two when it comes to runc exec? On our end, we distinguish these by prepending directives to the exec command at admission control so we don't actually need this support, but I could imagine it being useful for others.

I imagine all of that is largely unnecessary for probes, though they may want a different set of injections

One frustrating thing about our current setup of wrapping the runc command is that anything we do during runc create, we also need to implement for runc exec. Ex: if we inject an env var into a container by wrapping runc create then runc knows about it, but cri-o/containerd do not, so we also need to inject that env var into runc exec for probes to work. I'd imagine that the NRI create hook would sidestep this problem so we would care much less about hooking exec probes, but I'm sure we can find use cases for continuing to hook probes.

haircommander commented 1 year ago

In terms of parameters, is there any difference between the two when it comes to runc exec?

not really if you ignore the possibility of input or tty from the user (which doesn't change the actual command, but does significantly change what is setup for and by runc).