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

HCL to JSON api endpoint #4138

Merged
merged 11 commits into from
Apr 19, 2018
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
## 0.8.1 (Unreleased)

IMPROVEMENTS:
* api: Add /v1/jobs/parse api endpoint for rendering HCL jobs files as JSON [[GH-2782](https://github.com/hashicorp/nomad/issues/2782)]
* client: Create new process group on process startup. [[GH-3572](https://github.com/hashicorp/nomad/issues/3572)]

## 0.8.0 (April 12, 2018)
Expand Down
22 changes: 22 additions & 0 deletions api/jobs.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,11 +36,33 @@ type Jobs struct {
client *Client
}

// JobsParseRequest is used for arguments of the /vi/jobs/parse endpoint
type JobsParseRequest struct {
//JobHCL is an hcl jobspec
Copy link
Contributor

Choose a reason for hiding this comment

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

Put a space between forward slashes and the first letter

JobHCL string

//Canonicalize is a flag as to if the server should return default values
Copy link
Member

Choose a reason for hiding this comment

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

Put a newline between the previous field and the comment on the next one.

//for unset fields
Canonicalize bool
}

// Jobs returns a handle on the jobs endpoints.
func (c *Client) Jobs() *Jobs {
return &Jobs{client: c}
}

// Parse is used to convert the HCL repesentation of a Job to JSON server side.
// To parse the HCL client side see package github.com/hashicorp/nomad/jobspec
Copy link
Member

Choose a reason for hiding this comment

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

Put a period between multiple sentences in comments as godoc/godoc.org will run them together into one line.

func (j *Jobs) Parse(jobHCL string, canonicalize bool) (*Job, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Name it ParseHCL so we are futureproofed if we want to add a ParseJSON with canonicalize for people who want to just have defaults set.

var job Job
req := &JobsParseRequest{
JobHCL: jobHCL,
Canonicalize: canonicalize,
}
_, err := j.client.write("/v1/jobs/parse", req, &job, nil)
return &job, err
}

func (j *Jobs) Validate(job *Job, q *WriteOptions) (*JobValidateResponse, *WriteMeta, error) {
var resp JobValidateResponse
req := &JobValidateRequest{Job: job}
Expand Down
37 changes: 37 additions & 0 deletions api/jobs_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,43 @@ func TestJobs_Register(t *testing.T) {
}
}

func TestJobs_Parse(t *testing.T) {
t.Parallel()
c, s := makeClient(t, nil, nil)
defer s.Stop()

jobs := c.Jobs()

checkJob := func(job *Job, expected string) {
Copy link
Contributor

Choose a reason for hiding this comment

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

expected -> expectedRegion

if job == nil {
t.Fatal("job should not be nil")
}

region := job.Region

if region == nil {
if expected != "" {
t.Fatalf("expected job region to be '%s' but was unset", expected)
}
} else {
if expected != *region {
t.Fatalf("expected job region '%s', but got '%s'", expected, *region)
}
}
}
job, err := jobs.Parse(mock.HCL(), true)
if err != nil {
t.Fatalf("err: %s", err)
}
checkJob(job, "global")

job, err = jobs.Parse(mock.HCL(), false)
if err != nil {
t.Fatalf("err: %s", err)
}
checkJob(job, "")
}

func TestJobs_Validate(t *testing.T) {
t.Parallel()
c, s := makeClient(t, nil, nil)
Expand Down
1 change: 1 addition & 0 deletions command/agent/http.go
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,7 @@ func (s *HTTPServer) Shutdown() {
// registerHandlers is used to attach our handlers to the mux
func (s *HTTPServer) registerHandlers(enableDebug bool) {
s.mux.HandleFunc("/v1/jobs", s.wrap(s.JobsRequest))
s.mux.HandleFunc("/v1/jobs/parse", s.wrap(s.JobsParseRequest))
s.mux.HandleFunc("/v1/job/", s.wrap(s.JobSpecificRequest))

s.mux.HandleFunc("/v1/nodes", s.wrap(s.NodesRequest))
Expand Down
27 changes: 27 additions & 0 deletions command/agent/job_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (

"github.com/golang/snappy"
"github.com/hashicorp/nomad/api"
"github.com/hashicorp/nomad/jobspec"
"github.com/hashicorp/nomad/nomad/structs"
)

Expand Down Expand Up @@ -544,6 +545,32 @@ func (s *HTTPServer) jobDispatchRequest(resp http.ResponseWriter, req *http.Requ
return out, nil
}

// JobsParseRequest parses a hcl jobspec and returns a api.Job
func (s *HTTPServer) JobsParseRequest(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
if req.Method != http.MethodPut && req.Method != http.MethodPost {
return nil, CodedError(405, ErrInvalidMethod)
}

args := &api.JobsParseRequest{}
if err := decodeBody(req, &args); err != nil {
return nil, CodedError(400, err.Error())
}
if args.JobHCL == "" {
return nil, CodedError(400, "Job spec is empty")
}

jobfile := strings.NewReader(args.JobHCL)
jobStruct, err := jobspec.Parse(jobfile)
if err != nil {
return nil, CodedError(400, err.Error())
}

if args.Canonicalize {
jobStruct.Canonicalize()
Copy link
Contributor

Choose a reason for hiding this comment

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

Why not capture the warnings returned here and return it: https://github.com/hashicorp/nomad/blob/master/nomad/structs/structs.go#L906-L908

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure I follow. Canonicalize() does not return any errors? https://github.com/hashicorp/nomad/blob/master/api/jobs.go#L585

}
return jobStruct, nil
}

func ApiJobToStructJob(job *api.Job) *structs.Job {
job.Canonicalize()

Expand Down
32 changes: 32 additions & 0 deletions command/agent/job_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,38 @@ func TestHTTP_JobsRegister_Defaulting(t *testing.T) {
})
}

func TestHTTP_JobsParse(t *testing.T) {
t.Parallel()
httpTest(t, nil, func(s *TestAgent) {
buf := encodeReq(api.JobsParseRequest{JobHCL: mock.HCL()})
req, err := http.NewRequest("POST", "/v1/jobs/parse", buf)
if err != nil {
t.Fatalf("err: %v", err)
}

respW := httptest.NewRecorder()

obj, err := s.Server.JobsParseRequest(respW, req)
if err != nil {
t.Fatalf("err: %v", err)
}
if obj == nil {
t.Fatal("response should not be nil")
}

job := obj.(*api.Job)
expected := mock.Job()
if job.Name == nil || *job.Name != expected.Name {
t.Fatalf("job name is '%s', expected '%s'", *job.Name, expected.Name)
}

if job.Datacenters == nil ||
job.Datacenters[0] != expected.Datacenters[0] {
t.Fatalf("job datacenters is '%s', expected '%s'",
job.Datacenters[0], expected.Datacenters[0])
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Would suggest checking all fields of the returned job against expected instead of just these three fields.
We have a diff helper in

func (j *Job) Diff(other *Job, contextual bool) (*JobDiff, error) {
that we use in nomad plan and other places.

Copy link
Member Author

Choose a reason for hiding this comment

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

These are api.Job structs. Is there a helper to convert to a structs.Job?

Copy link
Contributor

Choose a reason for hiding this comment

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

func ApiJobToStructJob(job *api.Job) *structs.Job {

Copy link
Contributor

Choose a reason for hiding this comment

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

cautionary note - not sure if a direct translation like works okay when it comes to unset fields (esp if canonicalize is not called). so it may produce a diff complaining about empty list vs nil. I haven't tried it before, seem worth trying it and using it here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I spent some time looking at this and there are a few things:

  • Diff errors if IDs don't match (mock.Job() generates a unique ID)
  • Lots of unset fields yields a somewhat unuseful diff.
  • The mock.HCL() method doesn't quite mimic the mock.Job() fully.

At some point it feels like I'm testing more of the jobspec.Parse functionality than the api endpoint. Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's fair. It doesn't seem worth it here to use diff for this test given the above.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the pointer though, I learned more about the structs package 😃

})
}
func TestHTTP_JobQuery(t *testing.T) {
t.Parallel()
httpTest(t, nil, func(s *TestAgent) {
Expand Down
32 changes: 32 additions & 0 deletions nomad/mock/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,38 @@ func Node() *structs.Node {
return node
}

func HCL() string {
return `job "my-job" {
datacenters = ["dc1"]
type = "service"
constraint {
attribute = "${attr.kernel.name}"
value = "linux"
}

group "web" {
count = 10
restart {
attempts = 3
interval = "10m"
delay = "1m"
mode = "delay"
}
task "web" {
driver = "exec"
config {
command = "/bin/date"
}
resources {
cpu = 500
memory = 256
}
}
}
}
`
}

func Job() *structs.Job {
job := &structs.Job{
Region: "global",
Expand Down
77 changes: 77 additions & 0 deletions website/source/api/jobs.html.md
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,83 @@ $ curl \
}
```

## Parse Job

This endpoint will parse an hcl jobspec and produce the equivalent json encoded
Copy link
Contributor

Choose a reason for hiding this comment

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

HCL and JSON capitalized

job.

| Method | Path | Produces |
| ------ | ------------------------- | -------------------------- |
| `POST` | `/v1/jobs/parse` | `application/json` |

Copy link
Member

Choose a reason for hiding this comment

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

Add the standard blocking/acl section even though neither apply:

 | Blocking Queries | ACL Required               |
 | ---------------- | -------------------------- |
 | `No`             | `none`       |

Copy link
Member Author

Choose a reason for hiding this comment

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

✔️

The table below shows this endpoint's support for
[blocking queries](/api/index.html#blocking-queries) and
[required ACLs](/api/index.html#acls).

| Blocking Queries | ACL Required |
| ---------------- | ------------ |
| `NO` | `none` |

### Parameters

- `JobHCL` `(string: <required>)` - Specifies the HCL definition of the job
encoded in a JSON string.
- `Canonicalize` `(bool: false)` - Flag to enable setting any unset fields to
their default values.

## Sample Payload

```json
{
"JobHCL":"job \"example\" { type = \"service\" group \"cache\" {} }",
"Canonicalize": true
}
```

### Sample Request

```text
$ curl \
--request POST \
--data '{"Canonicalize": true, "JobHCL": "job \"my-job\" {}"}' \
https://localhost:4646/v1/jobs/parse
```

### Sample Response

```json
{
"AllAtOnce": false,
"Constraints": null,
"CreateIndex": 0,
"Datacenters": null,
"ID": "my-job",
"JobModifyIndex": 0,
"Meta": null,
"Migrate": null,
"ModifyIndex": 0,
"Name": "my-job",
"Namespace": "default",
"ParameterizedJob": null,
"ParentID": "",
"Payload": null,
"Periodic": null,
"Priority": 50,
"Region": "global",
"Reschedule": null,
"Stable": false,
"Status": "",
"StatusDescription": "",
"Stop": false,
"SubmitTime": null,
"TaskGroups": null,
"Type": "service",
"Update": null,
"VaultToken": "",
"Version": 0
}
```

## Read Job

This endpoint reads information about a single job for its specification and
Expand Down