-
Notifications
You must be signed in to change notification settings - Fork 28
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
base: add_ecs_blueprint
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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": "", | ||
}, | ||
} | ||
|
||
|
@@ -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, | ||
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)) | ||
|
@@ -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): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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). There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
||
|
@@ -411,7 +471,7 @@ def create_policy(self): | |
self.policy = t.add_resource( | ||
iam.ManagedPolicy( | ||
"ManagedPolicy", | ||
PolicyDocument=self.generate_policy(), | ||
PolicyDocument=policy_doc, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good catch! |
||
Roles=[self.role.Ref()], | ||
) | ||
) | ||
|
@@ -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() | ||
|
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.
nit: This comment isn't quite right - we don't let LogConfiguration be given any longer.