Break storage/memory from the StorageDriver API.

It is not a StorageDriver, rather it is the always-existing memory
storage.

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 {
+	return self.recentStats.InTimeRange(start, end, maxStats), nil
-		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
 }
 
 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 {

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/stretchr/testify/assert"
-	"github.com/google/cadvisor/storage/test"
+	"github.com/stretchr/testify/require"
 )
 
-type memoryTestStorageDriver struct {
+const containerName = "/container"
-	storage.StorageDriver
-}
 
-func (self *memoryTestStorageDriver) StatsEq(a, b *info.ContainerStats) bool {
+var (
-	return test.DefaultStatsEq(a, b)
+	containerRef = info.ContainerReference{Name: containerName}
-}
+	zero         time.Time
+)
 
-func runStorageTest(f func(test.TestStorageDriver, *testing.T), t *testing.T) {
+// Make stats with the specified identifier.
-	maxSize := 200
+func makeStat(i int) *info.ContainerStats {
-
+	return &info.ContainerStats{
-	for N := 10; N < maxSize; N += 10 {
+		Timestamp: zero.Add(time.Duration(i) * time.Second),
-		testDriver := &memoryTestStorageDriver{}
+		Cpu: info.CpuStats{
-		testDriver.StorageDriver = New(N, nil)
+			LoadAverage: int32(i),
-		f(testDriver, t)
+		},
 	}
 }
 
-func TestRetrievePartialRecentStats(t *testing.T) {
+func getRecentStats(t *testing.T, memoryStorage *InMemoryStorage, numStats int) []*info.ContainerStats {
-	runStorageTest(test.StorageDriverTestRetrievePartialRecentStats, t)
+	stats, err := memoryStorage.RecentStats(containerName, zero, zero, numStats)
+	require.Nil(t, err)
+	return stats
 }
 
-func TestRetrieveAllRecentStats(t *testing.T) {
+func TestAddStats(t *testing.T) {
-	runStorageTest(test.StorageDriverTestRetrieveAllRecentStats, 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) {
+func TestRecentStatsNoRecentStats(t *testing.T) {
-	runStorageTest(test.StorageDriverTestNoRecentStats, t)
+	memoryStorage := makeWithStats(0)
+
+	_, err := memoryStorage.RecentStats(containerName, zero, zero, 60)
+	assert.NotNil(t, err)
 }
 
-func TestRetrieveZeroStats(t *testing.T) {
+// Make an instance of InMemoryStorage with n stats.
-	runStorageTest(test.StorageDriverTestRetrieveZeroRecentStats, t)
+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)
 }