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