Merge pull request #2745 from hashicorp/b-2638-cpu_total_compute

Fix cpu_total_compute override
hashicorp · Jul 3, 2017 · da14944 · da14944
2 parents bde854c + ca38020
commit da14944
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 35 deletions.
diff --git a/client/fingerprint/cpu.go b/client/fingerprint/cpu.go
@@ -31,39 +31,48 @@ func (f *CPUFingerprint) Fingerprint(cfg *config.Config, node *structs.Node) (bo
 	}
 
 	if err := stats.Init(); err != nil {
-		err := fmt.Errorf("Unable to obtain CPU information: %v", err)
+		f.logger.Printf("[WARN] fingerprint.cpu: %v", err)
+	}
 
-		if cfg.CpuCompute != 0 {
-			f.logger.Printf("[DEBUG] fingerprint.cpu: %v. Using specified cpu compute %d", err, cfg.CpuCompute)
-			setResources(cfg.CpuCompute)
-			return true, nil
-		}
+	if cfg.CpuCompute != 0 {
+		setResources(cfg.CpuCompute)
+		return true, nil
+	}
 
-		f.logger.Printf("[ERR] fingerprint.cpu: %v", err)
-		f.logger.Printf("[INFO] fingerprint.cpu: cpu compute may be set manually"+
-			" using the client config option %q on machines where cpu information"+
-			" can not be automatically detected.", "cpu_total_compute")
+	if modelName := stats.CPUModelName(); modelName != "" {
+		node.Attributes["cpu.modelname"] = modelName
+	}
 
-		return false, err
+	if mhz := stats.CPUMHzPerCore(); mhz > 0 {
+		node.Attributes["cpu.frequency"] = fmt.Sprintf("%.0f", mhz)
+		f.logger.Printf("[DEBUG] fingerprint.cpu: frequency: %.0f MHz", mhz)
 	}
 
-	modelName := stats.CPUModelName()
-	if modelName != "" {
-		node.Attributes["cpu.modelname"] = modelName
+	if numCores := stats.CPUNumCores(); numCores > 0 {
+		node.Attributes["cpu.numcores"] = fmt.Sprintf("%d", numCores)
+		f.logger.Printf("[DEBUG] fingerprint.cpu: core count: %d", numCores)
 	}
 
-	mhz := stats.CPUMHzPerCore()
-	node.Attributes["cpu.frequency"] = fmt.Sprintf("%.0f", mhz)
-	f.logger.Printf("[DEBUG] fingerprint.cpu: frequency: %.0f MHz", mhz)
+	tt := int(stats.TotalTicksAvailable())
+	if cfg.CpuCompute > 0 {
+		f.logger.Printf("[DEBUG] fingerprint.cpu: Using specified cpu compute %d", cfg.CpuCompute)
+		tt = cfg.CpuCompute
+	}
 
-	numCores := stats.CPUNumCores()
-	node.Attributes["cpu.numcores"] = fmt.Sprintf("%d", numCores)
-	f.logger.Printf("[DEBUG] fingerprint.cpu: core count: %d", numCores)
+	// Return an error if no cpu was detected or explicitly set as this
+	// node would be unable to receive any allocations.
+	if tt == 0 {
+		return false, fmt.Errorf("cannot detect cpu total compute. "+
+			"CPU compute must be set manually using the client config option %q",
+			"cpu_total_compute")
+	}
 
-	tt := stats.TotalTicksAvailable()
+	node.Attributes["cpu.totalcompute"] = fmt.Sprintf("%d", tt)
 
-	node.Attributes["cpu.totalcompute"] = fmt.Sprintf("%.0f", tt)
+	if node.Resources == nil {
+		node.Resources = &structs.Resources{}
+	}
 
-	setResources(int(tt))
+	node.Resources.CPU = tt
 	return true, nil
 }
diff --git a/client/fingerprint/cpu_test.go b/client/fingerprint/cpu_test.go
@@ -40,3 +40,39 @@ func TestCPUFingerprint(t *testing.T) {
 	}
 
 }
+
+// TestCPUFingerprint_OverrideCompute asserts that setting cpu_total_compute in
+// the client config overrides the detected CPU freq (if any).
+func TestCPUFingerprint_OverrideCompute(t *testing.T) {
+	f := NewCPUFingerprint(testLogger())
+	node := &structs.Node{
+		Attributes: make(map[string]string),
+	}
+	cfg := &config.Config{}
+	ok, err := f.Fingerprint(cfg, node)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if !ok {
+		t.Fatalf("should apply")
+	}
+
+	// Get actual system CPU
+	origCPU := node.Resources.CPU
+
+	// Override it with a setting
+	cfg.CpuCompute = origCPU + 123
+
+	// Make sure the Fingerprinter applies the override
+	ok, err = f.Fingerprint(cfg, node)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if !ok {
+		t.Fatalf("should apply")
+	}
+
+	if node.Resources.CPU != cfg.CpuCompute {
+		t.Fatalf("expected override cpu of %d but found %d", cfg.CpuCompute, node.Resources.CPU)
+	}
+}
diff --git a/helper/stats/cpu.go b/helper/stats/cpu.go
@@ -5,6 +5,7 @@ import (
 	"math"
 	"sync"
 
+	multierror "github.com/hashicorp/go-multierror"
 	"github.com/shirou/gopsutil/cpu"
 )
 
@@ -20,15 +21,15 @@ var (
 
 func Init() error {
 	onceLer.Do(func() {
-		if cpuNumCores, initErr = cpu.Counts(true); initErr != nil {
-			initErr = fmt.Errorf("Unable to determine the number of CPU cores available: %v", initErr)
-			return
+		var merrs *multierror.Error
+		var err error
+		if cpuNumCores, err = cpu.Counts(true); err != nil {
+			merrs = multierror.Append(merrs, fmt.Errorf("Unable to determine the number of CPU cores available: %v", err))
 		}
 
 		var cpuInfo []cpu.InfoStat
-		if cpuInfo, initErr = cpu.Info(); initErr != nil {
-			initErr = fmt.Errorf("Unable to obtain CPU information: %v", initErr)
-			return
+		if cpuInfo, err = cpu.Info(); err != nil {
+			merrs = multierror.Append(merrs, fmt.Errorf("Unable to obtain CPU information: %v", initErr))
 		}
 
 		for _, cpu := range cpuInfo {
@@ -41,6 +42,9 @@ func Init() error {
 		// node to fall into a unique computed node class
 		cpuMhzPerCore = math.Floor(cpuMhzPerCore)
 		cpuTotalTicks = math.Floor(float64(cpuNumCores) * cpuMhzPerCore)
+
+		// Set any errors that occurred
+		initErr = merrs.ErrorOrNil()
 	})
 	return initErr
 }
@@ -60,8 +64,7 @@ func CPUModelName() string {
 	return cpuModelName
 }
 
-// TotalTicksAvailable calculates the total frequency available across all
-// cores
+// TotalTicksAvailable calculates the total Mhz available across all cores
 func TotalTicksAvailable() float64 {
 	return cpuTotalTicks
 }
diff --git a/website/source/docs/runtime/interpolation.html.md.erb b/website/source/docs/runtime/interpolation.html.md.erb
@@ -114,14 +114,20 @@ Below is a table documenting common node properties:
     <td><tt>${attr.cpu.arch}</tt></td>
     <td>CPU architecture of the client (e.g. <tt>amd64</tt>, <tt>386</tt>)</td>
   </tr>
-  <tr>
-    <td><tt>${attr.consul.datacenter}</tt></td>
-    <td>The Consul datacenter of the client (if Consul is found)</td>
-  </tr>
   <tr>
     <td><tt>${attr.cpu.numcores}</tt></td>
     <td>Number of CPU cores on the client</td>
   </tr>
+  <tr>
+    <td><tt>${attr.cpu.totalcompute}</tt></td>
+    <td>
+      <tt>cpu.frequency &times; cpu.numcores</tt> but may be overridden by <tt>client.cpu_total_compute</tt>
+    </td>
+  </tr>
+  <tr>
+    <td><tt>${attr.consul.datacenter}</tt></td>
+    <td>The Consul datacenter of the client (if Consul is found)</td>
+  </tr>
   <tr>
     <td><tt>${attr.driver.&lt;property&gt;}</tt></td>
     <td>See the [task drivers](/docs/drivers/index.html) for property documentation</td>