From f097c2b4ab6d1ec568ae1c6e53f6505cc657e3d4 Mon Sep 17 00:00:00 2001 From: Satnam Singh Date: Tue, 23 Sep 2014 11:58:44 -0700 Subject: [PATCH 1/7] A few minor Go style suggestions. --- api/handler.go | 6 ++--- client/client.go | 1 + container/container.go | 27 ++++++++++++-------- container/raw/handler.go | 32 +++++++++++------------ manager/container.go | 11 +++----- manager/container_test.go | 15 +++++------ manager/manager.go | 53 ++++++++++++++++++--------------------- manager/manager_test.go | 5 ++-- 8 files changed, 70 insertions(+), 80 deletions(-) diff --git a/api/handler.go b/api/handler.go index b45e74e5..8a2918c7 100644 --- a/api/handler.go +++ b/api/handler.go @@ -12,8 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Handler for /api/ - +// Package api provides a handler for /api/ package api import ( @@ -47,8 +46,7 @@ var supportedApiVersions map[string]struct{} = map[string]struct{}{ func RegisterHandlers(m manager.Manager) error { http.HandleFunc(apiResource, func(w http.ResponseWriter, r *http.Request) { - err := handleRequest(m, w, r) - if err != nil { + if err := handleRequest(m, w, r); err != nil { fmt.Fprintf(w, "%s", err) } }) diff --git a/client/client.go b/client/client.go index e47d1956..e129bb02 100644 --- a/client/client.go +++ b/client/client.go @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +// TODO(cAdvisor): Package comment. package cadvisor import ( diff --git a/container/container.go b/container/container.go index ba28854b..567a9892 100644 --- a/container/container.go +++ b/container/container.go @@ -12,27 +12,34 @@ // See the License for the specific language governing permissions and // limitations under the License. +// Pacakge container defines types for sub-container events and also +// defines an interface for container operation handlers. package container import "github.com/google/cadvisor/info" -// Listing types. -const ( - LIST_SELF = iota - LIST_RECURSIVE -) - +// ListType describes whether listing should be just for a +// specific container or performed recurisvely. type ListType int -// SubcontainerEvent types. const ( - SUBCONTAINER_ADD = iota - SUBCONTAINER_DELETE + ListSelf ListType = iota + ListRecursive ) +// SubcontainerEventType indicated whether the event +// specifies an addition or deletion. +type SubcontainerEventType int + +const ( + SubcontainerAdd SubcontainerEventType = iota + SubcontainerDelete +) + +// SubcontainerEvent represents a type SubcontainerEvent struct { // The type of event that occurred. - EventType int + EventType SubcontainerEventType // The full container name of the container where the event occurred. Name string diff --git a/container/raw/handler.go b/container/raw/handler.go index b777d9a4..cd42a050 100644 --- a/container/raw/handler.go +++ b/container/raw/handler.go @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +// TODO(cAdvosor): Package comment. package raw import ( @@ -164,8 +165,7 @@ func listDirectories(dirpath string, parent string, recursive bool, output map[s // List subcontainers if asked to. if recursive { - err := listDirectories(path.Join(dirpath, entry.Name()), name, true, output) - if err != nil { + if err := listDirectories(path.Join(dirpath, entry.Name()), name, true, output); err != nil { return err } } @@ -177,7 +177,7 @@ func listDirectories(dirpath string, parent string, recursive bool, output map[s func (self *rawContainerHandler) ListContainers(listType container.ListType) ([]info.ContainerReference, error) { containers := make(map[string]struct{}) for _, subsystem := range self.cgroupSubsystems.mounts { - err := listDirectories(path.Join(subsystem.Mountpoint, self.name), self.name, listType == container.LIST_RECURSIVE, containers) + err := listDirectories(path.Join(subsystem.Mountpoint, self.name), self.name, listType == container.ListRecursive, containers) if err != nil { return nil, err } @@ -204,8 +204,7 @@ func (self *rawContainerHandler) ListProcesses(listType container.ListType) ([]i } func (self *rawContainerHandler) watchDirectory(dir string, containerName string) error { - err := self.watcher.AddWatch(dir, inotify.IN_CREATE|inotify.IN_DELETE|inotify.IN_MOVE) - if err != nil { + if err := self.watcher.AddWatch(dir, inotify.IN_CREATE|inotify.IN_DELETE|inotify.IN_MOVE); err != nil { return err } self.watches[containerName] = struct{}{} @@ -228,16 +227,16 @@ func (self *rawContainerHandler) watchDirectory(dir string, containerName string func (self *rawContainerHandler) processEvent(event *inotify.Event, events chan container.SubcontainerEvent) error { // Convert the inotify event type to a container create or delete. - var eventType int + var eventType container.SubcontainerEventType switch { case (event.Mask & inotify.IN_CREATE) > 0: - eventType = container.SUBCONTAINER_ADD + eventType = container.SubcontainerAdd case (event.Mask & inotify.IN_DELETE) > 0: - eventType = container.SUBCONTAINER_DELETE + eventType = container.SubcontainerDelete case (event.Mask & inotify.IN_MOVED_FROM) > 0: - eventType = container.SUBCONTAINER_DELETE + eventType = container.SubcontainerDelete case (event.Mask & inotify.IN_MOVED_TO) > 0: - eventType = container.SUBCONTAINER_ADD + eventType = container.SubcontainerAdd default: // Ignore other events. return nil @@ -258,18 +257,17 @@ func (self *rawContainerHandler) processEvent(event *inotify.Event, events chan // Maintain the watch for the new or deleted container. switch { - case eventType == container.SUBCONTAINER_ADD: + case eventType == container.SubcontainerAdd: // If we've already seen this event, return. if _, ok := self.watches[containerName]; ok { return nil } // New container was created, watch it. - err := self.watchDirectory(event.Name, containerName) - if err != nil { + if err := self.watchDirectory(event.Name, containerName); err != nil { return err } - case eventType == container.SUBCONTAINER_DELETE: + case eventType == container.SubcontainerDelete: // If we've already seen this event, return. if _, ok := self.watches[containerName]; !ok { return nil @@ -277,8 +275,7 @@ func (self *rawContainerHandler) processEvent(event *inotify.Event, events chan delete(self.watches, containerName) // Container was deleted, stop watching for it. - err := self.watcher.RemoveWatch(event.Name) - if err != nil { + if err := self.watcher.RemoveWatch(event.Name); err != nil { return err } default: @@ -306,8 +303,7 @@ func (self *rawContainerHandler) WatchSubcontainers(events chan container.Subcon // Watch this container (all its cgroups) and all subdirectories. for _, mnt := range self.cgroupSubsystems.mounts { - err := self.watchDirectory(path.Join(mnt.Mountpoint, self.name), self.name) - if err != nil { + if err := self.watchDirectory(path.Join(mnt.Mountpoint, self.name), self.name); err != nil { return err } } diff --git a/manager/container.go b/manager/container.go index 4d4eee8b..2e910d8f 100644 --- a/manager/container.go +++ b/manager/container.go @@ -12,8 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Per-container manager. - +// Package mananger provides per-container manager support. package manager import ( @@ -80,8 +79,7 @@ func (c *containerData) GetInfo() (*containerInfo, error) { // Make a copy of the info for the user. c.lock.Lock() defer c.lock.Unlock() - ret := c.info - return &ret, nil + return &c.info, nil } func newContainerData(containerName string, driver storage.StorageDriver, handler container.ContainerHandler) (*containerData, error) { @@ -199,15 +197,14 @@ func (c *containerData) updateStats() error { if err != nil { return err } - err = c.storageDriver.AddStats(ref, stats) - if err != nil { + if err = c.storageDriver.AddStats(ref, stats); err != nil { return err } return nil } func (c *containerData) updateSubcontainers() error { - subcontainers, err := c.handler.ListContainers(container.LIST_SELF) + subcontainers, err := c.handler.ListContainers(container.ListSelf) if err != nil { return err } diff --git a/manager/container_test.go b/manager/container_test.go index cf0e654c..e0707dad 100644 --- a/manager/container_test.go +++ b/manager/container_test.go @@ -48,13 +48,12 @@ func TestUpdateSubcontainers(t *testing.T) { {Name: "/container/something"}, } cd, mockHandler, _ := newTestContainerData(t) - mockHandler.On("ListContainers", container.LIST_SELF).Return( + mockHandler.On("ListContainers", container.ListSelf).Return( subcontainers, nil, ) - err := cd.updateSubcontainers() - if err != nil { + if err := cd.updateSubcontainers(); err != nil { t.Fatal(err) } @@ -79,13 +78,12 @@ func TestUpdateSubcontainers(t *testing.T) { func TestUpdateSubcontainersWithError(t *testing.T) { cd, mockHandler, _ := newTestContainerData(t) - mockHandler.On("ListContainers", container.LIST_SELF).Return( + mockHandler.On("ListContainers", container.ListSelf).Return( []info.ContainerReference{}, fmt.Errorf("some error"), ) - err := cd.updateSubcontainers() - if err == nil { + if err := cd.updateSubcontainers(); err == nil { t.Fatal("updateSubcontainers should return error") } if len(cd.info.Subcontainers) != 0 { @@ -107,8 +105,7 @@ func TestUpdateStats(t *testing.T) { mockDriver.On("AddStats", info.ContainerReference{Name: mockHandler.Name}, stats).Return(nil) - err := cd.updateStats() - if err != nil { + if err := cd.updateStats(); err != nil { t.Fatal(err) } @@ -143,7 +140,7 @@ func TestGetInfo(t *testing.T) { spec, nil, ) - mockHandler.On("ListContainers", container.LIST_SELF).Return( + mockHandler.On("ListContainers", container.ListSelf).Return( subcontainers, nil, ) diff --git a/manager/manager.go b/manager/manager.go index b7f44975..9789294e 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +// TODO(cAdvisor): Package comment. package manager import ( @@ -30,6 +31,8 @@ import ( var globalHousekeepingInterval = flag.Duration("global_housekeeping_interval", 1*time.Minute, "Interval between global housekeepings") +// The Manager interface defines operations for starting a manager and getting +// container and machine information. type Manager interface { // Start the manager, blocks forever. Start() error @@ -47,6 +50,7 @@ type Manager interface { GetVersionInfo() (*info.VersionInfo, error) } +// New takes a driver and returns a new manager. func New(driver storage.StorageDriver) (Manager, error) { if driver == nil { return nil, fmt.Errorf("nil storage driver!") @@ -83,13 +87,11 @@ type manager struct { // Start the container manager. func (self *manager) Start() error { // Create root and then recover all containers. - err := self.createContainer("/") - if err != nil { + if err := self.createContainer("/"); err != nil { return err } glog.Infof("Starting recovery of all containers") - err = self.detectSubcontainers("/") - if err != nil { + if err := self.detectSubcontainers("/"); err != nil { return err } glog.Infof("Recovery completed") @@ -109,8 +111,7 @@ func (self *manager) Start() error { start := time.Now() // Check for new containers. - err = self.detectSubcontainers("/") - if err != nil { + if err := self.detectSubcontainers("/"); err != nil { glog.Errorf("Failed to detect containers: %s", err) } @@ -211,13 +212,11 @@ func (self *manager) SubcontainersInfo(containerName string, query *info.Contain func (m *manager) GetMachineInfo() (*info.MachineInfo, error) { // Copy and return the MachineInfo. - ret := m.machineInfo - return &ret, nil + return &m.machineInfo, nil } func (m *manager) GetVersionInfo() (*info.VersionInfo, error) { - ret := m.versionInfo - return &ret, nil + return &m.versionInfo, nil } // Create a container. @@ -237,8 +236,7 @@ func (m *manager) createContainer(containerName string) error { defer m.containersLock.Unlock() // Check that the container didn't already exist - _, ok := m.containers[containerName] - if ok { + if _, ok := m.containers[containerName]; ok { return true } @@ -293,9 +291,9 @@ func (m *manager) getContainersDiff(containerName string) (added []info.Containe // Get all subcontainers recursively. cont, ok := m.containers[containerName] if !ok { - return nil, nil, fmt.Errorf("Failed to find container %q while checking for new containers", containerName) + return nil, nil, fmt.Errorf("failed to find container %q while checking for new containers", containerName) } - allContainers, err := cont.handler.ListContainers(container.LIST_RECURSIVE) + allContainers, err := cont.handler.ListContainers(container.ListRecursive) if err != nil { return nil, nil, err } @@ -313,8 +311,7 @@ func (m *manager) getContainersDiff(containerName string) (added []info.Containe // Added containers for _, c := range allContainers { delete(allContainersSet, c.Name) - _, ok := m.containers[c.Name] - if !ok { + if _, ok := m.containers[c.Name]; !ok { added = append(added, c) } } @@ -338,15 +335,14 @@ func (m *manager) detectSubcontainers(containerName string) error { for _, cont := range added { err = m.createContainer(cont.Name) if err != nil { - glog.Errorf("Failed to create existing container: %s: %s", cont.Name, err) + glog.Errorf("failed to create existing container: %s: %s", cont.Name, err) } } // Remove the old containers. for _, cont := range removed { - err = m.destroyContainer(cont.Name) - if err != nil { - glog.Errorf("Failed to destroy existing container: %s: %s", cont.Name, err) + if err = m.destroyContainer(cont.Name); err != nil { + glog.Errorf("failed to destroy existing container: %s: %s", cont.Name, err) } } @@ -354,8 +350,8 @@ func (m *manager) detectSubcontainers(containerName string) error { } func (self *manager) processEvent(event container.SubcontainerEvent) error { - var err error = nil - return err + // TODO(cAdvisor): Why does this method always return nil? [satnam6502] + return nil } // Watches for new containers started in the system. Runs forever unless there is a setup error. @@ -373,27 +369,26 @@ func (self *manager) watchForNewContainers() error { // Register for new subcontainers. events := make(chan container.SubcontainerEvent, 16) - err := root.handler.WatchSubcontainers(events) - if err != nil { + if err := root.handler.WatchSubcontainers(events); err != nil { return err } // There is a race between starting the watch and new container creation so we do a detection before we read new containers. - err = self.detectSubcontainers("/") - if err != nil { + if err := self.detectSubcontainers("/"); err != nil { return err } // Listen to events from the container handler. + var err error for event := range events { switch { - case event.EventType == container.SUBCONTAINER_ADD: + case event.EventType == container.SubcontainerAdd: err = self.createContainer(event.Name) - case event.EventType == container.SUBCONTAINER_DELETE: + case event.EventType == container.SubcontainerDelete: err = self.destroyContainer(event.Name) } if err != nil { - glog.Warning("Failed to process watch event: %v", err) + glog.Warning("failed to process watch event: %v", err) } } diff --git a/manager/manager_test.go b/manager/manager_test.go index 37b35a84..3a207b51 100644 --- a/manager/manager_test.go +++ b/manager/manager_test.go @@ -83,7 +83,7 @@ func expectManagerWithContainers(containers []string, query *info.ContainerInfoR nil, ) - h.On("ListContainers", container.LIST_SELF).Return( + h.On("ListContainers", container.ListSelf).Return( []info.ContainerReference(nil), nil, ) @@ -176,8 +176,7 @@ func TestNew(t *testing.T) { } func TestNewNilManager(t *testing.T) { - _, err := New(nil) - if err == nil { + if _, err := New(nil); err == nil { t.Fatalf("Expected nil manager to return error") } } From cab052cc8aefae198926b3720d3d5e8cff31f478 Mon Sep 17 00:00:00 2001 From: Satnam Singh Date: Wed, 24 Sep 2014 10:03:56 -0700 Subject: [PATCH 2/7] Undo changes to if statements as requested by vmarmol. Fix typos in my changes. --- container/container.go | 7 +++---- container/raw/handler.go | 14 +++++++++----- manager/container.go | 4 ++-- manager/container_test.go | 6 ++++-- manager/manager.go | 24 +++++++++++++++--------- manager/manager_test.go | 3 ++- 6 files changed, 35 insertions(+), 23 deletions(-) diff --git a/container/container.go b/container/container.go index 567a9892..7060532d 100644 --- a/container/container.go +++ b/container/container.go @@ -12,14 +12,14 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Pacakge container defines types for sub-container events and also +// Package container defines types for sub-container events and also // defines an interface for container operation handlers. package container import "github.com/google/cadvisor/info" // ListType describes whether listing should be just for a -// specific container or performed recurisvely. +// specific container or performed recursively. type ListType int const ( @@ -27,8 +27,7 @@ const ( ListRecursive ) -// SubcontainerEventType indicated whether the event -// specifies an addition or deletion. +// SubcontainerEventType indicates an addition or deletion event. type SubcontainerEventType int const ( diff --git a/container/raw/handler.go b/container/raw/handler.go index cd42a050..7b1082bb 100644 --- a/container/raw/handler.go +++ b/container/raw/handler.go @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -// TODO(cAdvosor): Package comment. +// TODO(cAdvisor): Package comment. package raw import ( @@ -165,7 +165,8 @@ func listDirectories(dirpath string, parent string, recursive bool, output map[s // List subcontainers if asked to. if recursive { - if err := listDirectories(path.Join(dirpath, entry.Name()), name, true, output); err != nil { + err := listDirectories(path.Join(dirpath, entry.Name()), name, true, output) + if err != nil { return err } } @@ -204,7 +205,8 @@ func (self *rawContainerHandler) ListProcesses(listType container.ListType) ([]i } func (self *rawContainerHandler) watchDirectory(dir string, containerName string) error { - if err := self.watcher.AddWatch(dir, inotify.IN_CREATE|inotify.IN_DELETE|inotify.IN_MOVE); err != nil { + err := self.watcher.AddWatch(dir, inotify.IN_CREATE|inotify.IN_DELETE|inotify.IN_MOVE) + if err != nil { return err } self.watches[containerName] = struct{}{} @@ -264,7 +266,8 @@ func (self *rawContainerHandler) processEvent(event *inotify.Event, events chan } // New container was created, watch it. - if err := self.watchDirectory(event.Name, containerName); err != nil { + err := self.watchDirectory(event.Name, containerName) + if err != nil { return err } case eventType == container.SubcontainerDelete: @@ -303,7 +306,8 @@ func (self *rawContainerHandler) WatchSubcontainers(events chan container.Subcon // Watch this container (all its cgroups) and all subdirectories. for _, mnt := range self.cgroupSubsystems.mounts { - if err := self.watchDirectory(path.Join(mnt.Mountpoint, self.name), self.name); err != nil { + err := self.watchDirectory(path.Join(mnt.Mountpoint, self.name), self.name) + if err != nil { return err } } diff --git a/manager/container.go b/manager/container.go index 2e910d8f..904bb8e6 100644 --- a/manager/container.go +++ b/manager/container.go @@ -12,7 +12,6 @@ // See the License for the specific language governing permissions and // limitations under the License. -// Package mananger provides per-container manager support. package manager import ( @@ -197,7 +196,8 @@ func (c *containerData) updateStats() error { if err != nil { return err } - if err = c.storageDriver.AddStats(ref, stats); err != nil { + err = c.storageDriver.AddStats(ref, stats) + if err != nil { return err } return nil diff --git a/manager/container_test.go b/manager/container_test.go index e0707dad..ad838292 100644 --- a/manager/container_test.go +++ b/manager/container_test.go @@ -83,7 +83,8 @@ func TestUpdateSubcontainersWithError(t *testing.T) { fmt.Errorf("some error"), ) - if err := cd.updateSubcontainers(); err == nil { + err := cd.updateSubcontainers() + if err == nil { t.Fatal("updateSubcontainers should return error") } if len(cd.info.Subcontainers) != 0 { @@ -105,7 +106,8 @@ func TestUpdateStats(t *testing.T) { mockDriver.On("AddStats", info.ContainerReference{Name: mockHandler.Name}, stats).Return(nil) - if err := cd.updateStats(); err != nil { + err := cd.updateStats() + if err != nil { t.Fatal(err) } diff --git a/manager/manager.go b/manager/manager.go index 9789294e..3d2c12ce 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -87,11 +87,13 @@ type manager struct { // Start the container manager. func (self *manager) Start() error { // Create root and then recover all containers. - if err := self.createContainer("/"); err != nil { + err := self.createContainer("/") + if err != nil { return err } glog.Infof("Starting recovery of all containers") - if err := self.detectSubcontainers("/"); err != nil { + err = self.detectSubcontainers("/") + if err != nil { return err } glog.Infof("Recovery completed") @@ -111,7 +113,8 @@ func (self *manager) Start() error { start := time.Now() // Check for new containers. - if err := self.detectSubcontainers("/"); err != nil { + err := self.detectSubcontainers("/") + if err != nil { glog.Errorf("Failed to detect containers: %s", err) } @@ -235,8 +238,9 @@ func (m *manager) createContainer(containerName string) error { m.containersLock.Lock() defer m.containersLock.Unlock() - // Check that the container didn't already exist - if _, ok := m.containers[containerName]; ok { + // Check that the container didn't already exist\ + _, ok := m.containers[containerName] + if ok { return true } @@ -311,7 +315,8 @@ func (m *manager) getContainersDiff(containerName string) (added []info.Containe // Added containers for _, c := range allContainers { delete(allContainersSet, c.Name) - if _, ok := m.containers[c.Name]; !ok { + _, ok := m.containers[c.Name] + if !ok { added = append(added, c) } } @@ -341,7 +346,8 @@ func (m *manager) detectSubcontainers(containerName string) error { // Remove the old containers. for _, cont := range removed { - if err = m.destroyContainer(cont.Name); err != nil { + err = m.destroyContainer(cont.Name) + if err != nil { glog.Errorf("failed to destroy existing container: %s: %s", cont.Name, err) } } @@ -369,7 +375,8 @@ func (self *manager) watchForNewContainers() error { // Register for new subcontainers. events := make(chan container.SubcontainerEvent, 16) - if err := root.handler.WatchSubcontainers(events); err != nil { + err := root.handler.WatchSubcontainers(events) + if err != nil { return err } @@ -379,7 +386,6 @@ func (self *manager) watchForNewContainers() error { } // Listen to events from the container handler. - var err error for event := range events { switch { case event.EventType == container.SubcontainerAdd: diff --git a/manager/manager_test.go b/manager/manager_test.go index 3a207b51..1754a7b4 100644 --- a/manager/manager_test.go +++ b/manager/manager_test.go @@ -176,7 +176,8 @@ func TestNew(t *testing.T) { } func TestNewNilManager(t *testing.T) { - if _, err := New(nil); err == nil { + _, err := New(nil) + if err == nil { t.Fatalf("Expected nil manager to return error") } } From b4a7df6044e5f95aa361b85b498c9303b2ecfacb Mon Sep 17 00:00:00 2001 From: Satnam Singh Date: Wed, 24 Sep 2014 10:15:08 -0700 Subject: [PATCH 3/7] Undo changed to Errorf in manager.go re: capatalization. --- manager/manager.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/manager/manager.go b/manager/manager.go index 3d2c12ce..6578b584 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -340,7 +340,7 @@ func (m *manager) detectSubcontainers(containerName string) error { for _, cont := range added { err = m.createContainer(cont.Name) if err != nil { - glog.Errorf("failed to create existing container: %s: %s", cont.Name, err) + glog.Errorf("Failed to create existing container: %s: %s", cont.Name, err) } } @@ -348,7 +348,7 @@ func (m *manager) detectSubcontainers(containerName string) error { for _, cont := range removed { err = m.destroyContainer(cont.Name) if err != nil { - glog.Errorf("failed to destroy existing container: %s: %s", cont.Name, err) + glog.Errorf("Failed to destroy existing container: %s: %s", cont.Name, err) } } From db1a8ec47a2cd8a11ffbf287625a654ab7bacc99 Mon Sep 17 00:00:00 2001 From: Satnam Singh Date: Wed, 24 Sep 2014 10:16:05 -0700 Subject: [PATCH 4/7] And a few more capatalization undos. --- manager/manager.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/manager/manager.go b/manager/manager.go index 6578b584..e3e62bf3 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -370,7 +370,7 @@ func (self *manager) watchForNewContainers() error { root, ok = self.containers["/"] }() if !ok { - return fmt.Errorf("root container does not exist when watching for new containers") + return fmt.Errorf("Root container does not exist when watching for new containers") } // Register for new subcontainers. @@ -394,7 +394,7 @@ func (self *manager) watchForNewContainers() error { err = self.destroyContainer(event.Name) } if err != nil { - glog.Warning("failed to process watch event: %v", err) + glog.Warning("Failed to process watch event: %v", err) } } From 9ccf15fcf9bd987d65cd1b3497b86f5072e2e940 Mon Sep 17 00:00:00 2001 From: Satnam Singh Date: Wed, 24 Sep 2014 10:22:13 -0700 Subject: [PATCH 5/7] More if undos. --- api/handler.go | 6 ++++-- manager/container_test.go | 3 ++- manager/manager.go | 3 ++- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/api/handler.go b/api/handler.go index 8a2918c7..ff06819f 100644 --- a/api/handler.go +++ b/api/handler.go @@ -46,7 +46,8 @@ var supportedApiVersions map[string]struct{} = map[string]struct{}{ func RegisterHandlers(m manager.Manager) error { http.HandleFunc(apiResource, func(w http.ResponseWriter, r *http.Request) { - if err := handleRequest(m, w, r); err != nil { + err := handleRequest(m, w, r) + if err != nil { fmt.Fprintf(w, "%s", err) } }) @@ -170,7 +171,8 @@ func getContainerInfoRequest(body io.ReadCloser) (*info.ContainerInfoRequest, er query.NumStats = 64 decoder := json.NewDecoder(body) - if err := decoder.Decode(&query); err != nil && err != io.EOF { + err := decoder.Decode(&query) + if err != nil && err != io.EOF { return nil, fmt.Errorf("unable to decode the json value: %s", err) } diff --git a/manager/container_test.go b/manager/container_test.go index ad838292..2166f297 100644 --- a/manager/container_test.go +++ b/manager/container_test.go @@ -53,7 +53,8 @@ func TestUpdateSubcontainers(t *testing.T) { nil, ) - if err := cd.updateSubcontainers(); err != nil { + err := cd.updateSubcontainers() + if err != nil { t.Fatal(err) } diff --git a/manager/manager.go b/manager/manager.go index e3e62bf3..563899c7 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -381,7 +381,8 @@ func (self *manager) watchForNewContainers() error { } // There is a race between starting the watch and new container creation so we do a detection before we read new containers. - if err := self.detectSubcontainers("/"); err != nil { + err := self.detectSubcontainers("/") + if err != nil { return err } From b77bc54f5db244bdfc3a43081c8297b07704b391 Mon Sep 17 00:00:00 2001 From: Satnam Singh Date: Wed, 24 Sep 2014 10:26:53 -0700 Subject: [PATCH 6/7] And yet another if. --- container/raw/handler.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/container/raw/handler.go b/container/raw/handler.go index 7b1082bb..b5c736b7 100644 --- a/container/raw/handler.go +++ b/container/raw/handler.go @@ -278,7 +278,8 @@ func (self *rawContainerHandler) processEvent(event *inotify.Event, events chan delete(self.watches, containerName) // Container was deleted, stop watching for it. - if err := self.watcher.RemoveWatch(event.Name); err != nil { + err := self.watcher.RemoveWatch(event.Name) + if err != nil { return err } default: From 4d86f8d931308d1922eff90ef9d522b2173c6c7c Mon Sep 17 00:00:00 2001 From: Satnam Singh Date: Wed, 24 Sep 2014 10:29:04 -0700 Subject: [PATCH 7/7] Fix assignment error in haste. --- manager/manager.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/manager/manager.go b/manager/manager.go index 563899c7..d11b8450 100644 --- a/manager/manager.go +++ b/manager/manager.go @@ -381,7 +381,7 @@ func (self *manager) watchForNewContainers() error { } // There is a race between starting the watch and new container creation so we do a detection before we read new containers. - err := self.detectSubcontainers("/") + err = self.detectSubcontainers("/") if err != nil { return err }