feat(stepfunctions-tasks): EmrCreateClusterOptions support ebsRootVolumeIops, ebsRootVolumeThroughput and managedScalingPolicy

[phuhung273]

Issue # (if applicable)

Closes #33431

Reason for this change

Missing param in EmrCreateClusterOptions

Description of changes

• EmrCreateClusterOptions support
  • ebsRootVolumeIops
  • ebsRootVolumeThroughput
  • managedScalingPolicy
• Move validations to private validateProps function

ManagedScalingPolicy.ScalingStrategy is supported by awscli/API, but not yet supported by Step Function. Therefore not included in this PR

Describe any new or updated permissions being added

Add iam:CreateServiceLinkedRole permission for only EMR to create AWSServiceRoleForEMRCleanup as instructed by https://docs.aws.amazon.com/emr/latest/ManagementGuide/using-service-linked-roles-cleanup.html#create-service-linked-role

Description of how you validated changes

Unit + Integ

Checklist

• My code adheres to the CONTRIBUTING GUIDE and DESIGN GUIDELINES

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache-2.0 license

[aws-cdk-automation]

AWS CodeBuild CI Report

• CodeBuild project: AutoBuildv2Project1C6BFA3F-wQm2hXv2jqQv
• Commit ID: 96e6342
• Result: SUCCEEDED
• Build Logs (available for 30 days)

Powered by github-codebuild-logs, available on the AWS Serverless Application Repository

[kumvprat]

Thanks for raising the PR.

I have added inline comments where possible.
I have a high level question on the PR : It seems like the ebs root volume was already present as a property in the create emr cluster sfn task.

Is this change not impacting any other integration tests that might be testing emr cluster create task with any other ebs properties ?

[kumvprat]

Can we have a check here that the EMR cluster is created ? And also have a check that the cluster configuration match the cluster configuration provided in the step function step

[phuhung273]

Sure let me add, might take some time.

[phuhung273]

Verification added.

In case you have a question why we need 2 describeExecution calls, just found out that we cannot have below 2 things at the same time:

• Validate execution with .expect(ExpectedResult.objectLike({ status: 'SUCCEEDED', }))
• Retrieve clusterId with .getAttString('output.ClusterId')

Because when we do .getAttString('output.ClusterId'), the response return nothing besides from ClusterId, making status: 'SUCCEEDED' fail although the response actually have it. Thats why we need 2 describeExecution calls.

[kumvprat]

I see that the timeout is. :

totalTimeout: Duration.minutes(5)

Is this enough for the whole emr cluster to be created ?

[phuhung273]

Yes it is enough. It works everytime.

[phuhung273]

Seems like this statement from the doc When you launch a cluster, either for the first time or when the AWSServiceRoleForEMRCleanup service-linked role is not present, Amazon EMR creates the AWSServiceRoleForEMRCleanup service-linked role for you. is not true, at least in this case when StepFunction trigger it.

[kumvprat]

So we should add the permissions for creating service linked role, in an idempotent manner

Because it seems like the cluster is not created properly without the role, and requires the ability to create the service linked role for cluster creation

[kumvprat]

I think these are the required permissions for that, from the same link :

You must have permissions to create a service-linked role. For an example statement that adds this capability to the permissions policy of an IAM entity (such as a user, group, or role):

Add the following statement to the permissions policy for the IAM entity that needs to create the service-linked role.

{
"Sid": "ElasticMapReduceServiceLinkedRole",
"Effect": "Allow",
"Action": "iam:CreateServiceLinkedRole",
"Resource": "arn:aws:iam:::role/aws-service-role/elasticmapreduce.amazonaws.com/AWSServiceRoleForEMRCleanup*",
"Condition": {
"StringEquals": {
"iam:AWSServiceName": [
"elasticmapreduce.amazonaws.com",
"elasticmapreduce.amazonaws.com.cn"
]
}
}
}

[kumvprat]

Changes may not be exactly this but would need some changes in the policy creation during the task creation : maybe we should change the policies added here

Since when using this stepfunction task the user might utilize the emr cluster created from this task for running jobs => The cluster creation should be successful once the task completes
(I have not used this feature personally, and it seems you have. So I would like to ask if adding these set of permissions would make this functionality easier to use? )

[phuhung273]

Adding these set of permissions would make this functionality easier to use?

You're great mate. Can confirm that if StepFunction/StateMachine has iam:CreateServiceLinkedRole then user doesn't need to precreate AWSServiceRoleForEMRCleanup. Better user experience of course.

Idempotent: YES. I executed the job 2 times and no issue when the role exists.

[phuhung273]

Sorry I forgot these questions:

I have a high level question on the PR : It seems like the ebs root volume was already present as a property in the create emr cluster sfn task.

Only EBSsize prop exist, not IOPS+throughput

Is this change not impacting any other integration tests that might be testing emr cluster create task with any other ebs properties ?

Yes, new props will be undefined for current test/user. We know this since no change in other snapshots.

[phuhung273]

Still need security approval process but really appreciate @kumvprat for your prompt support so far 🚀 You're very helpful.

[kumvprat]

LGTM!!

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[mergify]

Thank you for contributing! Your pull request will be updated from main and then merged automatically (do not update manually, and be sure to allow changes to be pushed to your fork).

[github-actions]

Comments on closed issues and PRs are hard for our team to see.
If you need help, please open a new issue that references this one.