-
Notifications
You must be signed in to change notification settings - Fork 9.2k
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
service/batch: Support resource tagging and prevent differences with new secrets support in container properties #15470
Conversation
…new secrets support in container properties Reference: #12997 Reference: #15469 Reference: https://aws.amazon.com/about-aws/whats-new/2020/10/aws-batch-now-supports-custom-logging-configurations-swap-space-and-shared-memory/ Changes: ``` * data-source/aws_batch_compute_environment: Add `tags` attribute * data-source/aws_batch_job_queue: Add `tags` attribute * resource/aws_batch_compute_environment: Add `tags` argument * resource/aws_batch_job_definition: Add `tags` argument * resource/aws_batch_job_queue: Add `tags` argument BUG FIXES * resource/aws_batch_job_definition: Prevent unexpected plan difference for `container_properties` argument value with new secrets support ``` With the recent Batch service updates, the new secrets support in container properties would show as a confusing plan difference: ``` ~ container_properties = jsonencode( ~ { command = [ "echo", "test", ] - environment = [] -> null image = "busybox" memory = 128 - mountPoints = [] -> null - resourceRequirements = [] -> null - secrets = [] -> null - ulimits = [] -> null vcpus = 1 - volumes = [] -> null } ) ``` This augments the Batch equivalency to ensure the canonicalized API response of an empty secrets array matches no secrets configuration. While verifying the updates, this fixes the following two AWS GovCloud (US) test failures due to hardcoded partition handling: ``` === CONT TestAccDataSourceAwsBatchJobQueue_basic data_source_aws_batch_job_queue_test.go:16: Step 1/1 error: terraform failed: exit status 1 stderr: Error: Provider produced inconsistent result after apply When applying changes to aws_iam_role_policy_attachment.ecs_instance_role, provider "registry.terraform.io/-/aws" produced an unexpected new value for was present, but now absent. This is a bug in the provider, which should be reported in the provider's own issue tracker. Error: Provider produced inconsistent result after apply When applying changes to aws_iam_role_policy_attachment.aws_batch_service_role, provider "registry.terraform.io/-/aws" produced an unexpected new value for was present, but now absent. This is a bug in the provider, which should be reported in the provider's own issue tracker. === CONT TestAccDataSourceAwsBatchComputeEnvironment_basic data_source_aws_batch_compute_environment_test.go:16: Step 1/1 error: terraform failed: exit status 1 stderr: Error: Provider produced inconsistent result after apply When applying changes to aws_iam_role_policy_attachment.ecs_instance_role, provider "registry.terraform.io/-/aws" produced an unexpected new value for was present, but now absent. This is a bug in the provider, which should be reported in the provider's own issue tracker. Error: Provider produced inconsistent result after apply When applying changes to aws_iam_role_policy_attachment.aws_batch_service_role, provider "registry.terraform.io/-/aws" produced an unexpected new value for was present, but now absent. This is a bug in the provider, which should be reported in the provider's own issue tracker. ``` Output from acceptance testing in AWS Commercial: ``` --- PASS: TestAccAWSBatchComputeEnvironment_ComputeResources_DesiredVcpus_Computed (224.88s) --- PASS: TestAccAWSBatchComputeEnvironment_ComputeResources_MaxVcpus (105.82s) --- PASS: TestAccAWSBatchComputeEnvironment_ComputeResources_MinVcpus (213.34s) --- PASS: TestAccAWSBatchComputeEnvironment_createEc2 (51.12s) --- PASS: TestAccAWSBatchComputeEnvironment_createEc2WithoutComputeResources (32.16s) --- PASS: TestAccAWSBatchComputeEnvironment_createEc2WithTags (46.93s) --- PASS: TestAccAWSBatchComputeEnvironment_createSpot (54.38s) --- PASS: TestAccAWSBatchComputeEnvironment_createSpotWithAllocationStrategy (54.66s) --- PASS: TestAccAWSBatchComputeEnvironment_createSpotWithoutBidPercentage (22.83s) --- PASS: TestAccAWSBatchComputeEnvironment_createUnmanaged (46.86s) --- PASS: TestAccAWSBatchComputeEnvironment_createUnmanagedWithComputeResources (54.73s) --- PASS: TestAccAWSBatchComputeEnvironment_createWithNamePrefix (54.19s) --- PASS: TestAccAWSBatchComputeEnvironment_disappears (49.79s) --- PASS: TestAccAWSBatchComputeEnvironment_launchTemplate (62.07s) --- PASS: TestAccAWSBatchComputeEnvironment_Tags (107.51s) --- PASS: TestAccAWSBatchComputeEnvironment_updateComputeEnvironmentName (97.15s) --- PASS: TestAccAWSBatchComputeEnvironment_updateInstanceType (96.17s) --- PASS: TestAccAWSBatchComputeEnvironment_UpdateLaunchTemplate (110.14s) --- PASS: TestAccAWSBatchComputeEnvironment_updateState (92.78s) --- PASS: TestAccAWSBatchJobDefinition_basic (31.33s) --- PASS: TestAccAWSBatchJobDefinition_ContainerProperties_Advanced (30.82s) --- PASS: TestAccAWSBatchJobDefinition_Tags (51.78s) --- PASS: TestAccAWSBatchJobDefinition_updateForcesNewResource (36.03s) --- PASS: TestAccAWSBatchJobQueue_basic (142.93s) --- PASS: TestAccAWSBatchJobQueue_ComputeEnvironments_ExternalOrderUpdate (146.15s) --- PASS: TestAccAWSBatchJobQueue_disappears (159.82s) --- PASS: TestAccAWSBatchJobQueue_Priority (182.59s) --- PASS: TestAccAWSBatchJobQueue_State (146.39s) --- PASS: TestAccAWSBatchJobQueue_Tags (210.84s) --- PASS: TestAccDataSourceAwsBatchComputeEnvironment_basic (58.52s) --- PASS: TestAccDataSourceAwsBatchJobQueue_basic (145.37s) ``` Output from acceptance testing in AWS GovCloud (US): ``` --- PASS: TestAccAWSBatchComputeEnvironment_ComputeResources_DesiredVcpus_Computed (246.11s) --- PASS: TestAccAWSBatchComputeEnvironment_ComputeResources_MaxVcpus (143.57s) --- PASS: TestAccAWSBatchComputeEnvironment_ComputeResources_MinVcpus (271.07s) --- PASS: TestAccAWSBatchComputeEnvironment_createEc2 (52.91s) --- PASS: TestAccAWSBatchComputeEnvironment_createEc2WithoutComputeResources (24.92s) --- PASS: TestAccAWSBatchComputeEnvironment_createEc2WithTags (58.91s) --- PASS: TestAccAWSBatchComputeEnvironment_createSpot (62.24s) --- PASS: TestAccAWSBatchComputeEnvironment_createSpotWithAllocationStrategy (57.05s) --- PASS: TestAccAWSBatchComputeEnvironment_createSpotWithoutBidPercentage (22.58s) --- PASS: TestAccAWSBatchComputeEnvironment_createUnmanaged (51.34s) --- PASS: TestAccAWSBatchComputeEnvironment_createUnmanagedWithComputeResources (64.74s) --- PASS: TestAccAWSBatchComputeEnvironment_createWithNamePrefix (51.71s) --- PASS: TestAccAWSBatchComputeEnvironment_disappears (60.41s) --- PASS: TestAccAWSBatchComputeEnvironment_launchTemplate (77.36s) --- PASS: TestAccAWSBatchComputeEnvironment_Tags (121.87s) --- PASS: TestAccAWSBatchComputeEnvironment_updateComputeEnvironmentName (108.52s) --- PASS: TestAccAWSBatchComputeEnvironment_updateInstanceType (125.32s) --- PASS: TestAccAWSBatchComputeEnvironment_UpdateLaunchTemplate (110.03s) --- PASS: TestAccAWSBatchComputeEnvironment_updateState (91.09s) --- PASS: TestAccAWSBatchJobDefinition_basic (24.22s) --- PASS: TestAccAWSBatchJobDefinition_ContainerProperties_Advanced (24.59s) --- PASS: TestAccAWSBatchJobDefinition_Tags (67.28s) --- PASS: TestAccAWSBatchJobDefinition_updateForcesNewResource (36.09s) --- PASS: TestAccAWSBatchJobQueue_basic (150.57s) --- PASS: TestAccAWSBatchJobQueue_ComputeEnvironments_ExternalOrderUpdate (160.87s) --- PASS: TestAccAWSBatchJobQueue_disappears (139.57s) --- PASS: TestAccAWSBatchJobQueue_Priority (155.30s) --- PASS: TestAccAWSBatchJobQueue_State (182.97s) --- PASS: TestAccAWSBatchJobQueue_Tags (159.11s) --- PASS: TestAccDataSourceAwsBatchComputeEnvironment_basic (54.23s) --- PASS: TestAccDataSourceAwsBatchJobQueue_basic (139.32s) ```
Please note that the bug fix within this should be merged before version 3.10.0 is released, otherwise the updated AWS Go SDK will cause a large amount of configurations to show a new plan difference. Can split the pull request if necessary. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One comment, otherwise LGTM 🚀
--- PASS: TestAccAWSBatchComputeEnvironment_createSpotWithoutBidPercentage (36.88s)
--- PASS: TestAccAWSBatchComputeEnvironment_createEc2WithoutComputeResources (42.20s)
--- PASS: TestAccAWSBatchJobDefinition_basic (23.13s)
--- PASS: TestAccAWSBatchComputeEnvironment_createWithNamePrefix (77.91s)
--- PASS: TestAccAWSBatchComputeEnvironment_createUnmanaged (84.68s)
--- PASS: TestAccDataSourceAwsBatchComputeEnvironment_basic (91.64s)
--- PASS: TestAccAWSBatchComputeEnvironment_createEc2 (107.45s)
--- PASS: TestAccAWSBatchComputeEnvironment_createEc2WithTags (108.66s)
--- PASS: TestAccAWSBatchComputeEnvironment_launchTemplate (111.10s)
--- PASS: TestAccAWSBatchComputeEnvironment_createUnmanagedWithComputeResources (117.18s)
--- PASS: TestAccAWSBatchJobDefinition_ContainerProperties_Advanced (55.45s)
--- PASS: TestAccAWSBatchComputeEnvironment_createSpot (131.93s)
--- PASS: TestAccAWSBatchComputeEnvironment_disappears (141.42s)
--- PASS: TestAccAWSBatchComputeEnvironment_createSpotWithAllocationStrategy (141.50s)
--- PASS: TestAccAWSBatchComputeEnvironment_updateState (149.73s)
--- PASS: TestAccAWSBatchComputeEnvironment_UpdateLaunchTemplate (149.67s)
--- PASS: TestAccAWSBatchJobDefinition_updateForcesNewResource (73.34s)
--- PASS: TestAccAWSBatchComputeEnvironment_updateInstanceType (154.41s)
--- PASS: TestAccDataSourceAwsBatchJobQueue_basic (155.26s)
--- PASS: TestAccAWSBatchComputeEnvironment_updateComputeEnvironmentName (161.27s)
--- PASS: TestAccAWSBatchComputeEnvironment_Tags (129.23s)
--- PASS: TestAccAWSBatchJobDefinition_Tags (82.33s)
--- PASS: TestAccAWSBatchComputeEnvironment_ComputeResources_MaxVcpus (176.01s)
--- PASS: TestAccAWSBatchJobQueue_basic (152.89s)
--- PASS: TestAccAWSBatchJobQueue_Priority (138.27s)
--- PASS: TestAccAWSBatchJobQueue_ComputeEnvironments_ExternalOrderUpdate (143.87s)
--- PASS: TestAccAWSBatchJobQueue_disappears (148.98s)
--- PASS: TestAccAWSBatchComputeEnvironment_ComputeResources_MinVcpus (269.04s)
--- PASS: TestAccAWSBatchJobQueue_State (161.25s)
--- PASS: TestAccAWSBatchJobQueue_Tags (163.05s)
--- PASS: TestAccAWSBatchComputeEnvironment_ComputeResources_DesiredVcpus_Computed (293.34s)
testAccDataSourceAwsBatchJobQueueCheck(datasourceName, resourceName), | ||
resource.TestCheckResourceAttrPair(datasourceName, "arn", resourceName, "arn"), | ||
resource.TestCheckResourceAttrPair(datasourceName, "compute_environment_order.#", resourceName, "compute_environments.#"), | ||
resource.TestCheckResourceAttrPair(datasourceName, "name", resourceName, "name"), | ||
resource.TestCheckResourceAttrPair(datasourceName, "priority", resourceName, "priority"), | ||
resource.TestCheckResourceAttrPair(datasourceName, "state", resourceName, "state"), | ||
resource.TestCheckResourceAttrPair(datasourceName, "tags.%", resourceName, "tags.%"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
testAccDataSourceAwsBatchComputeEnvironmentCheck(datasourceName, resourceName), | ||
resource.TestCheckResourceAttrPair(datasourceName, "arn", resourceName, "arn"), | ||
resource.TestCheckResourceAttrPair(datasourceName, "compute_environment_name", resourceName, "compute_environment_name"), | ||
resource.TestCheckResourceAttrPair(datasourceName, "ecs_cluster_arn", resourceName, "ecs_cluster_arn"), | ||
resource.TestCheckResourceAttrPair(datasourceName, "service_role", resourceName, "service_role"), | ||
resource.TestCheckResourceAttrPair(datasourceName, "state", resourceName, "state"), | ||
resource.TestCheckResourceAttrPair(datasourceName, "tags.%", resourceName, "tags.%"), | ||
resource.TestCheckResourceAttrPair(datasourceName, "type", resourceName, "type"), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
Co-authored-by: Graham Davison <[email protected]>
This has been released in version 3.10.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! |
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! |
Community Note
Closes #15469
Reference: #12997
Reference: https://aws.amazon.com/about-aws/whats-new/2020/10/aws-batch-now-supports-custom-logging-configurations-swap-space-and-shared-memory/
Release note for CHANGELOG:
With the recent Batch service updates, the new secrets support in container properties would show as a confusing plan difference:
This augments the Batch equivalency to ensure the canonicalized API response of an empty secrets array matches no secrets configuration.
While verifying the updates, this fixes the following two AWS GovCloud (US) test failures due to hardcoded partition handling:
Output from acceptance testing in AWS Commercial:
Output from acceptance testing in AWS GovCloud (US):