r/mq_broker - allow updating engine version by DrFaust92 · Pull Request #12758 · hashicorp/terraform-provider-aws

DrFaust92 · 2020-04-10T11:19:40Z

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

Release note for CHANGELOG:

resouce_aws_mq_broker - allow updating engine version

Output from acceptance testing:

$ make testacc TESTARGS='-run=TestAccAWSMqBroker_updateEngineVersion'
--- PASS: TestAccAWSMqBroker_updateEngineVersion (1394.18s)

ewbankkit · 2020-04-10T16:08:49Z

Verified acceptance tests:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSMqBroker_updateEngineVersion'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAWSMqBroker_updateEngineVersion -timeout 120m
=== RUN   TestAccAWSMqBroker_updateEngineVersion
=== PAUSE TestAccAWSMqBroker_updateEngineVersion
=== CONT  TestAccAWSMqBroker_updateEngineVersion
--- PASS: TestAccAWSMqBroker_updateEngineVersion (1515.99s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1516.030s

ewbankkit · 2020-05-16T21:08:23Z

Verified acceptance test:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSMqBroker_updateEngineVersion'
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws/ -v -count 1 -parallel 20 -run=TestAccAWSMqBroker_updateEngineVersion -timeout 120m
=== RUN   TestAccAWSMqBroker_updateEngineVersion
=== PAUSE TestAccAWSMqBroker_updateEngineVersion
=== CONT  TestAccAWSMqBroker_updateEngineVersion
--- PASS: TestAccAWSMqBroker_updateEngineVersion (1466.44s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	1466.464s

DrFaust92 · 2020-07-10T15:15:08Z

testing refactor + rebase, test re-run:

--- PASS: TestAccAWSMqBroker_allFieldsDefaultVpc (1941.61s)
--- PASS: TestAccAWSMqBroker_allFieldsCustomVpc (2069.22s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_KmsKeyId (1153.99s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Disabled (1159.08s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Enabled (1149.53s)
--- PASS: TestAccAWSMqBroker_updateUsers (1638.19s)
--- PASS: TestAccAWSMqBroker_tags (1195.53s)
--- PASS: TestAccAWSMqBroker_updateSecurityGroup (1420.80s)
--- PASS: TestAccAWSMqBroker_disappears (1209.47s)
--- PASS: TestAccAWSMqBroker_updateEngineVersion (1425.13s)

szymonpk · 2020-10-02T10:20:27Z

Any chance we can have this merged?

Matiasmct · 2021-02-02T13:45:15Z

Any chance we can have this merged?

YakDriver · 2021-03-05T23:58:07Z

@DrFaust92 This looks like a very helpful PR. Now that some rework with #16108 is merged, if you have a chance to rebase, it would be great to also get this fix in.

DrFaust92 · 2021-03-06T07:02:34Z

Thanks @YakDriver rebased and reran tests:

--- PASS: TestAccAWSMqBroker_rabbitmq (541.35s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Disabled (1124.17s)
--- PASS: TestAccAWSMqBroker_basic (1155.99s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_KmsKeyId (1157.90s)
--- PASS: TestAccAWSMqBroker_disappears (1171.16s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Enabled (1200.44s)
--- PASS: TestAccAWSMqBroker_tags (1267.72s)
--- PASS: TestAccAWSMqBroker_updateSecurityGroup (1402.44s)
--- PASS: TestAccAWSMqBroker_updateEngineVersion (1409.84s)
--- PASS: TestAccAWSMqBroker_updateUsers (1594.24s)
--- PASS: TestAccAWSMqBroker_allFieldsDefaultVpc (1772.55s)
--- PASS: TestAccAWSMqBroker_allFieldsCustomVpc (1846.14s)

ewbankkit · 2021-03-07T00:23:53Z

Verified acceptance tests:

$ make testacc TEST=./aws/ TESTARGS='-run=TestAccAWSMqBroker_' ACCTEST_PARALLELISM=2
==> Checking that code complies with gofmt requirements...
TF_ACC=1 go test ./aws -v -count 1 -parallel 2 -run=TestAccAWSMqBroker_ -timeout 120m
=== RUN   TestAccAWSMqBroker_basic
=== PAUSE TestAccAWSMqBroker_basic
=== RUN   TestAccAWSMqBroker_allFieldsDefaultVpc
=== PAUSE TestAccAWSMqBroker_allFieldsDefaultVpc
=== RUN   TestAccAWSMqBroker_allFieldsCustomVpc
=== PAUSE TestAccAWSMqBroker_allFieldsCustomVpc
=== RUN   TestAccAWSMqBroker_EncryptionOptions_KmsKeyId
=== PAUSE TestAccAWSMqBroker_EncryptionOptions_KmsKeyId
=== RUN   TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Disabled
=== PAUSE TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Disabled
=== RUN   TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Enabled
=== PAUSE TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Enabled
=== RUN   TestAccAWSMqBroker_updateUsers
=== PAUSE TestAccAWSMqBroker_updateUsers
=== RUN   TestAccAWSMqBroker_tags
=== PAUSE TestAccAWSMqBroker_tags
=== RUN   TestAccAWSMqBroker_updateSecurityGroup
=== PAUSE TestAccAWSMqBroker_updateSecurityGroup
=== RUN   TestAccAWSMqBroker_updateEngineVersion
=== PAUSE TestAccAWSMqBroker_updateEngineVersion
=== RUN   TestAccAWSMqBroker_disappears
=== PAUSE TestAccAWSMqBroker_disappears
=== RUN   TestAccAWSMqBroker_rabbitmq
=== PAUSE TestAccAWSMqBroker_rabbitmq
=== CONT  TestAccAWSMqBroker_basic
=== CONT  TestAccAWSMqBroker_tags
=== CONT  TestAccAWSMqBroker_rabbitmq
--- PASS: TestAccAWSMqBroker_basic (1115.06s)
--- PASS: TestAccAWSMqBroker_tags (1178.00s)
=== CONT  TestAccAWSMqBroker_disappears
--- PASS: TestAccAWSMqBroker_rabbitmq (539.40s)
=== CONT  TestAccAWSMqBroker_updateEngineVersion
--- PASS: TestAccAWSMqBroker_disappears (1114.89s)
=== CONT  TestAccAWSMqBroker_updateSecurityGroup
--- PASS: TestAccAWSMqBroker_updateEngineVersion (1287.01s)
=== CONT  TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Disabled
--- PASS: TestAccAWSMqBroker_updateSecurityGroup (1344.09s)
=== CONT  TestAccAWSMqBroker_updateUsers
--- PASS: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Disabled (1075.88s)
=== CONT  TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Enabled
--- PASS: TestAccAWSMqBroker_updateUsers (1464.31s)
=== CONT  TestAccAWSMqBroker_allFieldsCustomVpc
--- PASS: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Enabled (1167.66s)
=== CONT  TestAccAWSMqBroker_EncryptionOptions_KmsKeyId
--- PASS: TestAccAWSMqBroker_EncryptionOptions_KmsKeyId (1149.59s)
=== CONT  TestAccAWSMqBroker_allFieldsDefaultVpc
--- PASS: TestAccAWSMqBroker_allFieldsDefaultVpc (1561.53s)
--- PASS: TestAccAWSMqBroker_allFieldsCustomVpc (1782.41s)
PASS
ok  	github.com/terraform-providers/terraform-provider-aws/aws	6883.803s

YakDriver · 2021-03-08T22:43:36Z

Ut-oh. Looks like a bug in our waiting-response logic. It didn't remove the label when you commented 3 days ago. 🤔

Anyway, thanks for working on this!

Co-authored-by: Kit Ewbank <Kit_Ewbank@hotmail.com>

YakDriver

Looks great! 🎉

Acceptance tests on commercial (us-west-2):

--- PASS: TestAccAWSMqBroker_rabbitmq (516.10s)
--- PASS: TestAccAWSMqBroker_throughputOptimized (1015.89s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_KmsKeyId (1100.35s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Disabled (1102.55s)
--- PASS: TestAccAWSMqBroker_tags (1120.97s)
--- PASS: TestAccAWSMqBroker_basic (1131.82s)
--- PASS: TestAccAWSMqBroker_disappears (1138.39s)
--- PASS: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Enabled (1146.32s)
--- PASS: TestAccAWSMqBroker_updateSecurityGroup (1340.00s)
--- PASS: TestAccAWSMqBroker_updateEngineVersion (1340.28s)
--- PASS: TestAccAWSMqBroker_updateUsers (1507.62s)
--- PASS: TestAccAWSMqBroker_allFieldsDefaultVpc (1797.68s)
--- PASS: TestAccAWSMqBroker_allFieldsCustomVpc (1818.05s)

On GovCloud (service not available):

--- SKIP: TestAccAWSMqBroker_updateUsers (1.23s)
--- SKIP: TestAccAWSMqBroker_rabbitmq (1.23s)
--- SKIP: TestAccAWSMqBroker_basic (1.23s)
--- SKIP: TestAccAWSMqBroker_allFieldsCustomVpc (1.23s)
--- SKIP: TestAccAWSMqBroker_disappears (1.23s)
--- SKIP: TestAccAWSMqBroker_updateEngineVersion (1.23s)
--- SKIP: TestAccAWSMqBroker_tags (1.23s)
--- SKIP: TestAccAWSMqBroker_updateSecurityGroup (1.23s)
--- SKIP: TestAccAWSMqBroker_throughputOptimized (1.23s)
--- SKIP: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Enabled (1.23s)
--- SKIP: TestAccAWSMqBroker_allFieldsDefaultVpc (1.23s)
--- SKIP: TestAccAWSMqBroker_EncryptionOptions_KmsKeyId (1.23s)
--- SKIP: TestAccAWSMqBroker_EncryptionOptions_UseAwsOwnedKey_Disabled (1.23s)

YakDriver · 2021-03-10T20:01:24Z

 				Check: resource.ComposeTestCheckFunc(
 					testAccCheckAwsMqBrokerExists(resourceName, &broker),
-					testAccCheckAwsMqBrokerDisappears(&broker),
+					testAccCheckResourceDisappears(testAccProvider, resourceAwsMqBroker(), resourceName),


YakDriver · 2021-03-10T20:01:59Z

 		m := make(map[string]interface{})
 		if instance.ConsoleURL != nil {
-			m["console_url"] = *instance.ConsoleURL
+			m["console_url"] = aws.StringValue(instance.ConsoleURL)


YakDriver · 2021-03-10T20:02:21Z

 		input.MaintenanceWindowStartTime = expandMqWeeklyStartTime(v.([]interface{}))
 	}
 	if v, ok := d.GetOk("security_groups"); ok && v.(*schema.Set).Len() > 0 {
-		input.SecurityGroups = expandStringSet(d.Get("security_groups").(*schema.Set))


Good catch!

ghost · 2021-03-12T01:02:19Z

This has been released in version 3.32.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 · 2021-04-10T17:10:12Z

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!

DrFaust92 requested a review from a team April 10, 2020 11:19

DrFaust92 mentioned this pull request Apr 10, 2020

Changing aws_mq_broker engine_version should not force resource replacement #11827

Closed

DrFaust92 force-pushed the r/mq_broker_engine_version_update branch from 7ceae87 to 65671ec Compare May 15, 2020 20:49

ghost added size/M Managed by automation to categorize the size of a PR. and removed size/S Managed by automation to categorize the size of a PR. labels May 15, 2020

DrFaust92 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 Jul 9, 2020

DrFaust92 force-pushed the r/mq_broker_engine_version_update branch from 918fa2e to 3de5c5f Compare July 9, 2020 21:30

ghost added size/XL Managed by automation to categorize the size of a PR. and removed size/M Managed by automation to categorize the size of a PR. labels Jul 10, 2020

DrFaust92 force-pushed the r/mq_broker_engine_version_update branch from 9c1fcc8 to af3ba33 Compare July 10, 2020 08:25

anGie44 force-pushed the master branch from 54db70d to 48ba2f7 Compare July 21, 2020 18:27

DrFaust92 force-pushed the r/mq_broker_engine_version_update branch from 8e42172 to dfd53d7 Compare October 3, 2020 10:04

Base automatically changed from master to main January 23, 2021 00:57

breathingdust requested a review from a team as a code owner January 23, 2021 00:57

YakDriver self-assigned this Mar 5, 2021

YakDriver added the waiting-response Maintainers are waiting on response from community or contributor. label Mar 5, 2021

YakDriver mentioned this pull request Mar 6, 2021

resource/aws_mq_broker: Add RabbitMQ engine type #16108

Merged

DrFaust92 force-pushed the r/mq_broker_engine_version_update branch from b5a73f4 to 995f0fe Compare March 6, 2021 06:57

ewbankkit reviewed Mar 6, 2021

View reviewed changes

Comment thread .changelog/12758.txt Outdated

YakDriver removed the waiting-response Maintainers are waiting on response from community or contributor. label Mar 8, 2021

YakDriver force-pushed the r/mq_broker_engine_version_update branch from 58100d5 to 120fa81 Compare March 10, 2021 19:33

DrFaust92 and others added 13 commits March 10, 2021 14:38

engine version is updatable

a7ad1fb

rebase + refactor disappears

aa4c18a

fix tests

e21dc02

refactor for std names

e83b724

refactor more names + tags tests to std

d9117a2

remvoe hashstring func

fc0b663

rebase

7e744a9

re-add update test

ceeb6db

minor changes

d812e83

fix disappears

a50bd18

changelog

2e734e1

Update .changelog/12758.txt

538af49

Co-authored-by: Kit Ewbank <Kit_Ewbank@hotmail.com>

r/mq_broker: Update test for standard names

253c647

YakDriver force-pushed the r/mq_broker_engine_version_update branch from 120fa81 to 253c647 Compare March 10, 2021 19:47

tests/mq_broker: Add partition service precheck

774c75f

YakDriver approved these changes Mar 10, 2021

View reviewed changes

YakDriver added this to the v3.32.0 milestone Mar 10, 2021

YakDriver merged commit 19b732b into hashicorp:main Mar 10, 2021

ghost locked as resolved and limited conversation to collaborators Apr 10, 2021

DrFaust92 deleted the r/mq_broker_engine_version_update branch April 15, 2021 10:47

Conversation

DrFaust92 commented Apr 10, 2020

Community Note

Uh oh!

ewbankkit commented Apr 10, 2020

Uh oh!

ewbankkit commented May 16, 2020

Uh oh!

DrFaust92 commented Jul 10, 2020

Uh oh!

szymonpk commented Oct 2, 2020

Uh oh!

Matiasmct commented Feb 2, 2021

Uh oh!

YakDriver commented Mar 5, 2021

Uh oh!

DrFaust92 commented Mar 6, 2021

Uh oh!

Uh oh!

ewbankkit commented Mar 7, 2021

Uh oh!

YakDriver commented Mar 8, 2021

Uh oh!

YakDriver left a comment

Choose a reason for hiding this comment

Uh oh!

YakDriver Mar 10, 2021

Choose a reason for hiding this comment

Uh oh!

YakDriver Mar 10, 2021

Choose a reason for hiding this comment

Uh oh!

YakDriver Mar 10, 2021

Choose a reason for hiding this comment

Uh oh!

ghost commented Mar 12, 2021

Uh oh!

ghost commented Apr 10, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants