Add CLI and API support for forcing rescheduling of failed allocs #4274

preetapan · 2018-05-09T21:06:27Z

This PR changes the /job//evaluate end point to support specifying a json payload EvalOptions. Setting ForceReschedule to true will reschedule any failed allocs irrespective of their reschedule eligibility at the time of calling the endpoint.

If the job does not have rescheduling enabled, setting ForceReschedule is a no-op.

Also added a new CLI nomad job eval <jobid> to allow operators to easily force an evaluation without needing to run/register the job with full job hcl. To force an evaluation from the CLI, operators can run nomad job eval -force-reschedule

…aluate to take a request payload

preetapan · 2018-05-09T21:07:11Z

~~@dadgar Documentation updates aren't in this PR yet, coming in a future commit~~
Docs committed in 2d0e273 and b5e18b6

dadgar · 2018-05-09T22:33:30Z

api/jobs.go

@@ -224,9 +224,14 @@ func (j *Jobs) Deregister(jobID string, purge bool, q *WriteOptions) (string, *W
 }

 // ForceEvaluate is used to force-evaluate an existing job.
-func (j *Jobs) ForceEvaluate(jobID string, q *WriteOptions) (string, *WriteMeta, error) {
+func (j *Jobs) ForceEvaluate(jobID string, opts EvalOptions, q *WriteOptions) (string, *WriteMeta, error) {


Worth renaming to EvaluateWithOpts?

I modeled this after JobDeregisterOptions.

I meant the name of the function. Since it can be more than just force given it takes a generic options object

dadgar · 2018-05-09T22:34:58Z

command/agent/job_endpoint.go

-		JobID: jobName,
+	var args structs.JobEvaluateRequest
+
+	// TODO(preetha) remove in 0.9


Use the COMPAT tag so we can grep our code base

dadgar · 2018-05-09T22:37:41Z

command/job_eval.go

+	helpText := `
+Usage: nomad job eval [options] <job_id>
+
+  Force an evaluation of the provided job id


...provided job ID. Forcing an evaluation will trigger the scheduler to re-evaluate the job. The force flags allow operators to force the scheduler to create new allocations under certain scenarios.

dadgar · 2018-05-09T22:38:08Z

command/job_eval.go

+Eval Options:
+
+  -force-reschedule
+    Force reschedule any failed allocations even if they are not currently


remove any and and add a period to then end.

dadgar · 2018-05-09T22:38:41Z

command/job_eval.go

+}
+
+func (c *JobEvalCommand) Synopsis() string {
+	return "Force evaluating a job using its job id"


Force an evaluation for the job

dadgar · 2018-05-09T22:44:12Z

nomad/job_endpoint.go

+		for _, alloc := range allocs {
+			taskGroup := job.LookupTaskGroup(alloc.TaskGroup)
+			// Forcing rescheduling is only allowed if task group has rescheduling enabled
+			if taskGroup == nil || taskGroup.ReschedulePolicy == nil || !taskGroup.ReschedulePolicy.Enabled() {


Can't we remove taskGroup.ReschedulePolicy == nil given that Enabled() handles the nil case

dadgar · 2018-05-09T22:45:35Z

website/source/api/jobs.html.md

+- `JobID` `(string: <required>)` - Specify the ID of the job in the JSON payload
+
+- `EvalOptions` `(<optional>)` - Specify additional options to be used during the forced evaluation.
+    - `ForceReschedule` `(bool: false)` - If set, any failed allocations of the job are rescheduled


Remove any�. For me it gives the connotation that all failed evals will be rescheduler but really we mean any failed alloc that doesn't already have a replacement

dadgar · 2018-05-09T22:45:55Z

website/source/api/jobs.html.md

 ### Sample Request

 ```text
 $ curl \
    --request POST \
+    [email protected] \


I think there should be a space between -d and @sample.json

dadgar · 2018-05-09T22:46:29Z

website/source/docs/commands/job/eval.html.md.erb

+## Usage
+
+```
+nomad job eval [options] <jobID>


dadgar · 2018-05-09T22:46:47Z

website/source/docs/commands/job/eval.html.md.erb

+
+## Eval Options
+
+* `-force-reschedule`: `force-reschedule` is used to force placement of any failed allocations.


Same comment on any

nickethier · 2018-05-10T17:31:06Z

command/job_eval.go

+	})
+}
+
+func (c *JobEvalCommand) Name() string { return "eval" }


return "job eval"

preetapan · 2018-05-10T20:41:40Z

@dadgar @nickethier ive addressed all the review comments, this is ready for another look

dadgar · 2018-05-11T17:36:52Z

api/jobs.go

@@ -224,9 +224,14 @@ func (j *Jobs) Deregister(jobID string, purge bool, q *WriteOptions) (string, *W
 }

 // ForceEvaluate is used to force-evaluate an existing job.
-func (j *Jobs) ForceEvaluate(jobID string, q *WriteOptions) (string, *WriteMeta, error) {
+func (j *Jobs) ForceEvaluate(jobID string, opts EvalOptions, q *WriteOptions) (string, *WriteMeta, error) {


I meant the name of the function. Since it can be more than just force given it takes a generic options object

dadgar · 2018-05-11T17:38:43Z

command/job_eval.go

+  -force-reschedule
+    Force reschedule failed allocations even if they are not currently
+    eligible for rescheduling.
+  -detach


Blank line above

dadgar · 2018-05-11T17:39:04Z

command/job_eval.go

+}
+
+func (c *JobEvalCommand) Synopsis() string {
+	return "Force an evaluation for the job using its job ID"


Remove " using its job ID"

dadgar · 2018-05-11T17:39:54Z

command/job_eval.go

+    Force reschedule failed allocations even if they are not currently
+    eligible for rescheduling.
+  -detach
+    Return immediately instead of entering monitor mode. After deployment


After deployment resume?

dadgar · 2018-05-11T17:40:46Z

command/job_eval.go

+		c.Ui.Error(fmt.Sprintf("Error evaluating job: %s", err))
+		return 1
+	}
+	c.Ui.Output(fmt.Sprintf("Created eval ID: %q ", limit(evalId, length)))


This should be in the detach if

dadgar · 2018-05-11T17:42:24Z

website/source/docs/commands/job/eval.html.md.erb

+
+## Eval Options
+
+* `-force-reschedule`: `force-reschedule` is used to force placement of failed allocations.


Add the detach and verbose flag

dadgar · 2018-05-21T22:07:33Z

api/jobs.go

@@ -233,6 +233,21 @@ func (j *Jobs) ForceEvaluate(jobID string, q *WriteOptions) (string, *WriteMeta,
 	return resp.EvalID, wm, nil
 }

+// ForceEvaluate is used to force-evaluate an existing job.


Wrong method name in doc

dadgar · 2018-05-21T22:10:21Z

website/source/docs/commands/job/eval.html.md.erb

+
+```
+$ nomad job eval job1
+Created eval ID: "6754c2e3-9abb-e7e9-dc92-76aab01751c8"


Can you update b/c of the new monitoring

jippi · 2018-05-22T06:43:30Z

command/agent/job_endpoint.go

-		JobID: jobName,
+	var args structs.JobEvaluateRequest
+
+	// TODO(preetha): remove in 0.9


0.10? i guess this code will go out in 0.9 :)

github-actions · 2023-03-04T02:17:14Z

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.

Preetha Appan added 5 commits May 8, 2018 17:26

Work in progress - force rescheduling of failed allocs

242cc19

Fix logic inversion in force rescheduling

3b7d23f

Add unit tests for forced rescheduling

268a99e

Added CLI for evaluating job given ID, and modified client API for ev…

1bad719

…aluate to take a request payload

fix test comment

4f9d92c

preetapan requested a review from dadgar May 9, 2018 21:06

Preetha Appan added 2 commits May 9, 2018 16:52

Documentation for evaluate endpoint

2d0e273

Docs for job eval CLI

b5e18b6

dadgar requested changes May 9, 2018

View reviewed changes

nickethier requested changes May 10, 2018

View reviewed changes

command/job_eval.go Outdated

})

}

func (c *JobEvalCommand) Name() string { return "eval" }

Copy link

Member

nickethier May 10, 2018

Choose a reason for hiding this comment

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

return "job eval"

Preetha Appan added 3 commits May 10, 2018 14:42

Code review feedback

879c2c9

Add support for monitoring evals, and -detach/-verbose support

53c05c5

unit test for job eval should detach

e2f13d2

dadgar requested changes May 11, 2018

View reviewed changes

Preetha Appan added 2 commits May 11, 2018 13:39

more review feedback

b2006cc

Add new method EvaluateWithOptions to avoid breaking go API client

ae5d8fd

dadgar approved these changes May 21, 2018

View reviewed changes

Preetha Appan added 2 commits May 21, 2018 17:20

Fix docs and method documentation in API

2ea09b8

remove extra return

a5ca379

preetapan merged commit 447527a into master May 21, 2018

preetapan deleted the f-force-rescheduling branch May 21, 2018 23:26

jippi reviewed May 22, 2018

View reviewed changes

github-actions bot locked as resolved and limited conversation to collaborators Mar 4, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add CLI and API support for forcing rescheduling of failed allocs #4274

Add CLI and API support for forcing rescheduling of failed allocs #4274

preetapan commented May 9, 2018

preetapan commented May 9, 2018 •

edited

Loading

dadgar May 9, 2018

preetapan May 10, 2018

dadgar May 11, 2018

dadgar May 9, 2018

dadgar May 9, 2018

dadgar May 9, 2018

dadgar May 9, 2018

dadgar May 9, 2018

dadgar May 9, 2018

dadgar May 9, 2018

dadgar May 9, 2018

dadgar May 9, 2018

nickethier May 10, 2018

preetapan commented May 10, 2018 •

edited

Loading

dadgar May 11, 2018

dadgar May 11, 2018

dadgar May 11, 2018

dadgar May 11, 2018

dadgar May 11, 2018

dadgar May 11, 2018

dadgar May 21, 2018

dadgar May 21, 2018

jippi May 22, 2018

github-actions bot commented Mar 4, 2023


		## Eval Options

		* `-force-reschedule`: `force-reschedule` is used to force placement of any failed allocations.

Add CLI and API support for forcing rescheduling of failed allocs #4274

Add CLI and API support for forcing rescheduling of failed allocs #4274

Conversation

preetapan commented May 9, 2018

preetapan commented May 9, 2018 • edited Loading

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

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

preetapan commented May 10, 2018 • edited Loading

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 Mar 4, 2023

preetapan commented May 9, 2018 •

edited

Loading

preetapan commented May 10, 2018 •

edited

Loading