Set static MAC address when creating ecs-bridge bridge #111
Conversation
plugins/ecs-bridge/engine/engine.go
Outdated
| "bridge create: unable to generate mac addr %s", err) | ||
| } | ||
|
|
||
| log.Infof("Setting ecs-bridge hardware addr (MAC) %v %v", bridge, mac) |
There was a problem hiding this comment.
Looking at command.go and engine.go, it seems the logging is done by command.go before calling the implementation of engine in engine.go. We may want to stick with that convention and remove the log statement here.
There was a problem hiding this comment.
I would prefer to keep this log statement. Is there any drawback to logging this? Historically we've generally had issues with these plugins under-logging and not telling us what they are doing, rather than the opposite.
There was a problem hiding this comment.
there's virtually no risk of this spamming the logfile, as this function is generally only called once per instance.
There was a problem hiding this comment.
We still log the error if the operation fails, correct?
There was a problem hiding this comment.
yes, but issues where there's an explicit failure are always the easiest to debug :)
There was a problem hiding this comment.
I still don't think this log statement provides any additional value, but don't have a strong opinion. Let's go with your approach.
| @@ -14,10 +14,13 @@ | |||
| package engine | |||
There was a problem hiding this comment.
We should add an integration test which creates a bridge using this engine interface, attaches a dummy interface with a lower MAC address than the bridge MAC address. and confirms that the bridge MAC address does not change. The current mock tests won't work (since we need a real interface). However given the urgency of this fix, I don't want us to delay this PR. I can also add such a test as part of our unified networking library program.
The ecs-bridge bridge currently is created without setting a static MAC address. This means that the bridge MAC address inherits the 'lowest' MAC address of all of it's connected veth interfaces. What this means for ECS tasks using awsvpc networking mode is that the MAC address of ecs-bridge can be changing as tasks are killed an added to an instance (on the ec2 launch type). This presents a race condition where some tasks might have an arpcache entry pointing to the old MAC address when a task is killed and the ecs-bridge MAC changes. In rare cases this can lead to brief periods of lost network connectivity within the local network on the host (such as requests to the ECS Task Metadata Endpoint or the ECS credentials endpoint). By setting a static MAC address at creation we disable this behavior of inheriting the 'lowest' MAC address. This means that the ecs-bridge MAC stays constant through the entire lifetime of the ECS container instance.
sparrc
force-pushed
the
create-bridge-w-rand-mac
branch
from
e208180 to
e360eaa
Compare
This pulls in a fix for an error that can occur when using awsvpc networking mode where a stale MAC address can cause temporary loss of network connectivity in a container. Pulls in the following PRs with the fix and comments: aws/amazon-ecs-cni-plugins#111 aws/amazon-ecs-cni-plugins#112
This pulls in a fix for an error that can occur when using awsvpc networking mode where a stale MAC address can cause temporary loss of network connectivity in a container. Pulls in the following PRs with the fix and comments: aws/amazon-ecs-cni-plugins#111 aws/amazon-ecs-cni-plugins#112
This pulls in a fix for an error that can occur when using awsvpc networking mode where a stale MAC address can cause temporary loss of network connectivity in a container. Pulls in the following PRs with the fix and comments: aws/amazon-ecs-cni-plugins#111 aws/amazon-ecs-cni-plugins#112
The ecs-bridge bridge currently is created without setting a static MAC address. This means that the bridge MAC address inherits the 'lowest' MAC address of all of it's connected veth interfaces.
What this means for ECS tasks using awsvpc networking mode is that the MAC address of ecs-bridge can be changing as tasks are killed an added to an instance (on the ec2 launch type).
This presents a race condition where some tasks might have an arpcache entry pointing to the old MAC address when a task is killed and the ecs-bridge MAC changes. In rare cases this can lead to brief periods of lost network connectivity within the local network on the host (such as requests to the ECS Task Metadata Endpoint or the ECS credentials endpoint).
By setting a static MAC address at creation we disable this behavior of inheriting the 'lowest' MAC address. This means that the ecs-bridge MAC stays constant through the entire lifetime of the ECS container instance.
Testing
Tested on a container instance using awsvpc network mode. Verified that MAC address is set and stays constant through many task starts/stops.
Ran ECS agent functional testing suite.
Description for the changelog
bugfix: Set static MAC address when creating ecs-bridge bridge
Licensing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.