Scaling API changes #7409

cgbaker · 2020-03-21T15:40:14Z

partially completes #7394

Includes:

new Scaling block on TaskGroup
Enforce policy min/max during job registration Enforce scaling.[min,max] when scheduling #7413
new RPCs:
- Job.Scale: short-cut for a Job., will be expanded later to also persist a scaling event
- Job.ScaleStatus: get the current counts for a job's task group. similar to Job.Summary, but with more autoscaler-specific information and an appropriate ACL capability
- Scaling.ListPolicies: list the autoscaling polices in a namespace
- Scaling.GetPolicy: get a specific autoscaling policy
ACL support and testing for the new RPC endpoints ACL support for scaling APIs #7418
HTTP endpoints and API support for the above RPC endpoints
new state store table, a view of the scaling policies contained in jobs
lifecycle to populate the scaling policy table during job lifecycle

Currently missing (will address after PR):

CLI support
documentation in website

…tp now)

…of scaling request

updating job scaling endpoints to match RFC, cleaning up the API object as well

… tg.Count, tg.Scaling.Min/Max are well-defined with reasonable defaults. - tg.Count defaults to tg.Scaling.Min if present (falls back on previous default of 1 if Scaling is absent) - Validate() enforces tg.Scaling.Min <= tg.Count <= tg.Scaling.Max modification in ApiScalingPolicyToStructs, api.TaskGroup.Validate so that defaults are handled for TaskGroup.Count and

nomad/structs/structs.go

- read-job-scaling - scale-job - list-scaling-policies - read-scaling-policy updated the read and right policy dispositions, added the new autoscaler disposition

added ACL protection in Job.Scale in Job.Scale, only perform a Job.Register if the Count was non-nil

…kages

that we were throwing away .Min if provided

jrasell

Test failures look very unrelated.
I have checked this out locally, built and tested against the autoscaler with excellent results!

🎈 🎉 🎈

cgbaker · 2020-03-24T16:02:03Z

two tests failures are both flaky tests, merging now. will put Changelog in another PR

lgfa29

Minor comments, but LGTM. Awesome work!

lgfa29 · 2020-03-24T14:57:21Z

acl/policy.go

+	PolicyRead       = "read"
+	PolicyList       = "list"
+	PolicyWrite      = "write"
+	PolicyAutoscaler = "autoscaler"


A bit of bikeshedding here, but the other policies are named as verbs, so should this be

PolicyAutoscale = "autoscale"

?

lgfa29 · 2020-03-24T15:12:45Z

api/jobs_test.go

+	resp, _, err := jobs.Info(*job.ID, nil)
+	require.NoError(err)
+	require.Equal(*resp.TaskGroups[0].Count, newCount)
+


There should be a test for count as nil since it's a unique use case and to make sure this behaviour doesn't changed by accident.

lgfa29 · 2020-03-24T15:16:02Z

api/scaling.go

+			m = int64(*tg.Count)
+		} else {
+			// this should not be at this point, but safeguard here just in case
+			m = 0


Maybe use m = 1 as safeguard to match https://github.com/hashicorp/nomad/pull/7409/files#diff-6765437a29d3652967c3a164dc7f5752R450?

lgfa29 · 2020-03-24T15:32:43Z

api/tasks_test.go

+	require.NotNil(tg.Count)
+	require.NotNil(tg.Scaling.Min)
+	require.Equal(int64(5), *tg.Scaling.Min)
+	require.Equal(*tg.Scaling.Min, int64(*tg.Count))


Maybe add a sanity check where Scaling.Min and Count are both set to different values and then check if those values are kept

lgfa29 · 2020-03-24T15:35:17Z

api/utils.go

@@ -26,6 +26,11 @@ func uint64ToPtr(u uint64) *uint64 {
 	return &u
 }

+// int64ToPtr returns the pointer to a int64
+func int64ToPtr(u int64) *int64 {
+	return &u


More bikeshedding: use i instead of u since this is acting on ints

lgfa29 · 2020-03-24T15:58:40Z

jobspec/test-fixtures/tg-scaling-policy.hcl

@@ -0,0 +1,14 @@
+job "elastic" {
+  group "group" {
+    scaling {


Should this fixture have min and max as well?

lgfa29 · 2020-03-24T15:58:59Z

jobspec/parse_test.go

+								"val": 5,
+								"f":   .1,
+							},
+							Enabled: helper.BoolToPtr(false),


Validate min and max as well.

lgfa29 · 2020-03-24T16:15:46Z

nomad/mock/mock.go

+		Policy: map[string]interface{}{
+			"a": "b",
+		},
+		Enabled:     true,


Add Min and Max

github-actions · 2023-01-14T02:13:18Z

I'm going to lock this pull request because it has been closed for 120 days ⏳. This helps our maintainers find and focus on the active contributions.
If you have found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

pruett deployed to Netlify Preview March 21, 2020 16:19 View deployment

pruett deployed to Netlify Preview March 21, 2020 16:20 View deployment

pruett deployed to Netlify Preview March 22, 2020 13:06 View deployment

pruett deployed to Netlify Preview March 23, 2020 11:00 View deployment

cgbaker force-pushed the scaling-api branch from 280149d to aa58b82 Compare March 23, 2020 11:29

pruett deployed to Netlify Preview March 23, 2020 11:35 View deployment

cgbaker force-pushed the scaling-api branch from aa58b82 to 2c7464b Compare March 23, 2020 12:43

cgbaker added this to the 0.11.0 milestone Mar 24, 2020

cgbaker force-pushed the scaling-api branch from 4639141 to cc1bfb6 Compare March 24, 2020 13:43

cgbaker and others added 20 commits March 24, 2020 13:55

WIP: adding ScalingPolicy to api/structs and state store

ee1b091

wip: upsert/delete scaling policies on job upsert/delete

0762386

wip: was incorrectly parsing ScalingPolicy

e6d75ca

wip: test for scaling policy parsing

7ba9b94

wip: added policy get endpoint, added UUID to policy

a715eb7

wip: working on job group scaling endpoint

8102849

wip: added job.scale rpc endpoint, needs explicit test (tested via ht…

1c9bac9

…tp now)

wip: add job scale endpoint in client

ef7cb0e

wip: add GET endpoint for job group scaling target

4406668

wip: added tests for client methods around group scaling

94381c0

wip: remove PolicyOverride from scaling request

16472c0

wip: added Enabled to ScalingPolicyListStub, removed JobID from body …

6b9c004

…of scaling request

wip: add scaling policies methods to the client

7544aaa

wip: add tests for job scale method

836acc1

wip: use testify in job scaling tests

9756c64

wip: removed some commented junk from scaling poc

c095f41

finished refactoring state store, schema, etc

3b4a1ae

wip: some tests still failing

66bf8dd

updating job scaling endpoints to match RFC, cleaning up the API object as well

scaling: ensure min and max int64s are in toplevel of block.

9243718

wip: scaling status return, almost done

03eb96a

cgbaker added 4 commits March 24, 2020 13:57

finished Job.ScaleStatus RPC, need to work on http endpoint

4759bc3

fixed http endpoints for job.register and job.scalestatus

2d88d57

scaling api: put api.* objects in agreement with structs.* objects

4156009

cgbaker force-pushed the scaling-api branch 2 times, most recently from b0f5ed9 to 906b14b Compare March 24, 2020 14:13

cgbaker commented Mar 24, 2020

View reviewed changes

nomad/structs/structs.go Show resolved Hide resolved

cgbaker added 6 commits March 24, 2020 14:38

added new ACL capabilities related to autoscaling:

f704a49

- read-job-scaling - scale-job - list-scaling-policies - read-scaling-policy updated the read and right policy dispositions, added the new autoscaler disposition

wip: ACL checking for RPC Job.ScaleStatus

ac5a166

made count optional during job scaling actions

d4f967c

added ACL protection in Job.Scale in Job.Scale, only perform a Job.Register if the Count was non-nil

add acl validation to Scaling.ListPolicies and Scaling.GetPolicy

ff652bf

added new int64ToPtr method to api/util to avoid pulling in other pac…

e831ec3

…kages

bad conversion between api.ScalingPolicy and structs.ScalingPolicy meant

4330fe8

that we were throwing away .Min if provided

cgbaker force-pushed the scaling-api branch from f46336d to 4330fe8 Compare March 24, 2020 14:43

cgbaker marked this pull request as ready for review March 24, 2020 14:46

jrasell approved these changes Mar 24, 2020

View reviewed changes

cgbaker merged commit 33360be into master Mar 24, 2020

cgbaker deleted the scaling-api branch March 24, 2020 16:02

lgfa29 approved these changes Mar 24, 2020

View reviewed changes

cgbaker mentioned this pull request Mar 24, 2020

more testing for scaling API #7474

Merged

github-actions bot locked as resolved and limited conversation to collaborators Jan 14, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Scaling API changes #7409

Scaling API changes #7409

cgbaker commented Mar 21, 2020 •

edited

Loading

jrasell left a comment

cgbaker commented Mar 24, 2020

lgfa29 left a comment

lgfa29 Mar 24, 2020

lgfa29 Mar 24, 2020

lgfa29 Mar 24, 2020

lgfa29 Mar 24, 2020

lgfa29 Mar 24, 2020

lgfa29 Mar 24, 2020

lgfa29 Mar 24, 2020

lgfa29 Mar 24, 2020

github-actions bot commented Jan 14, 2023

Scaling API changes #7409

Scaling API changes #7409

Conversation

cgbaker commented Mar 21, 2020 • edited Loading

jrasell left a comment

Choose a reason for hiding this comment

cgbaker commented Mar 24, 2020

lgfa29 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Jan 14, 2023

cgbaker commented Mar 21, 2020 •

edited

Loading