Skip to content

Commit

Permalink
jobspec: lower min cpu resources from 10->1
Browse files Browse the repository at this point in the history
Since CPU resources are usually a soft limit it is desirable to allow
setting it as low as possible to allow tasks to run only in "idle" time.

Setting it to 0 is still not allowed to avoid potential unintentional
side effects with allowing a zero value. While there may not be any side
effects this commit attempts to minimize risk by avoiding the issue.

This does *not* change the defaults.
  • Loading branch information
schmichael committed Sep 30, 2020
1 parent 235f938 commit 902b0b5
Show file tree
Hide file tree
Showing 6 changed files with 88 additions and 18 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ IMPROVEMENTS:
* core: Improved job deregistration error logging. [[GH-8745](https://github.com/hashicorp/nomad/issues/8745)]
* api: Added support for cancellation contexts to HTTP API. [[GH-8836](https://github.com/hashicorp/nomad/issues/8836)]
* driver/docker: upgrade pause container and detect architecture [[GH-8957](https://github.com/hashicorp/nomad/pull/8957)]
* jobspec: Lowered minimum CPU allowed from 10 to 1. [[GH-8996](https://github.com/hashicorp/nomad/issues/8996)]

BUG FIXES:

Expand Down
5 changes: 4 additions & 1 deletion api/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ func DefaultResources() *Resources {
// IN nomad/structs/structs.go and should be kept in sync.
func MinResources() *Resources {
return &Resources{
CPU: intToPtr(20),
CPU: intToPtr(1),
MemoryMB: intToPtr(10),
}
}
Expand Down Expand Up @@ -110,6 +110,9 @@ type NetworkResource struct {
}

func (n *NetworkResource) Canonicalize() {
// COMPAT(0.12) MBits is deprecated but this should not be removed
// until MBits is fully removed. Removing this *without* fully removing
// MBits would cause unnecessary job diffs and destructive updates.
if n.MBits == nil {
n.MBits = intToPtr(10)
}
Expand Down
17 changes: 17 additions & 0 deletions command/agent/job_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2849,6 +2849,23 @@ func TestJobs_ApiJobToStructsJobUpdate(t *testing.T) {
require.Equal(t, group2, *structsJob.TaskGroups[1].Update)
}

// TestJobs_Matching_Resources asserts:
// api.{Default,Min}Resources == structs.{Default,Min}Resources
//
// While this is an odd place to test that, this is where both are imported,
// validated, and converted.
func TestJobs_Matching_Resources(t *testing.T) {
t.Parallel()

// api.MinResources == structs.MinResources
structsMinRes := ApiResourcesToStructs(api.MinResources())
assert.Equal(t, structs.MinResources(), structsMinRes)

// api.DefaultResources == structs.DefaultResources
structsDefaultRes := ApiResourcesToStructs(api.DefaultResources())
assert.Equal(t, structs.DefaultResources(), structsDefaultRes)
}

// TestHTTP_JobValidate_SystemMigrate asserts that a system job with a migrate
// stanza fails to validate but does not panic (see #5477).
func TestHTTP_JobValidate_SystemMigrate(t *testing.T) {
Expand Down
17 changes: 1 addition & 16 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -2099,7 +2099,7 @@ func DefaultResources() *Resources {
// api/resources.go and should be kept in sync.
func MinResources() *Resources {
return &Resources{
CPU: 20,
CPU: 1,
MemoryMB: 10,
}
}
Expand Down Expand Up @@ -2223,11 +2223,6 @@ func (r *Resources) MeetsMinResources() error {
if r.MemoryMB < minResources.MemoryMB {
mErr.Errors = append(mErr.Errors, fmt.Errorf("minimum MemoryMB value is %d; got %d", minResources.MemoryMB, r.MemoryMB))
}
for i, n := range r.Networks {
if err := n.MeetsMinResources(); err != nil {
mErr.Errors = append(mErr.Errors, fmt.Errorf("network resource at index %d failed: %v", i, err))
}
}

return mErr.ErrorOrNil()
}
Expand Down Expand Up @@ -2444,16 +2439,6 @@ func (n *NetworkResource) Canonicalize() {
}
}

// MeetsMinResources returns an error if the resources specified are less than
// the minimum allowed.
func (n *NetworkResource) MeetsMinResources() error {
var mErr multierror.Error
if n.MBits < 1 {
mErr.Errors = append(mErr.Errors, fmt.Errorf("minimum MBits value is 1; got %d", n.MBits))
}
return mErr.ErrorOrNil()
}

// Copy returns a deep copy of the network resource
func (n *NetworkResource) Copy() *NetworkResource {
if n == nil {
Expand Down
61 changes: 61 additions & 0 deletions nomad/structs/structs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1378,6 +1378,67 @@ func TestTask_Validate(t *testing.T) {
}
}

func TestTask_Validate_Resources(t *testing.T) {
cases := []struct {
name string
res *Resources
}{
{
name: "Minimum",
res: MinResources(),
},
{
name: "Default",
res: DefaultResources(),
},
{
name: "Full",
res: &Resources{
CPU: 1000,
MemoryMB: 1000,
IOPS: 1000,
Networks: []*NetworkResource{
{
Mode: "host",
Device: "localhost",
CIDR: "127.0.0.0/8",
IP: "127.0.0.1",
MBits: 1000,
DNS: &DNSConfig{
Servers: []string{"localhost"},
Searches: []string{"localdomain"},
Options: []string{"ndots:5"},
},
ReservedPorts: []Port{
{
Label: "reserved",
Value: 1234,
To: 1234,
HostNetwork: "loopback",
},
},
DynamicPorts: []Port{
{
Label: "dynamic",
Value: 5678,
To: 5678,
HostNetwork: "loopback",
},
},
},
},
},
},
}

for i := range cases {
tc := cases[i]
t.Run(tc.name, func(t *testing.T) {
require.NoError(t, tc.res.Validate())
})
}
}

func TestTask_Validate_Services(t *testing.T) {
s1 := &Service{
Name: "service-name",
Expand Down
5 changes: 4 additions & 1 deletion vendor/github.com/hashicorp/nomad/api/resources.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 902b0b5

Please sign in to comment.