A few minor Go style suggestions.

This commit is contained in:
Satnam Singh 2014-09-23 11:58:44 -07:00
parent 1cbf0bc910
commit f097c2b4ab
8 changed files with 70 additions and 80 deletions

View File

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

View File

@ -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 (

View File

@ -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

View File

@ -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
}
}

View File

@ -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
}

View File

@ -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,
)

View File

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

View File

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