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

Support "local down" to stop a running local task #783

Merged
merged 12 commits into from
Jun 6, 2019
Merged
Show file tree
Hide file tree
Changes from 6 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
32 changes: 12 additions & 20 deletions ecs-cli/integ/e2e/local_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,11 +16,9 @@
package e2e

import (
"fmt"
"testing"

"github.com/aws/amazon-ecs-cli/ecs-cli/integ"
"github.com/aws/amazon-ecs-cli/ecs-cli/integ/stdout"
"github.com/stretchr/testify/require"
)

Expand All @@ -40,15 +38,7 @@ func TestECSLocal(t *testing.T) {
{
args: []string{"local", "ps"},
execute: func(t *testing.T, args []string) {
// Given
cmd := integ.GetCommand(args)

// When
out, err := cmd.Output()
require.NoErrorf(t, err, "Failed local ps", fmt.Sprintf("args=%v, stdout=%s, err=%v", args, string(out), err))

// Then
stdout := stdout.Stdout(out)
stdout := integ.RunCmd(t, args)
require.Equal(t, 1, len(stdout.Lines()), "Expected only the table header")
stdout.TestHasAllSubstrings(t, []string{
"CONTAINER ID",
Expand All @@ -64,18 +54,20 @@ func TestECSLocal(t *testing.T) {
{
args: []string{"local", "ps", "--json"},
execute: func(t *testing.T, args []string) {
// Given
cmd := integ.GetCommand(args)

// When
out, err := cmd.Output()
require.NoErrorf(t, err, "Failed local ps", fmt.Sprintf("args=%v, stdout=%s, err=%v", args, string(out), err))

// Then
stdout := stdout.Stdout(out)
stdout := integ.RunCmd(t, args)
stdout.TestHasAllSubstrings(t, []string{"[]"})
},
},
{
args: []string{"local", "down"},
execute: func(t *testing.T, args []string) {
stdout := integ.RunCmd(t, args)
stdout.TestHasAllSubstrings(t, []string{
"docker-compose.local.yml does not exist",
"ecs-local-network not found",
})
},
},
},
},
}
Expand Down
13 changes: 13 additions & 0 deletions ecs-cli/integ/runner.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,9 @@ import (
"strings"
"testing"
"time"

"github.com/aws/amazon-ecs-cli/ecs-cli/integ/stdout"
"github.com/stretchr/testify/require"
)

const (
Expand All @@ -46,6 +49,16 @@ func GetCommand(args []string) *exec.Cmd {
return exec.Command(cmdPath, args...)
}

// RunCmd runs a command with args and returns its Stdout.
func RunCmd(t *testing.T, args []string) stdout.Stdout {
Copy link
Contributor

Choose a reason for hiding this comment

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

👍🏻

cmd := GetCommand(args)

out, err := cmd.Output()
require.NoErrorf(t, err, "Failed running command", fmt.Sprintf("args=%v, stdout=%s, err=%v", args, string(out), err))

return stdout.Stdout(out)
}

// createTempCoverageFile creates a coverage file for a CLI command under $TMPDIR.
func createTempCoverageFile() (string, error) {
tmpfile, err := ioutil.TempFile("", "coverage-*.out")
Expand Down
115 changes: 115 additions & 0 deletions ecs-cli/modules/cli/local/down_app.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
// Copyright 2015-2019 Amazon.com, Inc. or its affiliates. All Rights Reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License"). You may
// not use this file except in compliance with the License. A copy of the
// License is located at
//
// http://aws.amazon.com/apache2.0/
//
// or in the "license" file accompanying this file. This file is distributed
// on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either
// express or implied. See the License for the specific language governing
// permissions and limitations under the License.

package local

import (
"fmt"
"os"
"os/exec"
"path/filepath"

"github.com/aws/amazon-ecs-cli/ecs-cli/modules/cli/local/network"
"github.com/aws/amazon-ecs-cli/ecs-cli/modules/commands/flags"
"github.com/docker/docker/api/types"
"github.com/docker/docker/api/types/filters"
"github.com/sirupsen/logrus"
"github.com/urfave/cli"
"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 a running local ECS task.
// If the user stops the last running task in the local network then also remove the network.
func Down(c *cli.Context) error {
defer func() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nice use of defer 👍🏻

docker := newDockerClient()
network.Teardown(docker)
}()

taskPath := c.String(flags.TaskDefinitionFileFlag)
taskARN := c.String(flags.TaskDefinitionArnFlag)

// TaskDefinitionFileFlag flag has priority over TaskDefinitionArnFlag if both are present
if taskPath != "" {
return handleDownWithFile(taskPath)
}
if taskARN != "" {
return handleDownWithARN(taskARN)
}
return handleDownWithCompose()
}

func handleDownWithFile(path string) error {
return handleDownWithFilters(filters.NewArgs(
filters.Arg("label", fmt.Sprintf("%s=%s", taskFilePathLabelKey, path)),
))
}

func handleDownWithARN(value string) error {
return handleDownWithFilters(filters.NewArgs(
filters.Arg("label", fmt.Sprintf("%s=%s", taskDefinitionARNLabelKey, value)),
))
}

func handleDownWithCompose() error {
wd, _ := os.Getwd()
if _, err := os.Stat(filepath.Join(wd, ecsLocalDockerComposeFileName)); os.IsNotExist(err) {
logrus.Warnf("Compose file %s does not exist in current directory", ecsLocalDockerComposeFileName)
return nil
}

logrus.Infof("Running Compose down on %s", ecsLocalDockerComposeFileName)
cmd := exec.Command("docker-compose", "-f", ecsLocalDockerComposeFileName, "down")
_, err := cmd.CombinedOutput()
if err != nil {
logrus.Fatalf("Failed to run docker-compose down due to %v", err)
}

logrus.Info("Stopped and removed containers successfully")
return nil
}

func handleDownWithFilters(args filters.Args) error {
docker := newDockerClient()

containers, err := docker.ContainerList(context.Background(), types.ContainerListOptions{
Filters: args,
All: true,
})
if err != nil {
logrus.Fatalf("Failed to list containers with args %v due to %v", args, err)
}
if len(containers) == 0 {
logrus.Warnf("No containers found with label %v", args.Get("label"))
return nil
}

for _, container := range containers {
if err = docker.ContainerStop(context.Background(), container.ID, nil); err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use a context with a timeout

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes! 🙈 done, moved docker.go to its own sub-package with an exported timeout constant.

logrus.Fatalf("Failed to stop container %s due to %v", container.ID, err)
}
logrus.Infof("Stopped container with id %s", container.ID)

if err = docker.ContainerRemove(context.Background(), container.ID, types.ContainerRemoveOptions{}); err != nil {
logrus.Fatalf("Failed to remove container %s due to %v", container.ID, err)
}
logrus.Infof("Removed container with id %s", container.ID)
}
return nil
}
31 changes: 17 additions & 14 deletions ecs-cli/modules/cli/local/network/teardown.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"time"

"github.com/docker/docker/api/types"
"github.com/docker/docker/client"
"github.com/sirupsen/logrus"
"golang.org/x/net/context"
)
Expand Down Expand Up @@ -49,34 +50,36 @@ type networkRemover interface {
}

// Teardown removes both the Local Endpoints container and the Local network created by Setup.
// If there are other containers running in the network besides the endpoints container, this function does nothing.
//
// If there is any unexpected errors, we exit the program with a fatal log.
// If there is no network or there are other containers running in the network besides the
// endpoints container, then do nothing. Otherwise, we exit the program with a fatal log.
func Teardown(dockerClient LocalEndpointsStopper) {
if hasRunningTasksInNetwork(dockerClient) {
if shouldSkipTeardown(dockerClient) {
return
}
logrus.Infof("The network %s has no more running tasks, stopping the endpoints containers...", EcsLocalNetworkName)

stopEndpointsContainer(dockerClient)
removeEndpointsContainer(dockerClient)
removeLocalNetwork(dockerClient)
}

// hasRunningTasksInNetwork returns true if there are other containers besides the
// endpoints container running in the local network, false otherwise.
func hasRunningTasksInNetwork(d networkInspector) bool {
// shouldSkipTeardown returns false if the network exists and there is no local task running, otherwise return true.
func shouldSkipTeardown(d networkInspector) bool {
ctx, cancel := context.WithTimeout(context.Background(), dockerTimeout)
defer cancel()

resp, err := d.NetworkInspect(ctx, EcsLocalNetworkName, types.NetworkInspectOptions{})
if err != nil {
if client.IsErrNotFound(err) {
// The network is gone so there are no containers running attached to it, skip teardown.
logrus.Warnf("Network %s not found, skipping network removal", EcsLocalNetworkName)
return true
}
logrus.Fatalf("Failed to inspect network %s due to %v", EcsLocalNetworkName, err)
}

if len(resp.Containers) > 1 {
// Has other containers running in the network
logrus.Infof("%d other task(s) running locally, skipping network removal.", len(resp.Containers)-1)
// Has other containers running in the network, skip teardown.
logrus.Infof("%d other task(s) running locally, skipping network removal", len(resp.Containers)-1)
Copy link
Contributor

Choose a reason for hiding this comment

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

How do we know that the number of running tasks is equal to len(resp.Containers)-1

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The LocalEndpoints container should always be running in the network, so I subtract it to show the actual number of running containers for ECS local.

Updated the comment to:
// Don't count the endpoints container part of the running containers

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. But tasks != containers. So I think it should say "other container(s) running locally"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I updated that too :)

return true
}

Expand All @@ -90,6 +93,7 @@ func hasRunningTasksInNetwork(d networkInspector) bool {
}
}

logrus.Infof("The network %s has no more running tasks", EcsLocalNetworkName)
return false
}

Expand All @@ -105,7 +109,7 @@ func stopEndpointsContainer(d containerStopper) {
}
logrus.Fatalf("Failed to stop %s container due to %v", localEndpointsContainerName, err)
}
logrus.Infof("Stopped the %s container successfully, removing it...", localEndpointsContainerName)
logrus.Infof("Stopped container with name %s", localEndpointsContainerName)
}

// removeEndpointsContainer removes the endpoints container.
Expand All @@ -128,8 +132,7 @@ func removeEndpointsContainer(d containerRemover) {
}
logrus.Fatalf("Failed to remove %s container due to %v", localEndpointsContainerName, err)
}
logrus.Infof("Removed the %s container successfully, removing the %s network...",
localEndpointsContainerName, EcsLocalNetworkName)
logrus.Infof("Removed container with name %s", localEndpointsContainerName)
}

func removeLocalNetwork(d networkRemover) {
Expand All @@ -144,5 +147,5 @@ func removeLocalNetwork(d networkRemover) {
}
logrus.Fatalf("Failed to remove %s network due to %v", EcsLocalNetworkName, err)
}
logrus.Infof("Removed the %s network successfully", EcsLocalNetworkName)
logrus.Infof("Removed network with name %s", EcsLocalNetworkName)
}
15 changes: 15 additions & 0 deletions ecs-cli/modules/cli/local/network/teardown_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,21 @@ func TestTeardown(t *testing.T) {
return mock
},
},
"no network": {
configureCalls: func(mock *mock_network.MockLocalEndpointsStopper) *mock_network.MockLocalEndpointsStopper {
gomock.InOrder(
mock.EXPECT().NetworkInspect(gomock.Any(), gomock.Eq(EcsLocalNetworkName), gomock.Any()).Return(
types.NetworkResource{},
notFoundErr{},
),
// Should not be invoked
mock.EXPECT().ContainerStop(gomock.Any(), gomock.Any(), gomock.Any()).Times(0),
mock.EXPECT().ContainerRemove(gomock.Any(), gomock.Any(), gomock.Any()).Times(0),
mock.EXPECT().NetworkRemove(gomock.Any(), gomock.Any()).Times(0),
)
return mock
},
},
}

for name, tc := range tests {
Expand Down
27 changes: 0 additions & 27 deletions ecs-cli/modules/cli/local/stop_app.go

This file was deleted.

24 changes: 18 additions & 6 deletions ecs-cli/modules/commands/local/local_command.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ func LocalCommand() cli.Command {
Subcommands: []cli.Command{
createCommand(),
upCommand(),
stopCommand(),
downCommand(),
psCommand(),
},
}
Expand All @@ -57,12 +57,24 @@ func upCommand() cli.Command {
}
}

// TODO This is a placeholder function used to test the teardown of the ECS local network.
func stopCommand() cli.Command {
func downCommand() cli.Command {
return cli.Command{
Name: "stop",
Usage: "Stop a running local ECS task.",
Action: local.Stop,
Name: "down",
Usage: "Stop and remove a locally running ECS task.",
Action: local.Down,
Description: `By default, stop and remove containers defined in "docker-compose.local.yml".
If an option is provided, find, stop, and remove containers matching the option.
When there are no more running local ECS tasks then also teardown the network created with "local up".`,
Flags: []cli.Flag{
cli.StringFlag{
Name: flags.TaskDefinitionFileFlag + ",f",
Usage: "The file `path` of the task definition to stop.",
},
cli.StringFlag{
Name: flags.TaskDefinitionArnFlag + ",a",
Usage: "The ARN of the task definition to stop.",
},
},
}
}

Expand Down