Skip to content

Commit 75ff6a0

Browse files
authored
Merge pull request aws#4830 from aviral92/managed-daemons-networking-fix
Fix for pre-mature deletion of veth pair for daemon-host network name…
2 parents 966c44d + 69d8757 commit 75ff6a0

File tree

3 files changed

+361
-20
lines changed

3 files changed

+361
-20
lines changed

ecs-agent/netlib/platform/cniconf_linux.go

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ const (
5454
VPCTunnelInterfaceTypeTap = "tap"
5555

5656
BridgeInterfaceName = "fargate-bridge"
57+
VethInterfaceType = "veth"
5758

5859
IPAMDataFileName = "eni-ipam.db"
5960

ecs-agent/netlib/platform/managed_linux.go

Lines changed: 65 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ import (
2222

2323
"github.com/aws/aws-sdk-go-v2/aws"
2424
"github.com/aws/aws-sdk-go-v2/service/ecs/types"
25+
cnins "github.com/containernetworking/plugins/pkg/ns"
2526
"github.com/pkg/errors"
2627
)
2728

@@ -34,6 +35,7 @@ const (
3435
InstanceIDResource = "instance-id"
3536
DefaultArg = "default"
3637
NetworkInterfaceDeviceName = "eth1" // default network interface name in the task network namespace.
38+
DaemonInterfaceName = "eth0" // daemon network interface name in the daemon network namespace.
3739
)
3840

3941
type managedLinux struct {
@@ -463,23 +465,27 @@ func (m *managedLinux) configureDaemonNetNS(ctx context.Context, taskID string,
463465
return err
464466
}
465467

466-
// Create MI-Bridge for daemon-bridge mode
467-
var cniNetConf []ecscni.PluginConfig
468-
cniNetConf = append(cniNetConf, createDaemonBridgePluginConfig(netNS.Path))
469-
add := true
470-
471-
_, err = m.common.executeCNIPlugin(ctx, add, cniNetConf...)
472-
if err != nil {
473-
err = errors.Wrap(err, "failed to setup daemon network namespace bridge")
474-
return err
475-
}
476-
477-
// Add NAT masquerade rule for external connectivity
478-
err = m.addDaemonBridgeNATRule()
479-
if err != nil {
480-
logger.Warn("Failed to add NAT rule for daemon-bridge", logger.Fields{
481-
loggerfield.Error: err,
482-
})
468+
// Create MI-Bridge for daemon-bridge mode only if not already configured
469+
if !m.isDaemonNamespaceConfigured(netNS.Path) {
470+
var cniNetConf []ecscni.PluginConfig
471+
cniNetConf = append(cniNetConf, createDaemonBridgePluginConfig(netNS.Path))
472+
add := true
473+
474+
_, err = m.common.executeCNIPlugin(ctx, add, cniNetConf...)
475+
if err != nil {
476+
err = errors.Wrap(err, "failed to setup daemon network namespace bridge")
477+
return err
478+
}
479+
480+
// Add NAT masquerade rule for external connectivity
481+
err = m.addDaemonBridgeNATRule()
482+
if err != nil {
483+
logger.Warn("Failed to add NAT rule for daemon-bridge", logger.Fields{
484+
loggerfield.Error: err,
485+
})
486+
}
487+
} else {
488+
logger.Info("Daemon namespace already configured, skipping CNI plugin setup")
483489
}
484490
}
485491
return nil
@@ -509,15 +515,27 @@ func (m *managedLinux) addDaemonBridgeNATRule() error {
509515

510516
// StopDaemonNetNS stops and cleans up a daemon network namespace.
511517
func (m *managedLinux) StopDaemonNetNS(ctx context.Context, netNS *tasknetworkconfig.NetworkNamespace) error {
518+
logger.Info("Starting StopDaemonNetNS", logger.Fields{
519+
"netNSPath": netNS.Path,
520+
"knownState": netNS.KnownState,
521+
})
512522

513523
// Cleanup bridge config(veth pair).
514524
var cniNetConf []ecscni.PluginConfig
515-
cniNetConf = append(cniNetConf, createBridgePluginConfig(netNS.Path))
525+
cniNetConf = append(cniNetConf, createDaemonBridgePluginConfig(netNS.Path))
516526
add := false
517527

518528
_, err := m.common.executeCNIPlugin(ctx, add, cniNetConf...)
519529
if err != nil {
520530
err = errors.Wrap(err, "failed to stop daemon network namespace bridge")
531+
logger.Error("StopDaemonNetNS failed", logger.Fields{
532+
"netNSPath": netNS.Path,
533+
loggerfield.Error: err,
534+
})
535+
} else {
536+
logger.Info("StopDaemonNetNS completed successfully", logger.Fields{
537+
"netNSPath": netNS.Path,
538+
})
521539
}
522540

523541
// NOTE: The daemon network namespace is intentionally not deleted here.
@@ -527,3 +545,32 @@ func (m *managedLinux) StopDaemonNetNS(ctx context.Context, netNS *tasknetworkco
527545

528546
return err
529547
}
548+
549+
// isDaemonNamespaceConfigured checks if the daemon namespace is already properly configured
550+
// by verifying that the veth interface exists in the daemon network namespace.
551+
func (m *managedLinux) isDaemonNamespaceConfigured(netNSPath string) bool {
552+
var configured bool
553+
554+
// Execute within the network namespace to check interfaces
555+
err := m.nsUtil.ExecInNSPath(netNSPath, func(_ cnins.NetNS) error {
556+
// Check if eth0 veth interface exists in daemon namespace
557+
link, err := m.netlink.LinkByName(DaemonInterfaceName)
558+
if err != nil {
559+
return errors.New("eth0 interface not found in daemon namespace")
560+
}
561+
562+
// Verify it's a veth interface
563+
if link.Type() != VethInterfaceType {
564+
return errors.New("eth0 is not a veth interface")
565+
}
566+
567+
configured = true
568+
return nil
569+
})
570+
571+
if err != nil || !configured {
572+
return false
573+
}
574+
575+
return true
576+
}

0 commit comments

Comments
 (0)