Merge pull request #2806 from iwankgb/fixes_2798_2792

Porting fixes to #2792 and #2798 to v0.38

Author: bobbypage

container/libcontainer/handler.go

@@ -29,14 +29,13 @@ import (
 	"strings"
 	"time"
 
-	"github.com/google/cadvisor/container"
-	info "github.com/google/cadvisor/info/v1"
-	"golang.org/x/sys/unix"
-
 	"github.com/opencontainers/runc/libcontainer"
 	"github.com/opencontainers/runc/libcontainer/cgroups"
-	fs2 "github.com/opencontainers/runc/libcontainer/cgroups/fs2"
+	"github.com/opencontainers/runc/libcontainer/cgroups/fs2"
 	"k8s.io/klog/v2"
+
+	"github.com/google/cadvisor/container"
+	info "github.com/google/cadvisor/info/v1"
 )
 
 var (
@@ -758,16 +757,6 @@ func (h *Handler) GetProcesses() ([]int, error) {
 	return pids, nil
 }
 
-func minUint32(x, y uint32) uint32 {
-	if x < y {
-		return x
-	}
-	return y
-}
-
-// var to allow unit tests to stub it out
-var numCpusFunc = getNumberOnlineCPUs
-
 // Convert libcontainer stats to info.ContainerStats.
 func setCPUStats(s *cgroups.Stats, ret *info.ContainerStats, withPerCPU bool) {
 	ret.Cpu.Usage.User = s.CpuStats.CpuUsage.UsageInUsermode
@@ -785,37 +774,7 @@ func setCPUStats(s *cgroups.Stats, ret *info.ContainerStats, withPerCPU bool) {
 		// cpuacct subsystem.
 		return
 	}
-
-	numPossible := uint32(len(s.CpuStats.CpuUsage.PercpuUsage))
-	// Note that as of https://patchwork.kernel.org/patch/8607101/ (kernel v4.7),
-	// the percpu usage information includes extra zero values for all additional
-	// possible CPUs. This is to allow statistic collection after CPU-hotplug.
-	// We intentionally ignore these extra zeroes.
-	numActual, err := numCpusFunc()
-	if err != nil {
-		klog.Errorf("unable to determine number of actual cpus; defaulting to maximum possible number: errno %v", err)
-		numActual = numPossible
-	}
-	if numActual > numPossible {
-		// The real number of cores should never be greater than the number of
-		// datapoints reported in cpu usage.
-		klog.Errorf("PercpuUsage had %v cpus, but the actual number is %v; ignoring extra CPUs", numPossible, numActual)
-	}
-	numActual = minUint32(numPossible, numActual)
-	ret.Cpu.Usage.PerCpu = make([]uint64, numActual)
-
-	for i := uint32(0); i < numActual; i++ {
-		ret.Cpu.Usage.PerCpu[i] = s.CpuStats.CpuUsage.PercpuUsage[i]
-	}
-
-}
-
-func getNumberOnlineCPUs() (uint32, error) {
-	var availableCPUs unix.CPUSet
-	if err := unix.SchedGetaffinity(0, &availableCPUs); err != nil {
-		return 0, err
-	}
-	return uint32(availableCPUs.Count()), nil
+	ret.Cpu.Usage.PerCpu = s.CpuStats.CpuUsage.PercpuUsage
 }
 
 func setDiskIoStats(s *cgroups.Stats, ret *info.ContainerStats) {

container/libcontainer/handler_test.go

@@ -96,18 +96,11 @@ const nanosecondsInSeconds = 1000000000
 // https://github.com/containerd/cgroups/pull/12
 const clockTicks = 100
 
-func TestMorePossibleCPUs(t *testing.T) {
-	realNumCPUs := uint32(8)
-	numCpusFunc = func() (uint32, error) {
-		return realNumCPUs, nil
-	}
-	possibleCPUs := uint32(31)
-
-	perCPUUsage := make([]uint64, possibleCPUs)
-	for i := uint32(0); i < realNumCPUs; i++ {
+func TestSetCPUStats(t *testing.T) {
+	perCPUUsage := make([]uint64, 31)
+	for i := uint32(0); i < 31; i++ {
 		perCPUUsage[i] = 8562955455524
 	}
-
 	s := &cgroups.Stats{
 		CpuStats: cgroups.CpuStats{
 			CpuUsage: cgroups.CpuUsage{
@@ -124,7 +117,7 @@ func TestMorePossibleCPUs(t *testing.T) {
 	expected := info.ContainerStats{
 		Cpu: info.CpuStats{
 			Usage: info.CpuUsage{
-				PerCpu: perCPUUsage[0:realNumCPUs],
+				PerCpu: perCPUUsage,
 				User:   s.CpuStats.CpuUsage.UsageInUsermode,
 				System: s.CpuStats.CpuUsage.UsageInKernelmode,
 				Total:  33802947350272,

utils/sysinfo/sysinfo.go

@@ -383,7 +383,7 @@ func getCoresInfo(sysFs sysfs.SysFs, cpuDirs []string) ([]info.Core, error) {
 	for _, cpuDir := range cpuDirs {
 		cpuID, err := getMatchedInt(cpuDirRegExp, cpuDir)
 		if err != nil {
-			return nil, fmt.Errorf("Unexpected format of CPU directory, cpuDirRegExp %s, cpuDir: %s", cpuDirRegExp, cpuDir)
+			return nil, fmt.Errorf("unexpected format of CPU directory, cpuDirRegExp %s, cpuDir: %s", cpuDirRegExp, cpuDir)
 		}
 		if !sysFs.IsCPUOnline(cpuDir) {
 			continue
@@ -401,9 +401,22 @@ func getCoresInfo(sysFs sysfs.SysFs, cpuDirs []string) ([]info.Core, error) {
 			return nil, err
 		}
 
+		rawPhysicalPackageID, err := sysFs.GetCPUPhysicalPackageID(cpuDir)
+		if os.IsNotExist(err) {
+			klog.Warningf("Cannot read physical package id for %s, physical_package_id file does not exist, err: %s", cpuDir, err)
+			continue
+		} else if err != nil {
+			return nil, err
+		}
+
+		physicalPackageID, err := strconv.Atoi(rawPhysicalPackageID)
+		if err != nil {
+			return nil, err
+		}
+
 		coreIDx := -1
 		for id, core := range cores {
-			if core.Id == physicalID {
+			if core.Id == physicalID && core.SocketID == physicalPackageID {
 				coreIDx = id
 			}
 		}
@@ -414,25 +427,14 @@ func getCoresInfo(sysFs sysfs.SysFs, cpuDirs []string) ([]info.Core, error) {
 		desiredCore := &cores[coreIDx]
 
 		desiredCore.Id = physicalID
+		desiredCore.SocketID = physicalPackageID
+
 		if len(desiredCore.Threads) == 0 {
 			desiredCore.Threads = []int{cpuID}
 		} else {
 			desiredCore.Threads = append(desiredCore.Threads, cpuID)
 		}
 
-		rawPhysicalPackageID, err := sysFs.GetCPUPhysicalPackageID(cpuDir)
-		if os.IsNotExist(err) {
-			klog.Warningf("Cannot read physical package id for %s, physical_package_id file does not exist, err: %s", cpuDir, err)
-			continue
-		} else if err != nil {
-			return nil, err
-		}
-
-		physicalPackageID, err := strconv.Atoi(rawPhysicalPackageID)
-		if err != nil {
-			return nil, err
-		}
-		desiredCore.SocketID = physicalPackageID
 	}
 	return cores, nil
 }