From 99a48d83106c3a57f011f0a74f339fa2f8bcf4a7 Mon Sep 17 00:00:00 2001 From: Victor Marmol Date: Tue, 17 Feb 2015 22:09:02 -0800 Subject: [PATCH] Make explicit the use of InMemoryStorage in Manager. The current structure is a remnant of when the in memory storage was one of the options for backing store. Today, we always have the memory storage with an optional "backend storage" plugin. This commit makes the InMemoryStorage use explicit. --- cadvisor.go | 4 ++-- manager/manager.go | 18 +++++++++--------- manager/manager_test.go | 37 ++++++++++++++++++------------------- storagedriver.go | 15 ++++++++++----- 4 files changed, 39 insertions(+), 35 deletions(-) diff --git a/cadvisor.go b/cadvisor.go index 8a43a687..dcdb951a 100644 --- a/cadvisor.go +++ b/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) } diff --git a/manager/manager.go b/manager/manager.go index 3b1c60c4..ff5f7615 100644 --- a/manager/manager.go +++ b/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 } diff --git a/manager/manager_test.go b/manager/manager_test.go index 2e380ce2..94c23509 100644 --- a/manager/manager_test.go +++ b/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) } diff --git a/storagedriver.go b/storagedriver.go index d784d579..0256a0cf 100644 --- a/storagedriver.go +++ b/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 }