Skip to content

Commit

Permalink
changes to make sure that Max is present and valid, to improve error …
Browse files Browse the repository at this point in the history
…messages

* made api.Scaling.Max a pointer, so we can detect (and complain) when it is neglected
* added checks to HCL parsing that it is present
* when Scaling.Max is absent/invalid, don't return extraneous error messages during validation
* tweak to multiregion handling to ensure that the count is valid on the interpolated regional jobs

resolves #8355
  • Loading branch information
cgbaker committed Jul 4, 2020
1 parent f015e02 commit 7f8176a
Show file tree
Hide file tree
Showing 5 changed files with 20 additions and 10 deletions.
2 changes: 1 addition & 1 deletion api/scaling.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ type ScalingPolicy struct {
Namespace string
Target map[string]string
Min *int64
Max int64
Max *int64
Policy map[string]interface{}
Enabled *bool
CreateIndex uint64
Expand Down
7 changes: 6 additions & 1 deletion command/agent/scaling_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -78,10 +78,15 @@ func (s *HTTPServer) scalingPolicyQuery(resp http.ResponseWriter, req *http.Requ
func ApiScalingPolicyToStructs(count int, ap *api.ScalingPolicy) *structs.ScalingPolicy {
p := structs.ScalingPolicy{
Enabled: *ap.Enabled,
Max: ap.Max,
Policy: ap.Policy,
Target: map[string]string{},
}
if ap.Max != nil {
p.Max = *ap.Max
} else {
// catch this in Validate
p.Max = -1
}
if ap.Min != nil {
p.Min = *ap.Min
} else {
Expand Down
3 changes: 3 additions & 0 deletions jobspec/parse_group.go
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,9 @@ func parseScalingPolicy(out **api.ScalingPolicy, list *ast.ObjectList) error {
if err := dec.Decode(m); err != nil {
return err
}
if result.Max == nil {
return fmt.Errorf("missing 'max'")
}

// If we have policy, then parse that
if o := listVal.Filter("policy"); len(o.Items) > 0 {
Expand Down
16 changes: 9 additions & 7 deletions nomad/structs/structs.go
Original file line number Diff line number Diff line change
Expand Up @@ -5948,19 +5948,21 @@ func (tg *TaskGroup) validateScalingPolicy(j *Job) error {

var mErr multierror.Error

if tg.Scaling.Min > tg.Scaling.Max {
// was invalid or not specified; don't bother testing anything else involving max
if tg.Scaling.Max < 0 {
mErr.Errors = append(mErr.Errors,
fmt.Errorf("Scaling policy invalid: maximum count must be specified and non-negative"))
} else if tg.Scaling.Max < tg.Scaling.Min {
mErr.Errors = append(mErr.Errors,
fmt.Errorf("Scaling policy invalid: maximum count must not be less than minimum count"))
}

if int64(tg.Count) < tg.Scaling.Min && !(j.IsMultiregion() && tg.Count == 0) {
} else if tg.Scaling.Max < int64(tg.Count) {
mErr.Errors = append(mErr.Errors,
fmt.Errorf("Scaling policy invalid: task group count must not be less than minimum count in scaling policy"))
fmt.Errorf("Scaling policy invalid: task group count must not be greater than maximum count in scaling policy"))
}

if tg.Scaling.Max < int64(tg.Count) {
if int64(tg.Count) < tg.Scaling.Min && !(j.IsMultiregion() && tg.Count == 0 && j.Region == "global") {
mErr.Errors = append(mErr.Errors,
fmt.Errorf("Scaling policy invalid: task group count must not be greater than maximum count in scaling policy"))
fmt.Errorf("Scaling policy invalid: task group count must not be less than minimum count in scaling policy"))
}

return mErr.ErrorOrNil()
Expand Down
2 changes: 1 addition & 1 deletion vendor/github.com/hashicorp/nomad/api/scaling.go

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

0 comments on commit 7f8176a

Please sign in to comment.