From ebd8dff840930c77391eb52ebf11362df72d87c0 Mon Sep 17 00:00:00 2001 From: Thiago Nache Carvalho Date: Tue, 12 Jan 2021 16:33:04 -0300 Subject: [PATCH 1/9] Support HCL2 on nomad package --- nomad/resource_job.go | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/nomad/resource_job.go b/nomad/resource_job.go index 45c82968..9ffddd20 100644 --- a/nomad/resource_job.go +++ b/nomad/resource_job.go @@ -11,10 +11,9 @@ import ( "time" "github.com/hashicorp/nomad/api" + "github.com/hashicorp/nomad/jobspec2" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" - - "github.com/hashicorp/nomad/jobspec" ) func resourceJob() *schema.Resource { @@ -587,7 +586,7 @@ func parseJobspec(raw string, is_json bool, vaultToken *string) (*api.Job, error if is_json { job, err = parseJSONJobspec(raw) } else { - job, err = jobspec.Parse(strings.NewReader(raw)) + job, err = jobspec2.Parse("", strings.NewReader(raw)) } if err != nil { return nil, fmt.Errorf("error parsing jobspec: %s", err) @@ -709,13 +708,13 @@ func jobTaskGroupsRaw(tgs []*api.TaskGroup) []interface{} { func jobspecDiffSuppress(k, old, new string, d *schema.ResourceData) bool { // TODO: does this need to consider is_json ??? // Parse the old job - oldJob, err := jobspec.Parse(strings.NewReader(old)) + oldJob, err := jobspec2.Parse("", strings.NewReader(old)) if err != nil { return false } // Parse the new job - newJob, err := jobspec.Parse(strings.NewReader(new)) + newJob, err := jobspec2.Parse("", strings.NewReader(new)) if err != nil { return false } From 80afaad1892e7b25d9e619a66a59f0ea4ae4fffa Mon Sep 17 00:00:00 2001 From: Thiago Nache Carvalho Date: Tue, 12 Jan 2021 19:15:15 -0300 Subject: [PATCH 2/9] Test the same version as in the example --- nomad/resource_job_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nomad/resource_job_test.go b/nomad/resource_job_test.go index 2054153a..2c5a90b7 100644 --- a/nomad/resource_job_test.go +++ b/nomad/resource_job_test.go @@ -2659,7 +2659,7 @@ job "foo-csi-controller" { driver = "docker" config { - image = "amazon/aws-ebs-csi-driver:latest" + image = "amazon/aws-ebs-csi-driver:v0.7.1" args = [ "controller", From ffa4082fe0909e8e8c13ccdd3a0d3c72ef5311e3 Mon Sep 17 00:00:00 2001 From: Thiago Nache Carvalho Date: Tue, 12 Jan 2021 20:40:26 -0300 Subject: [PATCH 3/9] Revert "Test the same version as in the example" This reverts commit 80afaad1892e7b25d9e619a66a59f0ea4ae4fffa. --- nomad/resource_job_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nomad/resource_job_test.go b/nomad/resource_job_test.go index 2c5a90b7..2054153a 100644 --- a/nomad/resource_job_test.go +++ b/nomad/resource_job_test.go @@ -2659,7 +2659,7 @@ job "foo-csi-controller" { driver = "docker" config { - image = "amazon/aws-ebs-csi-driver:v0.7.1" + image = "amazon/aws-ebs-csi-driver:latest" args = [ "controller", From e4d328c0ddbfdfdf4581166a56c462e7d6bc6264 Mon Sep 17 00:00:00 2001 From: Thiago Nache Carvalho Date: Tue, 12 Jan 2021 20:44:39 -0300 Subject: [PATCH 4/9] Fix CSI job specification for testing. It is not a boolean according to the [documentation](https://www.nomadproject.io/docs/job-specification/group) --- nomad/resource_job_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nomad/resource_job_test.go b/nomad/resource_job_test.go index 2054153a..b17bc3c7 100644 --- a/nomad/resource_job_test.go +++ b/nomad/resource_job_test.go @@ -2654,7 +2654,7 @@ resource "nomad_job" "test" { job "foo-csi-controller" { datacenters = ["dc1"] group "foo-controller" { - stop_after_client_disconnect = true + stop_after_client_disconnect = "90s" task "plugin" { driver = "docker" From bfa37708b54e4d8a8749eb9afd062927d9e8b11d Mon Sep 17 00:00:00 2001 From: Thiago Nache Carvalho Date: Tue, 12 Jan 2021 22:59:02 -0300 Subject: [PATCH 5/9] Pass jobspec as string instead of empty value --- nomad/resource_job.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nomad/resource_job.go b/nomad/resource_job.go index 9ffddd20..501220e5 100644 --- a/nomad/resource_job.go +++ b/nomad/resource_job.go @@ -586,7 +586,7 @@ func parseJobspec(raw string, is_json bool, vaultToken *string) (*api.Job, error if is_json { job, err = parseJSONJobspec(raw) } else { - job, err = jobspec2.Parse("", strings.NewReader(raw)) + job, err = jobspec2.Parse(raw, strings.NewReader(raw)) } if err != nil { return nil, fmt.Errorf("error parsing jobspec: %s", err) @@ -708,13 +708,13 @@ func jobTaskGroupsRaw(tgs []*api.TaskGroup) []interface{} { func jobspecDiffSuppress(k, old, new string, d *schema.ResourceData) bool { // TODO: does this need to consider is_json ??? // Parse the old job - oldJob, err := jobspec2.Parse("", strings.NewReader(old)) + oldJob, err := jobspec2.Parse(old, strings.NewReader(old)) if err != nil { return false } // Parse the new job - newJob, err := jobspec2.Parse("", strings.NewReader(new)) + newJob, err := jobspec2.Parse(new, strings.NewReader(new)) if err != nil { return false } From f67d27e5440afda4266fe5d16e35cc84554c564e Mon Sep 17 00:00:00 2001 From: Thiago Nache Carvalho Date: Wed, 13 Jan 2021 08:15:31 -0300 Subject: [PATCH 6/9] Add cases statements for json, hcl and hcl2 --- nomad/resource_job.go | 67 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 54 insertions(+), 13 deletions(-) diff --git a/nomad/resource_job.go b/nomad/resource_job.go index 501220e5..21edb4d7 100644 --- a/nomad/resource_job.go +++ b/nomad/resource_job.go @@ -11,6 +11,7 @@ import ( "time" "github.com/hashicorp/nomad/api" + "github.com/hashicorp/nomad/jobspec" "github.com/hashicorp/nomad/jobspec2" "github.com/hashicorp/terraform-plugin-sdk/helper/resource" "github.com/hashicorp/terraform-plugin-sdk/helper/schema" @@ -72,6 +73,12 @@ func resourceJob() *schema.Resource { Type: schema.TypeString, }, + "hcl2": { + Description: "If true, the `jobspec` will be parsed as HCL2 instead of HCL.", + Optional: true, + Type: schema.TypeBool, + }, + "json": { Description: "If true, the `jobspec` will be parsed as json instead of HCL.", Optional: true, @@ -239,7 +246,11 @@ func resourceJobRegister(d *schema.ResourceData, meta interface{}) error { // Get the jobspec itself jobspecRaw := d.Get("jobspec").(string) is_json := d.Get("json").(bool) - job, err := parseJobspec(jobspecRaw, is_json, providerConfig.vaultToken) + parserHCL2 := d.Get("hcl2").(bool) + if is_json && parserHCL2 { + return fmt.Errorf("invalid combination. is_json is %t and parserHCL2 is %t", is_json, parserHCL2) + } + job, err := parseJobspec(jobspecRaw, is_json, parserHCL2, providerConfig.vaultToken) if err != nil { return err } @@ -504,7 +515,11 @@ func resourceJobCustomizeDiff(d *schema.ResourceDiff, meta interface{}) error { } is_json := d.Get("json").(bool) - job, err := parseJobspec(newSpecRaw.(string), is_json, providerConfig.vaultToken) // catch syntax errors client-side during plan + parserHCL2 := d.Get("hcl2").(bool) + if is_json && parserHCL2 { + return fmt.Errorf("invalid combination. is_json is %t and parserHCL2 is %t", is_json, parserHCL2) + } + job, err := parseJobspec(newSpecRaw.(string), is_json, parserHCL2, providerConfig.vaultToken) // catch syntax errors client-side during plan if err != nil { return err } @@ -579,15 +594,19 @@ func resourceJobCustomizeDiff(d *schema.ResourceDiff, meta interface{}) error { return nil } -func parseJobspec(raw string, is_json bool, vaultToken *string) (*api.Job, error) { +func parseJobspec(raw string, is_json, parserHCL2 bool, vaultToken *string) (*api.Job, error) { var job *api.Job var err error - if is_json { + switch { + case is_json: job, err = parseJSONJobspec(raw) - } else { + case parserHCL2: job, err = jobspec2.Parse(raw, strings.NewReader(raw)) + default: + job, err = jobspec.Parse(strings.NewReader(raw)) } + if err != nil { return nil, fmt.Errorf("error parsing jobspec: %s", err) } @@ -706,16 +725,38 @@ func jobTaskGroupsRaw(tgs []*api.TaskGroup) []interface{} { // jobspecDiffSuppress is the DiffSuppressFunc used by the schema to // check if two jobspecs are equal. func jobspecDiffSuppress(k, old, new string, d *schema.ResourceData) bool { - // TODO: does this need to consider is_json ??? - // Parse the old job - oldJob, err := jobspec2.Parse(old, strings.NewReader(old)) - if err != nil { + var oldJob *api.Job + var newJob *api.Job + var oldErr error + var newErr error + + is_json := d.Get("json").(bool) + parserHCL2 := d.Get("hcl2").(bool) + if is_json && parserHCL2 { + log.Printf("invalid combination. is_json is %t and parserHCL2 is %t\n", is_json, parserHCL2) return false } - - // Parse the new job - newJob, err := jobspec2.Parse(new, strings.NewReader(new)) - if err != nil { + switch { + case is_json: + oldJob, oldErr = parseJSONJobspec(old) + newJob, newErr = parseJSONJobspec(new) + case parserHCL2: + oldJob, oldErr = jobspec2.Parse(old, strings.NewReader(old)) + newJob, newErr = jobspec2.Parse(new, strings.NewReader(new)) + default: + oldJob, oldErr = jobspec.Parse(strings.NewReader(old)) + newJob, newErr = jobspec.Parse(strings.NewReader(new)) + } + if oldErr != nil { + log.Println("error parsing old jobspec") + log.Printf("%v\n", oldJob) + log.Printf("%v", oldErr) + return false + } + if newErr != nil { + log.Println("error parsing new jobspec") + log.Printf("%v\n", newJob) + log.Printf("%v", newErr) return false } From c13ee64bd76b50cbb6d6473b373d8944333419c4 Mon Sep 17 00:00:00 2001 From: Thiago Nache Carvalho Date: Wed, 13 Jan 2021 08:15:51 -0300 Subject: [PATCH 7/9] Add a test case for hcl2 parser --- nomad/resource_job_test.go | 74 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 74 insertions(+) diff --git a/nomad/resource_job_test.go b/nomad/resource_job_test.go index b17bc3c7..efcaf1dd 100644 --- a/nomad/resource_job_test.go +++ b/nomad/resource_job_test.go @@ -497,6 +497,47 @@ func testResourceJob_parameterizedCheck(s *terraform.State) error { return nil } +func TestResourceJob_hcl2(t *testing.T) { + r.Test(t, r.TestCase{ + Providers: testProviders, + PreCheck: func() { testAccPreCheck(t); testCheckMinVersion(t, "0.11.0-beta1") }, + Steps: []r.TestStep{ + { + Config: testResourceJob_hcl2, + Check: testResourceJob_hcl2Check, + }, + }, + CheckDestroy: testResourceJob_checkDestroy("foo-hcl2"), + }) +} + +func testResourceJob_hcl2Check(s *terraform.State) error { + resourceState := s.Modules[0].Resources["nomad_job.hcl2"] + if resourceState == nil { + return errors.New("resource not found in state") + } + + instanceState := resourceState.Primary + if instanceState == nil { + return errors.New("resource has no primary instance") + } + + jobID := instanceState.ID + + providerConfig := testProvider.Meta().(ProviderConfig) + client := providerConfig.client + job, _, err := client.Jobs().Info(jobID, nil) + if err != nil { + return fmt.Errorf("error reading back job: %s", err) + } + + if got, want := *job.ID, jobID; got != want { + return fmt.Errorf("jobID is %q; want %q", got, want) + } + + return nil +} + var testResourceJob_parameterizedJob = ` resource "nomad_job" "parameterized" { jobspec = < Date: Wed, 13 Jan 2021 09:07:35 -0300 Subject: [PATCH 8/9] Update website markdown --- website/docs/r/job.html.markdown | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/website/docs/r/job.html.markdown b/website/docs/r/job.html.markdown index bd3d08de..50ccc1e7 100644 --- a/website/docs/r/job.html.markdown +++ b/website/docs/r/job.html.markdown @@ -87,6 +87,18 @@ nomad job run -output my-job.nomad > my-job.json Or you can also use the [`/v1/jobs/parse`](https://www.nomadproject.io/api-docs/jobs/#parse-job) API endpoint. +## HCL2 jobspec + +The input jobspec can also be provided as HCL2 instead of HCL by setting the +argument `hcl2` to `true`: + +```hcl +resource "nomad_job" "app" { + jobspec = file("${path.module}/jobspec.json") + hcl2 = true +} +``` + ## Argument Reference The following arguments are supported: @@ -110,3 +122,5 @@ The following arguments are supported: - `json` `(boolean: false)` - Set this to true if your jobspec is structured with JSON instead of the default HCL. + +- `hcl2` `(boolean: false)` - Set this to true if your jobspec is structured with HCL2 instead of the default HCL. From cd427b13348eedd516561bc20374a11cdab9d5f9 Mon Sep 17 00:00:00 2001 From: Luiz Aoqui Date: Wed, 13 Jan 2021 15:52:13 -0500 Subject: [PATCH 9/9] docs: minor improvements --- website/docs/r/job.html.markdown | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/website/docs/r/job.html.markdown b/website/docs/r/job.html.markdown index 50ccc1e7..e5bfd14d 100644 --- a/website/docs/r/job.html.markdown +++ b/website/docs/r/job.html.markdown @@ -89,12 +89,12 @@ API endpoint. ## HCL2 jobspec -The input jobspec can also be provided as HCL2 instead of HCL by setting the -argument `hcl2` to `true`: +The input jobspec can also be provided in the [HCL2 format](https://www.nomadproject.io/docs/job-specification/hcl2) +by setting the argument `hcl2` to `true`: ```hcl resource "nomad_job" "app" { - jobspec = file("${path.module}/jobspec.json") + jobspec = file("${path.module}/jobspec.hcl") hcl2 = true } ``` @@ -123,4 +123,5 @@ The following arguments are supported: - `json` `(boolean: false)` - Set this to true if your jobspec is structured with JSON instead of the default HCL. -- `hcl2` `(boolean: false)` - Set this to true if your jobspec is structured with HCL2 instead of the default HCL. +- `hcl2` `(boolean: false)` - Set this to true if your jobspec uses the HCL2 + format instead of the default HCL.