From 264d999b8be6b7dc8fb3036da87c8b8ba1fbb272 Mon Sep 17 00:00:00 2001 From: Charlie Voiselle Date: Tue, 7 Nov 2017 15:33:32 -0500 Subject: [PATCH 1/5] `memory` default is 10, not 300 Verified by creating sample job and removing memory from resources ``` Task "redis" is "running" Task Resources CPU Memory Disk IOPS Addresses 3/100 MHz 976 KiB/10 MiB 300 MiB 0 db: **.**.**.**:20824 ``` --- website/source/docs/job-specification/resources.html.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/website/source/docs/job-specification/resources.html.md b/website/source/docs/job-specification/resources.html.md index cbc03bfee8a..0ccccc32a12 100644 --- a/website/source/docs/job-specification/resources.html.md +++ b/website/source/docs/job-specification/resources.html.md @@ -49,7 +49,7 @@ job "docs" { - `iops` `(int: 0)` - Specifies the number of IOPS required given as a weight between 0-1000. -- `memory` `(int: 300)` - Specifies the memory required in MB +- `memory` `(int: 10)` - Specifies the memory required in MB - `network` ([Network][]: ) - Specifies the network requirements, including static and dynamic port allocations. From 7c98e466fc331cf3b1ecd088af1dabfe07fd7144 Mon Sep 17 00:00:00 2001 From: Charlie Voiselle Date: Tue, 7 Nov 2017 17:17:49 -0500 Subject: [PATCH 2/5] Rolled back documentation change in favor of correcting the default --- api/resources.go | 2 +- website/source/docs/job-specification/resources.html.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/api/resources.go b/api/resources.go index 8d3f27c6cd2..e9e165419ec 100644 --- a/api/resources.go +++ b/api/resources.go @@ -17,7 +17,7 @@ func (r *Resources) Canonicalize() { r.CPU = helper.IntToPtr(100) } if r.MemoryMB == nil { - r.MemoryMB = helper.IntToPtr(10) + r.MemoryMB = helper.IntToPtr(300) } if r.IOPS == nil { r.IOPS = helper.IntToPtr(0) diff --git a/website/source/docs/job-specification/resources.html.md b/website/source/docs/job-specification/resources.html.md index 0ccccc32a12..cbc03bfee8a 100644 --- a/website/source/docs/job-specification/resources.html.md +++ b/website/source/docs/job-specification/resources.html.md @@ -49,7 +49,7 @@ job "docs" { - `iops` `(int: 0)` - Specifies the number of IOPS required given as a weight between 0-1000. -- `memory` `(int: 10)` - Specifies the memory required in MB +- `memory` `(int: 300)` - Specifies the memory required in MB - `network` ([Network][]: ) - Specifies the network requirements, including static and dynamic port allocations. From 1cb52865bc76445eda2e1eb171992fa934eb83c2 Mon Sep 17 00:00:00 2001 From: Charlie Voiselle Date: Thu, 9 Nov 2017 10:10:11 -0500 Subject: [PATCH 3/5] Canonicalize task and remove merge with MinResources The current code would merge the job with the output of MinResources causing the nil to be replaced with MemoryMB=helper.IntToPtr(10). When later `Canonicalize`d, the 10 would cause the default of 300 not to be applied. Running `Canonicalize` on the task itself guarantees that CPU, MemoryMB, and IOPS is set, so we can remove the Merge withMinResources. --- api/tasks.go | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index c956aac307c..613aff5b3f1 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -186,7 +186,7 @@ func (s *Service) Canonicalize(t *Task, tg *TaskGroup, job *Job) { s.CheckRestart.Canonicalize() - // Canonicallize CheckRestart on Checks and merge Service.CheckRestart + // Canonicalize CheckRestart on Checks and merge Service.CheckRestart // into each check. for _, c := range s.Checks { c.CheckRestart.Canonicalize() @@ -374,9 +374,7 @@ type Task struct { func (t *Task) Canonicalize(tg *TaskGroup, job *Job) { min := MinResources() - min.Merge(t.Resources) - min.Canonicalize() - t.Resources = min + t.Resources.Canonicalize() if t.KillTimeout == nil { t.KillTimeout = helper.TimeToPtr(5 * time.Second) From 20ea6b25dcc3e1629bc7b05e6b09e0e53623db17 Mon Sep 17 00:00:00 2001 From: Charlie Voiselle Date: Thu, 9 Nov 2017 10:57:57 -0500 Subject: [PATCH 4/5] Adopted pattern from LogConfig to handle panic when no resource stanza at all In testing realized that Resources night not be present at all. Testing this case caused panic. Added in a means to collect clean defaults in that case. --- api/tasks.go | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index 613aff5b3f1..8dff2b582dc 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -186,7 +186,7 @@ func (s *Service) Canonicalize(t *Task, tg *TaskGroup, job *Job) { s.CheckRestart.Canonicalize() - // Canonicalize CheckRestart on Checks and merge Service.CheckRestart + // Canonicallize CheckRestart on Checks and merge Service.CheckRestart // into each check. for _, c := range s.Checks { c.CheckRestart.Canonicalize() @@ -373,8 +373,13 @@ type Task struct { } func (t *Task) Canonicalize(tg *TaskGroup, job *Job) { - min := MinResources() - t.Resources.Canonicalize() + if t.Resources == nil { + var r Resources + r.Canonicalize() + t.Resources = &r + } else { + t.Resources.Canonicalize() + } if t.KillTimeout == nil { t.KillTimeout = helper.TimeToPtr(5 * time.Second) From 892f34896b9d13e1132632d730b9fd362d0cf361 Mon Sep 17 00:00:00 2001 From: Charlie Voiselle Date: Thu, 9 Nov 2017 20:09:37 -0500 Subject: [PATCH 5/5] Simplified based on review comments --- api/tasks.go | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/api/tasks.go b/api/tasks.go index 8dff2b582dc..e35079628ce 100644 --- a/api/tasks.go +++ b/api/tasks.go @@ -374,13 +374,9 @@ type Task struct { func (t *Task) Canonicalize(tg *TaskGroup, job *Job) { if t.Resources == nil { - var r Resources - r.Canonicalize() - t.Resources = &r - } else { - t.Resources.Canonicalize() + t.Resources = &Resources{} } - + t.Resources.Canonicalize() if t.KillTimeout == nil { t.KillTimeout = helper.TimeToPtr(5 * time.Second) }