From 5109ce33565b2538d802dcc99858236fe3e2ca49 Mon Sep 17 00:00:00 2001 From: Jimmi Dyson Date: Mon, 1 Dec 2014 11:10:59 +0000 Subject: [PATCH] Fixes #2681: update to cadvisor 0.6.2 --- Godeps/Godeps.json | 8 +- .../google/cadvisor/client/client.go | 48 +++++- .../google/cadvisor/client/client_test.go | 8 +- .../github.com/google/cadvisor/info/advice.go | 34 ----- .../google/cadvisor/info/container.go | 137 +++++++++++------- .../google/cadvisor/info/container_test.go | 24 +-- .../google/cadvisor/info/test/datagen.go | 2 - .../google/cadvisor/info/version.go | 2 +- pkg/kubelet/cadvisor.go | 13 +- pkg/kubelet/kubelet_test.go | 15 +- 10 files changed, 160 insertions(+), 131 deletions(-) delete mode 100644 Godeps/_workspace/src/github.com/google/cadvisor/info/advice.go diff --git a/Godeps/Godeps.json b/Godeps/Godeps.json index 09dd3c8617a..3cf42eacc60 100644 --- a/Godeps/Godeps.json +++ b/Godeps/Godeps.json @@ -64,13 +64,13 @@ }, { "ImportPath": "github.com/google/cadvisor/client", - "Comment": "0.5.0", - "Rev": "8c4f650e62f096710da794e536de86e34447fca9" + "Comment": "0.6.2", + "Rev": "89088df70eca64cf9d6b9a23a3d2bc21a30916d6" }, { "ImportPath": "github.com/google/cadvisor/info", - "Comment": "0.5.0", - "Rev": "8c4f650e62f096710da794e536de86e34447fca9" + "Comment": "0.6.2", + "Rev": "89088df70eca64cf9d6b9a23a3d2bc21a30916d6" }, { "ImportPath": "github.com/google/gofuzz", diff --git a/Godeps/_workspace/src/github.com/google/cadvisor/client/client.go b/Godeps/_workspace/src/github.com/google/cadvisor/client/client.go index c1117d23252..5c016e05889 100644 --- a/Godeps/_workspace/src/github.com/google/cadvisor/client/client.go +++ b/Godeps/_workspace/src/github.com/google/cadvisor/client/client.go @@ -39,7 +39,7 @@ func NewClient(url string) (*Client, error) { } return &Client{ - baseUrl: fmt.Sprintf("%sapi/v1.1/", url), + baseUrl: fmt.Sprintf("%sapi/v1.2/", url), }, nil } @@ -80,6 +80,38 @@ func (self *Client) SubcontainersInfo(name string, query *info.ContainerInfoRequ return response, nil } +// Returns the JSON container information for the specified +// Docker container and request. +func (self *Client) DockerContainer(name string, query *info.ContainerInfoRequest) (cinfo info.ContainerInfo, err error) { + u := self.dockerInfoUrl(name) + ret := make(map[string]info.ContainerInfo) + if err = self.httpGetJsonData(&ret, query, u, fmt.Sprintf("Docker container info for %q", name)); err != nil { + return + } + if len(ret) != 1 { + err = fmt.Errorf("expected to only receive 1 Docker container: %+v", ret) + return + } + for _, cont := range ret { + cinfo = cont + } + return +} + +// Returns the JSON container information for all Docker containers. +func (self *Client) AllDockerContainers(query *info.ContainerInfoRequest) (cinfo []info.ContainerInfo, err error) { + u := self.dockerInfoUrl("/") + ret := make(map[string]info.ContainerInfo) + if err = self.httpGetJsonData(&ret, query, u, "all Docker containers info"); err != nil { + return + } + cinfo = make([]info.ContainerInfo, 0, len(ret)) + for _, cont := range ret { + cinfo = append(cinfo, cont) + } + return +} + func (self *Client) machineInfoUrl() string { return self.baseUrl + path.Join("machine") } @@ -92,6 +124,10 @@ func (self *Client) subcontainersInfoUrl(name string) string { return self.baseUrl + path.Join("subcontainers", name) } +func (self *Client) dockerInfoUrl(name string) string { + return self.baseUrl + path.Join("docker", name) +} + func (self *Client) httpGetJsonData(data, postData interface{}, url, infoName string) error { var resp *http.Response var err error @@ -106,17 +142,19 @@ func (self *Client) httpGetJsonData(data, postData interface{}, url, infoName st resp, err = http.Get(url) } if err != nil { - err = fmt.Errorf("unable to get %v: %v", infoName, err) - return err + return fmt.Errorf("unable to get %q: %v", infoName, err) + } + if resp == nil { + return fmt.Errorf("received empty response from %q", infoName) } defer resp.Body.Close() body, err := ioutil.ReadAll(resp.Body) if err != nil { - err = fmt.Errorf("unable to read all %v: %v", infoName, err) + err = fmt.Errorf("unable to read all %q: %v", infoName, err) return err } if err = json.Unmarshal(body, data); err != nil { - err = fmt.Errorf("unable to unmarshal %v (%v): %v", infoName, string(body), err) + err = fmt.Errorf("unable to unmarshal %q (%v): %v", infoName, string(body), err) return err } return nil diff --git a/Godeps/_workspace/src/github.com/google/cadvisor/client/client_test.go b/Godeps/_workspace/src/github.com/google/cadvisor/client/client_test.go index 35a75adb884..67bf3c2b6c0 100644 --- a/Godeps/_workspace/src/github.com/google/cadvisor/client/client_test.go +++ b/Godeps/_workspace/src/github.com/google/cadvisor/client/client_test.go @@ -57,7 +57,7 @@ func cadvisorTestClient(path string, expectedPostObj, expectedPostObjEmpty, repl } encoder := json.NewEncoder(w) encoder.Encode(replyObj) - } else if r.URL.Path == "/api/v1.1/machine" { + } else if r.URL.Path == "/api/v1.2/machine" { fmt.Fprint(w, `{"num_cores":8,"memory_capacity":31625871360}`) } else { w.WriteHeader(http.StatusNotFound) @@ -79,7 +79,7 @@ func TestGetMachineinfo(t *testing.T) { NumCores: 8, MemoryCapacity: 31625871360, } - client, server, err := cadvisorTestClient("/api/v1.1/machine", nil, nil, minfo, t) + client, server, err := cadvisorTestClient("/api/v1.2/machine", nil, nil, minfo, t) if err != nil { t.Fatalf("unable to get a client %v", err) } @@ -101,7 +101,7 @@ func TestGetContainerInfo(t *testing.T) { } containerName := "/some/container" cinfo := itest.GenerateRandomContainerInfo(containerName, 4, query, 1*time.Second) - client, server, err := cadvisorTestClient(fmt.Sprintf("/api/v1.1/containers%v", containerName), query, &info.ContainerInfoRequest{}, cinfo, t) + client, server, err := cadvisorTestClient(fmt.Sprintf("/api/v1.2/containers%v", containerName), query, &info.ContainerInfoRequest{}, cinfo, t) if err != nil { t.Fatalf("unable to get a client %v", err) } @@ -129,7 +129,7 @@ func TestGetSubcontainersInfo(t *testing.T) { *cinfo1, *cinfo2, } - client, server, err := cadvisorTestClient(fmt.Sprintf("/api/v1.1/subcontainers%v", containerName), query, &info.ContainerInfoRequest{}, response, t) + client, server, err := cadvisorTestClient(fmt.Sprintf("/api/v1.2/subcontainers%v", containerName), query, &info.ContainerInfoRequest{}, response, t) if err != nil { t.Fatalf("unable to get a client %v", err) } diff --git a/Godeps/_workspace/src/github.com/google/cadvisor/info/advice.go b/Godeps/_workspace/src/github.com/google/cadvisor/info/advice.go deleted file mode 100644 index 8084cf4741f..00000000000 --- a/Godeps/_workspace/src/github.com/google/cadvisor/info/advice.go +++ /dev/null @@ -1,34 +0,0 @@ -// 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 info - -// This struct describes one type of relationship between containers: One -// container, antagonist, interferes the performance of other -// containers, victims. -type Interference struct { - // Absolute name of the antagonist container name. This field - // should not be empty. - Antagonist string `json:"antagonist"` - - // The absolute path of the victims. This field should not be empty. - Victims []string `json:"victims"` - - // The name of the detector used to detect this antagonism. This field - // should not be empty - Detector string `json:"detector"` - - // Human readable description of this interference - Description string `json:"description,omitempty"` -} diff --git a/Godeps/_workspace/src/github.com/google/cadvisor/info/container.go b/Godeps/_workspace/src/github.com/google/cadvisor/info/container.go index 5eaacb59df0..5a4fbbd9b5b 100644 --- a/Godeps/_workspace/src/github.com/google/cadvisor/info/container.go +++ b/Godeps/_workspace/src/github.com/google/cadvisor/info/container.go @@ -53,10 +53,16 @@ type ContainerSpec struct { // Container reference contains enough information to uniquely identify a container type ContainerReference struct { - // The absolute name of the container. + // The absolute name of the container. This is unique on the machine. Name string `json:"name"` + // Other names by which the container is known within a certain namespace. + // This is unique within that namespace. Aliases []string `json:"aliases,omitempty"` + + // Namespace under which the aliases of a container are unique. + // An example of a namespace is "docker" for Docker containers. + Namespace string `json:"namespace,omitempty"` } // ContainerInfoQuery is used when users check a container info from the REST api. @@ -184,32 +190,30 @@ type DiskIoStats struct { IoServiced []PerDiskStats `json:"io_serviced,omitempty"` IoQueued []PerDiskStats `json:"io_queued,omitempty"` Sectors []PerDiskStats `json:"sectors,omitempty"` + IoServiceTime []PerDiskStats `json:"io_service_time,omitempty"` + IoWaitTime []PerDiskStats `json:"io_wait_time,omitempty"` + IoMerged []PerDiskStats `json:"io_merged,omitempty"` + IoTime []PerDiskStats `json:"io_time,omitempty"` } type MemoryStats struct { - // Memory limit, equivalent to "limit" in MemorySpec. - // Units: Bytes. - Limit uint64 `json:"limit,omitempty"` - - // Usage statistics. - // Current memory usage, this includes all memory regardless of when it was // accessed. // Units: Bytes. - Usage uint64 `json:"usage,omitempty"` + Usage uint64 `json:"usage"` // The amount of working set memory, this includes recently accessed memory, // dirty memory, and kernel memory. Working set is <= "usage". // Units: Bytes. - WorkingSet uint64 `json:"working_set,omitempty"` + WorkingSet uint64 `json:"working_set"` ContainerData MemoryStatsMemoryData `json:"container_data,omitempty"` HierarchicalData MemoryStatsMemoryData `json:"hierarchical_data,omitempty"` } type MemoryStatsMemoryData struct { - Pgfault uint64 `json:"pgfault,omitempty"` - Pgmajfault uint64 `json:"pgmajfault,omitempty"` + Pgfault uint64 `json:"pgfault"` + Pgmajfault uint64 `json:"pgmajfault"` } type NetworkStats struct { @@ -240,55 +244,74 @@ type FsStats struct { // Number of bytes that is consumed by the container on this filesystem. Usage uint64 `json:"usage"` + + // Number of reads completed + // This is the total number of reads completed successfully. + ReadsCompleted uint64 `json:"reads_completed"` + + // Number of reads merged + // Reads and writes which are adjacent to each other may be merged for + // efficiency. Thus two 4K reads may become one 8K read before it is + // ultimately handed to the disk, and so it will be counted (and queued) + // as only one I/O. This field lets you know how often this was done. + ReadsMerged uint64 `json:"reads_merged"` + + // Number of sectors read + // This is the total number of sectors read successfully. + SectorsRead uint64 `json:"sectors_read"` + + // Number of milliseconds spent reading + // This is the total number of milliseconds spent by all reads (as + // measured from __make_request() to end_that_request_last()). + ReadTime uint64 `json:"read_time"` + + // Number of writes completed + // This is the total number of writes completed successfully. + WritesCompleted uint64 `json:"writes_completed"` + + // Number of writes merged + // See the description of reads merged. + WritesMerged uint64 `json:"writes_merged"` + + // Number of sectors written + // This is the total number of sectors written successfully. + SectorsWritten uint64 `json:"sectors_written"` + + // Number of milliseconds spent writing + // This is the total number of milliseconds spent by all writes (as + // measured from __make_request() to end_that_request_last()). + WriteTime uint64 `json:"write_time"` + + // Number of I/Os currently in progress + // The only field that should go to zero. Incremented as requests are + // given to appropriate struct request_queue and decremented as they finish. + IoInProgress uint64 `json:"io_in_progress"` + + // Number of milliseconds spent doing I/Os + // This field increases so long as field 9 is nonzero. + IoTime uint64 `json:"io_time"` + + // weighted number of milliseconds spent doing I/Os + // This field is incremented at each I/O start, I/O completion, I/O + // merge, or read of these stats by the number of I/Os in progress + // (field 9) times the number of milliseconds spent doing I/O since the + // last update of this field. This can provide an easy measure of both + // I/O completion time and the backlog that may be accumulating. + WeightedIoTime uint64 `json:"weighted_io_time"` } type ContainerStats struct { // The time of this stat point. - Timestamp time.Time `json:"timestamp"` - Cpu *CpuStats `json:"cpu,omitempty"` - DiskIo DiskIoStats `json:"diskio,omitempty"` - Memory *MemoryStats `json:"memory,omitempty"` - Network *NetworkStats `json:"network,omitempty"` + Timestamp time.Time `json:"timestamp"` + Cpu CpuStats `json:"cpu,omitempty"` + DiskIo DiskIoStats `json:"diskio,omitempty"` + Memory MemoryStats `json:"memory,omitempty"` + Network NetworkStats `json:"network,omitempty"` + // Filesystem statistics Filesystem []FsStats `json:"filesystem,omitempty"` } -// Makes a deep copy of the ContainerStats and returns a pointer to the new -// copy. Copy() will allocate a new ContainerStats object if dst is nil. -func (self *ContainerStats) Copy(dst *ContainerStats) *ContainerStats { - if dst == nil { - dst = new(ContainerStats) - } - dst.Timestamp = self.Timestamp - if self.Cpu != nil { - if dst.Cpu == nil { - dst.Cpu = new(CpuStats) - } - // To make a deep copy of a slice, we need to copy every value - // in the slice. To make less memory allocation, we would like - // to reuse the slice in dst if possible. - percpu := dst.Cpu.Usage.PerCpu - if len(percpu) != len(self.Cpu.Usage.PerCpu) { - percpu = make([]uint64, len(self.Cpu.Usage.PerCpu)) - } - dst.Cpu.Usage = self.Cpu.Usage - dst.Cpu.Load = self.Cpu.Load - copy(percpu, self.Cpu.Usage.PerCpu) - dst.Cpu.Usage.PerCpu = percpu - } else { - dst.Cpu = nil - } - if self.Memory != nil { - if dst.Memory == nil { - dst.Memory = new(MemoryStats) - } - *dst.Memory = *self.Memory - } else { - dst.Memory = nil - } - return dst -} - func timeEq(t1, t2 time.Time, tolerance time.Duration) bool { // t1 should not be later than t2 if t1.After(t2) { @@ -328,12 +351,22 @@ func (a *ContainerStats) Eq(b *ContainerStats) bool { // Checks equality of the stats values. func (a *ContainerStats) StatsEq(b *ContainerStats) bool { + // TODO(vmarmol): Consider using this through reflection. if !reflect.DeepEqual(a.Cpu, b.Cpu) { return false } if !reflect.DeepEqual(a.Memory, b.Memory) { return false } + if !reflect.DeepEqual(a.DiskIo, b.DiskIo) { + return false + } + if !reflect.DeepEqual(a.Network, b.Network) { + return false + } + if !reflect.DeepEqual(a.Filesystem, b.Filesystem) { + return false + } return true } diff --git a/Godeps/_workspace/src/github.com/google/cadvisor/info/container_test.go b/Godeps/_workspace/src/github.com/google/cadvisor/info/container_test.go index bd730c19c01..2ff38e699b3 100644 --- a/Godeps/_workspace/src/github.com/google/cadvisor/info/container_test.go +++ b/Godeps/_workspace/src/github.com/google/cadvisor/info/container_test.go @@ -15,7 +15,6 @@ package info import ( - "reflect" "testing" "time" ) @@ -69,10 +68,7 @@ func TestStatsEndTime(t *testing.T) { } func createStats(cpuUsage, memUsage uint64, timestamp time.Time) *ContainerStats { - stats := &ContainerStats{ - Cpu: &CpuStats{}, - Memory: &MemoryStats{}, - } + stats := &ContainerStats{} stats.Cpu.Usage.PerCpu = []uint64{cpuUsage} stats.Cpu.Usage.Total = cpuUsage stats.Cpu.Usage.System = 0 @@ -81,21 +77,3 @@ func createStats(cpuUsage, memUsage uint64, timestamp time.Time) *ContainerStats stats.Timestamp = timestamp return stats } - -func TestContainerStatsCopy(t *testing.T) { - stats := createStats(100, 101, time.Now()) - shadowStats := stats.Copy(nil) - if !reflect.DeepEqual(stats, shadowStats) { - t.Errorf("Copy() returned different object") - } - stats.Cpu.Usage.PerCpu[0] = shadowStats.Cpu.Usage.PerCpu[0] + 1 - stats.Cpu.Load = shadowStats.Cpu.Load + 1 - stats.Memory.Usage = shadowStats.Memory.Usage + 1 - if reflect.DeepEqual(stats, shadowStats) { - t.Errorf("Copy() did not deeply copy the object") - } - stats = shadowStats.Copy(stats) - if !reflect.DeepEqual(stats, shadowStats) { - t.Errorf("Copy() returned different object") - } -} diff --git a/Godeps/_workspace/src/github.com/google/cadvisor/info/test/datagen.go b/Godeps/_workspace/src/github.com/google/cadvisor/info/test/datagen.go index bb0b2ff47e0..519e28c1a3d 100644 --- a/Godeps/_workspace/src/github.com/google/cadvisor/info/test/datagen.go +++ b/Godeps/_workspace/src/github.com/google/cadvisor/info/test/datagen.go @@ -31,8 +31,6 @@ func GenerateRandomStats(numStats, numCores int, duration time.Duration) []*info } for i := 0; i < numStats; i++ { stats := new(info.ContainerStats) - stats.Cpu = new(info.CpuStats) - stats.Memory = new(info.MemoryStats) stats.Timestamp = currentTime currentTime = currentTime.Add(duration) diff --git a/Godeps/_workspace/src/github.com/google/cadvisor/info/version.go b/Godeps/_workspace/src/github.com/google/cadvisor/info/version.go index 8ae9b22712f..771382ad3b6 100644 --- a/Godeps/_workspace/src/github.com/google/cadvisor/info/version.go +++ b/Godeps/_workspace/src/github.com/google/cadvisor/info/version.go @@ -15,4 +15,4 @@ package info // Version of cAdvisor. -const VERSION = "0.5.0" +const VERSION = "0.6.2" diff --git a/pkg/kubelet/cadvisor.go b/pkg/kubelet/cadvisor.go index 427f1cce220..fdabb7f3c0d 100644 --- a/pkg/kubelet/cadvisor.go +++ b/pkg/kubelet/cadvisor.go @@ -25,6 +25,7 @@ import ( // cadvisorInterface is an abstract interface for testability. It abstracts the interface of "github.com/google/cadvisor/client".Client. type cadvisorInterface interface { + DockerContainer(name string, req *cadvisor.ContainerInfoRequest) (cadvisor.ContainerInfo, error) ContainerInfo(name string, req *cadvisor.ContainerInfoRequest) (*cadvisor.ContainerInfo, error) MachineInfo() (*cadvisor.MachineInfo, error) } @@ -41,6 +42,16 @@ func (kl *Kubelet) statsFromContainerPath(cc cadvisorInterface, containerPath st return cinfo, nil } +// This method takes a Docker container's ID and returns the stats for the +// container. +func (kl *Kubelet) statsFromDockerContainer(cc cadvisorInterface, containerId string, req *cadvisor.ContainerInfoRequest) (*cadvisor.ContainerInfo, error) { + cinfo, err := cc.DockerContainer(containerId, 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() @@ -55,7 +66,7 @@ func (kl *Kubelet) GetContainerInfo(podFullName, uuid, containerName string, req if !found { return nil, fmt.Errorf("couldn't find container") } - return kl.statsFromContainerPath(cc, fmt.Sprintf("/docker/%s", dockerContainer.ID), req) + return kl.statsFromDockerContainer(cc, dockerContainer.ID, req) } // GetRootInfo returns stats (from Cadvisor) of current machine (root container). diff --git a/pkg/kubelet/kubelet_test.go b/pkg/kubelet/kubelet_test.go index 65e35b05e66..9ffc2297da6 100644 --- a/pkg/kubelet/kubelet_test.go +++ b/pkg/kubelet/kubelet_test.go @@ -873,6 +873,12 @@ func (c *mockCadvisorClient) ContainerInfo(name string, req *info.ContainerInfoR return args.Get(0).(*info.ContainerInfo), args.Error(1) } +// DockerContainer is a mock implementation of CadvisorInterface.DockerContainer. +func (c *mockCadvisorClient) DockerContainer(name string, req *info.ContainerInfoRequest) (info.ContainerInfo, error) { + args := c.Called(name, req) + return args.Get(0).(info.ContainerInfo), args.Error(1) +} + // MachineInfo is a mock implementation of CadvisorInterface.MachineInfo. func (c *mockCadvisorClient) MachineInfo() (*info.MachineInfo, error) { args := c.Called() @@ -882,7 +888,7 @@ func (c *mockCadvisorClient) MachineInfo() (*info.MachineInfo, error) { func TestGetContainerInfo(t *testing.T) { containerID := "ab2cdf" containerPath := fmt.Sprintf("/docker/%v", containerID) - containerInfo := &info.ContainerInfo{ + containerInfo := info.ContainerInfo{ ContainerReference: info.ContainerReference{ Name: containerPath, }, @@ -890,7 +896,7 @@ func TestGetContainerInfo(t *testing.T) { mockCadvisor := &mockCadvisorClient{} cadvisorReq := &info.ContainerInfoRequest{} - mockCadvisor.On("ContainerInfo", containerPath, cadvisorReq).Return(containerInfo, nil) + mockCadvisor.On("DockerContainer", containerID, cadvisorReq).Return(containerInfo, nil) kubelet, _, fakeDocker := newTestKubelet(t) kubelet.cadvisorClient = mockCadvisor @@ -961,13 +967,12 @@ func TestGetContainerInfoWithoutCadvisor(t *testing.T) { func TestGetContainerInfoWhenCadvisorFailed(t *testing.T) { containerID := "ab2cdf" - containerPath := fmt.Sprintf("/docker/%v", containerID) - containerInfo := &info.ContainerInfo{} + containerInfo := info.ContainerInfo{} mockCadvisor := &mockCadvisorClient{} cadvisorReq := &info.ContainerInfoRequest{} expectedErr := fmt.Errorf("some error") - mockCadvisor.On("ContainerInfo", containerPath, cadvisorReq).Return(containerInfo, expectedErr) + mockCadvisor.On("DockerContainer", containerID, cadvisorReq).Return(containerInfo, expectedErr) kubelet, _, fakeDocker := newTestKubelet(t) kubelet.cadvisorClient = mockCadvisor