Skip to content
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

Set device names while building task network config #4026

Merged
merged 2 commits into from
Nov 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions ecs-agent/netlib/common_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ func getTestInterfacesData() ([]*ecsacs.ElasticNetworkInterface, []networkinterf
Default: true,
KnownStatus: status.NetworkNone,
DesiredStatus: status.NetworkReadyPull,
DeviceName: "eth1",
},
{
ID: eniID2,
Expand All @@ -253,6 +254,7 @@ func getTestInterfacesData() ([]*ecsacs.ElasticNetworkInterface, []networkinterf
Index: int64(1),
KnownStatus: status.NetworkNone,
DesiredStatus: status.NetworkReadyPull,
DeviceName: "eth2",
},
}

Expand Down
2 changes: 2 additions & 0 deletions ecs-agent/netlib/model/networkinterface/networkinterface.go
Original file line number Diff line number Diff line change
Expand Up @@ -475,6 +475,7 @@ func New(
acsENI *ecsacs.ElasticNetworkInterface,
guestNetNSName string,
peerInterface *ecsacs.ElasticNetworkInterface,
macToName map[string]string,
) (*NetworkInterface, error) {
var err error

Expand Down Expand Up @@ -519,6 +520,7 @@ func New(
networkInterface.KnownStatus = status.NetworkNone
networkInterface.DesiredStatus = status.NetworkReadyPull
networkInterface.GuestNetNSName = guestNetNSName
networkInterface.setDeviceName(macToName)

return networkInterface, nil
}
Expand Down
2 changes: 2 additions & 0 deletions ecs-agent/netlib/network_builder.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -54,6 +55,7 @@ func NewNetworkBuilder(
platformString,
volumeAccessor,
stateDBDir,
netwrapper.NewNet(),
)
if err != nil {
return nil, errors.Wrap(err, "failed to instantiate network builder")
Expand Down
26 changes: 25 additions & 1 deletion ecs-agent/netlib/network_builder_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ package netlib
import (
"context"
"encoding/json"
"fmt"
"net"
"testing"

"github.com/aws/amazon-ecs-agent/ecs-agent/acs/model/ecsacs"
Expand All @@ -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"
)
Expand Down Expand Up @@ -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)
Expand Down
35 changes: 31 additions & 4 deletions ecs-agent/netlib/platform/common_linux.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down Expand Up @@ -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
Expand All @@ -85,6 +87,7 @@ func NewPlatform(
platformString string,
volumeAccessor volume.VolumeAccessor,
stateDBDirectory string,
netWrapper netwrapper.Net,
) (API, error) {
commonPlatform := common{
nsUtil: ecscni.NewNetNSUtil(),
Expand All @@ -94,6 +97,7 @@ func NewPlatform(
netlink: netlinkwrapper.New(),
stateDBDir: stateDBDirectory,
cniClient: ecscni.NewCNIClient([]string{CNIPluginPathDefault}),
net: netWrapper,
}

// TODO: implement remaining platforms - FoF, windows.
Expand Down Expand Up @@ -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.
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
Expand All @@ -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
}
Expand Down Expand Up @@ -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
}
43 changes: 41 additions & 2 deletions ecs-agent/netlib/platform/common_linux_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (
"errors"
"fmt"
"io/fs"
"net"
"os"
"path/filepath"
"testing"
Expand All @@ -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"

Expand All @@ -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)
}

Expand Down Expand Up @@ -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()
Expand Down
4 changes: 3 additions & 1 deletion ecs-agent/netlib/platform/containerd_windows.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
)

Expand All @@ -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
}

Expand Down