-
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
r/aws_batch_job_definition: Suppress unnecessary environment variable diffs #21834
r/aws_batch_job_definition: Suppress unnecessary environment variable diffs #21834
Conversation
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.
Welcome @ma2gedev 👋
It looks like this is your first Pull Request submission to the Terraform AWS Provider! If you haven’t already done so please make sure you have checked out our CONTRIBUTING guide and FAQ to make sure your contribution is adhering to best practice and has all the necessary elements in place for a successful approval.
Also take a look at our FAQ which details how we prioritize Pull Requests for inclusion.
Thanks again, and welcome to the community! 😃
@justinretzolk Hey Justin. Any word on the triage for this issue? We're encountering it (just as we encountered the issue w/ ECS tasks in the past) and it creates a noisy diff each time something small in the job definition changes. |
Hey @rbreslow 👋 Thank you for checking in on this, and apologies for taking so long to get back to you. Unfortunately, I'm not able to provide an estimate on when this will be merged/reviewed due to the potential of shifting priorities (we prioritize work by count of ":+1:" reactions, as well as a few other things). For more information on how we prioritize, check out our prioritization guide. |
Is there anyway I could assist with issue @justinretzolk? It is impacting us and making Terraform plans very hard to read for actual changes vs env var ordering issues. |
Yeah, such a small diff, depressing hasn't been resolved and we're in 2024. |
Hey all 👋 Thanks for checking in on this one. I suspect it's been a bit tougher to get votes on this one, because I imagine a lot of people ignore the noisy diff and move on. We have dedicated days for trying to get through smaller PRs like this, and I've got this PR on that list to try to get it reviewed a bit quicker. |
This bug causes our team significant pain, any news on this being merged? |
Hello, Any news on this bug ? It's still an issue for us, it make reading plan a real pain :( |
Same here, the diffs are quite annoying. |
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.
LGTM 🚀.
% make testacc TESTARGS='-run=TestAccBatchJobDefinition_' PKG=batch ACCTEST_PARALLELISM=3
make: Verifying source code with gofmt...
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go1.23.2 test ./internal/service/batch/... -v -count 1 -parallel 3 -run=TestAccBatchJobDefinition_ -timeout 360m
=== RUN TestAccBatchJobDefinition_tags
=== PAUSE TestAccBatchJobDefinition_tags
=== RUN TestAccBatchJobDefinition_tags_null
=== PAUSE TestAccBatchJobDefinition_tags_null
=== RUN TestAccBatchJobDefinition_tags_EmptyMap
=== PAUSE TestAccBatchJobDefinition_tags_EmptyMap
=== RUN TestAccBatchJobDefinition_tags_AddOnUpdate
=== PAUSE TestAccBatchJobDefinition_tags_AddOnUpdate
=== RUN TestAccBatchJobDefinition_tags_EmptyTag_OnCreate
=== PAUSE TestAccBatchJobDefinition_tags_EmptyTag_OnCreate
=== RUN TestAccBatchJobDefinition_tags_EmptyTag_OnUpdate_Add
=== PAUSE TestAccBatchJobDefinition_tags_EmptyTag_OnUpdate_Add
=== RUN TestAccBatchJobDefinition_tags_EmptyTag_OnUpdate_Replace
=== PAUSE TestAccBatchJobDefinition_tags_EmptyTag_OnUpdate_Replace
=== RUN TestAccBatchJobDefinition_tags_DefaultTags_providerOnly
=== PAUSE TestAccBatchJobDefinition_tags_DefaultTags_providerOnly
=== RUN TestAccBatchJobDefinition_tags_DefaultTags_nonOverlapping
=== PAUSE TestAccBatchJobDefinition_tags_DefaultTags_nonOverlapping
=== RUN TestAccBatchJobDefinition_tags_DefaultTags_overlapping
=== PAUSE TestAccBatchJobDefinition_tags_DefaultTags_overlapping
=== RUN TestAccBatchJobDefinition_tags_DefaultTags_updateToProviderOnly
=== PAUSE TestAccBatchJobDefinition_tags_DefaultTags_updateToProviderOnly
=== RUN TestAccBatchJobDefinition_tags_DefaultTags_updateToResourceOnly
=== PAUSE TestAccBatchJobDefinition_tags_DefaultTags_updateToResourceOnly
=== RUN TestAccBatchJobDefinition_tags_DefaultTags_emptyResourceTag
=== PAUSE TestAccBatchJobDefinition_tags_DefaultTags_emptyResourceTag
=== RUN TestAccBatchJobDefinition_tags_DefaultTags_emptyProviderOnlyTag
=== PAUSE TestAccBatchJobDefinition_tags_DefaultTags_emptyProviderOnlyTag
=== RUN TestAccBatchJobDefinition_tags_DefaultTags_nullOverlappingResourceTag
=== PAUSE TestAccBatchJobDefinition_tags_DefaultTags_nullOverlappingResourceTag
=== RUN TestAccBatchJobDefinition_tags_DefaultTags_nullNonOverlappingResourceTag
=== PAUSE TestAccBatchJobDefinition_tags_DefaultTags_nullNonOverlappingResourceTag
=== RUN TestAccBatchJobDefinition_tags_ComputedTag_OnCreate
=== PAUSE TestAccBatchJobDefinition_tags_ComputedTag_OnCreate
=== RUN TestAccBatchJobDefinition_tags_ComputedTag_OnUpdate_Add
=== PAUSE TestAccBatchJobDefinition_tags_ComputedTag_OnUpdate_Add
=== RUN TestAccBatchJobDefinition_tags_ComputedTag_OnUpdate_Replace
=== PAUSE TestAccBatchJobDefinition_tags_ComputedTag_OnUpdate_Replace
=== RUN TestAccBatchJobDefinition_basic
=== PAUSE TestAccBatchJobDefinition_basic
=== RUN TestAccBatchJobDefinition_attributes
=== PAUSE TestAccBatchJobDefinition_attributes
=== RUN TestAccBatchJobDefinition_disappears
=== PAUSE TestAccBatchJobDefinition_disappears
=== RUN TestAccBatchJobDefinition_PlatformCapabilities_ec2
=== PAUSE TestAccBatchJobDefinition_PlatformCapabilities_ec2
=== RUN TestAccBatchJobDefinition_PlatformCapabilitiesFargate_containerPropertiesDefaults
=== PAUSE TestAccBatchJobDefinition_PlatformCapabilitiesFargate_containerPropertiesDefaults
=== RUN TestAccBatchJobDefinition_PlatformCapabilities_fargate
=== PAUSE TestAccBatchJobDefinition_PlatformCapabilities_fargate
=== RUN TestAccBatchJobDefinition_ContainerProperties_advanced
=== PAUSE TestAccBatchJobDefinition_ContainerProperties_advanced
=== RUN TestAccBatchJobDefinition_ContainerProperties_minorUpdate
=== PAUSE TestAccBatchJobDefinition_ContainerProperties_minorUpdate
=== RUN TestAccBatchJobDefinition_propagateTags
=== PAUSE TestAccBatchJobDefinition_propagateTags
=== RUN TestAccBatchJobDefinition_ContainerProperties_EmptyField
=== PAUSE TestAccBatchJobDefinition_ContainerProperties_EmptyField
=== RUN TestAccBatchJobDefinition_NodeProperties_basic
=== PAUSE TestAccBatchJobDefinition_NodeProperties_basic
=== RUN TestAccBatchJobDefinition_NodeProperties_advanced
=== PAUSE TestAccBatchJobDefinition_NodeProperties_advanced
=== RUN TestAccBatchJobDefinition_EKSProperties_basic
=== PAUSE TestAccBatchJobDefinition_EKSProperties_basic
=== RUN TestAccBatchJobDefinition_EKSProperties_update
=== PAUSE TestAccBatchJobDefinition_EKSProperties_update
=== RUN TestAccBatchJobDefinition_EKSProperties_imagePullSecrets
=== PAUSE TestAccBatchJobDefinition_EKSProperties_imagePullSecrets
=== RUN TestAccBatchJobDefinition_createTypeContainerWithNodeProperties
=== PAUSE TestAccBatchJobDefinition_createTypeContainerWithNodeProperties
=== RUN TestAccBatchJobDefinition_createTypeMultiNodeWithContainerProperties
=== PAUSE TestAccBatchJobDefinition_createTypeMultiNodeWithContainerProperties
=== RUN TestAccBatchJobDefinition_schedulingPriority
=== PAUSE TestAccBatchJobDefinition_schedulingPriority
=== RUN TestAccBatchJobDefinition_emptyRetryStrategy
=== PAUSE TestAccBatchJobDefinition_emptyRetryStrategy
=== RUN TestAccBatchJobDefinition_ECSProperties_update
=== PAUSE TestAccBatchJobDefinition_ECSProperties_update
=== CONT TestAccBatchJobDefinition_tags
=== CONT TestAccBatchJobDefinition_attributes
=== CONT TestAccBatchJobDefinition_NodeProperties_advanced
--- PASS: TestAccBatchJobDefinition_NodeProperties_advanced (28.90s)
=== CONT TestAccBatchJobDefinition_createTypeMultiNodeWithContainerProperties
--- PASS: TestAccBatchJobDefinition_createTypeMultiNodeWithContainerProperties (4.80s)
=== CONT TestAccBatchJobDefinition_ECSProperties_update
--- PASS: TestAccBatchJobDefinition_attributes (47.05s)
=== CONT TestAccBatchJobDefinition_emptyRetryStrategy
--- PASS: TestAccBatchJobDefinition_ECSProperties_update (25.85s)
=== CONT TestAccBatchJobDefinition_schedulingPriority
--- PASS: TestAccBatchJobDefinition_emptyRetryStrategy (15.64s)
=== CONT TestAccBatchJobDefinition_tags_DefaultTags_updateToProviderOnly
--- PASS: TestAccBatchJobDefinition_tags (63.86s)
=== CONT TestAccBatchJobDefinition_basic
--- PASS: TestAccBatchJobDefinition_schedulingPriority (16.68s)
=== CONT TestAccBatchJobDefinition_tags_ComputedTag_OnUpdate_Replace
--- PASS: TestAccBatchJobDefinition_basic (16.53s)
=== CONT TestAccBatchJobDefinition_tags_ComputedTag_OnUpdate_Add
--- PASS: TestAccBatchJobDefinition_tags_DefaultTags_updateToProviderOnly (29.64s)
=== CONT TestAccBatchJobDefinition_tags_ComputedTag_OnCreate
--- PASS: TestAccBatchJobDefinition_tags_ComputedTag_OnUpdate_Replace (36.96s)
=== CONT TestAccBatchJobDefinition_tags_DefaultTags_emptyResourceTag
--- PASS: TestAccBatchJobDefinition_tags_ComputedTag_OnUpdate_Add (36.53s)
=== CONT TestAccBatchJobDefinition_tags_DefaultTags_updateToResourceOnly
--- PASS: TestAccBatchJobDefinition_tags_ComputedTag_OnCreate (25.36s)
=== CONT TestAccBatchJobDefinition_tags_DefaultTags_nullNonOverlappingResourceTag
--- PASS: TestAccBatchJobDefinition_tags_DefaultTags_emptyResourceTag (18.37s)
=== CONT TestAccBatchJobDefinition_tags_EmptyTag_OnUpdate_Add
--- PASS: TestAccBatchJobDefinition_tags_DefaultTags_nullNonOverlappingResourceTag (18.24s)
=== CONT TestAccBatchJobDefinition_tags_DefaultTags_overlapping
--- PASS: TestAccBatchJobDefinition_tags_DefaultTags_updateToResourceOnly (27.62s)
=== CONT TestAccBatchJobDefinition_tags_DefaultTags_nonOverlapping
--- PASS: TestAccBatchJobDefinition_tags_EmptyTag_OnUpdate_Add (43.22s)
=== CONT TestAccBatchJobDefinition_tags_DefaultTags_providerOnly
--- PASS: TestAccBatchJobDefinition_tags_DefaultTags_overlapping (48.17s)
=== CONT TestAccBatchJobDefinition_tags_EmptyTag_OnUpdate_Replace
--- PASS: TestAccBatchJobDefinition_tags_DefaultTags_nonOverlapping (48.15s)
=== CONT TestAccBatchJobDefinition_ContainerProperties_advanced
--- PASS: TestAccBatchJobDefinition_tags_EmptyTag_OnUpdate_Replace (28.97s)
=== CONT TestAccBatchJobDefinition_NodeProperties_basic
=== CONT TestAccBatchJobDefinition_ContainerProperties_EmptyField
--- PASS: TestAccBatchJobDefinition_NodeProperties_basic (18.20s)
--- PASS: TestAccBatchJobDefinition_tags_DefaultTags_providerOnly (62.73s)
=== CONT TestAccBatchJobDefinition_propagateTags
--- PASS: TestAccBatchJobDefinition_ContainerProperties_EmptyField (16.53s)
=== CONT TestAccBatchJobDefinition_ContainerProperties_minorUpdate
--- PASS: TestAccBatchJobDefinition_propagateTags (13.10s)
=== CONT TestAccBatchJobDefinition_EKSProperties_imagePullSecrets
--- PASS: TestAccBatchJobDefinition_ContainerProperties_advanced (71.43s)
=== CONT TestAccBatchJobDefinition_tags_DefaultTags_emptyProviderOnlyTag
--- PASS: TestAccBatchJobDefinition_EKSProperties_imagePullSecrets (16.18s)
=== CONT TestAccBatchJobDefinition_createTypeContainerWithNodeProperties
--- PASS: TestAccBatchJobDefinition_createTypeContainerWithNodeProperties (4.65s)
=== CONT TestAccBatchJobDefinition_tags_DefaultTags_nullOverlappingResourceTag
--- PASS: TestAccBatchJobDefinition_tags_DefaultTags_emptyProviderOnlyTag (18.04s)
=== CONT TestAccBatchJobDefinition_PlatformCapabilitiesFargate_containerPropertiesDefaults
--- PASS: TestAccBatchJobDefinition_tags_DefaultTags_nullOverlappingResourceTag (18.31s)
=== CONT TestAccBatchJobDefinition_PlatformCapabilities_fargate
--- PASS: TestAccBatchJobDefinition_PlatformCapabilitiesFargate_containerPropertiesDefaults (17.27s)
=== CONT TestAccBatchJobDefinition_PlatformCapabilities_ec2
--- PASS: TestAccBatchJobDefinition_PlatformCapabilities_fargate (17.19s)
=== CONT TestAccBatchJobDefinition_EKSProperties_update
--- PASS: TestAccBatchJobDefinition_PlatformCapabilities_ec2 (16.23s)
=== CONT TestAccBatchJobDefinition_EKSProperties_basic
--- PASS: TestAccBatchJobDefinition_ContainerProperties_minorUpdate (78.00s)
=== CONT TestAccBatchJobDefinition_tags_AddOnUpdate
--- PASS: TestAccBatchJobDefinition_EKSProperties_basic (16.37s)
=== CONT TestAccBatchJobDefinition_disappears
--- PASS: TestAccBatchJobDefinition_EKSProperties_update (26.30s)
=== CONT TestAccBatchJobDefinition_tags_EmptyTag_OnCreate
--- PASS: TestAccBatchJobDefinition_disappears (14.13s)
=== CONT TestAccBatchJobDefinition_tags_EmptyMap
--- PASS: TestAccBatchJobDefinition_tags_AddOnUpdate (28.64s)
=== CONT TestAccBatchJobDefinition_tags_null
--- PASS: TestAccBatchJobDefinition_tags_EmptyTag_OnCreate (32.29s)
--- PASS: TestAccBatchJobDefinition_tags_EmptyMap (22.32s)
--- PASS: TestAccBatchJobDefinition_tags_null (22.13s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/batch 381.731s
@ma2gedev Thanks for the contribution 🎉 👏. |
@ewbankkit Thank you very much 🎉 |
thank you for getting this in, will make a big difference to our team |
Thanks, it’s really appreciated ! |
This functionality has been released in v5.72.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. Thank you! |
@ewbankkit I just tried v5.72.1, but it didn't suppress environment variable diffs. This is because your commit b710358 does not contain the changes (52f6bfa) to |
I'm going to lock this pull request because it has been closed for 30 days ⏳. This helps our maintainers find and focus on the active issues. |
Summary
This PR changes ordering AWS Batch job definition's environment variables when
terraform-provider-aws
reads job definition through AWS API and serializes to state. As the result, terraform's plan shows environment variables list without noisy diff.Background
AWS Batch job definition's environment variables are showed as diff even if there is no change about it, because AWS API returns environment variables list without keeping order.
For example if job definition's memory is changed,
terraform plan
shows environment variables diff despite it is not changed. Like the following:Actually same problem existed in AWS ECS task definition, but it was solved with #11463 . That PR's approach was ordering environment variables list, therefore I follow the way to solve problem to keep code consistency.
Community Note
Relates #11463
Closes #34947
Output from acceptance testing:
This change is passed unit tests, and worked in my project, but I could not do acceptance tests.