Skip to content
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

Typo: taskRole should be corrected to executionRole #2015

Closed
quakeyy opened this issue Mar 14, 2019 · 3 comments · Fixed by #2165 · May be fixed by MechanicalRock/account-reaper#6
Closed

Typo: taskRole should be corrected to executionRole #2015

quakeyy opened this issue Mar 14, 2019 · 3 comments · Fixed by #2165 · May be fixed by MechanicalRock/account-reaper#6
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container bug This issue is a bug. needs-reproduction This issue needs reproduction. pr/needs-tests This PR is missing tests.

Comments

@quakeyy
Copy link

quakeyy commented Mar 14, 2019

The line below should use executionRole instead of taskRole.
https://github.com/awslabs/aws-cdk/blob/3117cd35e02b2bd5f0845ffd14277bbee80eb09d/packages/%40aws-cdk/aws-ecs/lib/ec2/ec2-event-rule-target.ts#L98

For task that pulls the image from ECR, this typo causes CloudWatch Event to not able to invoke the task.

Thanks.

@rix0rrr
Copy link
Contributor

rix0rrr commented Mar 15, 2019

I've gone back and forth on this, and I think there are cases in which it fails when we just put executionRole here. Fairly sure I used to have that and it didn't work for me. I appreciate that it might need to be executionRole in your case.

Can you share some of your code so that we can try to tease out in which cases it needs to be which value?

@rix0rrr rix0rrr added bug This issue is a bug. @aws-cdk/aws-ecs Related to Amazon Elastic Container pr/needs-tests This PR is missing tests. needs-reproduction This issue needs reproduction. labels Mar 15, 2019
@rix0rrr
Copy link
Contributor

rix0rrr commented Mar 15, 2019

cc @SoManyHs @clareliguori any insights?

I'm fairly sure I needed to put taskRole there to make the integ test that's currently in the repo work.

@quakeyy
Copy link
Author

quakeyy commented Mar 15, 2019

Here is my code for creating the CloudWatch Event triggered ECS Task:

const repo = Repository.import(this, 'Repo', { 
    repositoryName: 'xxxxxxxx'
});

const ec2TaskDefinition = new ecs.Ec2TaskDefinition(this, 'TaskDef', {
    networkMode: "bridge"
});

ec2TaskDefinition.addContainer('container', {
    image: ecs.ContainerImage.fromEcrRepository(repo),
    memoryLimitMiB: 128,
    command: [ "xxxxxxxx", "xxxxxxxx" ],
    logging: new ecs.AwsLogDriver(this, 'xxxxxxxx', { streamPrefix: 'xxxxxxxx' })
});

const target = new ecs.Ec2EventRuleTarget(this, 'EventTarget', {
    cluster,
    taskDefinition: ec2TaskDefinition,
    taskCount: 1
});

const rule = new events.EventRule(this, 'Rule', {
    enabled: true,
    description: "xxxxxxxx",
    scheduleExpression: 'rate(1 minute)'
});

rule.addTarget(target);

Another thing I think that is worth mentioning is that the "Task Role" created for my ECS Task has no policies attached to it so it is basically an empty role. I can see that is probably also why it does not do anything when passed to CloudWatch and I have to pass the "Execution Role" instead.

Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
@aws-cdk/aws-ecs Related to Amazon Elastic Container bug This issue is a bug. needs-reproduction This issue needs reproduction. pr/needs-tests This PR is missing tests.
Projects
None yet
3 participants