Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed test and moved constants into standalone func #3536

Merged
merged 4 commits into from
Nov 13, 2017
+53 −16
Merged

Fixed test and moved constants into standalone func #3536

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
622d3dd
Fixed test and moved constants into standalone func
angrycub Nov 13, 2017
904d171
gofmt and goimports
angrycub Nov 13, 2017
6c36272
gofmt and goimports nomad/structs/structs.go
angrycub Nov 13, 2017
c0f4f52
Review feedback + re-add dropped import
angrycub Nov 13, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions api/jobs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func TestJobs_Canonicalize(t *testing.T) {
{
KillTimeout: helper.TimeToPtr(5 * time.Second),
LogConfig: DefaultLogConfig(),
Resources: MinResources(),
Resources: DefaultResources(),
},
},
},
Expand Down Expand Up @@ -201,7 +201,7 @@ func TestJobs_Canonicalize(t *testing.T) {
{
Name: "task1",
LogConfig: DefaultLogConfig(),
Resources: MinResources(),
Resources: DefaultResources(),
KillTimeout: helper.TimeToPtr(5 * time.Second),
},
},
Expand Down Expand Up @@ -550,7 +550,7 @@ func TestJobs_Canonicalize(t *testing.T) {
{
Name: "task1",
LogConfig: DefaultLogConfig(),
Resources: MinResources(),
Resources: DefaultResources(),
KillTimeout: helper.TimeToPtr(5 * time.Second),
},
},
Expand Down Expand Up @@ -582,7 +582,7 @@ func TestJobs_Canonicalize(t *testing.T) {
{
Name: "task1",
LogConfig: DefaultLogConfig(),
Resources: MinResources(),
Resources: DefaultResources(),
KillTimeout: helper.TimeToPtr(5 * time.Second),
},
},
Expand Down
27 changes: 23 additions & 4 deletions api/resources.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was only used in unit tests and is now replaced by DefaultResources, so why do we need MinResources here again?

return &Resources{
CPU: helper.IntToPtr(100),
MemoryMB: helper.IntToPtr(10),
IOPS: helper.IntToPtr(0),
}

}

// Merge merges this resource with another resource.
Expand Down
29 changes: 23 additions & 6 deletions nomad/structs/structs.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,7 @@ import (

"github.com/gorhill/cronexpr"
"github.com/hashicorp/consul/api"
"github.com/hashicorp/go-multierror"
"github.com/hashicorp/go-version"
multierror "github.com/hashicorp/go-multierror"
"github.com/hashicorp/nomad/acl"
"github.com/hashicorp/nomad/helper"
"github.com/hashicorp/nomad/helper/args"
Expand Down Expand Up @@ -1222,8 +1221,24 @@ 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,
Expand Down Expand Up @@ -1269,15 +1284,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))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This hard codes 20 but should use the value from the min resources. Same for all others.
fmt.Errorf("minimum CPU value is %d; got %d", minResources.CPU, 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 {
Expand Down