Refactor common container GetSpec

- Pull out the root container cases, since they're only relevant in the
  raw container handler
- Pass parameters rather than depending on AbstractContainerInterface

container/common/helpers.go

@@ -25,7 +25,6 @@ import (
 
 	info "github.com/google/cadvisor/info/v1"
 	"github.com/google/cadvisor/utils"
-	"github.com/google/cadvisor/utils/machine"
 
 	"github.com/golang/glog"
 )
@@ -45,26 +44,10 @@ func DebugInfo(watches map[string][]string) map[string][]string {
 	return out
 }
 
-type AbstractContainerHandler interface {
+func GetSpec(cgroupPaths map[string]string, machineInfoFactory info.MachineInfoFactory, hasNetwork, hasFilesystem bool) (info.ContainerSpec, error) {
-	GetCgroupPaths() map[string]string
-	GetMachineInfoFactory() info.MachineInfoFactory
-	GetName() string
-	GetRootNetworkDevices() ([]info.NetInfo, error)
-	GetExternalMounts() []Mount
-	HasNetwork() bool
-	HasFilesystem() bool
-}
-
-func GetSpec(handler AbstractContainerHandler) (info.ContainerSpec, error) {
-	cgroupPaths := handler.GetCgroupPaths()
-	machineInfoFactory := handler.GetMachineInfoFactory()
-	name := handler.GetName()
-	externalMounts := handler.GetExternalMounts()
-
 	var spec info.ContainerSpec
 
-	// The raw driver assumes unified hierarchy containers.
+	// Assume unified hierarchy containers.
-
 	// Get the lowest creation time from all hierarchies as the container creation time.
 	now := time.Now()
 	lowestTime := now
@@ -120,51 +103,22 @@ func GetSpec(handler AbstractContainerHandler) (info.ContainerSpec, error) {
 	}
 
 	// Memory
-	if name == "/" {
+	memoryRoot, ok := cgroupPaths["memory"]
-		// Get memory and swap limits of the running machine
+	if ok {
-		memLimit, err := machine.GetMachineMemoryCapacity()
+		if utils.FileExists(memoryRoot) {
-		if err != nil {
-			glog.Warningf("failed to obtain memory limit for machine container")
-			spec.HasMemory = false
-		} else {
-			spec.Memory.Limit = uint64(memLimit)
-			// Spec is marked to have memory only if the memory limit is set
 			spec.HasMemory = true
-		}
+			spec.Memory.Limit = readUInt64(memoryRoot, "memory.limit_in_bytes")
-
+			spec.Memory.SwapLimit = readUInt64(memoryRoot, "memory.memsw.limit_in_bytes")
-		swapLimit, err := machine.GetMachineSwapCapacity()
-		if err != nil {
-			glog.Warningf("failed to obtain swap limit for machine container")
-		} else {
-			spec.Memory.SwapLimit = uint64(swapLimit)
-		}
-	} else {
-		memoryRoot, ok := cgroupPaths["memory"]
-		if ok {
-			if utils.FileExists(memoryRoot) {
-				spec.HasMemory = true
-				spec.Memory.Limit = readUInt64(memoryRoot, "memory.limit_in_bytes")
-				spec.Memory.SwapLimit = readUInt64(memoryRoot, "memory.memsw.limit_in_bytes")
-			}
 		}
 	}
 
-	spec.HasFilesystem = name == "/" || externalMounts != nil || handler.HasFilesystem()
+	spec.HasNetwork = hasNetwork
-
+	spec.HasFilesystem = hasFilesystem
-	spec.HasNetwork = handler.HasNetwork()
 
 	if blkioRoot, ok := cgroupPaths["blkio"]; ok && utils.FileExists(blkioRoot) {
 		spec.HasDiskIo = true
 	}
 
-	// Check physical network devices for root container.
-	nd, err := handler.GetRootNetworkDevices()
-	if err != nil {
-		return spec, err
-	}
-
-	spec.HasNetwork = spec.HasNetwork || len(nd) != 0
-
 	return spec, nil
 }
 

container/raw/handler.go

@@ -27,6 +27,7 @@ import (
 	"github.com/google/cadvisor/fs"
 	info "github.com/google/cadvisor/info/v1"
 	"github.com/google/cadvisor/utils"
+	"github.com/google/cadvisor/utils/machine"
 
 	"github.com/golang/glog"
 	"github.com/opencontainers/runc/libcontainer/cgroups"
@@ -65,30 +66,6 @@ type rawContainerHandler struct {
 	pid int
 }
 
-func (self *rawContainerHandler) GetCgroupPaths() map[string]string {
-	return self.cgroupPaths
-}
-
-func (self *rawContainerHandler) GetMachineInfoFactory() info.MachineInfoFactory {
-	return self.machineInfoFactory
-}
-
-func (self *rawContainerHandler) GetName() string {
-	return self.name
-}
-
-func (self *rawContainerHandler) GetExternalMounts() []common.Mount {
-	return self.externalMounts
-}
-
-func (self *rawContainerHandler) HasNetwork() bool {
-	return false
-}
-
-func (self *rawContainerHandler) HasFilesystem() bool {
-	return false
-}
-
 func isRootCgroup(name string) bool {
 	return name == "/"
 }
@@ -164,7 +141,41 @@ func (self *rawContainerHandler) Start() {}
 func (self *rawContainerHandler) Cleanup() {}
 
 func (self *rawContainerHandler) GetSpec() (info.ContainerSpec, error) {
-	return common.GetSpec(self)
+	const hasNetwork = false
+	hasFilesystem := isRootCgroup(self.name) || len(self.externalMounts) > 0
+	spec, err := common.GetSpec(self.cgroupPaths, self.machineInfoFactory, hasNetwork, hasFilesystem)
+	if err != nil {
+		return spec, err
+	}
+
+	if isRootCgroup(self.name) {
+		// Check physical network devices for root container.
+		nd, err := self.GetRootNetworkDevices()
+		if err != nil {
+			return spec, err
+		}
+		spec.HasNetwork = spec.HasNetwork || len(nd) != 0
+
+		// Get memory and swap limits of the running machine
+		memLimit, err := machine.GetMachineMemoryCapacity()
+		if err != nil {
+			glog.Warningf("failed to obtain memory limit for machine container")
+			spec.HasMemory = false
+		} else {
+			spec.Memory.Limit = uint64(memLimit)
+			// Spec is marked to have memory only if the memory limit is set
+			spec.HasMemory = true
+		}
+
+		swapLimit, err := machine.GetMachineSwapCapacity()
+		if err != nil {
+			glog.Warningf("failed to obtain swap limit for machine container")
+		} else {
+			spec.Memory.SwapLimit = uint64(swapLimit)
+		}
+	}
+
+	return spec, nil
 }
 
 func (self *rawContainerHandler) getFsStats(stats *info.ContainerStats) error {

container/rkt/handler.go

@@ -53,8 +53,7 @@ type rktContainerHandler struct {
 	// Whether this container has network isolation enabled.
 	hasNetwork bool
 
-	fsInfo         fs.FsInfo
+	fsInfo fs.FsInfo
-	externalMounts []common.Mount
 
 	rootFs string
 
@@ -76,33 +75,6 @@ type rktContainerHandler struct {
 	apiPod *rktapi.Pod
 }
 
-func (handler *rktContainerHandler) GetCgroupPaths() map[string]string {
-	return handler.cgroupPaths
-}
-
-func (handler *rktContainerHandler) GetMachineInfoFactory() info.MachineInfoFactory {
-	return handler.machineInfoFactory
-}
-
-func (handler *rktContainerHandler) GetName() string {
-	return handler.name
-}
-
-func (handler *rktContainerHandler) GetExternalMounts() []common.Mount {
-	return handler.externalMounts
-}
-
-func (handler *rktContainerHandler) HasNetwork() bool {
-	return handler.hasNetwork && !handler.ignoreMetrics.Has(container.NetworkUsageMetrics)
-}
-
-func (handler *rktContainerHandler) HasFilesystem() bool {
-	if !handler.ignoreMetrics.Has(container.DiskUsageMetrics) {
-		return true
-	}
-	return false
-}
-
 func newRktContainerHandler(name string, rktClient rktapi.PublicAPIClient, rktPath string, cgroupSubsystems *libcontainer.CgroupSubsystems, machineInfoFactory info.MachineInfoFactory, fsInfo fs.FsInfo, rootFs string, ignoreMetrics container.MetricSet) (container.ContainerHandler, error) {
 	aliases := make([]string, 1)
 	isPod := false
@@ -214,12 +186,6 @@ func (handler *rktContainerHandler) ContainerReference() (info.ContainerReferenc
 	}, nil
 }
 
-//Only the Raw handler will return something of value here
-func (handler *rktContainerHandler) GetRootNetworkDevices() ([]info.NetInfo, error) {
-	nd := []info.NetInfo{}
-	return nd, nil
-}
-
 func (handler *rktContainerHandler) Start() {
 	handler.fsHandler.Start()
 }
@@ -229,7 +195,9 @@ func (handler *rktContainerHandler) Cleanup() {
 }
 
 func (handler *rktContainerHandler) GetSpec() (info.ContainerSpec, error) {
-	return common.GetSpec(handler)
+	hasNetwork := handler.hasNetwork && !handler.ignoreMetrics.Has(container.NetworkUsageMetrics)
+	hasFilesystem := !handler.ignoreMetrics.Has(container.DiskUsageMetrics)
+	return common.GetSpec(handler.cgroupPaths, handler.machineInfoFactory, hasNetwork, hasFilesystem)
 }
 
 func (handler *rktContainerHandler) getFsStats(stats *info.ContainerStats) error {