Merge pull request #517 from vmarmol/clean

Make explicit the use of InMemoryStorage in Manager.

cadvisor.go

@@ -61,7 +61,7 @@ func main() {
 
 	setMaxProcs()
 
-	storageDriver, err := NewStorageDriver(*argDbDriver)
+	memoryStorage, err := NewMemoryStorage(*argDbDriver)
 	if err != nil {
 		glog.Fatalf("Failed to connect to database: %s", err)
 	}
@@ -71,7 +71,7 @@ func main() {
 		glog.Fatalf("Failed to create a system interface: %s", err)
 	}
 
-	containerManager, err := manager.New(storageDriver, sysFs)
+	containerManager, err := manager.New(memoryStorage, sysFs)
 	if err != nil {
 		glog.Fatalf("Failed to create a Container Manager: %s", err)
 	}

manager/manager.go

@@ -29,7 +29,7 @@ import (
 	"github.com/google/cadvisor/container"
 	"github.com/google/cadvisor/container/docker"
 	"github.com/google/cadvisor/info"
-	"github.com/google/cadvisor/storage"
+	"github.com/google/cadvisor/storage/memory"
 	"github.com/google/cadvisor/utils/cpuload"
 	"github.com/google/cadvisor/utils/sysfs"
 )
@@ -65,10 +65,10 @@ type Manager interface {
 	GetVersionInfo() (*info.VersionInfo, error)
 }
 
-// New takes a driver and returns a new manager.
-func New(driver storage.StorageDriver, sysfs sysfs.SysFs) (Manager, error) {
-	if driver == nil {
-		return nil, fmt.Errorf("nil storage driver!")
+// New takes a memory storage and returns a new manager.
+func New(memoryStorage *memory.InMemoryStorage, sysfs sysfs.SysFs) (Manager, error) {
+	if memoryStorage == nil {
+		return nil, fmt.Errorf("manager requires memory storage")
 	}
 
 	// Detect the container we are running on.
@@ -81,7 +81,7 @@ func New(driver storage.StorageDriver, sysfs sysfs.SysFs) (Manager, error) {
 	newManager := &manager{
 		containers:        make(map[namespacedContainerName]*containerData),
 		quitChannels:      make([]chan error, 0, 2),
-		storageDriver:     driver,
+		memoryStorage:     memoryStorage,
 		cadvisorContainer: selfContainer,
 	}
 
@@ -114,7 +114,7 @@ type namespacedContainerName struct {
 type manager struct {
 	containers             map[namespacedContainerName]*containerData
 	containersLock         sync.RWMutex
-	storageDriver          storage.StorageDriver
+	memoryStorage          *memory.InMemoryStorage
 	machineInfo            info.MachineInfo
 	versionInfo            info.VersionInfo
 	quitChannels           []chan error
@@ -250,7 +250,7 @@ func (self *manager) containerDataToContainerInfo(cont *containerData, query *in
 		return nil, err
 	}
 
-	stats, err := self.storageDriver.RecentStats(cinfo.Name, query.NumStats)
+	stats, err := self.memoryStorage.RecentStats(cinfo.Name, query.NumStats)
 	if err != nil {
 		return nil, err
 	}
@@ -380,7 +380,7 @@ func (m *manager) createContainer(containerName string) error {
 		return err
 	}
 	logUsage := *logCadvisorUsage && containerName == m.cadvisorContainer
-	cont, err := newContainerData(containerName, m.storageDriver, handler, m.loadReader, logUsage)
+	cont, err := newContainerData(containerName, m.memoryStorage, handler, m.loadReader, logUsage)
 	if err != nil {
 		return err
 	}

manager/manager_test.go

@@ -26,24 +26,21 @@ import (
 	"github.com/google/cadvisor/container/docker"
 	"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/google/cadvisor/utils/sysfs/fakesysfs"
 )
 
 // TODO(vmarmol): Refactor these tests.
 
 func createManagerAndAddContainers(
-	driver *stest.MockStorageDriver,
+	memoryStorage *memory.InMemoryStorage,
 	sysfs *fakesysfs.FakeSysFs,
 	containers []string,
 	f func(*container.MockContainerHandler),
 	t *testing.T,
 ) *manager {
-	if driver == nil {
-		driver = &stest.MockStorageDriver{}
-	}
 	container.ClearContainerHandlerFactories()
-	mif, err := New(driver, sysfs)
+	mif, err := New(memoryStorage, sysfs)
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -55,7 +52,7 @@ func createManagerAndAddContainers(
 				spec,
 				nil,
 			).Once()
-			cont, err := newContainerData(name, driver, mockHandler, nil, false)
+			cont, err := newContainerData(name, memoryStorage, mockHandler, nil, false)
 			if err != nil {
 				t.Fatal(err)
 			}
@@ -87,24 +84,25 @@ func expectManagerWithContainers(containers []string, query *info.ContainerInfoR
 		infosMap[container] = itest.GenerateRandomContainerInfo(container, 4, query, 1*time.Second)
 	}
 
-	driver := &stest.MockStorageDriver{}
+	memoryStorage := memory.New(query.NumStats, nil)
 	sysfs := &fakesysfs.FakeSysFs{}
 	m := createManagerAndAddContainers(
-		driver,
+		memoryStorage,
 		sysfs,
 		containers,
 		func(h *container.MockContainerHandler) {
 			cinfo := infosMap[h.Name]
-			stats := cinfo.Stats
+			ref, err := h.ContainerReference()
+			if err != nil {
+				t.Error(err)
+			}
+			for _, stat := range cinfo.Stats {
+				err = memoryStorage.AddStats(ref, stat)
+				if err != nil {
+					t.Error(err)
+				}
+			}
 			spec := cinfo.Spec
-			driver.On(
-				"RecentStats",
-				h.Name,
-				query.NumStats,
-			).Return(
-				stats,
-				nil,
-			)
 
 			h.On("ListContainers", container.ListSelf).Return(
 				[]info.ContainerReference(nil),
@@ -209,7 +207,8 @@ func TestDockerContainersInfo(t *testing.T) {
 }
 
 func TestNew(t *testing.T) {
-	manager, err := New(&stest.MockStorageDriver{}, &fakesysfs.FakeSysFs{})
+	memoryStorage := memory.New(60, nil)
+	manager, err := New(memoryStorage, &fakesysfs.FakeSysFs{})
 	if err != nil {
 		t.Fatalf("Expected manager.New to succeed: %s", err)
 	}

storagedriver.go

@@ -38,7 +38,8 @@ var argDbBufferDuration = flag.Duration("storage_driver_buffer_duration", 60*tim
 
 const statsRequestedByUI = 60
 
-func NewStorageDriver(driverName string) (*memory.InMemoryStorage, error) {
+// Creates a memory storage with an optional backend storage option.
+func NewMemoryStorage(backendStorageName string) (*memory.InMemoryStorage, error) {
 	var storageDriver *memory.InMemoryStorage
 	var backendStorage storage.StorageDriver
 	var err error
@@ -48,7 +49,7 @@ func NewStorageDriver(driverName string) (*memory.InMemoryStorage, error) {
 		// The UI requests the most recent 60 stats by default.
 		statsToCache = statsRequestedByUI
 	}
-	switch driverName {
+	switch backendStorageName {
 	case "":
 		backendStorage = nil
 	case "influxdb":
@@ -79,14 +80,18 @@ func NewStorageDriver(driverName string) (*memory.InMemoryStorage, error) {
 			*argDbTable,
 			*argDbName,
 		)
-
 	default:
-		err = fmt.Errorf("unknown database driver: %v", *argDbDriver)
+		err = fmt.Errorf("unknown backend storage driver: %v", *argDbDriver)
 	}
 	if err != nil {
 		return nil, err
 	}
-	glog.Infof("Caching %d recent stats in memory; using \"%v\" storage driver\n", statsToCache, driverName)
+	if backendStorageName != "" {
+		glog.Infof("Using backend storage type %q", backendStorageName)
+	} else {
+		glog.Infof("No backend storage selected")
+	}
+	glog.Info("Caching %d stats in memory", statsToCache)
 	storageDriver = memory.New(statsToCache, backendStorage)
 	return storageDriver, nil
 }