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

Add task-def and file flags to local down/ps #789

Merged
merged 7 commits into from
Jun 17, 2019
Merged
Show file tree
Hide file tree
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
1 change: 1 addition & 0 deletions ecs-cli/integ/cmd/compose.go
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ func TestServiceUp(t *testing.T, p *Project) {
"up",
"--cluster-config",
p.ConfigName,
"--create-log-groups",
iamhopaul123 marked this conversation as resolved.
Show resolved Hide resolved
}
cmd := integ.GetCommand(args)

Expand Down
27 changes: 27 additions & 0 deletions ecs-cli/modules/cli/local/create_app.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,12 +18,39 @@ package local
import (
"fmt"

"github.com/aws/amazon-ecs-cli/ecs-cli/modules/commands/flags"
"github.com/urfave/cli"
)

const (
// taskDefinitionLabelType represents the type of option used to
// transform a task definition to a compose file e.g. remoteFile, localFile.
// taskDefinitionLabelValue represents the value of the option
// e.g. file path, arn, family.
taskDefinitionLabelType = "ecsLocalTaskDefType"
taskDefinitionLabelValue = "ecsLocalTaskDefVal"
)

const (
localTaskDefType = "localFile"
remoteTaskDefType = "remoteFile"
)
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm also a little confused why these four constants are defined in create_app.go right now, as I don't see them being used in the create function. Perhaps it would make sense to move these constants to a different file still under the local package namespace?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the idea is that eventually Create() will eventually add these as Docker labels to the containers so that we know if the containers were started with a localFile or remoteFile.


const (
ecsLocalDockerComposeFileName = "docker-compose.local.yml"
Copy link
Contributor

@SoManyHs SoManyHs Jun 18, 2019

Choose a reason for hiding this comment

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

I have a different constant name for this: LocalOutDefaultFileName https://github.com/aws/amazon-ecs-cli/pull/785/files#diff-c72fc3b09ef311d6734cb6f6b9c9b9a6R36. Was this name discussed outside of the context of the Create PR?

)

func Create(c *cli.Context) {
// 1. read in task def (from file or arn)
// 2. parse task def into go object
// 3. write to docker-compose.local.yml file
fmt.Println("foo") // placeholder
}

func validateOptions(c *cli.Context) error {
Copy link
Contributor

Choose a reason for hiding this comment

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

Similarly, not sure why this function lives in this file?

Copy link
Contributor

Choose a reason for hiding this comment

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

The flags that are being validated apply to all the sub-commands (create, up, down, ps). Should this live in some other file? 🤔 I think we decided to move it in create_app.go because it's the first command that users will use for local but I can see how that's not a strong reason.

if (c.String(flags.TaskDefinitionFileFlag) != "") && (c.String(flags.TaskDefinitionTaskFlag) != "") {
return fmt.Errorf("%s and %s can not be used together",
flags.TaskDefinitionTaskFlag, flags.TaskDefinitionFileFlag)
}
return nil
}
39 changes: 26 additions & 13 deletions ecs-cli/modules/cli/local/down_app.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package local

import (
"fmt"
"os"
"os/exec"
"path/filepath"
Expand All @@ -28,12 +29,6 @@ import (
"golang.org/x/net/context"
)

// TODO These labels should be defined part of the local.Create workflow.
// Refactor to import these constants instead of re-defining them here.
const (
ecsLocalDockerComposeFileName = "docker-compose.local.yml"
)

// Down stops and removes running local ECS tasks.
// If the user stops the last running task in the local network then also remove the network.
func Down(c *cli.Context) error {
Expand All @@ -42,8 +37,28 @@ func Down(c *cli.Context) error {
network.Teardown(client)
}()

if err := validateOptions(c); err != nil {
logrus.Fatal(err.Error())
}

if c.String(flags.TaskDefinitionFileFlag) != "" {
return downLocalContainersWithFilters(filters.NewArgs(
filters.Arg("label", fmt.Sprintf("%s=%s", taskDefinitionLabelValue,
c.String(flags.TaskDefinitionFileFlag))),
filters.Arg("label", fmt.Sprintf("%s=%s", taskDefinitionLabelType, localTaskDefType)),
))
}
if c.String(flags.TaskDefinitionTaskFlag) != "" {
return downLocalContainersWithFilters(filters.NewArgs(
filters.Arg("label", fmt.Sprintf("%s=%s", taskDefinitionLabelValue,
c.String(flags.TaskDefinitionTaskFlag))),
filters.Arg("label", fmt.Sprintf("%s=%s", taskDefinitionLabelType, remoteTaskDefType)),
))
}
if c.Bool(flags.AllFlag) {
return downAllLocalContainers()
return downLocalContainersWithFilters(filters.NewArgs(
filters.Arg("label", taskDefinitionLabelValue),
))
}
return downComposeLocalContainers()
}
Expand All @@ -65,19 +80,17 @@ func downComposeLocalContainers() error {
return nil
}

func downAllLocalContainers() error {
func downLocalContainersWithFilters(args filters.Args) error {
ctx, cancel := context.WithTimeout(context.Background(), docker.TimeoutInS)
defer cancel()

client := docker.NewClient()
containers, err := client.ContainerList(ctx, types.ContainerListOptions{
Filters: filters.NewArgs(
filters.Arg("label", taskDefinitionLabelKey),
),
All: true,
Filters: args,
All: true,
})
if err != nil {
logrus.Fatalf("Failed to list containers with label=%s due to %v", taskDefinitionLabelKey, err)
logrus.Fatalf("Failed to list containers with filters %v due to %v", args, err)
}
if len(containers) == 0 {
logrus.Warn("No running ECS local tasks found")
Expand Down
39 changes: 23 additions & 16 deletions ecs-cli/modules/cli/local/ps_app.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,15 +31,6 @@ import (
"golang.org/x/net/context"
)

// TODO These labels should be defined part of the local.Create workflow.
// Refactor to import these constants instead of re-defining them here.
// Docker object labels associated with containers created with "ecs-cli local".
const (
// taskDefinitionLabelKey represents the value of the option used to
// transform a task definition to a compose file e.g. file path, arn, family.
taskDefinitionLabelKey = "ecsLocalTaskDefinition"
)

// Table formatting settings used by the Docker CLI.
// See https://github.com/docker/cli/blob/0904fbfc77dbd4b6296c56e68be573b889d049e3/cli/command/formatter/formatter.go#L74
const (
Expand All @@ -63,18 +54,34 @@ const (
// If the --all flag is provided, then list all local ECS task containers.
// If the --json flag is provided, then output the format as JSON instead.
func Ps(c *cli.Context) {
if err := validateOptions(c); err != nil {
logrus.Fatal(err.Error())
}
containers := listContainers(c)
displayContainers(c, containers)
}

func listContainers(c *cli.Context) []types.Container {
if !c.Bool(flags.AllFlag) {
return listLocalComposeContainers()
if c.String(flags.TaskDefinitionFileFlag) != "" {
return listContainersWithFilters(filters.NewArgs(
filters.Arg("label", fmt.Sprintf("%s=%s", taskDefinitionLabelValue,
c.String(flags.TaskDefinitionFileFlag))),
filters.Arg("label", fmt.Sprintf("%s=%s", taskDefinitionLabelType, localTaskDefType)),
))
}
if c.String(flags.TaskDefinitionTaskFlag) != "" {
return listContainersWithFilters(filters.NewArgs(
filters.Arg("label", fmt.Sprintf("%s=%s", taskDefinitionLabelValue,
c.String(flags.TaskDefinitionTaskFlag))),
filters.Arg("label", fmt.Sprintf("%s=%s", taskDefinitionLabelType, remoteTaskDefType)),
))
}
if c.Bool(flags.AllFlag) {
return listContainersWithFilters(filters.NewArgs(
filters.Arg("label", taskDefinitionLabelValue),
))
}
// Task containers running locally all have a local label
return listContainersWithFilters(filters.NewArgs(
filters.Arg("label", taskDefinitionLabelKey),
))
return listLocalComposeContainers()
}

func listLocalComposeContainers() []types.Container {
Expand Down Expand Up @@ -144,7 +151,7 @@ func displayAsTable(containers []types.Container) {
container.Status,
prettifyPorts(container.Ports),
prettifyNames(container.Names),
container.Labels[taskDefinitionLabelKey])
container.Labels[taskDefinitionLabelValue])
fmt.Fprintln(w, row)
}
w.Flush()
Expand Down
1 change: 1 addition & 0 deletions ecs-cli/modules/commands/flags/flags.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,7 @@ const (
// Local
TaskDefinitionFileFlag = "file"
TaskDefinitionArnFlag = "arn"
TaskDefinitionTaskFlag = "task-def"
LocalOutputFlag = "output"
JsonFlag = "json"
AllFlag = "all"
Expand Down
20 changes: 18 additions & 2 deletions ecs-cli/modules/commands/local/local_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,20 @@ func upCommand() cli.Command {
func downCommand() cli.Command {
return cli.Command{
Name: "down",
Usage: "Stop and remove a running local ECS task.",
Usage: "Stop and remove a running ECS task container.",
Action: local.Down,
Flags: []cli.Flag{
cli.BoolFlag{
Name: flags.AllFlag,
Usage: "Stop and remove all running local ECS tasks.",
Usage: "Stops and removes all running containers",
},
cli.StringFlag{
Name: flags.TaskDefinitionTaskFlag,
Usage: "Stops and removes all running containers matching the task family or ARN",
iamhopaul123 marked this conversation as resolved.
Show resolved Hide resolved
},
cli.StringFlag{
Name: flags.TaskDefinitionFileFlag,
Usage: "Stops and removes all running containers matching the task definition file path",
},
},
}
Expand All @@ -81,6 +89,14 @@ func psCommand() cli.Command {
Name: flags.AllFlag,
Usage: "Lists all running local ECS tasks.",
},
cli.StringFlag{
Name: flags.TaskDefinitionTaskFlag,
Usage: "Lists all running containers matching the task family or ARN",
},
cli.StringFlag{
Name: flags.TaskDefinitionFileFlag,
Usage: "Lists all running containers matching the task definition file path",
},
cli.BoolFlag{
Name: flags.JsonFlag,
Usage: "Output in JSON format.",
Expand Down