resource/aws_emr_cluster Add support for subnet_ids attribute #17219

kristerjaanhold · 2021-01-21T19:02: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" 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:

Add support for subnet_ids attribute for aws_emr_cluster.

Output from acceptance testing:

$ make testacc TESTARGS='-run=testAccAWSEmrClusterConfigMultipleSubnets'

...

github-actions

Welcome @kristerjaanhold 👋

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! 😃

kasparg · 2021-04-01T11:47:43Z

Hey @gdavison 👋 We're wondering if there's something we can do to bump this up? Checks are passing, code is there, this would be really useful to many people 🙂

gdavison

Thanks for the PR, @kristerjaanhold.

The new acceptance test step is currently failing, showing a plan difference. To run the specific test, you can run

$ make testacc TESTARGS='-run=TestAccAWSEMRCluster_basic'

To run all of the EMR Cluster tests, run

$ make testacc TESTARGS='-run=TestAccAWSEMRCluster_'

I've also added a number of changes to consider.

gdavison · 2021-02-26T01:51:25Z

aws/resource_aws_emr_cluster_test.go

+  applications  = ["Spark"]
+
+  ec2_attributes {
+    subnet_ids                        = [aws_subnet.test.id]


This test should have multiple subnets

gdavison · 2021-04-01T23:19:07Z

aws/resource_aws_emr_cluster_test.go

@@ -98,6 +98,17 @@ func TestAccAWSEMRCluster_basic(t *testing.T) {
 					resource.TestCheckResourceAttr(resourceName, "bootstrap_action.#", "0"),
 				),
 			},
+			{


This should have an Import test after the first step, as well as after, to ensure that terraform import will work in both cases

gdavison · 2021-04-01T23:20:30Z

aws/resource_aws_emr_cluster_test.go

+			{
+				Config: testAccAWSEmrClusterConfigMultipleSubnets(rName),
+				Check: resource.ComposeTestCheckFunc(
+					testAccCheckAWSEmrClusterExists(resourceName, &cluster),


For completeness, this step should include a call to testAccCheckAWSEmrClusterRecreated(), since both subnet_id and subnet_ids are ForceNew

gdavison · 2021-04-01T23:45:23Z

aws/resource_aws_emr_cluster_test.go

+					resource.TestCheckResourceAttr(resourceName, "step.#", "0"),
+					resource.TestCheckResourceAttrSet(resourceName, "arn"),
+					resource.TestCheckNoResourceAttr(resourceName, "additional_info"),
+					resource.TestCheckResourceAttr(resourceName, "bootstrap_action.#", "0"),


This should test that ec2_attributes.0. subnet_ids has the correct number of values and that the correct values are set and that ec2_attributes.0. subnet_id is not set. TypeSet values can be checked using resource.TestCheckTypeSetElemAttrPair.

In addition, in the first test step, the test should confirm that ec2_attributes.0. subnet_id is set to the correct value and that ec2_attributes.0. subnet_ids has zero elements.

gdavison · 2021-04-09T17:43:56Z

Hi @kristerjaanhold. We've had a lot of requests for this PR, so we'd like to get it merged in. If you'd like to continue working on it, let me know. Otherwise, I'll pick it up from here and make the needed changes, probably Monday, 12 April, Pacific time (UTC-7). I'll make sure that your contributions are still credited 🙂

kristerjaanhold · 2021-04-12T07:38:09Z

Hi @gdavison, feel free to pick it up from here. Thanks!

gdavison

Acceptance test results

--- PASS: TestAccAWSEMRCluster_disappears (836.36s)
--- PASS: TestAccAWSEMRCluster_configurationsJson (943.43s)
--- PASS: TestAccAWSEMRCluster_security_config (948.97s)
--- PASS: TestAccAWSEMRCluster_basic (987.39s)
--- PASS: TestAccAWSEMRCluster_Step_Basic (1009.17s)
--- PASS: TestAccAWSEMRCluster_InstanceFleet_master_only (1020.61s)
--- PASS: TestAccAWSEMRCluster_custom_ami_id (1055.85s)
--- PASS: TestAccAWSEMRCluster_Kerberos_ClusterDedicatedKdc (1071.61s)
--- PASS: TestAccAWSEMRCluster_CoreInstanceGroup_AutoscalingPolicy (1119.10s)
--- PASS: TestAccAWSEMRCluster_CoreInstanceGroup_InstanceCount (1157.16s)
--- PASS: TestAccAWSEMRCluster_MasterInstanceGroup_InstanceType (1200.88s)
--- PASS: TestAccAWSEMRCluster_Ec2Attributes_DefaultManagedSecurityGroups (1303.71s)
--- PASS: TestAccAWSEMRCluster_tags (1308.94s)
--- PASS: TestAccAWSEMRCluster_additionalInfo (451.51s)
--- PASS: TestAccAWSEMRCluster_MasterInstanceGroup_BidPrice (1432.37s)
--- PASS: TestAccAWSEMRCluster_ebs_config (644.82s)
--- PASS: TestAccAWSEMRCluster_CoreInstanceGroup_InstanceType (1504.15s)
--- PASS: TestAccAWSEMRCluster_MasterInstanceGroup_InstanceCount (1518.15s)
--- PASS: TestAccAWSEMRCluster_CoreInstanceGroup_BidPrice (1527.34s)
--- PASS: TestAccAWSEMRCluster_Step_Multiple (521.91s)
--- PASS: TestAccAWSEMRCluster_step_concurrency_level (576.21s)
--- PASS: TestAccAWSEMRCluster_CoreInstanceGroup_Name (1614.24s)
--- PASS: TestAccAWSEMRCluster_s3Logging (637.90s)
--- PASS: TestAccAWSEMRCluster_terminationProtected (664.23s)
--- PASS: TestAccAWSEMRCluster_MasterInstanceGroup_Name (1673.52s)
--- PASS: TestAccAWSEMRCluster_keepJob (559.41s)
--- PASS: TestAccAWSEMRCluster_InstanceFleet_basic (1780.47s)
--- PASS: TestAccAWSEMRCluster_root_volume_size (972.90s)
--- PASS: TestAccAWSEMRCluster_visibleToAllUsers (912.02s)
--- PASS: TestAccAWSEMRCluster_Step_ConfigMode (1087.24s)
--- PASS: TestAccAWSEMRCluster_bootstrap_ordering (1454.59s)

…ster

ghost · 2021-04-16T16:35:10Z

This has been released in version 3.37.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-05-16T17:10:42Z

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!

Update resource_aws_emr_cluster.go

e6af72e

ghost added size/XS Managed by automation to categorize the size of a PR. service/emr Issues and PRs that pertain to the emr service. labels Jan 21, 2021

github-actions bot added the needs-triage Waiting for first response or review from a maintainer. label Jan 21, 2021

github-actions bot reviewed Jan 21, 2021

View reviewed changes

Update documentation

1841fb6

ghost added the documentation Introduces or discusses updates to documentation. label Jan 21, 2021

Add testAccAWSEmrClusterConfigMultipleSubnets

c60332b

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

kristerjaanhold added 12 commits January 21, 2021 22:04

Update resource_aws_emr_cluster.go

9ce2cca

Update resource_aws_emr_cluster.go

c3f01a5

Update resource_aws_emr_cluster.go

b2b3652

Update resource_aws_emr_cluster.go

fb5bfd0

Update resource_aws_emr_cluster.go

5bd6664

Update resource_aws_emr_cluster.go

7f8b7e3

Update resource_aws_emr_cluster.go

9628e89

Update resource_aws_emr_cluster.go

91a71f6

Update resource_aws_emr_cluster.go

7b14e15

Update resource_aws_emr_cluster_test.go

f384828

Add ConflictsWith to subnet_id and subnet_ids

6b96ad5

Update resource_aws_emr_cluster.go

f14a5b1

kristerjaanhold marked this pull request as ready for review January 22, 2021 14:00

kristerjaanhold requested a review from a team as a code owner January 22, 2021 14:00

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

kasparg mentioned this pull request Feb 9, 2021

Possibility to Add multiple subnets on Emr #15911

Closed

kasparg mentioned this pull request Feb 9, 2021

Multiple subnet_id-s are not supported with EMR instance fleets #15970

Closed

gdavison self-assigned this Feb 26, 2021

gdavison requested changes Apr 1, 2021

View reviewed changes

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

ghost removed waiting-response Maintainers are waiting on response from community or contributor. labels Apr 12, 2021

gdavison approved these changes Apr 15, 2021

View reviewed changes

gdavison added a commit that referenced this pull request Apr 15, 2021

Merge pull request #17219 from kristerjaanhold/f-resource_aws_emr_clu…

228db50

…ster

gdavison merged commit 5d55d97 into hashicorp:main Apr 15, 2021

github-actions bot added this to the v3.37.0 milestone Apr 15, 2021

ghost locked as resolved and limited conversation to collaborators May 16, 2021

breathingdust removed the needs-triage Waiting for first response or review from a maintainer. label Sep 17, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

resource/aws_emr_cluster Add support for subnet_ids attribute #17219

resource/aws_emr_cluster Add support for subnet_ids attribute #17219

kristerjaanhold commented Jan 21, 2021 •

edited by gdavison

Loading

github-actions bot left a comment

kasparg commented Apr 1, 2021

gdavison left a comment

gdavison Feb 26, 2021

gdavison Apr 1, 2021

gdavison Apr 1, 2021

gdavison Apr 1, 2021

gdavison commented Apr 9, 2021

kristerjaanhold commented Apr 12, 2021

gdavison left a comment

ghost commented Apr 16, 2021

ghost commented May 16, 2021

resource/aws_emr_cluster Add support for subnet_ids attribute #17219

resource/aws_emr_cluster Add support for subnet_ids attribute #17219

Conversation

kristerjaanhold commented Jan 21, 2021 • edited by gdavison Loading

Community Note

github-actions bot left a comment

Choose a reason for hiding this comment

kasparg commented Apr 1, 2021

gdavison left a comment

Choose a reason for hiding this comment

gdavison Feb 26, 2021

Choose a reason for hiding this comment

gdavison Apr 1, 2021

Choose a reason for hiding this comment

gdavison Apr 1, 2021

Choose a reason for hiding this comment

gdavison Apr 1, 2021

Choose a reason for hiding this comment

gdavison commented Apr 9, 2021

kristerjaanhold commented Apr 12, 2021

gdavison left a comment

Choose a reason for hiding this comment

ghost commented Apr 16, 2021

ghost commented May 16, 2021

kristerjaanhold commented Jan 21, 2021 •

edited by gdavison

Loading