search

google / gvisor-containerd-shim

containerd shim for gVisor
https://gvisor.dev
Apache License 2.0
80 stars 28 forks source link

Support Epoller to forward oom score notifications. #56

Closed moricho closed 4 years ago

moricho commented 4 years ago

I investigated https://github.com/google/gvisor/issues/2433 . We probably can forward oom score notifications by using oom.Epoller (this is in the containerd/pkg/oom package) like runc's shim. ・https://github.com/containerd/containerd/blob/master/runtime/v2/runc/v2/service.go#L76-L80 ・https://github.com/containerd/containerd/blob/master/runtime/v2/runc/v2/service.go#L334-L336

While oom.Epoller is supported on containerd v1.3 , gvisor-containerd-shim depends on containerd v1.2 and there are several other changes we have to follow between v1.2 and v1.3.

Can we move to v1.2 to v1.3? Or should we use v1.2 and implement Epoller like runc's shim?

To: @ianlewis

ianlewis commented 4 years ago

Yeah, that's what I suspected would be the right way for it to work as well. I'm looking at upgrading packages from containerd 1.3 but some e2e tests fail. See #55

ianlewis commented 4 years ago

I posted on the PR but it looks like we'll have to stick with using packages from containerd 1.2 if we want to maintain compatibility with 1.2. Containerd seems to maintain backwards compatibility with shims built from previous versions but isn't forwards compatible with shims built from later versions. https://github.com/google/gvisor-containerd-shim/pull/55#issuecomment-616934492

It looks like we'll need to copy the code from epoll.go for the time being. https://github.com/containerd/containerd/blob/release/1.2/runtime/v2/runc/epoll.go

moricho commented 4 years ago

it looks like we'll have to stick with using packages from containerd 1.2 if we want to maintain compatibility with 1.2

It looks like we'll need to copy the code from epoll.go for the time being.

That makes sense. Thanks!