added support in batch for the allocation_strategy attribute #10894

sam-burrell · 2019-11-15T14:06:47Z

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" comments, they generate extra noise for pull request followers and do not help prioritize the request

Relates support new attribute

Release note for CHANGELOG:

Added support for the allocation_strategy attribute for aws_batch_compute_environment resource

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSBatchComputeEnvironment_createSpotWithAllocationStrategy'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./... -v -count 1 -parallel 20 -run=TestAccAWSBatchComputeEnvironment_createSpotWithAllocationStrategy -timeout 120m
?       github.com/terraform-providers/terraform-provider-aws   [no test files]
=== RUN   TestAccAWSBatchComputeEnvironment_createSpotWithAllocationStrategy
=== PAUSE TestAccAWSBatchComputeEnvironment_createSpotWithAllocationStrategy
=== CONT  TestAccAWSBatchComputeEnvironment_createSpotWithAllocationStrategy
--- PASS: TestAccAWSBatchComputeEnvironment_createSpotWithAllocationStrategy (77.92s)
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws       77.969s
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws/internal/flatmap      0.009s [no tests to run]
testing: warning: no tests to run
PASS
ok      github.com/terraform-providers/terraform-provider-aws/aws/internal/keyvaluetags 0.016s [no tests to run]

ryanwc · 2019-12-10T17:18:09Z

are there any updates on this? looks like it is almost good to go, just needs a review?

sam-burrell · 2019-12-10T17:27:47Z

We have been using this compiled fork in our production systems since I made this pull request. No problems so far.

bflad

Hi @sam-burrell 👋 Thank you for submitting this -- the implementation looks good from looking at the code changes, however we would prefer an additional acceptance test covering its usage to ensure it works as expected and to prevent future regressions with the functionality. See also the Contributing Guide section on enhancements. Please reach out if you have any questions or do not have time to implement the testing.

bflad · 2020-01-03T15:32:11Z

website/docs/r/batch_compute_environment.html.markdown

@@ -128,6 +128,7 @@ resource "aws_batch_compute_environment" "sample" {
 * `bid_percentage` - (Optional) Integer of minimum percentage that a Spot Instance price must be when compared with the On-Demand price for that instance type before instances are launched. For example, if your bid percentage is 20% (`20`), then the Spot price must be below 20% of the current On-Demand price for that EC2 instance. This parameter is required for SPOT compute environments.
 * `desired_vcpus` - (Optional) The desired number of EC2 vCPUS in the compute environment.
 * `ec2_key_pair` - (Optional) The EC2 key pair that is used for instances launched in the compute environment.
+* `allocation_strategy` - (Optional) The allocation strategy to use for the compute resource in case not enough instances of the best fitting instance type can be allocated. Valid items are `BEST_FIT_PROGRESSIVE`, `SPOT_CAPACITY_OPTIMIZED` or `BEST_FIT`. Defaults to `BEST_FIT`.


Nit: Can you please alphabetize this line item with the rest of the list? It might also be good to include a link to the Batch User Guide section on Allocation Strategies (https://docs.aws.amazon.com/batch/latest/userguide/allocation-strategies.html). Thanks.

many thanks for the review, I have done both of these steps hopefully good to go

aws/resource_aws_batch_compute_environment.go

…pute_environment/compute_resources

sam-burrell · 2020-01-03T17:16:50Z

@bflad thanks I have also rebased.

bflad

Looks great, @sam-burrell, thanks for the updates 🚀

Output from acceptance testing:

--- PASS: TestAccAWSBatchComputeEnvironment_createSpotWithoutBidPercentage (16.57s)
--- PASS: TestAccAWSBatchComputeEnvironment_createEc2WithoutComputeResources (16.91s)
--- PASS: TestAccAWSBatchComputeEnvironment_createWithNamePrefix (45.76s)
--- PASS: TestAccAWSBatchComputeEnvironment_createUnmanagedWithComputeResources (45.94s)
--- PASS: TestAccAWSBatchComputeEnvironment_createUnmanaged (51.64s)
--- PASS: TestAccAWSBatchComputeEnvironment_createEc2WithTags (54.95s)
--- PASS: TestAccAWSBatchComputeEnvironment_createSpot (55.43s)
--- PASS: TestAccAWSBatchComputeEnvironment_createEc2 (56.19s)
--- PASS: TestAccAWSBatchComputeEnvironment_updateState (57.01s)
--- PASS: TestAccAWSBatchComputeEnvironment_updateMaxvCpus (57.76s)
--- PASS: TestAccAWSBatchComputeEnvironment_launchTemplate (62.81s)
--- PASS: TestAccAWSBatchComputeEnvironment_createSpotWithAllocationStrategy (68.44s)
--- PASS: TestAccAWSBatchComputeEnvironment_updateInstanceType (77.81s)
--- PASS: TestAccAWSBatchComputeEnvironment_updateComputeEnvironmentName (79.77s)
--- PASS: TestAccAWSBatchComputeEnvironment_UpdateLaunchTemplate (88.16s)

ghost · 2020-01-10T17:02:16Z

This has been released in version 2.44.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 · 2020-03-27T17:50:30Z

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!

sam-burrell requested a review from a team November 15, 2019 14:06

ghost added needs-triage Waiting for first response or review from a maintainer. size/XS Managed by automation to categorize the size of a PR. service/batch Issues and PRs that pertain to the batch service. documentation Introduces or discusses updates to documentation. labels Nov 15, 2019

sam-burrell force-pushed the master branch from 72d06b7 to 40c3fb0 Compare November 15, 2019 14:41

bflad added enhancement Requests to existing resources that expand the functionality or scope. and removed needs-triage Waiting for first response or review from a maintainer. labels Nov 15, 2019

pgieser mentioned this pull request Nov 22, 2019

Support for allocation strategy attribute on AWS Batch compute environments #10980

Closed

bflad suggested changes Jan 3, 2020

View reviewed changes

bflad self-assigned this Jan 3, 2020

bflad added the waiting-response Maintainers are waiting on response from community or contributor. label Jan 3, 2020

ghost added tests PRs: expanded test coverage. Issues: expanded coverage, enhancements to test infrastructure. size/M Managed by automation to categorize the size of a PR. and removed size/XS Managed by automation to categorize the size of a PR. labels Jan 3, 2020

sam-burrell added 5 commits January 3, 2020 17:14

added support for the allocation_strategy attribute for aws_batch_com…

d24cf10

…pute_environment/compute_resources

added allocation_strategy to flattenBatchComputeResources

39ee0de

added validation

83adfde

added passing acceptance tests for allocation_strategy

f8d56a2

alphabetized line item and added link to awsvdocumentation

1a15595

sam-burrell force-pushed the master branch from b38ffdb to 1a15595 Compare January 3, 2020 17:15

ghost removed the waiting-response Maintainers are waiting on response from community or contributor. label Jan 3, 2020

sam-burrell requested a review from bflad January 3, 2020 17:17

rbreslow mentioned this pull request Jan 3, 2020

Add Batch Prediction Infrastructure raster-foundry/granary#68

Merged

bflad added this to the v2.44.0 milestone Jan 3, 2020

bflad approved these changes Jan 3, 2020

View reviewed changes

bflad merged commit 72dbbbb into hashicorp:master Jan 3, 2020

bflad added a commit that referenced this pull request Jan 3, 2020

Update CHANGELOG for #10894

0d0eea0

ghost locked and limited conversation to collaborators Mar 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

added support in batch for the allocation_strategy attribute #10894

added support in batch for the allocation_strategy attribute #10894

sam-burrell commented Nov 15, 2019 •

edited

Loading

ryanwc commented Dec 10, 2019

sam-burrell commented Dec 10, 2019

bflad left a comment

bflad Jan 3, 2020

sam-burrell Jan 3, 2020

sam-burrell commented Jan 3, 2020

bflad left a comment

ghost commented Jan 10, 2020

ghost commented Mar 27, 2020

added support in batch for the allocation_strategy attribute #10894

added support in batch for the allocation_strategy attribute #10894

Conversation

sam-burrell commented Nov 15, 2019 • edited Loading

Community Note

ryanwc commented Dec 10, 2019

sam-burrell commented Dec 10, 2019

bflad left a comment

Choose a reason for hiding this comment

bflad Jan 3, 2020

Choose a reason for hiding this comment

sam-burrell Jan 3, 2020

Choose a reason for hiding this comment

sam-burrell commented Jan 3, 2020

bflad left a comment

Choose a reason for hiding this comment

ghost commented Jan 10, 2020

ghost commented Mar 27, 2020

sam-burrell commented Nov 15, 2019 •

edited

Loading