From e38221b42c47f4e29e55e03217c408b10561a8d6 Mon Sep 17 00:00:00 2001 From: Brian Zambrano Date: Tue, 19 Jan 2016 21:10:48 -0700 Subject: [PATCH 1/6] Add a command line argument to set the logConfiguration First cut of configuring the ``--log-driver`` for from Empire. Details of this are listed in #704. In short, this allows controlling the logging driver when Empire starts up. Work remaining to do: - validate the values passed in on the command line - add support for log options --- cmd/empire/factories.go | 2 ++ cmd/empire/main.go | 13 ++++++++++--- scheduler/ecs/ecs.go | 36 +++++++++++++++++++++++++----------- 3 files changed, 37 insertions(+), 14 deletions(-) diff --git a/cmd/empire/factories.go b/cmd/empire/factories.go index a7f5f2e29..f52186546 100644 --- a/cmd/empire/factories.go +++ b/cmd/empire/factories.go @@ -89,6 +89,7 @@ func newECSScheduler(c *cli.Context) (scheduler.Scheduler, error) { InternalSubnetIDs: c.StringSlice(FlagEC2SubnetsPrivate), ExternalSubnetIDs: c.StringSlice(FlagEC2SubnetsPublic), ZoneID: c.String(FlagRoute53InternalZoneID), + LogDriver: c.String(FlagDockerLogDriver), } s, err := ecs.NewLoadBalancedScheduler(config) @@ -109,6 +110,7 @@ func newECSScheduler(c *cli.Context) (scheduler.Scheduler, error) { log.Println(fmt.Sprintf(" InternalSubnetIDs: %v", config.InternalSubnetIDs)) log.Println(fmt.Sprintf(" ExternalSubnetIDs: %v", config.ExternalSubnetIDs)) log.Println(fmt.Sprintf(" ZoneID: %v", config.ZoneID)) + log.Println(fmt.Sprintf(" LogDriver: %v", config.LogDriver)) return &scheduler.AttachedRunner{ Scheduler: s, diff --git a/cmd/empire/main.go b/cmd/empire/main.go index de8ffe001..476403090 100644 --- a/cmd/empire/main.go +++ b/cmd/empire/main.go @@ -27,9 +27,10 @@ const ( FlagDBPath = "path" FlagDB = "db" - FlagDockerSocket = "docker.socket" - FlagDockerCert = "docker.cert" - FlagDockerAuth = "docker.auth" + FlagDockerSocket = "docker.socket" + FlagDockerCert = "docker.cert" + FlagDockerAuth = "docker.auth" + FlagDockerLogDriver = "docker.logdriver" FlagAWSDebug = "aws.debug" FlagECSCluster = "ecs.cluster" @@ -166,6 +167,12 @@ var EmpireFlags = []cli.Flag{ Usage: "Path to a docker registry auth file (~/.dockercfg)", EnvVar: "DOCKER_AUTH_PATH", }, + cli.StringFlag{ + Name: FlagDockerLogDriver, + Value: "json-file", + Usage: "Log driver to use when running containers. Maps to the --log-driver docker cli arg.", + EnvVar: "DOCKER_LOG_DRIVER", + }, cli.BoolFlag{ Name: FlagAWSDebug, Usage: "Enable verbose debug output for AWS integration.", diff --git a/scheduler/ecs/ecs.go b/scheduler/ecs/ecs.go index 00739e592..117de9ce6 100644 --- a/scheduler/ecs/ecs.go +++ b/scheduler/ecs/ecs.go @@ -47,6 +47,7 @@ type Scheduler struct { cluster string ecs *ecsutil.Client + config Config } // Config holds configuration for generating a new ECS backed Scheduler @@ -78,6 +79,9 @@ type Config struct { // AWS configuration. AWS client.ConfigProvider + + // Log driver to use when starting ECS tasks. Maps to the --log-driver docker cli arg + LogDriver string } // NewScheduler returns a new Scehduler implementation that: @@ -91,12 +95,14 @@ func NewScheduler(config Config) (*Scheduler, error) { cluster: config.Cluster, serviceRole: config.ServiceRole, ecs: c, + config: config, } return &Scheduler{ cluster: config.Cluster, ProcessManager: pm, ecs: c, + config: config, }, nil } @@ -117,6 +123,7 @@ func NewLoadBalancedScheduler(config Config) (*Scheduler, error) { cluster: config.Cluster, serviceRole: config.ServiceRole, ecs: c, + config: config, } // Create the ELB Manager @@ -144,6 +151,7 @@ func NewLoadBalancedScheduler(config Config) (*Scheduler, error) { cluster: config.Cluster, ProcessManager: pm, ecs: c, + config: config, }, nil } @@ -308,6 +316,7 @@ type ecsProcessManager struct { cluster string serviceRole string ecs *ecsutil.Client + config Config } // CreateProcess creates an ECS service for the process. @@ -341,7 +350,7 @@ func (m *ecsProcessManager) Run(ctx context.Context, app *scheduler.App, process // createTaskDefinition creates a Task Definition in ECS for the service. func (m *ecsProcessManager) createTaskDefinition(ctx context.Context, app *scheduler.App, process *scheduler.Process) (*ecs.TaskDefinition, error) { - taskDef, err := taskDefinitionInput(process) + taskDef, err := taskDefinitionInput(process, m.config) if err != nil { return nil, err } @@ -479,7 +488,7 @@ func (m *ecsProcessManager) Scale(ctx context.Context, app string, process strin // taskDefinitionInput returns an ecs.RegisterTaskDefinitionInput suitable for // creating a task definition from a Process. -func taskDefinitionInput(p *scheduler.Process) (*ecs.RegisterTaskDefinitionInput, error) { +func taskDefinitionInput(p *scheduler.Process, config Config) (*ecs.RegisterTaskDefinitionInput, error) { args, err := shellwords.Parse(p.Command) if err != nil { return nil, err @@ -513,19 +522,24 @@ func taskDefinitionInput(p *scheduler.Process) (*ecs.RegisterTaskDefinitionInput labels[k] = aws.String(v) } + logConfig := &ecs.LogConfiguration{ + LogDriver: aws.String(config.LogDriver), + } + return &ecs.RegisterTaskDefinitionInput{ Family: aws.String(p.Type), ContainerDefinitions: []*ecs.ContainerDefinition{ &ecs.ContainerDefinition{ - Name: aws.String(p.Type), - Cpu: aws.Int64(int64(p.CPUShares)), - Command: command, - Image: aws.String(p.Image.String()), - Essential: aws.Bool(true), - Memory: aws.Int64(int64(p.MemoryLimit / MB)), - Environment: environment, - PortMappings: ports, - DockerLabels: labels, + Name: aws.String(p.Type), + Cpu: aws.Int64(int64(p.CPUShares)), + Command: command, + Image: aws.String(p.Image.String()), + Essential: aws.Bool(true), + Memory: aws.Int64(int64(p.MemoryLimit / MB)), + Environment: environment, + LogConfiguration: logConfig, + PortMappings: ports, + DockerLabels: labels, }, }, }, nil From e04a9f95a010003b43ceabb3aff41b9a1928d2ca Mon Sep 17 00:00:00 2001 From: Brian Zambrano Date: Wed, 20 Jan 2016 08:13:26 -0700 Subject: [PATCH 2/6] Add to gitignore for vim swap files --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 57fd86512..25a4ea4ff 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ build .env* +*.swp From 97ba13af64be841d64b2bc813c359cce33c6c92d Mon Sep 17 00:00:00 2001 From: Brian Zambrano Date: Wed, 20 Jan 2016 16:11:37 -0700 Subject: [PATCH 3/6] Updtes based on feedback Change the visibility/scope of `Config` and instead deal with `ecs.logConfiguration` in the places which need it. Also add support to accept multiple `ecs.logopt` fields which end up in the docker `--log-opt` arguments. --- cmd/empire/factories.go | 4 +++- cmd/empire/main.go | 27 ++++++++++++++++---------- pkg/ecsutil/ecs.go | 19 +++++++++++++++++++ scheduler/ecs/ecs.go | 42 ++++++++++++++++++++--------------------- 4 files changed, 59 insertions(+), 33 deletions(-) diff --git a/cmd/empire/factories.go b/cmd/empire/factories.go index f52186546..a0d8e1a7c 100644 --- a/cmd/empire/factories.go +++ b/cmd/empire/factories.go @@ -89,7 +89,8 @@ func newECSScheduler(c *cli.Context) (scheduler.Scheduler, error) { InternalSubnetIDs: c.StringSlice(FlagEC2SubnetsPrivate), ExternalSubnetIDs: c.StringSlice(FlagEC2SubnetsPublic), ZoneID: c.String(FlagRoute53InternalZoneID), - LogDriver: c.String(FlagDockerLogDriver), + LogDriver: c.String(FlagECSLogDriver), + LogOpts: c.StringSlice(FlagECSLogOpts), } s, err := ecs.NewLoadBalancedScheduler(config) @@ -111,6 +112,7 @@ func newECSScheduler(c *cli.Context) (scheduler.Scheduler, error) { log.Println(fmt.Sprintf(" ExternalSubnetIDs: %v", config.ExternalSubnetIDs)) log.Println(fmt.Sprintf(" ZoneID: %v", config.ZoneID)) log.Println(fmt.Sprintf(" LogDriver: %v", config.LogDriver)) + log.Println(fmt.Sprintf(" LogOpts: %v", config.LogOpts)) return &scheduler.AttachedRunner{ Scheduler: s, diff --git a/cmd/empire/main.go b/cmd/empire/main.go index 476403090..8370b4668 100644 --- a/cmd/empire/main.go +++ b/cmd/empire/main.go @@ -27,14 +27,15 @@ const ( FlagDBPath = "path" FlagDB = "db" - FlagDockerSocket = "docker.socket" - FlagDockerCert = "docker.cert" - FlagDockerAuth = "docker.auth" - FlagDockerLogDriver = "docker.logdriver" + FlagDockerSocket = "docker.socket" + FlagDockerCert = "docker.cert" + FlagDockerAuth = "docker.auth" FlagAWSDebug = "aws.debug" FlagECSCluster = "ecs.cluster" FlagECSServiceRole = "ecs.service.role" + FlagECSLogDriver = "ecs.logdriver" + FlagECSLogOpts = "ecs.logopt" FlagELBSGPrivate = "elb.sg.private" FlagELBSGPublic = "elb.sg.public" @@ -167,12 +168,6 @@ var EmpireFlags = []cli.Flag{ Usage: "Path to a docker registry auth file (~/.dockercfg)", EnvVar: "DOCKER_AUTH_PATH", }, - cli.StringFlag{ - Name: FlagDockerLogDriver, - Value: "json-file", - Usage: "Log driver to use when running containers. Maps to the --log-driver docker cli arg.", - EnvVar: "DOCKER_LOG_DRIVER", - }, cli.BoolFlag{ Name: FlagAWSDebug, Usage: "Enable verbose debug output for AWS integration.", @@ -190,6 +185,18 @@ var EmpireFlags = []cli.Flag{ Usage: "The IAM Role to use for managing ECS", EnvVar: "EMPIRE_ECS_SERVICE_ROLE", }, + cli.StringFlag{ + Name: FlagECSLogDriver, + Value: "json-file", + Usage: "Log driver to use when running containers. Maps to the --log-driver docker cli arg", + EnvVar: "EMPIRE_ECS_LOG_DRIVER", + }, + cli.StringSliceFlag{ + Name: FlagECSLogOpts, + Value: &cli.StringSlice{}, + Usage: "Log driver to options. Maps to the --log-opt docker cli arg", + EnvVar: "EMPIRE_ECS_LOG_OPT", + }, cli.StringFlag{ Name: FlagELBSGPrivate, Value: "", diff --git a/pkg/ecsutil/ecs.go b/pkg/ecsutil/ecs.go index 03f87c31d..7a5184242 100644 --- a/pkg/ecsutil/ecs.go +++ b/pkg/ecsutil/ecs.go @@ -2,8 +2,10 @@ package ecsutil import ( "fmt" + "strings" "time" + "github.com/aws/aws-sdk-go/aws" "github.com/aws/aws-sdk-go/aws/client" "github.com/aws/aws-sdk-go/service/ecs" "github.com/remind101/pkg/trace" @@ -237,3 +239,20 @@ func (c *limitedClient) DescribeTasks(ctx context.Context, input *ecs.DescribeTa Tasks: tasks, }, nil } + +func NewLogConfiguration(logDriver string, logOpts []string) *ecs.LogConfiguration { + + logOptions := make(map[string]*string) + + for _, opt := range logOpts { + logOpt := strings.SplitN(opt, "=", 2) + if len(logOpt) == 2 { + logOptions[logOpt[0]] = &logOpt[1] + } + } + + return &ecs.LogConfiguration{ + LogDriver: aws.String(logDriver), + Options: logOptions, + } +} diff --git a/scheduler/ecs/ecs.go b/scheduler/ecs/ecs.go index 117de9ce6..75e7fd8cc 100644 --- a/scheduler/ecs/ecs.go +++ b/scheduler/ecs/ecs.go @@ -47,7 +47,6 @@ type Scheduler struct { cluster string ecs *ecsutil.Client - config Config } // Config holds configuration for generating a new ECS backed Scheduler @@ -82,6 +81,9 @@ type Config struct { // Log driver to use when starting ECS tasks. Maps to the --log-driver docker cli arg LogDriver string + + // Log options to use when starting ECS tasks. Maps to the --log-opt docker cli arg + LogOpts []string } // NewScheduler returns a new Scehduler implementation that: @@ -89,20 +91,20 @@ type Config struct { // * Creates services with ECS. func NewScheduler(config Config) (*Scheduler, error) { c := ecsutil.NewClient(config.AWS) + l := ecsutil.NewLogConfiguration(config.LogDriver, config.LogOpts) // Create the ECS Scheduler var pm ProcessManager = &ecsProcessManager{ - cluster: config.Cluster, - serviceRole: config.ServiceRole, - ecs: c, - config: config, + cluster: config.Cluster, + serviceRole: config.ServiceRole, + ecs: c, + logConfiguration: l, } return &Scheduler{ cluster: config.Cluster, ProcessManager: pm, ecs: c, - config: config, }, nil } @@ -117,13 +119,14 @@ func NewLoadBalancedScheduler(config Config) (*Scheduler, error) { } c := ecsutil.NewClient(config.AWS) + l := ecsutil.NewLogConfiguration(config.LogDriver, config.LogOpts) // Create the ECS Scheduler var pm ProcessManager = &ecsProcessManager{ - cluster: config.Cluster, - serviceRole: config.ServiceRole, - ecs: c, - config: config, + cluster: config.Cluster, + serviceRole: config.ServiceRole, + ecs: c, + logConfiguration: l, } // Create the ELB Manager @@ -151,7 +154,6 @@ func NewLoadBalancedScheduler(config Config) (*Scheduler, error) { cluster: config.Cluster, ProcessManager: pm, ecs: c, - config: config, }, nil } @@ -313,10 +315,10 @@ var _ ProcessManager = &ecsProcessManager{} // ecsProcessManager is an implementation of the ProcessManager interface that // creates ECS services for Processes. type ecsProcessManager struct { - cluster string - serviceRole string - ecs *ecsutil.Client - config Config + cluster string + serviceRole string + ecs *ecsutil.Client + logConfiguration *ecs.LogConfiguration } // CreateProcess creates an ECS service for the process. @@ -350,7 +352,7 @@ func (m *ecsProcessManager) Run(ctx context.Context, app *scheduler.App, process // createTaskDefinition creates a Task Definition in ECS for the service. func (m *ecsProcessManager) createTaskDefinition(ctx context.Context, app *scheduler.App, process *scheduler.Process) (*ecs.TaskDefinition, error) { - taskDef, err := taskDefinitionInput(process, m.config) + taskDef, err := taskDefinitionInput(process, m.logConfiguration) if err != nil { return nil, err } @@ -488,7 +490,7 @@ func (m *ecsProcessManager) Scale(ctx context.Context, app string, process strin // taskDefinitionInput returns an ecs.RegisterTaskDefinitionInput suitable for // creating a task definition from a Process. -func taskDefinitionInput(p *scheduler.Process, config Config) (*ecs.RegisterTaskDefinitionInput, error) { +func taskDefinitionInput(p *scheduler.Process, logConfiguration *ecs.LogConfiguration) (*ecs.RegisterTaskDefinitionInput, error) { args, err := shellwords.Parse(p.Command) if err != nil { return nil, err @@ -522,10 +524,6 @@ func taskDefinitionInput(p *scheduler.Process, config Config) (*ecs.RegisterTask labels[k] = aws.String(v) } - logConfig := &ecs.LogConfiguration{ - LogDriver: aws.String(config.LogDriver), - } - return &ecs.RegisterTaskDefinitionInput{ Family: aws.String(p.Type), ContainerDefinitions: []*ecs.ContainerDefinition{ @@ -537,7 +535,7 @@ func taskDefinitionInput(p *scheduler.Process, config Config) (*ecs.RegisterTask Essential: aws.Bool(true), Memory: aws.Int64(int64(p.MemoryLimit / MB)), Environment: environment, - LogConfiguration: logConfig, + LogConfiguration: logConfiguration, PortMappings: ports, DockerLabels: labels, }, From 85d5c29b0ade0af68e7ae3338143b7390900f1a6 Mon Sep 17 00:00:00 2001 From: Brian Zambrano Date: Thu, 21 Jan 2016 15:16:11 -0700 Subject: [PATCH 4/6] Bump to latest ECS AMI --- docs/cloudformation.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/cloudformation.json b/docs/cloudformation.json index 5ea1c3dec..d0d572ef3 100644 --- a/docs/cloudformation.json +++ b/docs/cloudformation.json @@ -17,7 +17,7 @@ "AmiId" : { "Type": "AWS::EC2::Image::Id", "Description": "AMI Id. Defaults to the official ECS Optimized Linux.", - "Default": "ami-e2b1f988" + "Default": "ami-e7acf78d" }, "KeyName": { "Type": "AWS::EC2::KeyPair::KeyName", From a332d1fd0e5e8f40224662465a9b35d959a28efe Mon Sep 17 00:00:00 2001 From: Brian Zambrano Date: Thu, 21 Jan 2016 15:16:42 -0700 Subject: [PATCH 5/6] Add tags to Empire minions This makes it more obvious what's running in the list of EC2 instances --- docs/cloudformation.json | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/docs/cloudformation.json b/docs/cloudformation.json index d0d572ef3..1c61f63a5 100644 --- a/docs/cloudformation.json +++ b/docs/cloudformation.json @@ -407,7 +407,14 @@ "LaunchConfigurationName": { "Ref": "LaunchConfiguration" }, "MinSize": "1", "MaxSize": { "Ref": "MaxCapacity" }, - "DesiredCapacity": { "Ref": "DesiredCapacity" } + "DesiredCapacity": { "Ref": "DesiredCapacity" }, + "Tags": [ + { + "Key": "Name", + "Value": "Empire minion", + "PropagateAtLaunch": "true" + } + ] } }, From b8c980175b5ccd51817434e93986c6b999acdbc4 Mon Sep 17 00:00:00 2001 From: Brian Zambrano Date: Thu, 21 Jan 2016 16:49:17 -0700 Subject: [PATCH 6/6] Move LogConfiguration construction to `cmd/empire` Also add some validation around the log driver options. Restrict to syslog and json-file for now since the other options would have other consequences which aren't fully tested. --- cmd/empire/factories.go | 12 ++++++++---- pkg/ecsutil/ecs.go | 10 ++++++++++ scheduler/ecs/ecs.go | 13 ++++--------- 3 files changed, 22 insertions(+), 13 deletions(-) diff --git a/cmd/empire/factories.go b/cmd/empire/factories.go index a0d8e1a7c..e12125078 100644 --- a/cmd/empire/factories.go +++ b/cmd/empire/factories.go @@ -14,6 +14,7 @@ import ( "github.com/remind101/empire" "github.com/remind101/empire/events/sns" "github.com/remind101/empire/pkg/dockerutil" + "github.com/remind101/empire/pkg/ecsutil" "github.com/remind101/empire/pkg/runner" "github.com/remind101/empire/scheduler" "github.com/remind101/empire/scheduler/ecs" @@ -80,6 +81,11 @@ func newScheduler(c *cli.Context) (scheduler.Scheduler, error) { } func newECSScheduler(c *cli.Context) (scheduler.Scheduler, error) { + + logDriver := c.String(FlagECSLogDriver) + logOpts := c.StringSlice(FlagECSLogOpts) + logConfiguration := ecsutil.NewLogConfiguration(logDriver, logOpts) + config := ecs.Config{ AWS: newConfigProvider(c), Cluster: c.String(FlagECSCluster), @@ -89,8 +95,7 @@ func newECSScheduler(c *cli.Context) (scheduler.Scheduler, error) { InternalSubnetIDs: c.StringSlice(FlagEC2SubnetsPrivate), ExternalSubnetIDs: c.StringSlice(FlagEC2SubnetsPublic), ZoneID: c.String(FlagRoute53InternalZoneID), - LogDriver: c.String(FlagECSLogDriver), - LogOpts: c.StringSlice(FlagECSLogOpts), + LogConfiguration: logConfiguration, } s, err := ecs.NewLoadBalancedScheduler(config) @@ -111,8 +116,7 @@ func newECSScheduler(c *cli.Context) (scheduler.Scheduler, error) { log.Println(fmt.Sprintf(" InternalSubnetIDs: %v", config.InternalSubnetIDs)) log.Println(fmt.Sprintf(" ExternalSubnetIDs: %v", config.ExternalSubnetIDs)) log.Println(fmt.Sprintf(" ZoneID: %v", config.ZoneID)) - log.Println(fmt.Sprintf(" LogDriver: %v", config.LogDriver)) - log.Println(fmt.Sprintf(" LogOpts: %v", config.LogOpts)) + log.Println(fmt.Sprintf(" LogConfiguration: %v", logConfiguration)) return &scheduler.AttachedRunner{ Scheduler: s, diff --git a/pkg/ecsutil/ecs.go b/pkg/ecsutil/ecs.go index 7a5184242..810aa2888 100644 --- a/pkg/ecsutil/ecs.go +++ b/pkg/ecsutil/ecs.go @@ -240,6 +240,11 @@ func (c *limitedClient) DescribeTasks(ctx context.Context, input *ecs.DescribeTa }, nil } +var validLogDrivers = map[string]bool{ + "json-file": true, + "syslog": true, +} + func NewLogConfiguration(logDriver string, logOpts []string) *ecs.LogConfiguration { logOptions := make(map[string]*string) @@ -251,6 +256,11 @@ func NewLogConfiguration(logDriver string, logOpts []string) *ecs.LogConfigurati } } + _, ok := validLogDrivers[logDriver] + if !ok { + logDriver = "json-file" + } + return &ecs.LogConfiguration{ LogDriver: aws.String(logDriver), Options: logOptions, diff --git a/scheduler/ecs/ecs.go b/scheduler/ecs/ecs.go index 75e7fd8cc..4d7126d9b 100644 --- a/scheduler/ecs/ecs.go +++ b/scheduler/ecs/ecs.go @@ -79,11 +79,8 @@ type Config struct { // AWS configuration. AWS client.ConfigProvider - // Log driver to use when starting ECS tasks. Maps to the --log-driver docker cli arg - LogDriver string - - // Log options to use when starting ECS tasks. Maps to the --log-opt docker cli arg - LogOpts []string + // Log configuraton for ECS tasks + LogConfiguration *ecs.LogConfiguration } // NewScheduler returns a new Scehduler implementation that: @@ -91,14 +88,13 @@ type Config struct { // * Creates services with ECS. func NewScheduler(config Config) (*Scheduler, error) { c := ecsutil.NewClient(config.AWS) - l := ecsutil.NewLogConfiguration(config.LogDriver, config.LogOpts) // Create the ECS Scheduler var pm ProcessManager = &ecsProcessManager{ cluster: config.Cluster, serviceRole: config.ServiceRole, ecs: c, - logConfiguration: l, + logConfiguration: config.LogConfiguration, } return &Scheduler{ @@ -119,14 +115,13 @@ func NewLoadBalancedScheduler(config Config) (*Scheduler, error) { } c := ecsutil.NewClient(config.AWS) - l := ecsutil.NewLogConfiguration(config.LogDriver, config.LogOpts) // Create the ECS Scheduler var pm ProcessManager = &ecsProcessManager{ cluster: config.Cluster, serviceRole: config.ServiceRole, ecs: c, - logConfiguration: l, + logConfiguration: config.LogConfiguration, } // Create the ELB Manager