-
Notifications
You must be signed in to change notification settings - Fork 632
Route for TMDS access on host mode on IPv6-only instances #4633
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
Conversation
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.
minor feedback, with 1 overall suggestion about having this in the shared library
@@ -136,6 +143,12 @@ func (e *Engine) PreStart() error { | |||
if err != nil { | |||
return engineError("could not disable ipv6 router advertisements", err) | |||
} | |||
// Add a local route for TMDS if there are no IPv4 default routes | |||
log.Info("pre-start: adding routes for TMDS access on IPv6-only host if applicable") |
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.
log.Info("pre-start: adding routes for TMDS access on IPv6-only host if applicable") | |
log.Info("Pre-start: adding routes for TMDS access on IPv6-only host if applicable") |
|
||
netutils "github.com/aws/amazon-ecs-agent/ecs-agent/utils/net" | ||
"github.com/aws/amazon-ecs-agent/ecs-agent/utils/netlinkwrapper" | ||
"github.com/cihub/seelog" |
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.
nit: seelog goes into the third block.
return &TMDSRouteManagerForIPv6Only{nl: netlinkwrapper.New(), tmdsAddr: addr}, nil | ||
} | ||
|
||
// Creates a route to route TMDS traffic to loopback interface. |
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.
nit: godoc starts with the element it describes: https://go.dev/blog/godoc, applicable across the PR.
return nil | ||
} | ||
|
||
seelog.Info("Detected IPv6-only instance: adding route to route TMDS traffic to loopback") |
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.
seelog.Info("Detected IPv6-only instance: adding route to route TMDS traffic to loopback") | |
seelog.Info("Detected IPv6-only instance: adding route to direct TMDS traffic to loopback") |
minor suggestion, "... route to route ... " can be confusing.
@@ -0,0 +1,131 @@ | |||
// Copyright 2015-2016 Amazon.com, Inc. or its affiliates. All Rights Reserved. |
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.
I think it would be useful to have this in the shared library
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.
also update the copyright header btw :)
|
||
if err := nl.RouteAdd(route); err != nil { | ||
if os.IsExist(err) { | ||
seelog.Infof("Route %+v already exists", route) |
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.
seelog.Infof("Route %+v already exists", route) | |
seelog.Debugf("Route %+v already exists", route) |
minor suggestion, to reduce log noise
// Get the loopback interface. | ||
lo, err := nl.LinkByName("lo") | ||
if err != nil { | ||
return nil, fmt.Errorf("error getting lo interface: %v", err) |
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.
return nil, fmt.Errorf("error getting lo interface: %v", err) | |
return nil, fmt.Errorf("error getting lo interface: %w", err) |
to facilitate unwrapping as needed.
Summary
Task Metadata Server (TMDS) is not reachable from host network on IPv6-only instances. Calls fail with
Couldn't connect to server
error.On IPv4-compatible instances, TMDS access from host network works based on an
iptables
PREROUTING table rule to redirect traffic destined to169.254.170.2:80
to127.0.0.1:51679
. However, on an IPv6-only instance there are no IPv4 routes at all which causes the calls to fail even before they hit the PREROUTING table rule.This change solves this problem by creating a route for TMDS traffic to route it to loopback interface. The route is created and cleaned up by
ecs-init
during pre-start and post-stop phases, respectively. The route is created only on IPv6-only instances. The created route is an equivalent ofip route add 169.254.170.2/32 dev lo
.Implementation details
tmdsRouteManagerForIPv6Only
interface is added as a new dependency of ecs-init'sengine
. The interface provides methods to create and remove the route for TMDS access on host network on IPv6-only instances.engine
's pre-start and post-stop methods are updated to calltmdsRouteManagerForIPv6Only
'sCreateRoute
andRemoveRoute
methods, respectively.type TMDSRouteManagerForIPv6Only struct
is added that implementstmdsRouteManagerForIPv6Only
. It'sCreateRoute
andRemoveRoute
methods both check if the instance is IPv6-only and create/remove the route usingnetlink
library if so. To check IPv6-only status of the instance, the methods useHasDefaultRoute
function fromecs-agent/utils/net
package. So, this change makesecs-init
depend onecs-agent
module for the first time. This change also makesnetlink
a dependency ofecs-init
.Testing
Built ecs-init rpm and ran it on an IPv6-only instance. Observed the following.
169.254.170.2 dev lo scope link
is created when ecs-init starts.169.254.170.2
address.Did the same on a dual-stack instance and observed the following.
169.254.170.2
is created when ecs-init starts.New tests cover the changes: yes
Description for the changelog
Enhancement: Create a route for TMDS access on host mode on IPv6-only instances.
Additional Information
Does this PR include breaking model changes? If so, Have you added transformation functions?
No
Does this PR include the addition of new environment variables in the README?
No
Licensing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.