-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
f/aws_mq_broker: Allow custom timeouts for common operations #27035
f/aws_mq_broker: Allow custom timeouts for common operations #27035
Conversation
Community NoteVoting for Prioritization
For Submitters
|
a8c1d97
to
ff5e8ba
Compare
For peripheral AWS regions it is common that AmazonMQ RabbitMQ, for instance, takes more than 30min for a create or even a broker update when a cluster requires a full reboot. Since there is already support in the code, the API is just not exposed, this is mostly about allowing terraform code to configure the currently static timeouts. Nothing else changes, the defaults continue to be 30m, so it is backwards compatible.
ff5e8ba
to
1d98f02
Compare
Acceptance test output: % make testacc TESTARGS='-run=TestAccMQBroker_basic\|TestAccMQBroker_disappears' PKG=mq ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/mq/... -v -count 1 -parallel 2 -run=TestAccMQBroker_basic\|TestAccMQBroker_disappears -timeout 180m === RUN TestAccMQBroker_basic === PAUSE TestAccMQBroker_basic === RUN TestAccMQBroker_disappears === PAUSE TestAccMQBroker_disappears === CONT TestAccMQBroker_basic === CONT TestAccMQBroker_disappears --- PASS: TestAccMQBroker_disappears (1103.82s) --- PASS: TestAccMQBroker_basic (1106.80s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/mq 1110.900s
…_tags'. Acceptance test output: % make testacc TESTARGS='-run=TestAccMQBroker_basic\|TestAccMQBroker_tags' PKG=mq ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/mq/... -v -count 1 -parallel 2 -run=TestAccMQBroker_basic\|TestAccMQBroker_tags -timeout 180m === RUN TestAccMQBroker_basic === PAUSE TestAccMQBroker_basic === RUN TestAccMQBroker_tags === PAUSE TestAccMQBroker_tags === CONT TestAccMQBroker_basic === CONT TestAccMQBroker_tags --- PASS: TestAccMQBroker_basic (1044.14s) --- PASS: TestAccMQBroker_tags (1083.50s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/mq 1087.952s
Acceptance test output: % make testacc TESTARGS='-run=TestAccMQBroker_' PKG=mq ACCTEST_PARALLELISM=2 ==> Checking that code complies with gofmt requirements... TF_ACC=1 go test ./internal/service/mq/... -v -count 1 -parallel 2 -run=TestAccMQBroker_ -timeout 180m === RUN TestAccMQBroker_basic === PAUSE TestAccMQBroker_basic === RUN TestAccMQBroker_disappears === PAUSE TestAccMQBroker_disappears === RUN TestAccMQBroker_tags === PAUSE TestAccMQBroker_tags === RUN TestAccMQBroker_throughputOptimized === PAUSE TestAccMQBroker_throughputOptimized === RUN TestAccMQBroker_AllFields_defaultVPC === PAUSE TestAccMQBroker_AllFields_defaultVPC === RUN TestAccMQBroker_AllFields_customVPC === PAUSE TestAccMQBroker_AllFields_customVPC === RUN TestAccMQBroker_EncryptionOptions_kmsKeyID === PAUSE TestAccMQBroker_EncryptionOptions_kmsKeyID === RUN TestAccMQBroker_EncryptionOptions_managedKeyDisabled === PAUSE TestAccMQBroker_EncryptionOptions_managedKeyDisabled === RUN TestAccMQBroker_EncryptionOptions_managedKeyEnabled === PAUSE TestAccMQBroker_EncryptionOptions_managedKeyEnabled === RUN TestAccMQBroker_Update_users === PAUSE TestAccMQBroker_Update_users === RUN TestAccMQBroker_Update_securityGroup === PAUSE TestAccMQBroker_Update_securityGroup === RUN TestAccMQBroker_Update_engineVersion === PAUSE TestAccMQBroker_Update_engineVersion === RUN TestAccMQBroker_Update_hostInstanceType === PAUSE TestAccMQBroker_Update_hostInstanceType === RUN TestAccMQBroker_RabbitMQ_basic === PAUSE TestAccMQBroker_RabbitMQ_basic === RUN TestAccMQBroker_RabbitMQ_logs === PAUSE TestAccMQBroker_RabbitMQ_logs === RUN TestAccMQBroker_RabbitMQ_validationAuditLog === PAUSE TestAccMQBroker_RabbitMQ_validationAuditLog === RUN TestAccMQBroker_RabbitMQ_cluster === PAUSE TestAccMQBroker_RabbitMQ_cluster === RUN TestAccMQBroker_ldap === PAUSE TestAccMQBroker_ldap === CONT TestAccMQBroker_basic === CONT TestAccMQBroker_Update_users --- PASS: TestAccMQBroker_basic (1073.77s) === CONT TestAccMQBroker_RabbitMQ_logs --- PASS: TestAccMQBroker_Update_users (1548.15s) === CONT TestAccMQBroker_ldap --- PASS: TestAccMQBroker_RabbitMQ_logs (661.31s) === CONT TestAccMQBroker_RabbitMQ_cluster --- PASS: TestAccMQBroker_RabbitMQ_cluster (875.34s) === CONT TestAccMQBroker_RabbitMQ_validationAuditLog --- PASS: TestAccMQBroker_ldap (1081.11s) === CONT TestAccMQBroker_AllFields_customVPC --- PASS: TestAccMQBroker_AllFields_customVPC (1908.95s) === CONT TestAccMQBroker_EncryptionOptions_managedKeyEnabled --- PASS: TestAccMQBroker_RabbitMQ_validationAuditLog (856.59s) === CONT TestAccMQBroker_EncryptionOptions_managedKeyDisabled --- PASS: TestAccMQBroker_EncryptionOptions_managedKeyEnabled (1100.57s) === CONT TestAccMQBroker_EncryptionOptions_kmsKeyID --- PASS: TestAccMQBroker_EncryptionOptions_managedKeyDisabled (1039.41s) === CONT TestAccMQBroker_Update_hostInstanceType --- PASS: TestAccMQBroker_EncryptionOptions_kmsKeyID (1094.46s) === CONT TestAccMQBroker_RabbitMQ_basic --- PASS: TestAccMQBroker_RabbitMQ_basic (787.21s) === CONT TestAccMQBroker_throughputOptimized --- PASS: TestAccMQBroker_Update_hostInstanceType (1591.05s) === CONT TestAccMQBroker_AllFields_defaultVPC --- PASS: TestAccMQBroker_throughputOptimized (856.89s) === CONT TestAccMQBroker_tags --- PASS: TestAccMQBroker_tags (1120.47s) === CONT TestAccMQBroker_disappears --- PASS: TestAccMQBroker_AllFields_defaultVPC (1778.68s) === CONT TestAccMQBroker_Update_engineVersion --- PASS: TestAccMQBroker_disappears (1047.85s) === CONT TestAccMQBroker_Update_securityGroup --- PASS: TestAccMQBroker_Update_engineVersion (1425.14s) --- PASS: TestAccMQBroker_Update_securityGroup (1472.88s) PASS ok github.com/hashicorp/terraform-provider-aws/internal/service/mq 10752.440s
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=TestAccMQBroker_' PKG=mq ACCTEST_PARALLELISM=2
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./internal/service/mq/... -v -count 1 -parallel 2 -run=TestAccMQBroker_ -timeout 180m
=== RUN TestAccMQBroker_basic
=== PAUSE TestAccMQBroker_basic
=== RUN TestAccMQBroker_disappears
=== PAUSE TestAccMQBroker_disappears
=== RUN TestAccMQBroker_tags
=== PAUSE TestAccMQBroker_tags
=== RUN TestAccMQBroker_throughputOptimized
=== PAUSE TestAccMQBroker_throughputOptimized
=== RUN TestAccMQBroker_AllFields_defaultVPC
=== PAUSE TestAccMQBroker_AllFields_defaultVPC
=== RUN TestAccMQBroker_AllFields_customVPC
=== PAUSE TestAccMQBroker_AllFields_customVPC
=== RUN TestAccMQBroker_EncryptionOptions_kmsKeyID
=== PAUSE TestAccMQBroker_EncryptionOptions_kmsKeyID
=== RUN TestAccMQBroker_EncryptionOptions_managedKeyDisabled
=== PAUSE TestAccMQBroker_EncryptionOptions_managedKeyDisabled
=== RUN TestAccMQBroker_EncryptionOptions_managedKeyEnabled
=== PAUSE TestAccMQBroker_EncryptionOptions_managedKeyEnabled
=== RUN TestAccMQBroker_Update_users
=== PAUSE TestAccMQBroker_Update_users
=== RUN TestAccMQBroker_Update_securityGroup
=== PAUSE TestAccMQBroker_Update_securityGroup
=== RUN TestAccMQBroker_Update_engineVersion
=== PAUSE TestAccMQBroker_Update_engineVersion
=== RUN TestAccMQBroker_Update_hostInstanceType
=== PAUSE TestAccMQBroker_Update_hostInstanceType
=== RUN TestAccMQBroker_RabbitMQ_basic
=== PAUSE TestAccMQBroker_RabbitMQ_basic
=== RUN TestAccMQBroker_RabbitMQ_logs
=== PAUSE TestAccMQBroker_RabbitMQ_logs
=== RUN TestAccMQBroker_RabbitMQ_validationAuditLog
=== PAUSE TestAccMQBroker_RabbitMQ_validationAuditLog
=== RUN TestAccMQBroker_RabbitMQ_cluster
=== PAUSE TestAccMQBroker_RabbitMQ_cluster
=== RUN TestAccMQBroker_ldap
=== PAUSE TestAccMQBroker_ldap
=== CONT TestAccMQBroker_basic
=== CONT TestAccMQBroker_Update_users
--- PASS: TestAccMQBroker_basic (1073.77s)
=== CONT TestAccMQBroker_RabbitMQ_logs
--- PASS: TestAccMQBroker_Update_users (1548.15s)
=== CONT TestAccMQBroker_ldap
--- PASS: TestAccMQBroker_RabbitMQ_logs (661.31s)
=== CONT TestAccMQBroker_RabbitMQ_cluster
--- PASS: TestAccMQBroker_RabbitMQ_cluster (875.34s)
=== CONT TestAccMQBroker_RabbitMQ_validationAuditLog
--- PASS: TestAccMQBroker_ldap (1081.11s)
=== CONT TestAccMQBroker_AllFields_customVPC
--- PASS: TestAccMQBroker_AllFields_customVPC (1908.95s)
=== CONT TestAccMQBroker_EncryptionOptions_managedKeyEnabled
--- PASS: TestAccMQBroker_RabbitMQ_validationAuditLog (856.59s)
=== CONT TestAccMQBroker_EncryptionOptions_managedKeyDisabled
--- PASS: TestAccMQBroker_EncryptionOptions_managedKeyEnabled (1100.57s)
=== CONT TestAccMQBroker_EncryptionOptions_kmsKeyID
--- PASS: TestAccMQBroker_EncryptionOptions_managedKeyDisabled (1039.41s)
=== CONT TestAccMQBroker_Update_hostInstanceType
--- PASS: TestAccMQBroker_EncryptionOptions_kmsKeyID (1094.46s)
=== CONT TestAccMQBroker_RabbitMQ_basic
--- PASS: TestAccMQBroker_RabbitMQ_basic (787.21s)
=== CONT TestAccMQBroker_throughputOptimized
--- PASS: TestAccMQBroker_Update_hostInstanceType (1591.05s)
=== CONT TestAccMQBroker_AllFields_defaultVPC
--- PASS: TestAccMQBroker_throughputOptimized (856.89s)
=== CONT TestAccMQBroker_tags
--- PASS: TestAccMQBroker_tags (1120.47s)
=== CONT TestAccMQBroker_disappears
--- PASS: TestAccMQBroker_AllFields_defaultVPC (1778.68s)
=== CONT TestAccMQBroker_Update_engineVersion
--- PASS: TestAccMQBroker_disappears (1047.85s)
=== CONT TestAccMQBroker_Update_securityGroup
--- PASS: TestAccMQBroker_Update_engineVersion (1425.14s)
--- PASS: TestAccMQBroker_Update_securityGroup (1472.88s)
PASS
ok github.com/hashicorp/terraform-provider-aws/internal/service/mq 10752.440s
@marceloboeira Thanks for the contribution 🎉 👏. |
@ewbankkit thanks for the review and fixes! Let me know if there is anything else to be done from my side to get all checks to green. |
This functionality has been released in v4.36.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! |
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. |
Description
For peripheral AWS regions it is commong that AmazonMQ RabbitMQ for instance, takes more than 30min for a create or even a broker update when a cluster requires a full reboot.
Since there is already support in the code, just the API is not exposed, this is mostly about allowing terraform code to configure the currently static timeouts
Nothing else changes, the defaults continue to be 30m, so it is backwards compatible.
Testing
Building the provider locally with timeouts works now, before it would error saying it was invalid to setup that: