Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
samjkon committed Nov 2, 2023
1 parent 8cf80a1 commit ffdedf8
Show file tree
Hide file tree
Showing 3 changed files with 36 additions and 11 deletions.

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

2 changes: 1 addition & 1 deletion ecs-agent/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ require (
github.com/pkg/errors v0.9.1
github.com/prometheus/client_model v0.2.0
github.com/prometheus/common v0.26.0
github.com/sirupsen/logrus v1.9.0
github.com/stretchr/testify v1.8.2
github.com/vishvananda/netlink v1.2.1-beta.2
go.etcd.io/bbolt v1.3.6
Expand Down Expand Up @@ -50,6 +49,7 @@ require (
github.com/opencontainers/image-spec v1.0.3-0.20211202183452-c5a74bcca799 // indirect
github.com/patrickmn/go-cache v2.1.0+incompatible // indirect
github.com/pmezard/go-difflib v1.0.0 // indirect
github.com/sirupsen/logrus v1.9.0 // indirect
github.com/vishvananda/netns v0.0.0-20210104183010-2eb08e3e575f // indirect
go.opencensus.io v0.24.0 // indirect
golang.org/x/mod v0.10.0 // indirect
Expand Down
42 changes: 32 additions & 10 deletions 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"
"github.com/aws/amazon-ecs-agent/ecs-agent/metrics"
mock_metrics "github.com/aws/amazon-ecs-agent/ecs-agent/metrics/mocks"
"testing"

"github.com/aws/amazon-ecs-agent/ecs-agent/acs/model/ecsacs"
Expand Down Expand Up @@ -95,8 +97,11 @@ func testNetworkBuilder_StartAWSVPC(t *testing.T) {

ctx := context.TODO()
platformAPI := mock_platform.NewMockAPI(ctrl)
metricsFactory := mock_metrics.NewMockEntryFactory(ctrl)
mockEntry := mock_metrics.NewMockEntry(ctrl)
netBuilder := &networkBuilder{
platformAPI: platformAPI,
platformAPI: platformAPI,
metricsFactory: metricsFactory,
}

// Single ENI use case without AppMesh and service connect configs.
Expand All @@ -108,7 +113,7 @@ func testNetworkBuilder_StartAWSVPC(t *testing.T) {
netNS.DesiredState = status.NetworkReadyPull
t.Run("single-eni-default", func(*testing.T) {
gomock.InOrder(
getExpectedCalls_StartAWSVPC(ctx, platformAPI, netNS)...,
getExpectedCalls_StartAWSVPC(ctx, platformAPI, metricsFactory, mockEntry, netNS)...,
)
netBuilder.Start(ctx, ecs.NetworkModeAwsvpc, taskID, netNS)
})
Expand All @@ -119,9 +124,10 @@ func testNetworkBuilder_StartAWSVPC(t *testing.T) {
// Placeholder data.
ContainerName: "appmesh-envoy",
}
mockEntry = mock_metrics.NewMockEntry(ctrl)
t.Run("single-eni-appmesh-readypull", func(*testing.T) {
gomock.InOrder(
getExpectedCalls_StartAWSVPC(ctx, platformAPI, netNS)...,
getExpectedCalls_StartAWSVPC(ctx, platformAPI, metricsFactory, mockEntry, netNS)...,
)
netBuilder.Start(ctx, ecs.NetworkModeAwsvpc, taskID, netNS)
})
Expand All @@ -130,9 +136,10 @@ func testNetworkBuilder_StartAWSVPC(t *testing.T) {
// The appmesh configuration should get executed now.
netNS.KnownState = status.NetworkReadyPull
netNS.DesiredState = status.NetworkReady
mockEntry = mock_metrics.NewMockEntry(ctrl)
t.Run("single-eni-appmesh-ready", func(*testing.T) {
gomock.InOrder(
getExpectedCalls_StartAWSVPC(ctx, platformAPI, netNS)...,
getExpectedCalls_StartAWSVPC(ctx, platformAPI, metricsFactory, mockEntry, netNS)...,
)
netBuilder.Start(ctx, ecs.NetworkModeAwsvpc, taskID, netNS)
})
Expand All @@ -143,9 +150,10 @@ func testNetworkBuilder_StartAWSVPC(t *testing.T) {
netNS.ServiceConnectConfig = &serviceconnect.ServiceConnectConfig{
ServiceConnectContainerName: "ecs-service-connect",
}
mockEntry = mock_metrics.NewMockEntry(ctrl)
t.Run("single-eni-serviceconnect-ready", func(*testing.T) {
gomock.InOrder(
getExpectedCalls_StartAWSVPC(ctx, platformAPI, netNS)...,
getExpectedCalls_StartAWSVPC(ctx, platformAPI, metricsFactory, mockEntry, netNS)...,
)
netBuilder.Start(ctx, ecs.NetworkModeAwsvpc, taskID, netNS)
})
Expand All @@ -154,29 +162,32 @@ func testNetworkBuilder_StartAWSVPC(t *testing.T) {
// In this case, the ServiceConnect configuration should not be executed.
netNS.KnownState = status.NetworkReadyPull
netNS.DesiredState = status.NetworkReady
mockEntry = mock_metrics.NewMockEntry(ctrl)
t.Run("single-eni-serviceconnect-readypull", func(*testing.T) {
gomock.InOrder(
getExpectedCalls_StartAWSVPC(ctx, platformAPI, netNS)...,
getExpectedCalls_StartAWSVPC(ctx, platformAPI, metricsFactory, mockEntry, netNS)...,
)
netBuilder.Start(ctx, ecs.NetworkModeAwsvpc, taskID, netNS)
})

// Single netns with multi interface case.
_, taskNetConfig = getSingleNetNSMultiIfaceAWSVPCTestData(taskID)
netNS = taskNetConfig.GetPrimaryNetNS()
mockEntry = mock_metrics.NewMockEntry(ctrl)
t.Run("multi-eni-default", func(*testing.T) {
gomock.InOrder(
getExpectedCalls_StartAWSVPC(ctx, platformAPI, netNS)...,
getExpectedCalls_StartAWSVPC(ctx, platformAPI, metricsFactory, mockEntry, netNS)...,
)
netBuilder.Start(ctx, ecs.NetworkModeAwsvpc, taskID, netNS)
})

// Desired state = DELETED. There should be no expected calls to
// platform APIs for this case.
netNS.DesiredState = status.NetworkDeleted
mockEntry = mock_metrics.NewMockEntry(ctrl)
t.Run("deleted", func(*testing.T) {
gomock.InOrder(
getExpectedCalls_StartAWSVPC(ctx, platformAPI, netNS)...,
getExpectedCalls_StartAWSVPC(ctx, platformAPI, metricsFactory, mockEntry, netNS)...,
)
netBuilder.Start(ctx, ecs.NetworkModeAwsvpc, taskID, netNS)
})
Expand All @@ -190,14 +201,23 @@ func testNetworkBuilder_StartAWSVPC(t *testing.T) {
func getExpectedCalls_StartAWSVPC(
ctx context.Context,
platformAPI *mock_platform.MockAPI,
metricsFactory *mock_metrics.MockEntryFactory,
mockEntry *mock_metrics.MockEntry,
netNS *tasknetworkconfig.NetworkNamespace,
) []*gomock.Call {
var calls []*gomock.Call

calls = append(calls,
metricsFactory.EXPECT().New(metrics.BuildNetworkNamespaceMetricName).Return(mockEntry).Times(1),
mockEntry.EXPECT().WithFields(gomock.Any()).Return(mockEntry).Times(1))

// Start() should not be invoked when desired state = DELETED.
if netNS.DesiredState == status.NetworkDeleted {
return nil
calls = append(calls,
mockEntry.EXPECT().Done(gomock.Any()).Times(1))
return calls
}

var calls []*gomock.Call
// Network namespace creation and DNS config files creation is to happen
// only while transitioning from NONE to READY_PULL.
if netNS.KnownState == status.NetworkNone &&
Expand Down Expand Up @@ -231,5 +251,7 @@ func getExpectedCalls_StartAWSVPC(
}
}

calls = append(calls, mockEntry.EXPECT().Done(nil).Times(1))

return calls
}

0 comments on commit ffdedf8

Please sign in to comment.