From 622d3ddb92ea7e656ef831641c02024cb5a5d6d1 Mon Sep 17 00:00:00 2001
From: Charlie Voiselle <charlie.voiselle@gmail.com>
Date: Mon, 13 Nov 2017 12:05:30 -0500
Subject: [PATCH] Fixed test and moved constants into standalone func

In #3520, work was done to true up the defaults for Nomad resource
stanzas with the documentation.  This fixes the tests that I
accidentally broke in the process.  Some questions were raised about
using dynamic elements as part of expects, which is why I opted to
copy the MinResources pattern.   During this refactor I also noticed
that structs.go had a similar issue and an inconsistent minium for CPU.
---
 api/jobs_test.go         |  8 ++++----
 api/resources.go         | 27 +++++++++++++++++++++++----
 nomad/structs/structs.go | 27 +++++++++++++++++++++++----
 3 files changed, 50 insertions(+), 12 deletions(-)

diff --git a/api/jobs_test.go b/api/jobs_test.go
index 90e51ddfc2f..de4ae634dfb 100644
--- a/api/jobs_test.go
+++ b/api/jobs_test.go
@@ -139,7 +139,7 @@ func TestJobs_Canonicalize(t *testing.T) {
 							{
 								KillTimeout: helper.TimeToPtr(5 * time.Second),
 								LogConfig:   DefaultLogConfig(),
-								Resources:   MinResources(),
+								Resources:   DefaultResources(),
 							},
 						},
 					},
@@ -201,7 +201,7 @@ func TestJobs_Canonicalize(t *testing.T) {
 							{
 								Name:        "task1",
 								LogConfig:   DefaultLogConfig(),
-								Resources:   MinResources(),
+								Resources:   DefaultResources(),
 								KillTimeout: helper.TimeToPtr(5 * time.Second),
 							},
 						},
@@ -550,7 +550,7 @@ func TestJobs_Canonicalize(t *testing.T) {
 							{
 								Name:        "task1",
 								LogConfig:   DefaultLogConfig(),
-								Resources:   MinResources(),
+								Resources:   DefaultResources(),
 								KillTimeout: helper.TimeToPtr(5 * time.Second),
 							},
 						},
@@ -582,7 +582,7 @@ func TestJobs_Canonicalize(t *testing.T) {
 							{
 								Name:        "task1",
 								LogConfig:   DefaultLogConfig(),
-								Resources:   MinResources(),
+								Resources:   DefaultResources(),
 								KillTimeout: helper.TimeToPtr(5 * time.Second),
 							},
 						},
diff --git a/api/resources.go b/api/resources.go
index e9e165419ec..be415a4a5c1 100644
--- a/api/resources.go
+++ b/api/resources.go
@@ -12,28 +12,47 @@ type Resources struct {
 	Networks []*NetworkResource
 }
 
+// Canonicalize will supply missing values in the cases
+// where they are not provided.
 func (r *Resources) Canonicalize() {
+	defaultResources := DefaultResources()
 	if r.CPU == nil {
-		r.CPU = helper.IntToPtr(100)
+		r.CPU = defaultResources.CPU
 	}
 	if r.MemoryMB == nil {
-		r.MemoryMB = helper.IntToPtr(300)
+		r.MemoryMB = defaultResources.MemoryMB
 	}
 	if r.IOPS == nil {
-		r.IOPS = helper.IntToPtr(0)
+		r.IOPS = defaultResources.IOPS
 	}
 	for _, n := range r.Networks {
 		n.Canonicalize()
 	}
 }
 
+// DefaultResources is a small resources object that contains the
+// default resources requests that we will provide to an object.
+// ---  THIS FUNCTION IS REPLICATED IN nomad/structs/structs.go 
+// and should be kept in sync.
+func DefaultResources() *Resources {
+	return &Resources{
+		CPU:      helper.IntToPtr(100),
+		MemoryMB: helper.IntToPtr(300),
+		IOPS:     helper.IntToPtr(0),
+	}
+}
+
+// MinResources is a small resources object that contains the
+// absolute minimum resources that we will provide to an object.
+// This should not be confused with the defaults which are 
+// provided in DefaultResources() ---  THIS LOGIC IS REPLICATED 
+// IN nomad/structs/structs.go and should be kept in sync.
 func MinResources() *Resources {
 	return &Resources{
 		CPU:      helper.IntToPtr(100),
 		MemoryMB: helper.IntToPtr(10),
 		IOPS:     helper.IntToPtr(0),
 	}
-
 }
 
 // Merge merges this resource with another resource.
diff --git a/nomad/structs/structs.go b/nomad/structs/structs.go
index 3b8b7ca496d..4efe8e60a09 100644
--- a/nomad/structs/structs.go
+++ b/nomad/structs/structs.go
@@ -1222,8 +1222,25 @@ const (
 	BytesInMegabyte = 1024 * 1024
 )
 
-// DefaultResources returns the default resources for a task.
+
+// DefaultResources is a small resources object that contains the
+// default resources requests that we will provide to an object.
+// ---  THIS FUNCTION IS REPLICATED IN api/resources.go and should 
+// be kept in sync.
 func DefaultResources() *Resources {
+	return &Resources{
+		CPU:      100,
+		MemoryMB: 300,
+		IOPS:     0,
+	}
+}
+
+// MinResources is a small resources object that contains the
+// absolute minimum resources that we will provide to an object.
+// This should not be confused with the defaults which are 
+// provided in Canonicalize() ---  THIS FUNCTION IS REPLICATED IN 
+// api/resources.go and should be kept in sync.
+func MinResources() *Resources {
 	return &Resources{
 		CPU:      100,
 		MemoryMB: 10,
@@ -1269,15 +1286,17 @@ func (r *Resources) Canonicalize() {
 
 // MeetsMinResources returns an error if the resources specified are less than
 // the minimum allowed.
+// This is based on the minimums defined in the Resources type
 func (r *Resources) MeetsMinResources() error {
 	var mErr multierror.Error
-	if r.CPU < 20 {
+	minResources := MinResources()
+	if r.CPU < minResources.CPU {
 		mErr.Errors = append(mErr.Errors, fmt.Errorf("minimum CPU value is 20; got %d", r.CPU))
 	}
-	if r.MemoryMB < 10 {
+	if r.MemoryMB < minResources.MemoryMB {
 		mErr.Errors = append(mErr.Errors, fmt.Errorf("minimum MemoryMB value is 10; got %d", r.MemoryMB))
 	}
-	if r.IOPS < 0 {
+	if r.IOPS < minResources.IOPS {
 		mErr.Errors = append(mErr.Errors, fmt.Errorf("minimum IOPS value is 0; got %d", r.IOPS))
 	}
 	for i, n := range r.Networks {