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

AWS Glue worker_type feature #9115

Merged
merged 5 commits into from
Nov 20, 2019
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
49 changes: 40 additions & 9 deletions aws/resource_aws_glue_job.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package aws
import (
"fmt"
"log"
"regexp"

"github.com/aws/aws-sdk-go/aws"
"github.com/aws/aws-sdk-go/service/glue"
Expand All @@ -25,7 +26,7 @@ func resourceAwsGlueJob() *schema.Resource {
Type: schema.TypeInt,
Optional: true,
Computed: true,
ConflictsWith: []string{"max_capacity"},
ConflictsWith: []string{"max_capacity", "number_of_workers", "worker_type"},
Deprecated: "Please use attribute `max_capacity' instead. This attribute might be removed in future releases.",
ValidateFunc: validation.IntAtLeast(2),
},
Expand Down Expand Up @@ -86,7 +87,7 @@ func resourceAwsGlueJob() *schema.Resource {
Type: schema.TypeFloat,
Optional: true,
Computed: true,
ConflictsWith: []string{"allocated_capacity"},
ConflictsWith: []string{"allocated_capacity", "number_of_workers", "worker_type"},
},
"max_retries": {
Type: schema.TypeInt,
Expand All @@ -113,6 +114,18 @@ func resourceAwsGlueJob() *schema.Resource {
Type: schema.TypeString,
Optional: true,
},
"worker_type": {
Type: schema.TypeString,
Optional: true,
ConflictsWith: []string{"allocated_capacity", "max_capacity", "allocated_capacity"},
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: allocated_capacity is declared twice

ValidateFunc: validation.StringMatch(regexp.MustCompile(`^Standard|G.1X|G.2X`), "Must be one of Standard, G.1X or G.2X. See https://docs.aws.amazon.com/glue/latest/dg/aws-glue-api-jobs-job.html"),
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: We typically prefer to use the validation.StringInSlice() functionality coupled with available AWS Go SDK constants for this type of validation, to limit the usage of regular expressions while also setting us up for future auto-generation opportunities, e.g.

Suggested change
ValidateFunc: validation.StringMatch(regexp.MustCompile(`^Standard|G.1X|G.2X`), "Must be one of Standard, G.1X or G.2X. See https://docs.aws.amazon.com/glue/latest/dg/aws-glue-api-jobs-job.html"),
ValidateFunc: validation.StringInSlice([]string{
glue.WorkerTypeG1x,
glue.WorkerTypeG2x,
glue.WorkerTypeStandard,
}, false),

Copy link
Contributor

@jValdron jValdron Nov 20, 2019

Choose a reason for hiding this comment

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

This is not formatted through make fmt. There is two spaces vetween ValidateFunc: and validation. It's currently making all PR tests fail.

Here's the diff after pulling the latest master and running make fmt:

diff --git a/aws/resource_aws_glue_job.go b/aws/resource_aws_glue_job.go
index 19468747d..4e2aa2945 100644
--- a/aws/resource_aws_glue_job.go
+++ b/aws/resource_aws_glue_job.go
@@ -122,7 +122,7 @@ func resourceAwsGlueJob() *schema.Resource {
                                Type:          schema.TypeString,
                                Optional:      true,
                                ConflictsWith: []string{"allocated_capacity", "max_capacity"},
-                               ValidateFunc:  validation.StringInSlice([]string{
+                               ValidateFunc: validation.StringInSlice([]string{
                                        glue.WorkerTypeG1x,
                                        glue.WorkerTypeG2x,
                                        glue.WorkerTypeStandard,

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I ran go fmt and not make fmt from the root per the CONTRIBUTING.md's Enhancement/Bugfix to a Resource -> Well-formed Code section. I can run make fmt and push to the PR.

Copy link
Contributor

@jValdron jValdron Nov 20, 2019

Choose a reason for hiding this comment

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

No problem, fixed by 4e449b0, master is now passing

Copy link
Contributor Author

Choose a reason for hiding this comment

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

thank you, sorry for an inconvenience!

},
"number_of_workers": {
Type: schema.TypeInt,
Optional: true,
ConflictsWith: []string{"allocated_capacity", "max_capacity", "allocated_capacity"},
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: allocated_capacity is declared twice

ValidateFunc: validation.IntAtLeast(2),
},
},
}
}
Expand Down Expand Up @@ -167,6 +180,14 @@ func resourceAwsGlueJobCreate(d *schema.ResourceData, meta interface{}) error {
input.SecurityConfiguration = aws.String(v.(string))
}

if v, ok := d.GetOk("worker_type"); ok {
input.WorkerType = aws.String(v.(string))
}

if v, ok := d.GetOk("number_of_workers"); ok {
input.NumberOfWorkers = aws.Int64(int64(v.(int)))
}

log.Printf("[DEBUG] Creating Glue Job: %s", input)
_, err := conn.CreateJob(input)
if err != nil {
Expand Down Expand Up @@ -225,6 +246,9 @@ func resourceAwsGlueJobRead(d *schema.ResourceData, meta interface{}) error {
return fmt.Errorf("error setting security_configuration: %s", err)
}

d.Set("worker_type", job.WorkerType)
d.Set("number_of_workers", int(aws.Int64Value(job.NumberOfWorkers)))

// TODO: Deprecated fields - remove in next major version
d.Set("allocated_capacity", int(aws.Int64Value(job.AllocatedCapacity)))

Expand All @@ -240,13 +264,16 @@ func resourceAwsGlueJobUpdate(d *schema.ResourceData, meta interface{}) error {
Timeout: aws.Int64(int64(d.Get("timeout").(int))),
}

if v, ok := d.GetOk("max_capacity"); ok {
jobUpdate.MaxCapacity = aws.Float64(v.(float64))
}

if d.HasChange("allocated_capacity") {
jobUpdate.MaxCapacity = aws.Float64(float64(d.Get("allocated_capacity").(int)))
log.Printf("[WARN] Using deprecated `allocated_capacity' attribute.")
if v, ok := d.GetOk("number_of_workers"); ok {
jobUpdate.NumberOfWorkers = aws.Int64(int64(v.(int)))
} else {
if v, ok := d.GetOk("max_capacity"); ok {
jobUpdate.MaxCapacity = aws.Float64(v.(float64))
}
if d.HasChange("allocated_capacity") {
jobUpdate.MaxCapacity = aws.Float64(float64(d.Get("allocated_capacity").(int)))
log.Printf("[WARN] Using deprecated `allocated_capacity' attribute.")
}
}

if v, ok := d.GetOk("connections"); ok {
Expand Down Expand Up @@ -279,6 +306,10 @@ func resourceAwsGlueJobUpdate(d *schema.ResourceData, meta interface{}) error {
jobUpdate.SecurityConfiguration = aws.String(v.(string))
}

if v, ok := d.GetOk("worker_type"); ok {
jobUpdate.WorkerType = aws.String(v.(string))
}

input := &glue.UpdateJobInput{
JobName: aws.String(d.Id()),
JobUpdate: jobUpdate,
Expand Down
60 changes: 60 additions & 0 deletions aws/resource_aws_glue_job_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,47 @@ func TestAccAWSGlueJob_SecurityConfiguration(t *testing.T) {
})
}

func TestAccAWSGlueJob_WorkerType(t *testing.T) {
var job glue.Job

rName := fmt.Sprintf("tf-acc-test-%s", acctest.RandString(5))
resourceName := "aws_glue_job.test"

resource.ParallelTest(t, resource.TestCase{
PreCheck: func() { testAccPreCheck(t) },
Providers: testAccProviders,
CheckDestroy: testAccCheckAWSGlueJobDestroy,
Steps: []resource.TestStep{
{
Config: testAccAWSGlueJobConfig_WorkerType(rName, "Standard"),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSGlueJobExists(resourceName, &job),
resource.TestCheckResourceAttr(resourceName, "worker_type", "Standard"),
),
},
{
Config: testAccAWSGlueJobConfig_WorkerType(rName, "G.1X"),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSGlueJobExists(resourceName, &job),
resource.TestCheckResourceAttr(resourceName, "worker_type", "G.1X"),
),
},
{
Config: testAccAWSGlueJobConfig_WorkerType(rName, "G.2X"),
Check: resource.ComposeTestCheckFunc(
testAccCheckAWSGlueJobExists(resourceName, &job),
resource.TestCheckResourceAttr(resourceName, "worker_type", "G.2X"),
),
},
{
ResourceName: resourceName,
ImportState: true,
ImportStateVerify: true,
},
},
})
}

func TestAccAWSGlueJob_PythonShell(t *testing.T) {
var job glue.Job

Expand Down Expand Up @@ -742,6 +783,25 @@ resource "aws_glue_job" "test" {
`, testAccAWSGlueJobConfig_Base(rName), rName, securityConfiguration)
}

func testAccAWSGlueJobConfig_WorkerType(rName string, workerType string) string {
return fmt.Sprintf(`
%s

resource "aws_glue_job" "test" {
name = "%s"
role_arn = "${aws_iam_role.test.arn}"
worker_type = "%s"
number_of_workers = 10

command {
script_location = "testscriptlocation"
}

depends_on = ["aws_iam_role_policy_attachment.test"]
}
`, testAccAWSGlueJobConfig_Base(rName), rName, workerType)
}

func testAccAWSGlueJobConfig_PythonShell(rName string) string {
return fmt.Sprintf(`
%s
Expand Down
2 changes: 2 additions & 0 deletions website/docs/r/glue_job.html.markdown
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,8 @@ be removed in future releases, please use `max_capacity` instead.
* `role_arn` – (Required) The ARN of the IAM role associated with this job.
* `timeout` – (Optional) The job timeout in minutes. The default is 2880 minutes (48 hours).
* `security_configuration` - (Optional) The name of the Security Configuration to be associated with the job.
* `worker_type` - (Optional) The type of predefined worker that is allocated when a job runs. Accepts a value of Standard, G.1X, or G.2X.
* `number_of_workers` - (Optional) The number of workers of a defined workerType that are allocated when a job runs.

### command Argument Reference

Expand Down