From 487867bd013664ad05f16e64a33a4b9737615794 Mon Sep 17 00:00:00 2001 From: Tim Hockin Date: Sun, 19 Oct 2014 20:54:52 -0700 Subject: [PATCH] Extract cadvisor code to cadvisor.go --- pkg/kubelet/cadvisor.go | 76 +++++++++++++++++++++++++++++++++++++ pkg/kubelet/kubelet.go | 67 ++------------------------------ pkg/kubelet/kubelet_test.go | 15 +++----- 3 files changed, 85 insertions(+), 73 deletions(-) create mode 100644 pkg/kubelet/cadvisor.go diff --git a/pkg/kubelet/cadvisor.go b/pkg/kubelet/cadvisor.go new file mode 100644 index 0000000000..427f1cce22 --- /dev/null +++ b/pkg/kubelet/cadvisor.go @@ -0,0 +1,76 @@ +/* +Copyright 2014 Google Inc. All rights reserved. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package kubelet + +import ( + "fmt" + + "github.com/GoogleCloudPlatform/kubernetes/pkg/kubelet/dockertools" + cadvisor "github.com/google/cadvisor/info" +) + +// cadvisorInterface is an abstract interface for testability. It abstracts the interface of "github.com/google/cadvisor/client".Client. +type cadvisorInterface interface { + ContainerInfo(name string, req *cadvisor.ContainerInfoRequest) (*cadvisor.ContainerInfo, error) + MachineInfo() (*cadvisor.MachineInfo, error) +} + +// This method takes a container's absolute path and returns the stats for the +// container. The container's absolute path refers to its hierarchy in the +// cgroup file system. e.g. The root container, which represents the whole +// machine, has path "/"; all docker containers have path "/docker/" +func (kl *Kubelet) statsFromContainerPath(cc cadvisorInterface, containerPath string, req *cadvisor.ContainerInfoRequest) (*cadvisor.ContainerInfo, error) { + cinfo, err := cc.ContainerInfo(containerPath, req) + if err != nil { + return nil, err + } + return cinfo, nil +} + +// GetContainerInfo returns stats (from Cadvisor) for a container. +func (kl *Kubelet) GetContainerInfo(podFullName, uuid, containerName string, req *cadvisor.ContainerInfoRequest) (*cadvisor.ContainerInfo, error) { + cc := kl.GetCadvisorClient() + if cc == nil { + return nil, nil + } + dockerContainers, err := dockertools.GetKubeletDockerContainers(kl.dockerClient, false) + if err != nil { + return nil, err + } + dockerContainer, found, _ := dockerContainers.FindPodContainer(podFullName, uuid, containerName) + if !found { + return nil, fmt.Errorf("couldn't find container") + } + return kl.statsFromContainerPath(cc, fmt.Sprintf("/docker/%s", dockerContainer.ID), req) +} + +// GetRootInfo returns stats (from Cadvisor) of current machine (root container). +func (kl *Kubelet) GetRootInfo(req *cadvisor.ContainerInfoRequest) (*cadvisor.ContainerInfo, error) { + cc := kl.GetCadvisorClient() + if cc == nil { + return nil, fmt.Errorf("no cadvisor connection") + } + return kl.statsFromContainerPath(cc, "/", req) +} + +func (kl *Kubelet) GetMachineInfo() (*cadvisor.MachineInfo, error) { + cc := kl.GetCadvisorClient() + if cc == nil { + return nil, fmt.Errorf("no cadvisor connection") + } + return cc.MachineInfo() +} diff --git a/pkg/kubelet/kubelet.go b/pkg/kubelet/kubelet.go index bb2e9c4377..fedfe26b89 100644 --- a/pkg/kubelet/kubelet.go +++ b/pkg/kubelet/kubelet.go @@ -17,7 +17,6 @@ limitations under the License. package kubelet import ( - "errors" "fmt" "io" "net/http" @@ -37,7 +36,6 @@ import ( "github.com/GoogleCloudPlatform/kubernetes/pkg/volume" "github.com/fsouza/go-dockerclient" "github.com/golang/glog" - "github.com/google/cadvisor/info" ) const defaultChanSize = 1024 @@ -47,12 +45,6 @@ const minShares = 2 const sharesPerCPU = 1024 const milliCPUToCPU = 1000 -// CadvisorInterface is an abstract interface for testability. It abstracts the interface of "github.com/google/cadvisor/client".Client. -type CadvisorInterface interface { - ContainerInfo(name string, req *info.ContainerInfoRequest) (*info.ContainerInfo, error) - MachineInfo() (*info.MachineInfo, error) -} - // SyncHandler is an interface implemented by Kubelet, for testability type SyncHandler interface { SyncPods([]api.BoundPod) error @@ -131,19 +123,19 @@ type Kubelet struct { pullBurst int // Optional, no statistics will be available if omitted - cadvisorClient CadvisorInterface + cadvisorClient cadvisorInterface cadvisorLock sync.RWMutex } // SetCadvisorClient sets the cadvisor client in a thread-safe way. -func (kl *Kubelet) SetCadvisorClient(c CadvisorInterface) { +func (kl *Kubelet) SetCadvisorClient(c cadvisorInterface) { kl.cadvisorLock.Lock() defer kl.cadvisorLock.Unlock() kl.cadvisorClient = c } // GetCadvisorClient gets the cadvisor client. -func (kl *Kubelet) GetCadvisorClient() CadvisorInterface { +func (kl *Kubelet) GetCadvisorClient() cadvisorInterface { kl.cadvisorLock.RLock() defer kl.cadvisorLock.RUnlock() return kl.cadvisorClient @@ -740,25 +732,6 @@ func (kl *Kubelet) syncLoop(updates <-chan PodUpdate, handler SyncHandler) { } } -func getCadvisorContainerInfoRequest(req *info.ContainerInfoRequest) *info.ContainerInfoRequest { - ret := &info.ContainerInfoRequest{ - NumStats: req.NumStats, - } - return ret -} - -// This method takes a container's absolute path and returns the stats for the -// container. The container's absolute path refers to its hierarchy in the -// cgroup file system. e.g. The root container, which represents the whole -// machine, has path "/"; all docker containers have path "/docker/" -func (kl *Kubelet) statsFromContainerPath(cc CadvisorInterface, containerPath string, req *info.ContainerInfoRequest) (*info.ContainerInfo, error) { - cinfo, err := cc.ContainerInfo(containerPath, getCadvisorContainerInfoRequest(req)) - if err != nil { - return nil, err - } - return cinfo, nil -} - // GetKubeletContainerLogs returns logs from the container // The second parameter of GetPodInfo and FindPodContainer methods represents pod UUID, which is allowed to be blank func (kl *Kubelet) GetKubeletContainerLogs(podFullName, containerName, tail string, follow bool, stdout, stderr io.Writer) error { @@ -789,40 +762,6 @@ func (kl *Kubelet) GetPodInfo(podFullName, uuid string) (api.PodInfo, error) { return dockertools.GetDockerPodInfo(kl.dockerClient, manifest, podFullName, uuid) } -// GetContainerInfo returns stats (from Cadvisor) for a container. -func (kl *Kubelet) GetContainerInfo(podFullName, uuid, containerName string, req *info.ContainerInfoRequest) (*info.ContainerInfo, error) { - cc := kl.GetCadvisorClient() - if cc == nil { - return nil, nil - } - dockerContainers, err := dockertools.GetKubeletDockerContainers(kl.dockerClient, false) - if err != nil { - return nil, err - } - dockerContainer, found, _ := dockerContainers.FindPodContainer(podFullName, uuid, containerName) - if !found { - return nil, errors.New("couldn't find container") - } - return kl.statsFromContainerPath(cc, fmt.Sprintf("/docker/%s", dockerContainer.ID), req) -} - -// GetRootInfo returns stats (from Cadvisor) of current machine (root container). -func (kl *Kubelet) GetRootInfo(req *info.ContainerInfoRequest) (*info.ContainerInfo, error) { - cc := kl.GetCadvisorClient() - if cc == nil { - return nil, fmt.Errorf("no cadvisor connection") - } - return kl.statsFromContainerPath(cc, "/", req) -} - -func (kl *Kubelet) GetMachineInfo() (*info.MachineInfo, error) { - cc := kl.GetCadvisorClient() - if cc == nil { - return nil, fmt.Errorf("no cadvisor connection") - } - return cc.MachineInfo() -} - func (kl *Kubelet) healthy(podFullName, podUUID string, currentState api.PodState, container api.Container, dockerContainer *docker.APIContainers) (health.Status, error) { // Give the container 60 seconds to start up. if container.LivenessProbe == nil { diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 0ca6174dfa..044a467d61 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -847,8 +847,7 @@ func TestGetContainerInfo(t *testing.T) { } mockCadvisor := &mockCadvisorClient{} - req := &info.ContainerInfoRequest{} - cadvisorReq := getCadvisorContainerInfoRequest(req) + cadvisorReq := &info.ContainerInfoRequest{} mockCadvisor.On("ContainerInfo", containerPath, cadvisorReq).Return(containerInfo, nil) kubelet, _, fakeDocker := newTestKubelet(t) @@ -862,7 +861,7 @@ func TestGetContainerInfo(t *testing.T) { }, } - stats, err := kubelet.GetContainerInfo("qux", "", "foo", req) + stats, err := kubelet.GetContainerInfo("qux", "", "foo", cadvisorReq) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -882,8 +881,7 @@ func TestGetRootInfo(t *testing.T) { fakeDocker := dockertools.FakeDockerClient{} mockCadvisor := &mockCadvisorClient{} - req := &info.ContainerInfoRequest{} - cadvisorReq := getCadvisorContainerInfoRequest(req) + cadvisorReq := &info.ContainerInfoRequest{} mockCadvisor.On("ContainerInfo", containerPath, cadvisorReq).Return(containerInfo, nil) kubelet := Kubelet{ @@ -894,7 +892,7 @@ func TestGetRootInfo(t *testing.T) { } // If the container name is an empty string, then it means the root container. - _, err := kubelet.GetRootInfo(req) + _, err := kubelet.GetRootInfo(cadvisorReq) if err != nil { t.Errorf("unexpected error: %v", err) } @@ -925,8 +923,7 @@ func TestGetContainerInfoWhenCadvisorFailed(t *testing.T) { containerInfo := &info.ContainerInfo{} mockCadvisor := &mockCadvisorClient{} - req := &info.ContainerInfoRequest{} - cadvisorReq := getCadvisorContainerInfoRequest(req) + cadvisorReq := &info.ContainerInfoRequest{} expectedErr := fmt.Errorf("some error") mockCadvisor.On("ContainerInfo", containerPath, cadvisorReq).Return(containerInfo, expectedErr) @@ -941,7 +938,7 @@ func TestGetContainerInfoWhenCadvisorFailed(t *testing.T) { }, } - stats, err := kubelet.GetContainerInfo("qux", "uuid", "foo", req) + stats, err := kubelet.GetContainerInfo("qux", "uuid", "foo", cadvisorReq) if stats != nil { t.Errorf("non-nil stats on error") }