-
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
F/aws emr cluster #17706
F/aws emr cluster #17706
Conversation
@@ -325,6 +325,7 @@ The following arguments are supported: | |||
* `security_configuration` - (Optional) The security configuration name to attach to the EMR cluster. Only valid for EMR clusters with `release_label` 4.8.0 or greater | |||
* `core_instance_group` - (Optional) Configuration block to use an [Instance Group](https://docs.aws.amazon.com/emr/latest/ManagementGuide/emr-instance-group-configuration.html#emr-plan-instance-groups) for the [core node type](https://docs.aws.amazon.com/emr/latest/ManagementGuide/emr-master-core-task-nodes.html#emr-plan-core). | |||
* `core_instance_fleet` - (Optional) Configuration block to use an [Instance Fleet](https://docs.aws.amazon.com/emr/latest/ManagementGuide/emr-instance-fleet.html) for the core node type. Cannot be specified if any `core_instance_group` configuration blocks are set. Detailed below. | |||
* `log_encryption_kms_key_id` - (Optional) The AWS KMS master key ID used for encrypting log files. This attribute is only available with EMR version 5.30.0 and later, excluding EMR 6.0.0. |
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.
As per https://docs.aws.amazon.com/emr/latest/APIReference/API_JobFlowDetail.html,
this is Customer Master Key (CMK). Could you please replace master key
with customer master key (CMK)
to align with AWS documentation?
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.
Thanks, Updated
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.
Thanks. Looks good to me.
@@ -2970,16 +2995,17 @@ resource "aws_emr_cluster" "tf-test-cluster" { | |||
instance_type = "c4.large" | |||
} | |||
|
|||
log_uri = "s3://${aws_s3_bucket.test.bucket}/" |
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.
Hi @nikhil-goenka 👋 Thank you for submitting this enhancement. Can you please create a new acceptance test and test configuration to verify this change instead of modifying the existing one? This helps us ensure there are no unexpected regressions. Please see also: https://github.com/hashicorp/terraform-provider-aws/blob/main/docs/contributing/contribution-checklists.md#enhancementbugfix-to-a-resource
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.
Got the logic, updated
Hi Team, |
Hi all, any updates on this please? |
@bflad @pradeepbhadani any chance to review this again please? :) |
Hello guys. Is there any new information about this? Any update is appreciated. |
I am not the approver on this project. but the change looks good to me. Reaching out to HashiCorp Community on getting this reviewed. |
21b307c
to
ea9f0cf
Compare
ea9f0cf
to
b8e0055
Compare
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.
Looks good! 👍
Output of acceptance tests (us-west-2
):
--- PASS: TestAccAWSEMRCluster_basic (394.16s)
--- PASS: TestAccAWSEMRCluster_Step_Basic (495.96s)
--- PASS: TestAccAWSEMRCluster_InstanceFleet_master_only (522.43s)
--- PASS: TestAccAWSEMRCluster_security_config (540.73s)
--- PASS: TestAccAWSEMRCluster_step_concurrency_level (560.61s)
--- PASS: TestAccAWSEMRCluster_ebs_config (575.45s)
--- PASS: TestAccAWSEMRCluster_custom_ami_id (579.43s)
--- PASS: TestAccAWSEMRCluster_CoreInstanceGroup_AutoscalingPolicy (677.97s)
--- PASS: TestAccAWSEMRCluster_s3LogEncryption (701.55s)
--- PASS: TestAccAWSEMRCluster_Kerberos_ClusterDedicatedKdc (720.66s)
--- PASS: TestAccAWSEMRCluster_Ec2Attributes_DefaultManagedSecurityGroups (864.27s)
--- PASS: TestAccAWSEMRCluster_MasterInstanceGroup_InstanceType (888.21s)
--- PASS: TestAccAWSEMRCluster_MasterInstanceGroup_Name (914.91s)
--- PASS: TestAccAWSEMRCluster_CoreInstanceGroup_Name (965.39s)
--- PASS: TestAccAWSEMRCluster_keepJob (422.17s)
--- PASS: TestAccAWSEMRCluster_MasterInstanceGroup_BidPrice (983.19s)
--- PASS: TestAccAWSEMRCluster_root_volume_size (1028.63s)
--- PASS: TestAccAWSEMRCluster_terminationProtected (560.45s)
--- PASS: TestAccAWSEMRCluster_s3Logging (541.94s)
--- PASS: TestAccAWSEMRCluster_CoreInstanceGroup_BidPrice (1065.99s)
--- PASS: TestAccAWSEMRCluster_MasterInstanceGroup_InstanceCount (1095.81s)
--- PASS: TestAccAWSEMRCluster_disappears (521.71s)
--- PASS: TestAccAWSEMRCluster_configurationsJson (520.61s)
--- PASS: TestAccAWSEMRCluster_additionalInfo (400.55s)
--- PASS: TestAccAWSEMRCluster_Step_Multiple (476.43s)
--- PASS: TestAccAWSEMRCluster_InstanceFleet_basic (1260.41s)
--- PASS: TestAccAWSEMRCluster_tags (873.52s)
--- PASS: TestAccAWSEMRCluster_CoreInstanceGroup_InstanceCount (1278.96s)
--- PASS: TestAccAWSEMRCluster_visibleToAllUsers (805.84s)
--- PASS: TestAccAWSEMRCluster_Step_ConfigMode (859.46s)
--- PASS: TestAccAWSEMRCluster_CoreInstanceGroup_InstanceType (906.93s)
--- PASS: TestAccAWSEMRCluster_bootstrap_ordering (1277.84s)
Output of acceptance tests (GovCloud
):
(Failures unrelated to these changes and stem from differences on GovCloud downloading bootstrap action.)
--- FAIL: TestAccAWSEMRCluster_s3Logging (262.62s)
--- FAIL: TestAccAWSEMRCluster_InstanceFleet_master_only (310.53s)
--- FAIL: TestAccAWSEMRCluster_InstanceFleet_basic (322.48s)
--- PASS: TestAccAWSEMRCluster_Kerberos_ClusterDedicatedKdc (399.96s)
--- PASS: TestAccAWSEMRCluster_ebs_config (474.33s)
--- PASS: TestAccAWSEMRCluster_custom_ami_id (480.35s)
--- PASS: TestAccAWSEMRCluster_Step_Multiple (488.69s)
--- PASS: TestAccAWSEMRCluster_step_concurrency_level (497.75s)
--- FAIL: TestAccAWSEMRCluster_bootstrap_ordering (275.12s)
--- PASS: TestAccAWSEMRCluster_Step_Basic (548.95s)
--- PASS: TestAccAWSEMRCluster_security_config (563.88s)
--- PASS: TestAccAWSEMRCluster_basic (580.93s)
--- PASS: TestAccAWSEMRCluster_keepJob (397.85s)
--- PASS: TestAccAWSEMRCluster_visibleToAllUsers (823.01s)
--- PASS: TestAccAWSEMRCluster_CoreInstanceGroup_Name (831.01s)
--- PASS: TestAccAWSEMRCluster_root_volume_size (857.41s)
--- PASS: TestAccAWSEMRCluster_terminationProtected (541.65s)
--- PASS: TestAccAWSEMRCluster_MasterInstanceGroup_Name (873.37s)
--- PASS: TestAccAWSEMRCluster_MasterInstanceGroup_InstanceType (883.61s)
--- PASS: TestAccAWSEMRCluster_tags (923.43s)
--- PASS: TestAccAWSEMRCluster_Step_ConfigMode (933.06s)
--- PASS: TestAccAWSEMRCluster_additionalInfo (407.19s)
--- PASS: TestAccAWSEMRCluster_CoreInstanceGroup_InstanceCount (505.34s)
--- PASS: TestAccAWSEMRCluster_configurationsJson (446.65s)
--- PASS: TestAccAWSEMRCluster_CoreInstanceGroup_AutoscalingPolicy (564.35s)
--- PASS: TestAccAWSEMRCluster_disappears (500.07s)
--- PASS: TestAccAWSEMRCluster_MasterInstanceGroup_InstanceCount (1076.35s)
--- PASS: TestAccAWSEMRCluster_Ec2Attributes_DefaultManagedSecurityGroups (762.44s)
--- PASS: TestAccAWSEMRCluster_s3LogEncryption (1166.72s)
--- PASS: TestAccAWSEMRCluster_CoreInstanceGroup_InstanceType (761.26s)
--- PASS: TestAccAWSEMRCluster_MasterInstanceGroup_BidPrice (822.30s)
--- PASS: TestAccAWSEMRCluster_CoreInstanceGroup_BidPrice (950.06s)
This functionality has been released in v3.62.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. |
Community Note
Relates OR Closes #17062
Output from acceptance testing: