Set device names while building task network config (aws#4026)

* Set device names while building task network config Devices names are the names assigned to the interfaces on the host. For ex, eth1. We need to specify what device names should be assigned to each interface when they are being configured by the CNI plugins.
tshan2001 · Nov 17, 2023 · 72a3c4a · 72a3c4a
1 parent 403b4ce
commit 72a3c4a
Show file tree

Hide file tree

Showing 8 changed files with 108 additions and 8 deletions.
diff --git a/...thub.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface/networkinterface.go b/...thub.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface/networkinterface.go
diff --git a/ecs-agent/netlib/common_test.go b/ecs-agent/netlib/common_test.go
@@ -229,6 +229,7 @@ func getTestInterfacesData() ([]*ecsacs.ElasticNetworkInterface, []networkinterf
 			Default:                      true,
 			KnownStatus:                  status.NetworkNone,
 			DesiredStatus:                status.NetworkReadyPull,
+			DeviceName:                   "eth1",
 		},
 		{
 			ID:         eniID2,
@@ -253,6 +254,7 @@ func getTestInterfacesData() ([]*ecsacs.ElasticNetworkInterface, []networkinterf
 			Index:                        int64(1),
 			KnownStatus:                  status.NetworkNone,
 			DesiredStatus:                status.NetworkReadyPull,
+			DeviceName:                   "eth2",
 		},
 	}
 

diff --git a/ecs-agent/netlib/model/networkinterface/networkinterface.go b/ecs-agent/netlib/model/networkinterface/networkinterface.go
@@ -475,6 +475,7 @@ func New(
 	acsENI *ecsacs.ElasticNetworkInterface,
 	guestNetNSName string,
 	peerInterface *ecsacs.ElasticNetworkInterface,
+	macToName map[string]string,
 ) (*NetworkInterface, error) {
 	var err error
 
@@ -519,6 +520,7 @@ func New(
 	networkInterface.KnownStatus = status.NetworkNone
 	networkInterface.DesiredStatus = status.NetworkReadyPull
 	networkInterface.GuestNetNSName = guestNetNSName
+	networkInterface.setDeviceName(macToName)
 
 	return networkInterface, nil
 }

diff --git a/ecs-agent/netlib/network_builder.go b/ecs-agent/netlib/network_builder.go
@@ -25,6 +25,7 @@ import (
 	"github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/status"
 	"github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/tasknetworkconfig"
 	"github.com/aws/amazon-ecs-agent/ecs-agent/netlib/platform"
+	"github.com/aws/amazon-ecs-agent/ecs-agent/utils/netwrapper"
 	"github.com/aws/amazon-ecs-agent/ecs-agent/volume"
 
 	"github.com/pkg/errors"
@@ -54,6 +55,7 @@ func NewNetworkBuilder(
 		platformString,
 		volumeAccessor,
 		stateDBDir,
+		netwrapper.NewNet(),
 	)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to instantiate network builder")

diff --git a/ecs-agent/netlib/network_builder_linux_test.go b/ecs-agent/netlib/network_builder_linux_test.go
@@ -19,6 +19,8 @@ package netlib
 import (
 	"context"
 	"encoding/json"
+	"fmt"
+	"net"
 	"testing"
 
 	"github.com/aws/amazon-ecs-agent/ecs-agent/acs/model/ecsacs"
@@ -32,7 +34,9 @@ import (
 	"github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/tasknetworkconfig"
 	"github.com/aws/amazon-ecs-agent/ecs-agent/netlib/platform"
 	mock_platform "github.com/aws/amazon-ecs-agent/ecs-agent/netlib/platform/mocks"
+	mock_netwrapper "github.com/aws/amazon-ecs-agent/ecs-agent/utils/netwrapper/mocks"
 
+	"github.com/aws/aws-sdk-go/aws"
 	"github.com/golang/mock/gomock"
 	"github.com/stretchr/testify/require"
 )
@@ -66,13 +70,33 @@ func TestNetworkBuilder_Start(t *testing.T) {
 func getTestFunc(dataGenF func(string) (input *ecsacs.Task, expected tasknetworkconfig.TaskNetworkConfig)) func(*testing.T) {
 
 	return func(t *testing.T) {
+		ctrl := gomock.NewController(t)
+		defer ctrl.Finish()
+
 		// Create a networkBuilder for the warmpool platform.
-		netBuilder, err := NewNetworkBuilder(platform.WarmpoolPlatform, nil, nil, data.Client{}, "")
+		mockNet := mock_netwrapper.NewMockNet(ctrl)
+		platformAPI, err := platform.NewPlatform(platform.WarmpoolPlatform, nil, "", mockNet)
 		require.NoError(t, err)
+		netBuilder := &networkBuilder{
+			platformAPI: platformAPI,
+		}
 
 		// Generate input task payload and a reference to verify the output with.
 		taskPayload, expectedConfig := dataGenF(taskID)
 
+		var ifaces []net.Interface
+		idx := 1
+		for _, eni := range taskPayload.ElasticNetworkInterfaces {
+			hw, err := net.ParseMAC(aws.StringValue(eni.MacAddress))
+			require.NoError(t, err)
+			ifaces = append(ifaces, net.Interface{
+				HardwareAddr: hw,
+				Name:         fmt.Sprintf("eth%d", idx),
+			})
+			idx += 1
+		}
+		mockNet.EXPECT().Interfaces().Return(ifaces, nil).Times(1)
+
 		// Invoke networkBuilder function for building the task network config.
 		actualConfig, err := netBuilder.BuildTaskNetworkConfiguration(taskID, taskPayload)
 		require.NoError(t, err)

diff --git a/ecs-agent/netlib/platform/common_linux.go b/ecs-agent/netlib/platform/common_linux.go
@@ -33,6 +33,7 @@ import (
 	"github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/tasknetworkconfig"
 	"github.com/aws/amazon-ecs-agent/ecs-agent/utils/ioutilwrapper"
 	"github.com/aws/amazon-ecs-agent/ecs-agent/utils/netlinkwrapper"
+	"github.com/aws/amazon-ecs-agent/ecs-agent/utils/netwrapper"
 	"github.com/aws/amazon-ecs-agent/ecs-agent/utils/oswrapper"
 	"github.com/aws/amazon-ecs-agent/ecs-agent/volume"
 
@@ -77,6 +78,7 @@ type common struct {
 	netlink            netlinkwrapper.NetLink
 	stateDBDir         string
 	cniClient          ecscni.CNI
+	net                netwrapper.Net
 }
 
 // NewPlatform creates an implementation of the platform API depending on the
@@ -85,6 +87,7 @@ func NewPlatform(
 	platformString string,
 	volumeAccessor volume.VolumeAccessor,
 	stateDBDirectory string,
+	netWrapper netwrapper.Net,
 ) (API, error) {
 	commonPlatform := common{
 		nsUtil:             ecscni.NewNetNSUtil(),
@@ -94,6 +97,7 @@ func NewPlatform(
 		netlink:            netlinkwrapper.New(),
 		stateDBDir:         stateDBDirectory,
 		cniClient:          ecscni.NewCNIClient([]string{CNIPluginPathDefault}),
+		net:                netWrapper,
 	}
 
 	// TODO: implement remaining platforms - FoF, windows.
@@ -155,6 +159,10 @@ func (c *common) buildAWSVPCNetworkNamespaces(taskID string,
 		return nil, errors.New("interfaces list cannot be empty")
 	}
 
+	macToNames, err := c.interfacesMACToName()
+	if err != nil {
+		return nil, err
+	}
 	// If task payload has only one interface, the network configuration is
 	// straight forward. It will have only one network namespace containing
 	// the corresponding network interface.
@@ -166,7 +174,8 @@ func (c *common) buildAWSVPCNetworkNamespaces(taskID string,
 		primaryNetNS, err := c.buildNetNS(taskID,
 			0,
 			taskPayload.ElasticNetworkInterfaces,
-			taskPayload.ProxyConfiguration)
+			taskPayload.ProxyConfiguration,
+			macToNames)
 		if err != nil {
 			return nil, err
 		}
@@ -217,7 +226,7 @@ func (c *common) buildAWSVPCNetworkNamespaces(taskID string,
 			continue
 		}
 
-		netNS, err := c.buildNetNS(taskID, nsIndex, ifaces, nil)
+		netNS, err := c.buildNetNS(taskID, nsIndex, ifaces, nil, macToNames)
 		if err != nil {
 			return nil, err
 		}
@@ -232,12 +241,13 @@ func (c *common) buildNetNS(
 	taskID string,
 	index int,
 	networkInterfaces []*ecsacs.ElasticNetworkInterface,
-	proxyConfig *ecsacs.ProxyConfiguration) (*tasknetworkconfig.NetworkNamespace, error) {
+	proxyConfig *ecsacs.ProxyConfiguration,
+	macToName map[string]string) (*tasknetworkconfig.NetworkNamespace, error) {
 	var primaryIF *networkinterface.NetworkInterface
 	var ifaces []*networkinterface.NetworkInterface
 	lowestIdx := int64(indexHighValue)
 	for _, ni := range networkInterfaces {
-		iface, err := networkinterface.New(ni, "", nil)
+		iface, err := networkinterface.New(ni, "", nil, macToName)
 		if err != nil {
 			return nil, err
 		}
@@ -608,3 +618,20 @@ func (c *common) configureServiceConnect(
 
 	return nil
 }
+
+// interfacesMACToName lists all network interfaces on the host inside the default
+// netns and returns a mac address to device name map.
+func (c *common) interfacesMACToName() (map[string]string, error) {
+	links, err := c.net.Interfaces()
+	if err != nil {
+		return nil, err
+	}
+
+	// Build a map of interface MAC address to name on the host.
+	macToName := make(map[string]string)
+	for _, link := range links {
+		macToName[link.HardwareAddr.String()] = link.Name
+	}
+
+	return macToName, nil
+}
diff --git a/ecs-agent/netlib/platform/common_linux_test.go b/ecs-agent/netlib/platform/common_linux_test.go
@@ -21,6 +21,7 @@ import (
 	"errors"
 	"fmt"
 	"io/fs"
+	"net"
 	"os"
 	"path/filepath"
 	"testing"
@@ -33,6 +34,7 @@ import (
 	"github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/tasknetworkconfig"
 	mock_ioutilwrapper "github.com/aws/amazon-ecs-agent/ecs-agent/utils/ioutilwrapper/mocks"
 	mock_netlinkwrapper "github.com/aws/amazon-ecs-agent/ecs-agent/utils/netlinkwrapper/mocks"
+	mock_netwrapper "github.com/aws/amazon-ecs-agent/ecs-agent/utils/netwrapper/mocks"
 	mock_oswrapper "github.com/aws/amazon-ecs-agent/ecs-agent/utils/oswrapper/mocks"
 	mock_volume "github.com/aws/amazon-ecs-agent/ecs-agent/volume/mocks"
 
@@ -58,10 +60,10 @@ const (
 )
 
 func TestNewPlatform(t *testing.T) {
-	_, err := NewPlatform(WarmpoolPlatform, nil, "")
+	_, err := NewPlatform(WarmpoolPlatform, nil, "", nil)
 	assert.NoError(t, err)
 
-	_, err = NewPlatform("invalid-platform", nil, "")
+	_, err = NewPlatform("invalid-platform", nil, "", nil)
 	assert.Error(t, err)
 }
 
@@ -205,6 +207,43 @@ func TestCommon_ConfigureInterface(t *testing.T) {
 	t.Run("create-branch-eni", testBranchENIConfiguration)
 }
 
+// TestInterfacesMACToName verifies interfacesMACToName behaves as expected.
+func TestInterfacesMACToName(t *testing.T) {
+	ctrl := gomock.NewController(t)
+	defer ctrl.Finish()
+
+	mockNet := mock_netwrapper.NewMockNet(ctrl)
+	commonPlatform := &common{
+		net: mockNet,
+	}
+
+	// Prepare test data.
+	testMac, err := net.ParseMAC(trunkENIMac)
+	require.NoError(t, err)
+	testIface := []net.Interface{
+		{
+			HardwareAddr: testMac,
+			Name:         "eth1",
+		},
+	}
+	expected := map[string]string{
+		trunkENIMac: "eth1",
+	}
+
+	// Positive case.
+	mockNet.EXPECT().Interfaces().Return(testIface, nil).Times(1)
+	actual, err := commonPlatform.interfacesMACToName()
+	require.NoError(t, err)
+	require.Equal(t, expected, actual)
+
+	// Negative case.
+	testErr := errors.New("no interfaces to chat with")
+	mockNet.EXPECT().Interfaces().Return(testIface, testErr).Times(1)
+	_, err = commonPlatform.interfacesMACToName()
+	require.Error(t, err)
+	require.Equal(t, testErr, err)
+}
+
 func testRegularENIConfiguration(t *testing.T) {
 	ctrl := gomock.NewController(t)
 	defer ctrl.Finish()

diff --git a/ecs-agent/netlib/platform/containerd_windows.go b/ecs-agent/netlib/platform/containerd_windows.go
@@ -25,6 +25,7 @@ import (
 	"github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/networkinterface"
 	"github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/serviceconnect"
 	"github.com/aws/amazon-ecs-agent/ecs-agent/netlib/model/tasknetworkconfig"
+	"github.com/aws/amazon-ecs-agent/ecs-agent/utils/netwrapper"
 	"github.com/aws/amazon-ecs-agent/ecs-agent/volume"
 )
 
@@ -36,7 +37,8 @@ type common struct {
 func NewPlatform(
 	platformString string,
 	volumeAccessor volume.VolumeAccessor,
-	stateDBDirectory string) (API, error) {
+	stateDBDirectory string,
+	netWrapper netwrapper.Net) (API, error) {
 	return nil, nil
 }