Teardowns the local network on "local stop" #775
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,148 @@ | ||
| // 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 network | ||
|
|
||
| import ( | ||
| "strings" | ||
| "time" | ||
|
|
||
| "github.com/docker/docker/api/types" | ||
| "github.com/sirupsen/logrus" | ||
| "golang.org/x/net/context" | ||
| ) | ||
|
|
||
| // LocalEndpointsStopper groups the Docker NetworkInspect, ContainerStop, ContainerRemove, and NetworkRemove functions. | ||
| // | ||
| // These functions can be used together to remove a network once unwanted containers are stopped. | ||
| type LocalEndpointsStopper interface { | ||
| networkInspector | ||
| containerStopper | ||
| containerRemover | ||
| networkRemover | ||
| } | ||
|
|
||
| type networkInspector interface { | ||
| NetworkInspect(ctx context.Context, networkID string, options types.NetworkInspectOptions) (types.NetworkResource, error) | ||
| } | ||
|
|
||
| type containerStopper interface { | ||
| ContainerStop(ctx context.Context, containerID string, timeout *time.Duration) error | ||
| } | ||
|
|
||
| type containerRemover interface { | ||
| ContainerRemove(ctx context.Context, containerID string, options types.ContainerRemoveOptions) error | ||
| } | ||
|
|
||
| type networkRemover interface { | ||
| NetworkRemove(ctx context.Context, networkID string) error | ||
| } | ||
|
|
||
| // 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. | ||
| func Teardown(dockerClient LocalEndpointsStopper) { | ||
| if hasRunningTasksInNetwork(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 { | ||
| ctx, cancel := context.WithTimeout(context.Background(), dockerTimeout) | ||
| defer cancel() | ||
|
|
||
| resp, err := d.NetworkInspect(ctx, EcsLocalNetworkName, types.NetworkInspectOptions{}) | ||
| if err != nil { | ||
| 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) | ||
| return true | ||
|
There was a problem hiding this comment. I'd suggest adding a log statement here so that users have more visibility in the case where teardown is being skipped because they had launched other containers in the network There was a problem hiding this comment. Done! Added |
||
| } | ||
|
|
||
| for _, container := range resp.Containers { | ||
| if container.Name != localEndpointsContainerName { | ||
| // The only container running in the network is a task without the endpoints container. | ||
| // This scenario should not happen unless the user themselves stopped the endpoints container. | ||
| logrus.Warnf("The %s container is running in the %s network without the %s container, please stop it first", | ||
| container.Name, EcsLocalNetworkName, localEndpointsContainerName) | ||
| return true | ||
| } | ||
| } | ||
|
|
||
| return false | ||
| } | ||
|
|
||
| func stopEndpointsContainer(d containerStopper) { | ||
| ctx, cancel := context.WithTimeout(context.Background(), dockerTimeout) | ||
| defer cancel() | ||
|
|
||
| err := d.ContainerStop(ctx, localEndpointsContainerName, nil) | ||
| if err != nil { | ||
| if strings.Contains(strings.ToLower(err.Error()), "no such container") { | ||
|
There was a problem hiding this comment. Great question, looks like the comment below wasn't precise enough :P This error happens if the user stopped all their task containers with We don't want to I updated the comment to There was a problem hiding this comment. An alternative is to not invoke However, I think that approach pushes complexity upwards instead of downwards (into the function) so I don't prefer it as much. Also if you run There was a problem hiding this comment. Got it... my only concern is that this is yet another place where we are using string matching on errors returned by Docker :/ There was a problem hiding this comment. Yeah, I agree but I think Docker locked themselves and can't change their error messages otherwise they'll break everyone's code :P Here is a similar example from the agent: https://github.com/aws/amazon-ecs-agent/blob/24c71162368b3a9090689532e5c035fe2bccf133/agent/dockerclient/dockerapi/docker_client.go#L693 There was a problem hiding this comment. This is my least favorite thing in Go... |
||
| // The containers in the network were already stopped by the user using "docker stop", do nothing. | ||
| return | ||
| } | ||
| logrus.Fatalf("Failed to stop %s container due to %v", localEndpointsContainerName, err) | ||
| } | ||
| logrus.Infof("Stopped the %s container successfully, removing it...", localEndpointsContainerName) | ||
| } | ||
|
|
||
| // removeEndpointsContainer removes the endpoints container. | ||
| // | ||
| // If we do not remove the container, then the user will receive a "network not found" error on using "local up". | ||
| // Here is a sample scenario: | ||
| // 1) User runs "local up" and creates a new local network with an endpoints container. | ||
| // 2) User runs "local down" and stops the endpoints container but does not remove it, however the network is removed. | ||
| // 3) User runs "local up" again and creates a new local network but re-starts the old endpoints container. | ||
| // The old endpoints container tries to connect to the network created in step 1) and fails. | ||
| func removeEndpointsContainer(d containerRemover) { | ||
| ctx, cancel := context.WithTimeout(context.Background(), dockerTimeout) | ||
| defer cancel() | ||
|
|
||
| err := d.ContainerRemove(ctx, localEndpointsContainerName, types.ContainerRemoveOptions{}) | ||
| if err != nil { | ||
| if strings.Contains(strings.ToLower(err.Error()), "no such container") { | ||
| // The containers in the network were already removed by the user using "docker rm", do nothing. | ||
| return | ||
| } | ||
| 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) | ||
| } | ||
|
|
||
| func removeLocalNetwork(d networkRemover) { | ||
| ctx, cancel := context.WithTimeout(context.Background(), dockerTimeout) | ||
| defer cancel() | ||
|
|
||
| err := d.NetworkRemove(ctx, EcsLocalNetworkName) | ||
| if err != nil { | ||
| if strings.Contains(strings.ToLower(err.Error()), "no such network") { | ||
| // The network was removed, do nothing. | ||
| return | ||
| } | ||
| logrus.Fatalf("Failed to remove %s network due to %v", EcsLocalNetworkName, err) | ||
| } | ||
| logrus.Infof("Removed the %s network successfully", EcsLocalNetworkName) | ||
| } | ||
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.
Very minor nit: this change and the other one in teardown.go probably shouldn't be in the "generate mocks" commit.
If you plan to squash all changes to one commit prior to merge, then don't worry about it