Merge pull request #525 from vmarmol/time-based

Add Start and End to ContainerInfoRequests.

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) {
+				if expectedPostObj.NumStats != expectedPostObjEmpty.NumStats ||
-					t.Errorf("Received unexpected object: %+v", expectedPostObjEmpty)
+					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)
 	}

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 {

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) {
+func newContainerData(containerName string, memoryStorage *memory.InMemoryStorage, handler container.ContainerHandler, loadReader cpuload.CpuLoadReader, logUsage bool) (*containerData, error) {
-	if driver == nil {
+	if memoryStorage == nil {
-		return nil, fmt.Errorf("nil storage driver")
+		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
 	}

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{}
+	memoryStorage := memory.New(60, nil)
-	ret, err := newContainerData(containerName, mockDriver, mockHandler, nil, false)
+	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)
+	ret, mockHandler, memoryStorage := setupContainerData(t, spec)
-	return ret, mockHandler, mockDriver
+	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)
 }
 

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
 	}

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