feat(stepfunctions-tasks): runtime role in EmrAddStep #27736

msambol · 2023-10-27T20:16:14Z

Here is the result of running the step function deployed by the integration test. The step completed successfully with the IAM role.

Closes #27691.

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

...nteg/test/aws-stepfunctions-tasks/test/emr/integ.emr-create-cluster-add-step-runtime-role.ts

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/emr-add-step.ts

msambol · 2023-10-27T21:19:14Z

@mattliemAWS if you're interested.

lpizzinidev

Thanks!
The implementation looks good.
Some adjustments are needed on documentation and tests in my opinion.

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/emr-add-step.ts

packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-add-step.test.ts

packages/aws-cdk-lib/aws-stepfunctions-tasks/README.md

...nteg/test/aws-stepfunctions-tasks/test/emr/integ.emr-create-cluster-add-step-runtime-role.ts

lpizzinidev

Thanks for the follow-up.
I think that we should automate as much as possible on the integration test to make it maintainable and re-usable as a reference.
Some smaller adjustments are needed on documentation.

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/emr-add-step.ts

packages/aws-cdk-lib/aws-stepfunctions-tasks/README.md

packages/aws-cdk-lib/aws-stepfunctions-tasks/test/emr/emr-add-step.test.ts

...nteg/test/aws-stepfunctions-tasks/test/emr/integ.emr-create-cluster-add-step-runtime-role.ts

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/emr-add-step.ts

msambol · 2023-10-29T22:00:40Z

...nteg/test/aws-stepfunctions-tasks/test/emr/integ.emr-create-cluster-add-step-runtime-role.ts

+          }
+      }
+    }`),
+});


@lpizzinidev I added this as well so no prereqs are required. Step function succeeds after deploying stack as-is. Good shout.

lpizzinidev

Thanks 💪

lpizzinidev · 2023-10-30T13:26:56Z

...nteg/test/aws-stepfunctions-tasks/test/emr/integ.emr-create-cluster-add-step-runtime-role.ts

+          }
+      }
+    }`),
+});


vinayak-kukreja · 2023-12-05T23:55:33Z

Hey @msambol , thank you for this contribution. I am seeing the integ test fails for EmrCreateCluster. I am seeing,

        "StateChangeReason": {
          "Code": "VALIDATION_ERROR",
          "Message": "EMR service role arn:aws:iam::<AccountId>:role/aws-cdk-emr-add-step-runt-EmrCreateClusterServiceRo-xZEbpPmy7it3 is invalid"
        },

Could you confirm if its working on your end once again?

msambol · 2023-12-06T00:01:54Z

Hey @msambol , thank you for this contribution. I am seeing the integ test fails for EmrCreateCluster. I am seeing,
        "StateChangeReason": {
          "Code": "VALIDATION_ERROR",
          "Message": "EMR service role arn:aws:iam::<AccountId>:role/aws-cdk-emr-add-step-runt-EmrCreateClusterServiceRo-xZEbpPmy7it3 is invalid"
        },
Could you confirm if its working on your end once again?

Can I see your code?

aws-cdk-automation · 2023-12-06T00:09:10Z

This PR has been in the MERGE CONFLICTS state for 3 weeks, and looks abandoned. To keep this PR from being closed, please continue work on it. If not, it will automatically be closed in a week.

msambol · 2023-12-06T00:11:28Z

It was working. I'll take a look this weekend.

msambol · 2023-12-06T15:57:23Z

@vinayak-kukreja I think there is a bug with the EMR create cluster process right now. It won't let me create a cluster with a perfectly-acceptable EMR service role. I'm checking with the EMR team.

vinayak-kukreja · 2023-12-06T17:09:12Z

Can I see your code?

Hey @msambol , I just deployed the integ test mentioned in this PR. The deployment succeeds but when I run the first command mentioned in steps within the integ test, it fails the create task. Let me know if I can provide any other information.

msambol · 2023-12-06T17:21:24Z

Mine failed as well. Something is off with EMR cluster creation. I reached out to the EMR team.

vinayak-kukreja · 2023-12-06T22:47:06Z

Ok, thank you. Let me know if you hear anything from them. If not, I can reach out too.

msambol · 2023-12-11T17:20:25Z

@vinayak-kukreja I addressed the cluster creation issues in #28327. Once that's merged, I'll rebase and make sure this integration test succeeds.

paulhcsun · 2023-12-11T23:03:02Z

Hi @msambol, your fix for ecr cluster creation #28327 has been approved and merged.

msambol · 2023-12-11T23:05:41Z

@paulhcsun Rebasing now and re-running integ test 👍🏼

aws-cdk-automation · 2023-12-12T00:14:12Z

AWS CodeBuild CI Report

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

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

msambol · 2023-12-12T00:30:02Z

@paulhcsun This has been updated and confirmed working.

paulhcsun

Thanks for updating and confirming the tests are working. Great work and thanks for the contribution @msambol!

mergify · 2023-12-12T18:46:15Z

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).

aws-cdk-automation requested a review from a team October 27, 2023 20:16

github-actions bot added feature-request A feature should be added or improved. p2 admired-contributor [Pilot] contributed between 13-24 PRs to the CDK labels Oct 27, 2023

msambol commented Oct 27, 2023

View reviewed changes

...nteg/test/aws-stepfunctions-tasks/test/emr/integ.emr-create-cluster-add-step-runtime-role.ts Show resolved Hide resolved

msambol commented Oct 27, 2023

View reviewed changes

packages/aws-cdk-lib/aws-stepfunctions-tasks/lib/emr/emr-add-step.ts Outdated Show resolved Hide resolved

msambol force-pushed the sfn-emr-role branch from 1ce971b to eb16583 Compare October 27, 2023 20:42

aws-cdk-automation added the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 27, 2023

lpizzinidev suggested changes Oct 28, 2023

View reviewed changes

lpizzinidev suggested changes Oct 29, 2023

View reviewed changes

aws-cdk-automation removed the pr/needs-community-review This PR needs a review from a Trusted Community Member or Core Team Member. label Oct 29, 2023

msambol changed the title ~~feat(stepfunctions-tasks): add support to EmrAddStep for runtime role~~ feat(stepfunctions-tasks): add support for runtime role to EmrAddStep Oct 29, 2023

msambol commented Oct 29, 2023

View reviewed changes

lpizzinidev approved these changes Oct 30, 2023

View reviewed changes

...nteg/test/aws-stepfunctions-tasks/test/emr/integ.emr-create-cluster-add-step-runtime-role.ts

}

}

}`),

});

Copy link

Contributor

lpizzinidev Oct 30, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍

aws-cdk-automation added the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Oct 30, 2023

github-actions bot mentioned this pull request Nov 1, 2023

Monthly PR metrics report - October 2023 #27786

Closed

kaizencc changed the title ~~feat(stepfunctions-tasks): add support for runtime role to EmrAddStep~~ feat(stepfunctions-tasks): runtime role in EmrAddStep Dec 7, 2023

github-actions bot added the effort/medium Medium work item – several days of effort label Dec 7, 2023

vinayak-kukreja self-assigned this Dec 7, 2023

msambol added 15 commits December 11, 2023 15:03

feat(stepfunctions-tasks): add support to EmrAddStep for runtime role

64be93b

fix import order

2cef154

fix readme

bd9568a

minor doc edits

d397944

fix name

f7cc503

add feedback from Luca

f6a0cc5

second round of feedback

31e5ef8

update executionRole naming

b03280a

update comment

4ad9fea

ignore arn linting error

4c1f693

fix readme lint errors

4970459

this time

73dcb2b

new files

9c64d95

new

ed75bcf

more

139148a

verify int test

e3c70a7

msambol force-pushed the sfn-emr-role branch from bd12e83 to e3c70a7 Compare December 11, 2023 23:47

paulhcsun approved these changes Dec 12, 2023

View reviewed changes

mergify bot merged commit 314fbfa into aws:main Dec 12, 2023
10 checks passed

aws-cdk-automation removed the pr/needs-maintainer-review This PR needs a review from a Core Team Member label Dec 12, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(stepfunctions-tasks): runtime role in EmrAddStep #27736

feat(stepfunctions-tasks): runtime role in EmrAddStep #27736

msambol commented Oct 27, 2023 •

edited

msambol commented Oct 27, 2023

lpizzinidev left a comment

lpizzinidev left a comment

msambol Oct 29, 2023

lpizzinidev Oct 30, 2023

lpizzinidev left a comment

lpizzinidev Oct 30, 2023

vinayak-kukreja commented Dec 5, 2023

msambol commented Dec 6, 2023

aws-cdk-automation commented Dec 6, 2023

msambol commented Dec 6, 2023

msambol commented Dec 6, 2023

vinayak-kukreja commented Dec 6, 2023

msambol commented Dec 6, 2023

vinayak-kukreja commented Dec 6, 2023

msambol commented Dec 11, 2023

paulhcsun commented Dec 11, 2023

msambol commented Dec 11, 2023

aws-cdk-automation commented Dec 12, 2023

msambol commented Dec 12, 2023

paulhcsun left a comment

mergify bot commented Dec 12, 2023

feat(stepfunctions-tasks): runtime role in EmrAddStep #27736

feat(stepfunctions-tasks): runtime role in EmrAddStep #27736

Conversation

msambol commented Oct 27, 2023 • edited

msambol commented Oct 27, 2023

lpizzinidev left a comment

Choose a reason for hiding this comment

lpizzinidev left a comment

Choose a reason for hiding this comment

msambol Oct 29, 2023

Choose a reason for hiding this comment

lpizzinidev Oct 30, 2023

Choose a reason for hiding this comment

lpizzinidev left a comment

Choose a reason for hiding this comment

lpizzinidev Oct 30, 2023

Choose a reason for hiding this comment

vinayak-kukreja commented Dec 5, 2023

msambol commented Dec 6, 2023

aws-cdk-automation commented Dec 6, 2023

msambol commented Dec 6, 2023

msambol commented Dec 6, 2023

vinayak-kukreja commented Dec 6, 2023

msambol commented Dec 6, 2023

vinayak-kukreja commented Dec 6, 2023

msambol commented Dec 11, 2023

paulhcsun commented Dec 11, 2023

msambol commented Dec 11, 2023

aws-cdk-automation commented Dec 12, 2023

AWS CodeBuild CI Report

msambol commented Dec 12, 2023

paulhcsun left a comment

Choose a reason for hiding this comment

mergify bot commented Dec 12, 2023

msambol commented Oct 27, 2023 •

edited