pkg: prevent oom watcher from depending on shim pkg#11433
pkg: prevent oom watcher from depending on shim pkg#11433AkihiroSuda merged 1 commit intocontainerd:mainfrom
Conversation
Signed-off-by: Lei Liu <[email protected]>
|
Hi @zhaodiaoer. Thanks for your PR. I'm waiting for a containerd member to verify that this patch is reasonable to test. If it is, they should reply with Once the patch is verified, the new status will be reflected by the I understand the commands that are listed here. DetailsInstructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
| // New returns an epoll implementation that listens to OOM events | ||
| // from a container's cgroups. | ||
| func New(publisher shim.Publisher) (oom.Watcher, error) { | ||
| func New(publisher events.Publisher) (oom.Watcher, error) { |
There was a problem hiding this comment.
Do we know if there was a specific reason for picking the shim.Publisher interface? Because it looks like they're not identical; the shim.Publisher also implements io.Closer;
Lines 48 to 52 in 00b357b
There was a problem hiding this comment.
☝️ introduced in ae87730, which also updated the NewOOMEpoller (which I think is what's this New function is now)
There was a problem hiding this comment.
If Close is not called, makes sense to reduce the interface. Its harder to expand it later, but calling Close on a passed in Publisher would never be the cleanest solution anyway.
There was a problem hiding this comment.
Ah, yes (sorry I had to jump out after I posted); I was indeed looking if any of the other methods were called anywhere. I didn't spot that being the case at an initial glance. The commit was as far as I got to see if there was something I missed, but that's where I left off ❤️
|
/cherry-pick release/2.0 |
|
@AkihiroSuda: new pull request created: #11439 DetailsIn response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
Compared with package
pkg/shim, packagecore/eventsis more suitable as the dependency for the oom watcher functions(pkg/oom).