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

Add platform capability to Batch job definition #16850

Merged
merged 14 commits into from
Apr 19, 2021

Conversation

gmazelier
Copy link
Contributor

@gmazelier gmazelier commented Dec 19, 2020

Community Note

  • Please vote on this pull request by adding a 👍 reaction to the original pull request comment to help the community and maintainers prioritize this request
  • Please do not leave "+1" or other comments that do not add relevant new information or questions, they generate extra noise for pull request followers and do not help prioritize the request

Relates #16592

Release note for CHANGELOG:

resource/aws_batch_job_definition: add platform capabilities support (#16850)

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSBatchJobDefinition_' ACCTEST_PARALLELISM=4   
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 4 -run=TestAccAWSBatchJobDefinition_ -timeout 120m
=== RUN   TestAccAWSBatchJobDefinition_basic
=== PAUSE TestAccAWSBatchJobDefinition_basic
=== RUN   TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2
=== PAUSE TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2
=== RUN   TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate
=== PAUSE TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate
=== RUN   TestAccAWSBatchJobDefinition_ContainerProperties_Advanced
=== PAUSE TestAccAWSBatchJobDefinition_ContainerProperties_Advanced
=== RUN   TestAccAWSBatchJobDefinition_updateForcesNewResource
=== PAUSE TestAccAWSBatchJobDefinition_updateForcesNewResource
=== RUN   TestAccAWSBatchJobDefinition_Tags
=== PAUSE TestAccAWSBatchJobDefinition_Tags
=== CONT  TestAccAWSBatchJobDefinition_basic
=== CONT  TestAccAWSBatchJobDefinition_updateForcesNewResource
=== CONT  TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate
=== CONT  TestAccAWSBatchJobDefinition_ContainerProperties_Advanced
--- PASS: TestAccAWSBatchJobDefinition_basic (43.89s)
=== CONT  TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2
--- PASS: TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate (45.65s)
=== CONT  TestAccAWSBatchJobDefinition_Tags
--- PASS: TestAccAWSBatchJobDefinition_ContainerProperties_Advanced (47.09s)
--- PASS: TestAccAWSBatchJobDefinition_updateForcesNewResource (70.40s)
--- PASS: TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2 (40.08s)
--- PASS: TestAccAWSBatchJobDefinition_Tags (86.78s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       134.167s

@gmazelier gmazelier requested a review from a team as a code owner December 19, 2020 18:55
@ghost ghost added size/M Managed by automation to categorize the size of a PR. service/batch Issues and PRs that pertain to the batch service. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. labels Dec 19, 2020
@gmazelier gmazelier marked this pull request as draft December 19, 2020 18:56
@github-actions github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Dec 19, 2020
@ghost ghost added size/XL Managed by automation to categorize the size of a PR. documentation Introduces or discusses updates to documentation. size/L Managed by automation to categorize the size of a PR. and removed size/M Managed by automation to categorize the size of a PR. size/XL Managed by automation to categorize the size of a PR. labels Dec 19, 2020
@gmazelier gmazelier force-pushed the r/batch_fargate_job_definition branch from b20dddd to 52c1c15 Compare December 20, 2020 11:35
@gmazelier gmazelier changed the title [WIP] Add platform capability to Batch job definition Add platform capability to Batch job definition Dec 20, 2020
@gmazelier gmazelier marked this pull request as ready for review December 20, 2020 17:22
@gmazelier gmazelier force-pushed the r/batch_fargate_job_definition branch from 52c1c15 to 533ea28 Compare January 6, 2021 10:01
aws/resource_aws_batch_job_definition.go Outdated Show resolved Hide resolved
@@ -61,6 +130,7 @@ The following arguments are supported:
* `container_properties` - (Optional) A valid [container properties](http://docs.aws.amazon.com/batch/latest/APIReference/API_RegisterJobDefinition.html)
provided as a single valid JSON document. This parameter is required if the `type` parameter is `container`.
* `parameters` - (Optional) Specifies the parameter substitution placeholders to set in the job definition.
* `platform_capability` - (Optional) The platform capabilities required by the job definition. If no value is specified, it defaults to `EC2`. Jobs run on Fargate resources specify `FARGATE`.
Copy link
Collaborator

Choose a reason for hiding this comment

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

according the the basic test check there is no value if one is not specified:

resource.TestCheckResourceAttr(resourceName, "platform_capability.#", "0"),
how does this conform with the above docs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's maybe unclear but the platformCapabilities is optional and if not specified defaults to EC2, that means job will be only able to run on an EC2 compute environment. This idea of the test check is to be sure that this do not introduce a new behaviour and modify existing job definitions.

See AWS documentation: https://docs.aws.amazon.com/batch/latest/userguide/job_definition_parameters.html#job-definition-parameters-platform-capabilities

@awsiv
Copy link
Contributor

awsiv commented Jan 6, 2021

Awesome! Looking forward too this

@dmccaffery
Copy link

Anything left to do to see this make its way into the next release? We're really looking forward to it. Happy to lend a hand if there is outstanding blockers.

Base automatically changed from master to main January 23, 2021 01:00
@mfoti
Copy link

mfoti commented Apr 3, 2021

any news on this?

@ewbankkit ewbankkit self-assigned this Apr 19, 2021
gmazelier and others added 4 commits April 19, 2021 09:26
…_capabilities'.

Acceptance test output:

% make testacc TEST=./aws TESTARGS='-run=TestAccAWSBatchJobDefinition_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSBatchJobDefinition_ -timeout 180m
=== RUN   TestAccAWSBatchJobDefinition_basic
=== PAUSE TestAccAWSBatchJobDefinition_basic
=== RUN   TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2
=== PAUSE TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2
=== RUN   TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate
=== PAUSE TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate
=== RUN   TestAccAWSBatchJobDefinition_ContainerProperties_Advanced
=== PAUSE TestAccAWSBatchJobDefinition_ContainerProperties_Advanced
=== RUN   TestAccAWSBatchJobDefinition_updateForcesNewResource
=== PAUSE TestAccAWSBatchJobDefinition_updateForcesNewResource
=== RUN   TestAccAWSBatchJobDefinition_Tags
=== PAUSE TestAccAWSBatchJobDefinition_Tags
=== CONT  TestAccAWSBatchJobDefinition_basic
=== CONT  TestAccAWSBatchJobDefinition_Tags
=== CONT  TestAccAWSBatchJobDefinition_updateForcesNewResource
=== CONT  TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate
=== CONT  TestAccAWSBatchJobDefinition_ContainerProperties_Advanced
=== CONT  TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2
--- PASS: TestAccAWSBatchJobDefinition_basic (15.37s)
--- PASS: TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2 (15.44s)
--- PASS: TestAccAWSBatchJobDefinition_ContainerProperties_Advanced (19.44s)
--- PASS: TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate (19.81s)
--- PASS: TestAccAWSBatchJobDefinition_updateForcesNewResource (24.59s)
--- PASS: TestAccAWSBatchJobDefinition_Tags (30.90s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	33.926s
@ewbankkit ewbankkit force-pushed the r/batch_fargate_job_definition branch from 60fb3aa to 5a65b23 Compare April 19, 2021 14:18
Acceptance test output:

% make testacc TEST=./aws TESTARGS='-run=TestAccAWSBatchJobDefinition_disappears'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSBatchJobDefinition_disappears -timeout 180m
=== RUN   TestAccAWSBatchJobDefinition_disappears
=== PAUSE TestAccAWSBatchJobDefinition_disappears
=== CONT  TestAccAWSBatchJobDefinition_disappears
--- PASS: TestAccAWSBatchJobDefinition_disappears (9.67s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	14.046s
Acceptance test output:

% make testacc TEST=./aws TESTARGS='-run=TestAccAWSBatchJobDefinition_basic'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSBatchJobDefinition_basic -timeout 180m
=== RUN   TestAccAWSBatchJobDefinition_basic
=== PAUSE TestAccAWSBatchJobDefinition_basic
=== CONT  TestAccAWSBatchJobDefinition_basic
--- PASS: TestAccAWSBatchJobDefinition_basic (12.62s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	15.829s
Acceptance test output:

% make testacc TEST=./aws TESTARGS='-run=TestAccAWSBatchJobDefinition_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSBatchJobDefinition_ -timeout 180m
=== RUN   TestAccAWSBatchJobDefinition_basic
=== PAUSE TestAccAWSBatchJobDefinition_basic
=== RUN   TestAccAWSBatchJobDefinition_disappears
=== PAUSE TestAccAWSBatchJobDefinition_disappears
=== RUN   TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2
=== PAUSE TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2
=== RUN   TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate
=== PAUSE TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate
=== RUN   TestAccAWSBatchJobDefinition_ContainerProperties_Advanced
=== PAUSE TestAccAWSBatchJobDefinition_ContainerProperties_Advanced
=== RUN   TestAccAWSBatchJobDefinition_updateForcesNewResource
=== PAUSE TestAccAWSBatchJobDefinition_updateForcesNewResource
=== RUN   TestAccAWSBatchJobDefinition_Tags
=== PAUSE TestAccAWSBatchJobDefinition_Tags
=== CONT  TestAccAWSBatchJobDefinition_basic
=== CONT  TestAccAWSBatchJobDefinition_ContainerProperties_Advanced
=== CONT  TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2
=== CONT  TestAccAWSBatchJobDefinition_disappears
=== CONT  TestAccAWSBatchJobDefinition_Tags
=== CONT  TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate
=== CONT  TestAccAWSBatchJobDefinition_updateForcesNewResource
--- PASS: TestAccAWSBatchJobDefinition_disappears (13.05s)
--- PASS: TestAccAWSBatchJobDefinition_ContainerProperties_Advanced (15.72s)
--- PASS: TestAccAWSBatchJobDefinition_basic (15.84s)
--- PASS: TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2 (16.82s)
--- PASS: TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate (18.69s)
--- PASS: TestAccAWSBatchJobDefinition_updateForcesNewResource (25.88s)
--- PASS: TestAccAWSBatchJobDefinition_Tags (31.97s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	35.037s
@ghost ghost added size/XL Managed by automation to categorize the size of a PR. and removed size/L Managed by automation to categorize the size of a PR. labels Apr 19, 2021
Copy link
Contributor

@ewbankkit ewbankkit left a comment

Choose a reason for hiding this comment

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

LGTM.

Commercial
% make testacc TEST=./aws TESTARGS='-run=TestAccAWSBatchJobDefinition_'                                                                     
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSBatchJobDefinition_ -timeout 180m
=== RUN   TestAccAWSBatchJobDefinition_basic
=== PAUSE TestAccAWSBatchJobDefinition_basic
=== RUN   TestAccAWSBatchJobDefinition_disappears
=== PAUSE TestAccAWSBatchJobDefinition_disappears
=== RUN   TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2
=== PAUSE TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2
=== RUN   TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate
=== PAUSE TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate
=== RUN   TestAccAWSBatchJobDefinition_ContainerProperties_Advanced
=== PAUSE TestAccAWSBatchJobDefinition_ContainerProperties_Advanced
=== RUN   TestAccAWSBatchJobDefinition_updateForcesNewResource
=== PAUSE TestAccAWSBatchJobDefinition_updateForcesNewResource
=== RUN   TestAccAWSBatchJobDefinition_Tags
=== PAUSE TestAccAWSBatchJobDefinition_Tags
=== CONT  TestAccAWSBatchJobDefinition_basic
=== CONT  TestAccAWSBatchJobDefinition_ContainerProperties_Advanced
=== CONT  TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2
=== CONT  TestAccAWSBatchJobDefinition_disappears
=== CONT  TestAccAWSBatchJobDefinition_Tags
=== CONT  TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate
=== CONT  TestAccAWSBatchJobDefinition_updateForcesNewResource
--- PASS: TestAccAWSBatchJobDefinition_disappears (13.05s)
--- PASS: TestAccAWSBatchJobDefinition_ContainerProperties_Advanced (15.72s)
--- PASS: TestAccAWSBatchJobDefinition_basic (15.84s)
--- PASS: TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2 (16.82s)
--- PASS: TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate (18.69s)
--- PASS: TestAccAWSBatchJobDefinition_updateForcesNewResource (25.88s)
--- PASS: TestAccAWSBatchJobDefinition_Tags (31.97s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	35.037s
GovCloud
% make testacc TEST=./aws TESTARGS='-run=TestAccAWSBatchJobDefinition_'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 20 -run=TestAccAWSBatchJobDefinition_ -timeout 180m
=== RUN   TestAccAWSBatchJobDefinition_basic
=== PAUSE TestAccAWSBatchJobDefinition_basic
=== RUN   TestAccAWSBatchJobDefinition_disappears
=== PAUSE TestAccAWSBatchJobDefinition_disappears
=== RUN   TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2
=== PAUSE TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2
=== RUN   TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate
=== PAUSE TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate
=== RUN   TestAccAWSBatchJobDefinition_ContainerProperties_Advanced
=== PAUSE TestAccAWSBatchJobDefinition_ContainerProperties_Advanced
=== RUN   TestAccAWSBatchJobDefinition_updateForcesNewResource
=== PAUSE TestAccAWSBatchJobDefinition_updateForcesNewResource
=== RUN   TestAccAWSBatchJobDefinition_Tags
=== PAUSE TestAccAWSBatchJobDefinition_Tags
=== CONT  TestAccAWSBatchJobDefinition_basic
=== CONT  TestAccAWSBatchJobDefinition_ContainerProperties_Advanced
=== CONT  TestAccAWSBatchJobDefinition_Tags
=== CONT  TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2
=== CONT  TestAccAWSBatchJobDefinition_disappears
=== CONT  TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate
=== CONT  TestAccAWSBatchJobDefinition_updateForcesNewResource
--- PASS: TestAccAWSBatchJobDefinition_disappears (19.36s)
--- PASS: TestAccAWSBatchJobDefinition_basic (19.73s)
--- PASS: TestAccAWSBatchJobDefinition_ContainerProperties_Advanced (19.86s)
--- PASS: TestAccAWSBatchJobDefinition_PlatformCapabilities_Fargate (23.62s)
--- PASS: TestAccAWSBatchJobDefinition_PlatformCapabilities_EC2 (23.80s)
--- PASS: TestAccAWSBatchJobDefinition_updateForcesNewResource (36.87s)
--- PASS: TestAccAWSBatchJobDefinition_Tags (47.88s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	50.807s

@ewbankkit
Copy link
Contributor

@gmazelier Thanks for the contribution 👏.
I took the liberty of making a few minor changes to get this one merged quickly.
The new attribute is now named platform_capabilities to match the AWS API and is Optional (and ForceNew).
I verified that if no value is specified on creation, no value is returned on read so we do not have to worry about backwards compatibility for existing configurations.

@gmazelier
Copy link
Contributor Author

@ewbankkit, excellent, thank you!

@ewbankkit ewbankkit merged commit b7c1c57 into hashicorp:main Apr 19, 2021
@github-actions github-actions bot added this to the v3.38.0 milestone Apr 19, 2021
@ewbankkit ewbankkit removed the needs-triage Waiting for first response or review from a maintainer. label Apr 19, 2021
@ghost
Copy link

ghost commented Apr 30, 2021

This has been released in version 3.38.0 of the Terraform AWS provider. Please see the Terraform documentation on provider versioning or reach out if you need any assistance upgrading.

For further feature requests or bug reports with this functionality, please create a new GitHub issue following the template for triage. Thanks!

@ghost
Copy link

ghost commented May 19, 2021

I'm going to lock this issue because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues.

If you feel this issue should be reopened, we encourage creating a new issue linking back to this one for added context. Thanks!

@ghost ghost locked as resolved and limited conversation to collaborators May 19, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Introduces or discusses updates to documentation. service/batch Issues and PRs that pertain to the batch service. size/XL Managed by automation to categorize the size of a PR. tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants