diff --git a/client/client_test.go b/client/client_test.go index db972d32..6ed088fb 100644 --- a/client/client_test.go +++ b/client/client_test.go @@ -44,16 +44,19 @@ func testGetJsonData( return nil } -func cadvisorTestClient(path string, expectedPostObj, expectedPostObjEmpty, replyObj interface{}, t *testing.T) (*Client, *httptest.Server, error) { +func cadvisorTestClient(path string, expectedPostObj *info.ContainerInfoRequest, replyObj interface{}, t *testing.T) (*Client, *httptest.Server, error) { ts := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { if r.URL.Path == path { if expectedPostObj != nil { + expectedPostObjEmpty := new(info.ContainerInfoRequest) decoder := json.NewDecoder(r.Body) if err := decoder.Decode(expectedPostObjEmpty); err != nil { t.Errorf("Received invalid object: %v", err) } - if !reflect.DeepEqual(expectedPostObj, expectedPostObjEmpty) { - t.Errorf("Received unexpected object: %+v", expectedPostObjEmpty) + if expectedPostObj.NumStats != expectedPostObjEmpty.NumStats || + expectedPostObj.Start.Unix() != expectedPostObjEmpty.Start.Unix() || + expectedPostObj.End.Unix() != expectedPostObjEmpty.End.Unix() { + t.Errorf("Received unexpected object: %+v, expected: %+v", expectedPostObjEmpty, expectedPostObj) } } encoder := json.NewEncoder(w) @@ -88,7 +91,7 @@ func TestGetMachineinfo(t *testing.T) { }, }, } - client, server, err := cadvisorTestClient("/api/v1.2/machine", nil, nil, minfo, t) + client, server, err := cadvisorTestClient("/api/v1.2/machine", nil, minfo, t) if err != nil { t.Fatalf("unable to get a client %v", err) } @@ -110,7 +113,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.2/containers%v", containerName), query, &info.ContainerInfoRequest{}, cinfo, t) + client, server, err := cadvisorTestClient(fmt.Sprintf("/api/v1.2/containers%v", containerName), query, cinfo, t) if err != nil { t.Fatalf("unable to get a client %v", err) } @@ -125,7 +128,7 @@ func TestGetContainerInfo(t *testing.T) { } } -// Test a requesy failing +// Test a request failing func TestRequestFails(t *testing.T) { errorText := "there was an error" // Setup a server that simply fails. @@ -162,7 +165,7 @@ func TestGetSubcontainersInfo(t *testing.T) { *cinfo1, *cinfo2, } - client, server, err := cadvisorTestClient(fmt.Sprintf("/api/v1.2/subcontainers%v", containerName), query, &info.ContainerInfoRequest{}, response, t) + client, server, err := cadvisorTestClient(fmt.Sprintf("/api/v1.2/subcontainers%v", containerName), query, response, t) if err != nil { t.Fatalf("unable to get a client %v", err) } diff --git a/info/container.go b/info/container.go index b3816336..5349444b 100644 --- a/info/container.go +++ b/info/container.go @@ -76,6 +76,14 @@ type ContainerReference struct { type ContainerInfoRequest struct { // Max number of stats to return. NumStats int `json:"num_stats,omitempty"` + + // Start time for which to query information. + // If ommitted, the beginning of time is assumed. + Start time.Time `json:"start,omitempty"` + + // End time for which to query information. + // If ommitted, current time is assumed. + End time.Time `json:"end,omitempty"` } type ContainerInfo struct { diff --git a/manager/container.go b/manager/container.go index 53f746df..e2bc8373 100644 --- a/manager/container.go +++ b/manager/container.go @@ -25,7 +25,7 @@ import ( "github.com/golang/glog" "github.com/google/cadvisor/container" "github.com/google/cadvisor/info" - "github.com/google/cadvisor/storage" + "github.com/google/cadvisor/storage/memory" "github.com/google/cadvisor/summary" "github.com/google/cadvisor/utils/cpuload" ) @@ -47,7 +47,7 @@ type containerInfo struct { type containerData struct { handler container.ContainerHandler info containerInfo - storageDriver storage.StorageDriver + memoryStorage *memory.InMemoryStorage lock sync.Mutex loadReader cpuload.CpuLoadReader summaryReader *summary.StatsSummary @@ -107,9 +107,9 @@ func (c *containerData) DerivedStats() (info.DerivedStats, error) { return c.summaryReader.DerivedStats() } -func newContainerData(containerName string, driver storage.StorageDriver, handler container.ContainerHandler, loadReader cpuload.CpuLoadReader, logUsage bool) (*containerData, error) { - if driver == nil { - return nil, fmt.Errorf("nil storage driver") +func newContainerData(containerName string, memoryStorage *memory.InMemoryStorage, handler container.ContainerHandler, loadReader cpuload.CpuLoadReader, logUsage bool) (*containerData, error) { + if memoryStorage == nil { + return nil, fmt.Errorf("nil memory storage") } if handler == nil { return nil, fmt.Errorf("nil container handler") @@ -121,7 +121,7 @@ func newContainerData(containerName string, driver storage.StorageDriver, handle cont := &containerData{ handler: handler, - storageDriver: driver, + memoryStorage: memoryStorage, housekeepingInterval: *HousekeepingInterval, loadReader: loadReader, logUsage: logUsage, @@ -145,7 +145,8 @@ func newContainerData(containerName string, driver storage.StorageDriver, handle // Determine when the next housekeeping should occur. func (self *containerData) nextHousekeeping(lastHousekeeping time.Time) time.Time { if *allowDynamicHousekeeping { - stats, err := self.storageDriver.RecentStats(self.info.Name, 2) + var empty time.Time + stats, err := self.memoryStorage.RecentStats(self.info.Name, empty, empty, 2) if err != nil { if self.allowErrorLogging() { glog.Warningf("Failed to get RecentStats(%q) while determining the next housekeeping: %v", self.info.Name, err) @@ -200,7 +201,8 @@ func (c *containerData) housekeeping() { // Log usage if asked to do so. if c.logUsage { const numSamples = 60 - stats, err := c.storageDriver.RecentStats(c.info.Name, numSamples) + var empty time.Time + stats, err := c.memoryStorage.RecentStats(c.info.Name, empty, empty, numSamples) if err != nil { if c.allowErrorLogging() { glog.Infof("[%s] Failed to get recent stats for logging usage: %v", c.info.Name, err) @@ -311,7 +313,7 @@ func (c *containerData) updateStats() error { } return err } - err = c.storageDriver.AddStats(ref, stats) + err = c.memoryStorage.AddStats(ref, stats) if err != nil { return err } diff --git a/manager/container_test.go b/manager/container_test.go index 1d23a506..57a0cad7 100644 --- a/manager/container_test.go +++ b/manager/container_test.go @@ -25,32 +25,33 @@ import ( "github.com/google/cadvisor/container" "github.com/google/cadvisor/info" itest "github.com/google/cadvisor/info/test" - stest "github.com/google/cadvisor/storage/test" + "github.com/google/cadvisor/storage/memory" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) const containerName = "/container" // Create a containerData instance for a test. -func setupContainerData(t *testing.T, spec info.ContainerSpec) (*containerData, *container.MockContainerHandler, *stest.MockStorageDriver) { +func setupContainerData(t *testing.T, spec info.ContainerSpec) (*containerData, *container.MockContainerHandler, *memory.InMemoryStorage) { mockHandler := container.NewMockContainerHandler(containerName) mockHandler.On("GetSpec").Return( spec, nil, ) - mockDriver := &stest.MockStorageDriver{} - ret, err := newContainerData(containerName, mockDriver, mockHandler, nil, false) + memoryStorage := memory.New(60, nil) + ret, err := newContainerData(containerName, memoryStorage, mockHandler, nil, false) if err != nil { t.Fatal(err) } - return ret, mockHandler, mockDriver + return ret, mockHandler, memoryStorage } // Create a containerData instance for a test and add a default GetSpec mock. -func newTestContainerData(t *testing.T) (*containerData, *container.MockContainerHandler, *stest.MockStorageDriver) { +func newTestContainerData(t *testing.T) (*containerData, *container.MockContainerHandler, *memory.InMemoryStorage) { spec := itest.GenerateRandomContainerSpec(4) - ret, mockHandler, mockDriver := setupContainerData(t, spec) - return ret, mockHandler, mockDriver + ret, mockHandler, memoryStorage := setupContainerData(t, spec) + return ret, mockHandler, memoryStorage } func TestUpdateSubcontainers(t *testing.T) { @@ -114,23 +115,29 @@ func TestUpdateSubcontainersWithErrorOnDeadContainer(t *testing.T) { mockHandler.AssertExpectations(t) } +func checkNumStats(t *testing.T, memoryStorage *memory.InMemoryStorage, numStats int) { + var empty time.Time + stats, err := memoryStorage.RecentStats(containerName, empty, empty, -1) + require.Nil(t, err) + assert.Len(t, stats, numStats) +} + func TestUpdateStats(t *testing.T) { statsList := itest.GenerateRandomStats(1, 4, 1*time.Second) stats := statsList[0] - cd, mockHandler, mockDriver := newTestContainerData(t) + cd, mockHandler, memoryStorage := newTestContainerData(t) mockHandler.On("GetStats").Return( stats, nil, ) - mockDriver.On("AddStats", info.ContainerReference{Name: mockHandler.Name}, stats).Return(nil) - err := cd.updateStats() if err != nil { t.Fatal(err) } + checkNumStats(t, memoryStorage, 1) mockHandler.AssertExpectations(t) } diff --git a/manager/manager.go b/manager/manager.go index d10ecc33..78cd9e0f 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -300,7 +300,7 @@ func (self *manager) containerDataToContainerInfo(cont *containerData, query *in return nil, err } - stats, err := self.memoryStorage.RecentStats(cinfo.Name, query.NumStats) + stats, err := self.memoryStorage.RecentStats(cinfo.Name, query.Start, query.End, query.NumStats) if err != nil { return nil, err } diff --git a/storage/memory/memory.go b/storage/memory/memory.go index 758ebf08..e19afb4a 100644 --- a/storage/memory/memory.go +++ b/storage/memory/memory.go @@ -17,12 +17,14 @@ package memory import ( "fmt" "sync" + "time" "github.com/golang/glog" "github.com/google/cadvisor/info" "github.com/google/cadvisor/storage" ) +// TODO(vmarmol): See about refactoring this class, we have an unecessary redirection of containerStorage and InMemoryStorage. // containerStorage is used to store per-container information type containerStorage struct { ref info.ContainerReference @@ -40,22 +42,10 @@ func (self *containerStorage) AddStats(stats *info.ContainerStats) error { return nil } -func (self *containerStorage) RecentStats(numStats int) ([]*info.ContainerStats, error) { +func (self *containerStorage) RecentStats(start, end time.Time, maxStats int) ([]*info.ContainerStats, error) { self.lock.RLock() defer self.lock.RUnlock() - if self.recentStats.Size() < numStats || numStats < 0 { - numStats = self.recentStats.Size() - } - - // Stats in the recentStats list are stored in reverse chronological - // order, i.e. most recent stats is in the front. - // numStats will always <= recentStats.Size() so that there will be - // always at least numStats available stats to retrieve. We traverse - // the recentStats list from its head so that the returned slice will be in chronological - // order. The order of the returned slice is not specified by the - // StorageDriver interface, so it is not necessary for other storage - // drivers to return the slice in the same order. - return self.recentStats.FirstN(numStats), nil + return self.recentStats.InTimeRange(start, end, maxStats), nil } func newContainerStore(ref info.ContainerReference, maxNumStats int) *containerStorage { @@ -97,7 +87,7 @@ func (self *InMemoryStorage) AddStats(ref info.ContainerReference, stats *info.C return cstore.AddStats(stats) } -func (self *InMemoryStorage) RecentStats(name string, numStats int) ([]*info.ContainerStats, error) { +func (self *InMemoryStorage) RecentStats(name string, start, end time.Time, maxStats int) ([]*info.ContainerStats, error) { var cstore *containerStorage var ok bool err := func() error { @@ -112,7 +102,7 @@ func (self *InMemoryStorage) RecentStats(name string, numStats int) ([]*info.Con return nil, err } - return cstore.RecentStats(numStats) + return cstore.RecentStats(start, end, maxStats) } func (self *InMemoryStorage) Close() error { diff --git a/storage/memory/memory_test.go b/storage/memory/memory_test.go index 0516efef..12d51c9d 100644 --- a/storage/memory/memory_test.go +++ b/storage/memory/memory_test.go @@ -16,42 +16,82 @@ package memory import ( "testing" + "time" "github.com/google/cadvisor/info" - "github.com/google/cadvisor/storage" - "github.com/google/cadvisor/storage/test" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) -type memoryTestStorageDriver struct { - storage.StorageDriver -} +const containerName = "/container" -func (self *memoryTestStorageDriver) StatsEq(a, b *info.ContainerStats) bool { - return test.DefaultStatsEq(a, b) -} +var ( + containerRef = info.ContainerReference{Name: containerName} + zero time.Time +) -func runStorageTest(f func(test.TestStorageDriver, *testing.T), t *testing.T) { - maxSize := 200 - - for N := 10; N < maxSize; N += 10 { - testDriver := &memoryTestStorageDriver{} - testDriver.StorageDriver = New(N, nil) - f(testDriver, t) +// Make stats with the specified identifier. +func makeStat(i int) *info.ContainerStats { + return &info.ContainerStats{ + Timestamp: zero.Add(time.Duration(i) * time.Second), + Cpu: info.CpuStats{ + LoadAverage: int32(i), + }, } } -func TestRetrievePartialRecentStats(t *testing.T) { - runStorageTest(test.StorageDriverTestRetrievePartialRecentStats, t) +func getRecentStats(t *testing.T, memoryStorage *InMemoryStorage, numStats int) []*info.ContainerStats { + stats, err := memoryStorage.RecentStats(containerName, zero, zero, numStats) + require.Nil(t, err) + return stats } -func TestRetrieveAllRecentStats(t *testing.T) { - runStorageTest(test.StorageDriverTestRetrieveAllRecentStats, t) +func TestAddStats(t *testing.T) { + memoryStorage := New(60, nil) + + assert := assert.New(t) + assert.Nil(memoryStorage.AddStats(containerRef, makeStat(0))) + assert.Nil(memoryStorage.AddStats(containerRef, makeStat(1))) + assert.Nil(memoryStorage.AddStats(containerRef, makeStat(2))) + assert.Nil(memoryStorage.AddStats(containerRef, makeStat(0))) + containerRef2 := info.ContainerReference{ + Name: "/container2", + } + assert.Nil(memoryStorage.AddStats(containerRef2, makeStat(0))) + assert.Nil(memoryStorage.AddStats(containerRef2, makeStat(1))) } -func TestNoRecentStats(t *testing.T) { - runStorageTest(test.StorageDriverTestNoRecentStats, t) +func TestRecentStatsNoRecentStats(t *testing.T) { + memoryStorage := makeWithStats(0) + + _, err := memoryStorage.RecentStats(containerName, zero, zero, 60) + assert.NotNil(t, err) } -func TestRetrieveZeroStats(t *testing.T) { - runStorageTest(test.StorageDriverTestRetrieveZeroRecentStats, t) +// Make an instance of InMemoryStorage with n stats. +func makeWithStats(n int) *InMemoryStorage { + memoryStorage := New(60, nil) + + for i := 0; i < n; i++ { + memoryStorage.AddStats(containerRef, makeStat(i)) + } + return memoryStorage +} + +func TestRecentStatsGetZeroStats(t *testing.T) { + memoryStorage := makeWithStats(10) + + assert.Len(t, getRecentStats(t, memoryStorage, 0), 0) +} + +func TestRecentStatsGetSomeStats(t *testing.T) { + memoryStorage := makeWithStats(10) + + assert.Len(t, getRecentStats(t, memoryStorage, 5), 5) +} + +func TestRecentStatsGetAllStats(t *testing.T) { + memoryStorage := makeWithStats(10) + + assert.Len(t, getRecentStats(t, memoryStorage, -1), 10) }