From 4861405904cf7cba88d91c9be4fa4b177c7fbd7d Mon Sep 17 00:00:00 2001 From: "Tim St. Clair" Date: Thu, 14 Apr 2016 16:57:52 -0700 Subject: [PATCH] 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 | 64 ++++++------------------------------- container/raw/handler.go | 61 ++++++++++++++++++++--------------- container/rkt/handler.go | 40 +++-------------------- 3 files changed, 49 insertions(+), 116 deletions(-) diff --git a/container/common/helpers.go b/container/common/helpers.go index 14c1fe16..f6ffe366 100644 --- a/container/common/helpers.go +++ b/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 { - 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() - +func GetSpec(cgroupPaths map[string]string, machineInfoFactory info.MachineInfoFactory, hasNetwork, hasFilesystem bool) (info.ContainerSpec, error) { 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 == "/" { - // 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 + memoryRoot, ok := cgroupPaths["memory"] + if ok { + if utils.FileExists(memoryRoot) { 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) - } - } 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.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 = handler.HasNetwork() + spec.HasNetwork = hasNetwork + spec.HasFilesystem = hasFilesystem 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 } diff --git a/container/raw/handler.go b/container/raw/handler.go index acaaa2a7..314320a4 100644 --- a/container/raw/handler.go +++ b/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 { diff --git a/container/rkt/handler.go b/container/rkt/handler.go index 324f1db9..5effc1f9 100644 --- a/container/rkt/handler.go +++ b/container/rkt/handler.go @@ -53,8 +53,7 @@ type rktContainerHandler struct { // Whether this container has network isolation enabled. hasNetwork bool - fsInfo fs.FsInfo - externalMounts []common.Mount + fsInfo fs.FsInfo 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 {