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.

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

cli: forward request for job validation to nomad leader #14065

Merged
merged 1 commit into from
Aug 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions .changelog/14065.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
```release-note:bug
cli: Fixed a bug where job validation requeset was not sent to leader
```
14 changes: 9 additions & 5 deletions nomad/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -464,10 +464,8 @@ func getSignalConstraint(signals []string) *structs.Constraint {
}
}

// Summary retrieves the summary of a job
func (j *Job) Summary(args *structs.JobSummaryRequest,
reply *structs.JobSummaryResponse) error {

// Summary retrieves the summary of a job.
func (j *Job) Summary(args *structs.JobSummaryRequest, reply *structs.JobSummaryResponse) error {
if done, err := j.srv.forward("Job.Summary", args, args, reply); done {
return err
}
Expand Down Expand Up @@ -511,8 +509,14 @@ func (j *Job) Summary(args *structs.JobSummaryRequest,
return j.srv.blockingRPC(&opts)
}

// Validate validates a job
// Validate validates a job.
//
// Must forward to the leader, because only the leader will have a live Vault
// client with which to validate vault tokens.
func (j *Job) Validate(args *structs.JobValidateRequest, reply *structs.JobValidateResponse) error {
if done, err := j.srv.forward("Job.Validate", args, args, reply); done {
return err
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think request allows for stale right? But if we would were to allow validation without forwarding (like a local check) we could downgrade Vault validation errors to warnings. But since (I think?) stale requests are not allowed here we're probably good as it is.

Copy link
Member Author

Choose a reason for hiding this comment

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

Nope! The JobValidateRequest rpc embeds a WriteRequest, which has no notion of stale

}
defer metrics.MeasureSince([]string{"nomad", "job", "validate"}, time.Now())

// defensive check; http layer and RPC requester should ensure namespaces are set consistently
Expand Down
2 changes: 1 addition & 1 deletion website/content/api-docs/validate.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ types.
## Validate Job

This endpoint validates a Nomad job file. The local Nomad agent forwards the
request to a server. In the event a server can't be reached the agent verifies
request to the leader. In the event the leader cannot be reached the agent verifies
the job file locally but skips validating driver configurations.

~> This endpoint accepts a **JSON job file**, not an HCL job file.
Expand Down