Changed SimpleECSService to use LogGroup #15
Conversation
Now SimpleECSService and SimpleFargateService are consistently using LogGroup instead of LogConfiguration, making it really simple.
|
The awslogs driver will work with ecs-agent version > 1.9.0, which should be fine in most cases. |
There was a problem hiding this comment.
Mostly a small nit and then you should remove the task execution stuff, otherwise the log config stuff looks good. We should probably, in the long run, make it more configurable (maybe allow a log configuration, and if that's not given use the LogGroup setting to automatically use a Cloudwatch logs LogGroup with the awslogs) but this is good for now!
| 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.
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.
| iam.ManagedPolicy( | ||
| "ManagedPolicy", | ||
| PolicyDocument=self.generate_policy(), | ||
| PolicyDocument=policy_doc, |
| log_config = self.get_variables()["LogConfiguration"] | ||
| return log_config or NoValue | ||
| """ | ||
| If LogConfiguration is given, go with it. Otherwise if LogGroup is given, |
There was a problem hiding this comment.
nit: This comment isn't quite right - we don't let LogConfiguration be given any longer.
|
Thank you! will make the changes. I will look more into the AWS documentation. |
Now SimpleECSService and SimpleFargateService are consistently
using LogGroup instead of LogConfiguration, making it really simple.
For other drivers like syslogd, user can subclass the SimpleECSService.