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) }