Register container handler factories in manager.New().

This lets us simplify the startup of the manager.
derekwaynecarr · Mar 3, 2015 · 7e7c433 · 7e7c433
1 parent c1f572f
commit 7e7c433
Show file tree

Hide file tree

Showing 3 changed files with 39 additions and 52 deletions.
diff --git a/cadvisor.go b/cadvisor.go
@@ -27,8 +27,6 @@ import (
 	auth "github.com/abbot/go-http-auth"
 	"github.com/golang/glog"
 	"github.com/google/cadvisor/api"
-	"github.com/google/cadvisor/container/docker"
-	"github.com/google/cadvisor/container/raw"
 	"github.com/google/cadvisor/healthz"
 	"github.com/google/cadvisor/info"
 	"github.com/google/cadvisor/manager"
@@ -76,16 +74,6 @@ func main() {
 		glog.Fatalf("Failed to create a Container Manager: %s", err)
 	}
 
-	// Register Docker.
-	if err := docker.Register(containerManager); err != nil {
-		glog.Errorf("Docker registration failed: %v.", err)
-	}
-
-	// Register the raw driver.
-	if err := raw.Register(containerManager); err != nil {
-		glog.Fatalf("Raw registration failed: %v.", err)
-	}
-
 	// Basic health handler.
 	if err := healthz.RegisterHandler(); err != nil {
 		glog.Fatalf("Failed to register healthz handler: %s", err)

diff --git a/manager/manager.go b/manager/manager.go
@@ -28,6 +28,7 @@ import (
 	"github.com/golang/glog"
 	"github.com/google/cadvisor/container"
 	"github.com/google/cadvisor/container/docker"
+	"github.com/google/cadvisor/container/raw"
 	"github.com/google/cadvisor/events"
 	"github.com/google/cadvisor/info"
 	"github.com/google/cadvisor/storage/memory"
@@ -115,6 +116,18 @@ func New(memoryStorage *memory.InMemoryStorage, sysfs sysfs.SysFs) (Manager, err
 
 	newManager.eventHandler = events.NewEventManager()
 
+	// Register Docker container factory.
+	err = docker.Register(newManager)
+	if err != nil {
+		glog.Errorf("Docker container factory registration failed: %v.", err)
+	}
+
+	// Register the raw driver.
+	err = raw.Register(newManager)
+	if err != nil {
+		return nil, fmt.Errorf("registration of the raw container factory failed: %v", err)
+	}
+
 	return newManager, nil
 }
 

diff --git a/manager/manager_test.go b/manager/manager_test.go
@@ -40,38 +40,35 @@ func createManagerAndAddContainers(
 	t *testing.T,
 ) *manager {
 	container.ClearContainerHandlerFactories()
-	mif, err := New(memoryStorage, sysfs)
-	if err != nil {
-		t.Fatal(err)
-	}
-	if ret, ok := mif.(*manager); ok {
-		for _, name := range containers {
-			mockHandler := container.NewMockContainerHandler(name)
-			spec := itest.GenerateRandomContainerSpec(4)
-			mockHandler.On("GetSpec").Return(
-				spec,
-				nil,
-			).Once()
-			cont, err := newContainerData(name, memoryStorage, mockHandler, nil, false)
-			if err != nil {
-				t.Fatal(err)
-			}
-			ret.containers[namespacedContainerName{
-				Name: name,
+	mif := &manager{
+		containers:    make(map[namespacedContainerName]*containerData),
+		quitChannels:  make([]chan error, 0, 2),
+		memoryStorage: memoryStorage,
+	}
+	for _, name := range containers {
+		mockHandler := container.NewMockContainerHandler(name)
+		spec := itest.GenerateRandomContainerSpec(4)
+		mockHandler.On("GetSpec").Return(
+			spec,
+			nil,
+		).Once()
+		cont, err := newContainerData(name, memoryStorage, mockHandler, nil, false)
+		if err != nil {
+			t.Fatal(err)
+		}
+		mif.containers[namespacedContainerName{
+			Name: name,
+		}] = cont
+		// Add Docker containers under their namespace.
+		if strings.HasPrefix(name, "/docker") {
+			mif.containers[namespacedContainerName{
+				Namespace: docker.DockerNamespace,
+				Name:      strings.TrimPrefix(name, "/docker/"),
 			}] = cont
-			// Add Docker containers under their namespace.
-			if strings.HasPrefix(name, "/docker") {
-				ret.containers[namespacedContainerName{
-					Namespace: docker.DockerNamespace,
-					Name:      strings.TrimPrefix(name, "/docker/"),
-				}] = cont
-			}
-			f(mockHandler)
 		}
-		return ret
+		f(mockHandler)
 	}
-	t.Fatal("Wrong type")
-	return nil
+	return mif
 }
 
 // Expect a manager with the specified containers and query. Returns the manager, map of ContainerInfo objects,
@@ -206,17 +203,6 @@ func TestDockerContainersInfo(t *testing.T) {
 	}
 }
 
-func TestNew(t *testing.T) {
-	memoryStorage := memory.New(60, nil)
-	manager, err := New(memoryStorage, &fakesysfs.FakeSysFs{})
-	if err != nil {
-		t.Fatalf("Expected manager.New to succeed: %s", err)
-	}
-	if manager == nil {
-		t.Fatalf("Expected returned manager to not be nil")
-	}
-}
-
 func TestNewNilManager(t *testing.T) {
 	_, err := New(nil, nil)
 	if err == nil {