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

Changed SimpleECSService to use LogGroup #15

Open
wants to merge 1 commit into
base: add_ecs_blueprint
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
77 changes: 70 additions & 7 deletions stacker_blueprints/ecs.py
Original file line number Diff line number Diff line change
Expand Up @@ -328,10 +328,11 @@ class SimpleECSService(Blueprint):
"task.",
"default": {},
},
"LogConfiguration": {
"type": TroposphereType(ecs.LogConfiguration, optional=True),
"description": "An optional log configuration object.",
"default": None,
"LogGroup": {
"type": str,
"description": "An optional CloudWatch LogGroup name to send logs "
"to.",
"default": "",
},
}

Expand Down Expand Up @@ -375,10 +376,29 @@ def environment(self):

return env_list

@property
def log_group(self):
return self.get_variables()["LogGroup"]

@property
def log_configuration(self):
log_config = self.get_variables()["LogConfiguration"]
return log_config or NoValue
"""
If LogConfiguration is given, go with it. Otherwise if LogGroup is given,
Copy link
Member

Choose a reason for hiding this comment

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

nit: This comment isn't quite right - we don't let LogConfiguration be given any longer.

add a quick log group based on awslogs driver.
Returns:

"""
if not self.log_group:
return NoValue

return ecs.LogConfiguration(
LogDriver="awslogs",
Options={
"awslogs-group": self.log_group,
"awslogs-region": Region,
"awslogs-stream-prefix": self.service_name,
}
)

def add_output(self, key, value):
self.template.add_output(Output(key, Value=value))
Expand All @@ -398,6 +418,46 @@ def create_role(self):
self.add_output("RoleArn", self.role.GetAtt("Arn"))
self.add_output("RoleId", self.role.GetAtt("RoleId"))

def create_task_execution_role(self):
Copy link
Member

Choose a reason for hiding this comment

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

So this actually won't work in the EC2 version of ECS. TaskExecutionRoles are meant to be used with Fargate from my understanding, and with EC2 the effective task execution role is the one that is applied to your Container Instance's (EC2) InstanceProfile.

So basically we just need to include the policy statements that you would normally include here in the Instance role/policy (which isn't something we can do here).

Copy link
Member

Choose a reason for hiding this comment

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

t = self.template

self.task_execution_role = t.add_resource(
iam.Role(
"TaskExecutionRole",
AssumeRolePolicyDocument=get_ecs_task_assumerole_policy(),
)
)

t.add_output(
Output(
"TaskExecutionRoleName",
Value=self.task_execution_role.Ref()
)
)

t.add_output(
Output(
"TaskExecutionRoleArn",
Value=self.task_execution_role.GetAtt("Arn")
)
)

def create_task_execution_role_policy(self):
t = self.template

policy_name = Sub("${AWS::StackName}-task-exeuction-role-policy")

self.task_execution_role_policy = t.add_resource(
iam.PolicyType(
"TaskExecutionRolePolicy",
PolicyName=policy_name,
PolicyDocument=ecs_task_execution_policy(
log_group=self.log_group
),
Roles=[self.task_execution_role.Ref()],
)
)

def generate_policy_document(self):
return None

Expand All @@ -411,7 +471,7 @@ def create_policy(self):
self.policy = t.add_resource(
iam.ManagedPolicy(
"ManagedPolicy",
PolicyDocument=self.generate_policy(),
PolicyDocument=policy_doc,
Copy link
Member

Choose a reason for hiding this comment

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

Good catch!

Roles=[self.role.Ref()],
)
)
Expand Down Expand Up @@ -463,7 +523,10 @@ def create_service(self):
self.add_output("ServiceName", self.service.GetAtt("Name"))

def create_template(self):
self.create_task_execution_role()
self.create_task_execution_role_policy()
self.create_role()
self.create_policy()
self.create_task_definition()
self.create_service()