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

Make POD container last OOM victim #4647

Merged
merged 2 commits into from
Feb 20, 2015
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
19 changes: 18 additions & 1 deletion pkg/kubelet/kubelet.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ const minShares = 2
const sharesPerCPU = 1024
const milliCPUToCPU = 1000

// The oom_score_adj of the POD infrastructure container. The default is 0, so
// any value below that makes it *less* likely to get OOM killed.
const podOomScoreAdj = -100

// SyncHandler is an interface implemented by Kubelet, for testability
type SyncHandler interface {
SyncPods([]api.BoundPod) error
Expand Down Expand Up @@ -938,7 +942,20 @@ func (kl *Kubelet) createPodInfraContainer(pod *api.BoundPod) (dockertools.Docke
if ref != nil {
record.Eventf(ref, "pulled", "Successfully pulled image %q", container.Image)
}
return kl.runContainer(pod, container, nil, "", "")
id, err := kl.runContainer(pod, container, nil, "", "")
if err != nil {
return "", err
}

// Set OOM score of POD container to lower than those of the other
// containers in the pod. This ensures that it is killed only as a last
// resort.
containerInfo, err := kl.dockerClient.InspectContainer(string(id))
if err != nil {
return "", err
}

return id, util.ApplyOomScoreAdj(containerInfo.State.Pid, podOomScoreAdj)
}

func (kl *Kubelet) pullImage(img string, ref *api.ObjectReference) error {
Expand Down
14 changes: 7 additions & 7 deletions pkg/kubelet/kubelet_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -437,7 +437,7 @@ func TestSyncPodsWithTerminationLog(t *testing.T) {
}
kubelet.drainWorkers()
verifyCalls(t, fakeDocker, []string{
"list", "create", "start", "list", "inspect_container", "inspect_image", "list", "create", "start"})
"list", "create", "start", "inspect_container", "list", "inspect_container", "inspect_image", "list", "create", "start"})

fakeDocker.Lock()
parts := strings.Split(fakeDocker.Container.HostConfig.Binds[0], ":")
Expand Down Expand Up @@ -497,7 +497,7 @@ func TestSyncPodsCreatesNetAndContainer(t *testing.T) {
kubelet.drainWorkers()

verifyCalls(t, fakeDocker, []string{
"list", "create", "start", "list", "inspect_container", "inspect_image", "list", "create", "start"})
"list", "create", "start", "inspect_container", "list", "inspect_container", "inspect_image", "list", "create", "start"})

fakeDocker.Lock()

Expand Down Expand Up @@ -547,7 +547,7 @@ func TestSyncPodsCreatesNetAndContainerPullsImage(t *testing.T) {
kubelet.drainWorkers()

verifyCalls(t, fakeDocker, []string{
"list", "create", "start", "list", "inspect_container", "inspect_image", "list", "create", "start"})
"list", "create", "start", "inspect_container", "list", "inspect_container", "inspect_image", "list", "create", "start"})

fakeDocker.Lock()

Expand All @@ -563,7 +563,7 @@ func TestSyncPodsCreatesNetAndContainerPullsImage(t *testing.T) {
fakeDocker.Unlock()
}

func TestSyncPodsWithNetCreatesContainer(t *testing.T) {
func TestSyncPodsWithPodInfraCreatesContainer(t *testing.T) {
kubelet, fakeDocker := newTestKubelet(t)
fakeDocker.ContainerList = []docker.APIContainers{
{
Expand Down Expand Up @@ -604,7 +604,7 @@ func TestSyncPodsWithNetCreatesContainer(t *testing.T) {
fakeDocker.Unlock()
}

func TestSyncPodsWithNetCreatesContainerCallsHandler(t *testing.T) {
func TestSyncPodsWithPodInfraCreatesContainerCallsHandler(t *testing.T) {
kubelet, fakeDocker := newTestKubelet(t)
fakeHttp := fakeHTTP{}
kubelet.httpClient = &fakeHttp
Expand Down Expand Up @@ -661,7 +661,7 @@ func TestSyncPodsWithNetCreatesContainerCallsHandler(t *testing.T) {
}
}

func TestSyncPodsDeletesWithNoNetContainer(t *testing.T) {
func TestSyncPodsDeletesWithNoPodInfraContainer(t *testing.T) {
kubelet, fakeDocker := newTestKubelet(t)
fakeDocker.ContainerList = []docker.APIContainers{
{
Expand Down Expand Up @@ -692,7 +692,7 @@ func TestSyncPodsDeletesWithNoNetContainer(t *testing.T) {
kubelet.drainWorkers()

verifyCalls(t, fakeDocker, []string{
"list", "stop", "create", "start", "list", "list", "inspect_container", "inspect_image", "list", "create", "start"})
"list", "stop", "create", "start", "inspect_container", "list", "list", "inspect_container", "inspect_image", "list", "create", "start"})

// A map iteration is used to delete containers, so must not depend on
// order here.
Expand Down
18 changes: 16 additions & 2 deletions pkg/kubelet/runonce_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -97,14 +97,28 @@ func TestRunOnce(t *testing.T) {
label: "syncPod",
container: docker.Container{
Config: &docker.Config{Image: "someimage"},
State: docker.State{Running: true},
State: docker.State{Running: true, Pid: 42},
},
},
{
label: "syncPod",
container: docker.Container{
Config: &docker.Config{Image: "someimage"},
State: docker.State{Running: true},
State: docker.State{Running: true, Pid: 42},
},
},
{
label: "syncPod",
container: docker.Container{
Config: &docker.Config{Image: "someimage"},
State: docker.State{Running: true, Pid: 42},
},
},
{
label: "syncPod",
container: docker.Container{
Config: &docker.Config{Image: "someimage"},
State: docker.State{Running: true, Pid: 42},
},
},
},
Expand Down
2 changes: 1 addition & 1 deletion pkg/kubelet/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ func (s *KubeletServer) Run(_ []string) error {
s.EtcdServerList = util.StringList{}
}

if err := util.ApplyOomScoreAdj(s.OOMScoreAdj); err != nil {
if err := util.ApplyOomScoreAdj(0, s.OOMScoreAdj); err != nil {
glog.Info(err)
}

Expand Down
2 changes: 1 addition & 1 deletion pkg/proxy/server/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ func (s *ProxyServer) AddFlags(fs *pflag.FlagSet) {

// Run runs the specified ProxyServer. This should never exit.
func (s *ProxyServer) Run(_ []string) error {
if err := util.ApplyOomScoreAdj(s.OOMScoreAdj); err != nil {
if err := util.ApplyOomScoreAdj(0, s.OOMScoreAdj); err != nil {
glog.Info(err)
}

Expand Down
18 changes: 14 additions & 4 deletions pkg/util/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -175,14 +175,24 @@ func CompileRegexps(regexpStrings []string) ([]*regexp.Regexp, error) {
return regexps, nil
}

// Writes 'value' to /proc/self/oom_score_adj.
func ApplyOomScoreAdj(value int) error {
// Writes 'value' to /proc/<pid>/oom_score_adj. PID = 0 means self
func ApplyOomScoreAdj(pid int, value int) error {
if value < -1000 || value > 1000 {
return fmt.Errorf("invalid value(%d) specified for oom_score_adj. Values must be within the range [-1000, 1000]", value)
}
if pid < 0 {
return fmt.Errorf("invalid PID %d specified for oom_score_adj", pid)
}

var pidStr string
if pid == 0 {
pidStr = "self"
} else {
pidStr = strconv.Itoa(pid)
}

if err := ioutil.WriteFile("/proc/self/oom_score_adj", []byte(strconv.Itoa(value)), 0700); err != nil {
fmt.Errorf("failed to set oom_score_adj to %d - %q", value, err)
if err := ioutil.WriteFile(path.Join("/proc", pidStr, "oom_score_adj"), []byte(strconv.Itoa(value)), 0700); err != nil {
fmt.Errorf("failed to set oom_score_adj to %d: %v", value, err)
}

return nil
Expand Down