From 3e75f2effba11e8e847e92738de36a338071365c Mon Sep 17 00:00:00 2001 From: derekwaynecarr Date: Thu, 21 Jul 2016 18:31:36 -0400 Subject: [PATCH] Eviction manager needs to start as runtime dependent module --- pkg/kubelet/eviction/eviction_manager.go | 3 +- pkg/kubelet/eviction/types.go | 8 +++++- pkg/kubelet/kubelet.go | 7 +++-- pkg/kubelet/kubelet_cadvisor.go | 13 +++++++++ pkg/kubelet/kubelet_cadvisor_test.go | 35 ++++++++++++++++++++++++ 5 files changed, 62 insertions(+), 4 deletions(-) diff --git a/pkg/kubelet/eviction/eviction_manager.go b/pkg/kubelet/eviction/eviction_manager.go index f4ef16e5c5d..9cee8dd37b3 100644 --- a/pkg/kubelet/eviction/eviction_manager.go +++ b/pkg/kubelet/eviction/eviction_manager.go @@ -101,8 +101,9 @@ func (m *managerImpl) Admit(attrs *lifecycle.PodAdmitAttributes) lifecycle.PodAd } // Start starts the control loop to observe and response to low compute resources. -func (m *managerImpl) Start(podFunc ActivePodsFunc, monitoringInterval time.Duration) { +func (m *managerImpl) Start(diskInfoProvider DiskInfoProvider, podFunc ActivePodsFunc, monitoringInterval time.Duration) error { go wait.Until(func() { m.synchronize(podFunc) }, monitoringInterval, wait.NeverStop) + return nil } // IsUnderMemoryPressure returns true if the node is under memory pressure. diff --git a/pkg/kubelet/eviction/types.go b/pkg/kubelet/eviction/types.go index 185860722af..4f9ba2d6168 100644 --- a/pkg/kubelet/eviction/types.go +++ b/pkg/kubelet/eviction/types.go @@ -65,12 +65,18 @@ type Threshold struct { // Manager evaluates when an eviction threshold for node stability has been met on the node. type Manager interface { // Start starts the control loop to monitor eviction thresholds at specified interval. - Start(podFunc ActivePodsFunc, monitoringInterval time.Duration) + Start(diskInfoProvider DiskInfoProvider, podFunc ActivePodsFunc, monitoringInterval time.Duration) error // IsUnderMemoryPressure returns true if the node is under memory pressure. IsUnderMemoryPressure() bool } +// DiskInfoProvider is responsible for informing the manager how disk is configured. +type DiskInfoProvider interface { + // HasDedicatedImageFs returns true if the imagefs is on a separate device from the rootfs. + HasDedicatedImageFs() (bool, error) +} + // KillPodFunc kills a pod. // The pod status is updated, and then it is killed with the specified grace period. // This function must block until either the pod is killed or an error is encountered. diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index 03c704d64c9..1123fadeec3 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -926,7 +926,11 @@ func (kl *Kubelet) initializeModules() error { // initializeRuntimeDependentModules will initialize internal modules that require the container runtime to be up. func (kl *Kubelet) initializeRuntimeDependentModules() { if err := kl.cadvisor.Start(); err != nil { - kl.runtimeState.setInternalError(fmt.Errorf("Failed to start cAdvisor %v", err)) + kl.runtimeState.setInternalError(fmt.Errorf("failed to start cAdvisor %v", err)) + } + // eviction manager must start after cadvisor because it needs to know if the container runtime has a dedicated imagefs + if err := kl.evictionManager.Start(kl, kl.getActivePods, evictionMonitoringPeriod); err != nil { + kl.runtimeState.setInternalError(fmt.Errorf("failed to start eviction manager %v", err)) } } @@ -961,7 +965,6 @@ func (kl *Kubelet) Run(updates <-chan kubetypes.PodUpdate) { // Start component sync loops. kl.statusManager.Start() kl.probeManager.Start() - kl.evictionManager.Start(kl.getActivePods, evictionMonitoringPeriod) // Start the pod lifecycle event generator. kl.pleg.Start() diff --git a/pkg/kubelet/kubelet_cadvisor.go b/pkg/kubelet/kubelet_cadvisor.go index c61f89b04c3..be1876a84a5 100644 --- a/pkg/kubelet/kubelet_cadvisor.go +++ b/pkg/kubelet/kubelet_cadvisor.go @@ -45,6 +45,19 @@ func (kl *Kubelet) GetContainerInfo(podFullName string, podUID types.UID, contai return &ci, nil } +// HasDedicatedImageFs returns true if the imagefs has a dedicated device. +func (kl *Kubelet) HasDedicatedImageFs() (bool, error) { + imageFsInfo, err := kl.ImagesFsInfo() + if err != nil { + return false, err + } + rootFsInfo, err := kl.RootFsInfo() + if err != nil { + return false, err + } + return imageFsInfo.Device != rootFsInfo.Device, nil +} + // GetContainerInfoV2 returns stats (from Cadvisor) for containers. func (kl *Kubelet) GetContainerInfoV2(name string, options cadvisorapiv2.RequestOptions) (map[string]cadvisorapiv2.ContainerInfo, error) { return kl.cadvisor.ContainerInfoV2(name, options) diff --git a/pkg/kubelet/kubelet_cadvisor_test.go b/pkg/kubelet/kubelet_cadvisor_test.go index 03d69d1768d..a39bad872cb 100644 --- a/pkg/kubelet/kubelet_cadvisor_test.go +++ b/pkg/kubelet/kubelet_cadvisor_test.go @@ -21,6 +21,7 @@ import ( "testing" cadvisorapi "github.com/google/cadvisor/info/v1" + cadvisorapiv2 "github.com/google/cadvisor/info/v2" kubecontainer "k8s.io/kubernetes/pkg/kubelet/container" kubecontainertest "k8s.io/kubernetes/pkg/kubelet/container/testing" ) @@ -232,3 +233,37 @@ func TestGetContainerInfoWithNoMatchingContainers(t *testing.T) { } mockCadvisor.AssertExpectations(t) } + +func TestHasDedicatedImageFs(t *testing.T) { + testCases := map[string]struct { + imageFsInfo cadvisorapiv2.FsInfo + rootFsInfo cadvisorapiv2.FsInfo + expected bool + }{ + "has-dedicated-image-fs": { + imageFsInfo: cadvisorapiv2.FsInfo{Device: "123"}, + rootFsInfo: cadvisorapiv2.FsInfo{Device: "456"}, + expected: true, + }, + "has-unified-image-fs": { + imageFsInfo: cadvisorapiv2.FsInfo{Device: "123"}, + rootFsInfo: cadvisorapiv2.FsInfo{Device: "123"}, + expected: false, + }, + } + for testName, testCase := range testCases { + testKubelet := newTestKubelet(t, false /* controllerAttachDetachEnabled */) + kubelet := testKubelet.kubelet + mockCadvisor := testKubelet.fakeCadvisor + mockCadvisor.On("Start").Return(nil) + mockCadvisor.On("ImagesFsInfo").Return(testCase.imageFsInfo, nil) + mockCadvisor.On("RootFsInfo").Return(testCase.rootFsInfo, nil) + actual, err := kubelet.HasDedicatedImageFs() + if err != nil { + t.Errorf("case: %s, unexpected error: %v", testName, err) + } + if actual != testCase.expected { + t.Errorf("case: %s, expected: %v, actual: %v", testName, testCase.expected, actual) + } + } +}