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

Refactor oom watcher to allow greater test coverage #86702

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 4 additions & 1 deletion pkg/kubelet/kubelet.go
Expand Up @@ -472,7 +472,10 @@ func NewMainKubelet(kubeCfg *kubeletconfiginternal.KubeletConfiguration,

containerRefManager := kubecontainer.NewRefManager()

oomWatcher := oomwatcher.NewWatcher(kubeDeps.Recorder)
oomWatcher, err := oomwatcher.NewWatcher(kubeDeps.Recorder)
if err != nil {
return nil, err
}

clusterDNS := make([]net.IP, 0, len(kubeCfg.ClusterDNS))
for _, ipEntry := range kubeCfg.ClusterDNS {
Expand Down
2 changes: 2 additions & 0 deletions pkg/kubelet/oom/BUILD
Expand Up @@ -68,11 +68,13 @@ go_test(
"@io_bazel_rules_go//go/platform:android": [
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/client-go/tools/record:go_default_library",
"//vendor/github.com/google/cadvisor/utils/oomparser:go_default_library",
"//vendor/github.com/stretchr/testify/assert:go_default_library",
],
"@io_bazel_rules_go//go/platform:linux": [
"//staging/src/k8s.io/api/core/v1:go_default_library",
"//staging/src/k8s.io/client-go/tools/record:go_default_library",
"//vendor/github.com/google/cadvisor/utils/oomparser:go_default_library",
"//vendor/github.com/stretchr/testify/assert:go_default_library",
],
"//conditions:default": [],
Expand Down
32 changes: 22 additions & 10 deletions pkg/kubelet/oom/oom_watcher_linux.go
Expand Up @@ -30,29 +30,41 @@ import (
"github.com/google/cadvisor/utils/oomparser"
)

type streamer interface {
StreamOoms(chan<- *oomparser.OomInstance)
}

var _ streamer = &oomparser.OomParser{}

type realWatcher struct {
recorder record.EventRecorder
recorder record.EventRecorder
oomStreamer streamer
}

var _ Watcher = &realWatcher{}

// NewWatcher creates and initializes a OOMWatcher based on parameters.
func NewWatcher(recorder record.EventRecorder) Watcher {
return &realWatcher{
recorder: recorder,
// NewWatcher creates and initializes a OOMWatcher backed by Cadvisor as
// the oom streamer.
func NewWatcher(recorder record.EventRecorder) (Watcher, error) {
oomStreamer, err := oomparser.New()
if err != nil {
return nil, err
}

watcher := &realWatcher{
recorder: recorder,
oomStreamer: oomStreamer,
}

return watcher, nil
}

const systemOOMEvent = "SystemOOM"

// Start watches for system oom's and records an event for every system oom encountered.
func (ow *realWatcher) Start(ref *v1.ObjectReference) error {
oomLog, err := oomparser.New()
if err != nil {
return err
}
outStream := make(chan *oomparser.OomInstance, 10)
go oomLog.StreamOoms(outStream)
go ow.oomStreamer.StreamOoms(outStream)

go func() {
defer runtime.HandleCrash()
Expand Down
19 changes: 13 additions & 6 deletions pkg/kubelet/oom/oom_watcher_linux_test.go
Expand Up @@ -19,19 +19,26 @@ package oom
import (
"testing"

"github.com/stretchr/testify/assert"

v1 "k8s.io/api/core/v1"
"k8s.io/client-go/tools/record"

"github.com/google/cadvisor/utils/oomparser"
"github.com/stretchr/testify/assert"
)

// TestBasic verifies that the OOMWatch works without error.
func TestBasic(t *testing.T) {
fakeRecorder := &record.FakeRecorder{}
node := &v1.ObjectReference{}
oomWatcher := NewWatcher(fakeRecorder)
assert.NoError(t, oomWatcher.Start(node))

// TODO: Improve this test once cadvisor exports events.EventChannel as an interface
// and thereby allow using a mock version of cadvisor.
// TODO: Substitute this `oomStreamer` out for a fake, and then write
// more comprehensive unit tests of the actual behavior.
oomStreamer, err := oomparser.New()
assert.NoError(t, err)

oomWatcher := &realWatcher{
recorder: fakeRecorder,
oomStreamer: oomStreamer,
}
assert.NoError(t, oomWatcher.Start(node))
}
4 changes: 2 additions & 2 deletions pkg/kubelet/oom/oom_watcher_unsupported.go
Expand Up @@ -28,8 +28,8 @@ type oomWatcherUnsupported struct{}
var _ Watcher = new(oomWatcherUnsupported)

// NewWatcher creates a fake one here
func NewWatcher(_ record.EventRecorder) Watcher {
return &oomWatcherUnsupported{}
func NewWatcher(_ record.EventRecorder) (Watcher, error) {
return &oomWatcherUnsupported{}, nil
}

func (ow *oomWatcherUnsupported) Start(_ *v1.ObjectReference) error {
Expand Down