Skip to content

Commit

Permalink
Remove ENI ID which is used for RWMutex
Browse files Browse the repository at this point in the history
  • Loading branch information
@xxx0624
xxx0624 committed Sep 3, 2024
1 parent c1ec72f commit 0f2cd58
Show file tree
Hide file tree
Showing 7 changed files with 49 additions and 128 deletions.
1 change: 0 additions & 1 deletion agent/handlers/v4/tmdsstate.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,6 @@ func (s *TMDSAgentState) getTaskMetadata(v3EndpointID string, includeTags bool)
Path: task.GetNetworkNamespace(),
NetworkInterfaces: []*tmdsv4.NetworkInterface{
{
ENIID: "eni-fake-id",
DeviceName: "ethx",
},
},
Expand Down
62 changes: 21 additions & 41 deletions ...dor/github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/fault/v1/handlers/handlers.go
View file

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

2 changes: 0 additions & 2 deletions agent/vendor/github.com/aws/amazon-ecs-agent/ecs-agent/tmds/handlers/v4/state/response.go
View file

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

65 changes: 22 additions & 43 deletions ecs-agent/tmds/handlers/fault/v1/handlers/handlers.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,7 @@ const (

type FaultHandler struct {
// mutexMap is used to avoid multiple clients to manipulate same resource at same
// time. The 'key' is the ENI ID or the network namespace path and 'value' is the
// RWMutex.
// time. The 'key' is the the network namespace path and 'value' is the RWMutex.
// Using concurrent map here because the handler is shared by all requests.
mutexMap sync.Map
AgentState state.AgentState
Expand Down Expand Up @@ -96,8 +95,7 @@ func (h *FaultHandler) StartNetworkBlackholePort() func(http.ResponseWriter, *ht
return
}

// the network blackhole port fault would be injected to given task network
// namespace.
// To avoid multiple requests to manipulate same network resource
networkNSPath := taskMetadata.TaskNetworkConfig.NetworkNamespaces[0].Path
rwMu := h.loadLock(networkNSPath)
rwMu.Lock()
Expand Down Expand Up @@ -148,6 +146,7 @@ func (h *FaultHandler) StopNetworkBlackHolePort() func(http.ResponseWriter, *htt
return
}

// To avoid multiple requests to manipulate same network resource
networkNSPath := taskMetadata.TaskNetworkConfig.NetworkNamespaces[0].Path
rwMu := h.loadLock(networkNSPath)
rwMu.Lock()
Expand Down Expand Up @@ -198,6 +197,7 @@ func (h *FaultHandler) CheckNetworkBlackHolePort() func(http.ResponseWriter, *ht
return
}

// To avoid multiple requests to manipulate same network resource
networkNSPath := taskMetadata.TaskNetworkConfig.NetworkNamespaces[0].Path
rwMu := h.loadLock(networkNSPath)
rwMu.RLock()
Expand Down Expand Up @@ -243,13 +243,9 @@ func (h *FaultHandler) StartNetworkLatency() func(http.ResponseWriter, *http.Req
return
}

// the network latency fault would be injected to given elastic network
// interface/ENI.
eniID := taskMetadata.TaskNetworkConfig.
NetworkNamespaces[0].
NetworkInterfaces[0].
ENIID
rwMu := h.loadLock(eniID)
// To avoid multiple requests to manipulate same network resource
networkNSPath := taskMetadata.TaskNetworkConfig.NetworkNamespaces[0].Path
rwMu := h.loadLock(networkNSPath)
rwMu.Lock()
defer rwMu.Unlock()

Expand Down Expand Up @@ -294,11 +290,9 @@ func (h *FaultHandler) StopNetworkLatency() func(http.ResponseWriter, *http.Requ
return
}

eniID := taskMetadata.TaskNetworkConfig.
NetworkNamespaces[0].
NetworkInterfaces[0].
ENIID
rwMu := h.loadLock(eniID)
// To avoid multiple requests to manipulate same network resource
networkNSPath := taskMetadata.TaskNetworkConfig.NetworkNamespaces[0].Path
rwMu := h.loadLock(networkNSPath)
rwMu.Lock()
defer rwMu.Unlock()

Expand Down Expand Up @@ -343,11 +337,9 @@ func (h *FaultHandler) CheckNetworkLatency() func(http.ResponseWriter, *http.Req
return
}

eniID := taskMetadata.TaskNetworkConfig.
NetworkNamespaces[0].
NetworkInterfaces[0].
ENIID
rwMu := h.loadLock(eniID)
// To avoid multiple requests to manipulate same network resource
networkNSPath := taskMetadata.TaskNetworkConfig.NetworkNamespaces[0].Path
rwMu := h.loadLock(networkNSPath)
rwMu.RLock()
defer rwMu.RUnlock()

Expand Down Expand Up @@ -391,13 +383,9 @@ func (h *FaultHandler) StartNetworkPacketLoss() func(http.ResponseWriter, *http.
return
}

// the network packet loss fault would be injected to given elastic network
// interface/ENI.
eniID := taskMetadata.TaskNetworkConfig.
NetworkNamespaces[0].
NetworkInterfaces[0].
ENIID
rwMu := h.loadLock(eniID)
// To avoid multiple requests to manipulate same network resource
networkNSPath := taskMetadata.TaskNetworkConfig.NetworkNamespaces[0].Path
rwMu := h.loadLock(networkNSPath)
rwMu.Lock()
defer rwMu.Unlock()

Expand Down Expand Up @@ -442,11 +430,9 @@ func (h *FaultHandler) StopNetworkPacketLoss() func(http.ResponseWriter, *http.R
return
}

eniID := taskMetadata.TaskNetworkConfig.
NetworkNamespaces[0].
NetworkInterfaces[0].
ENIID
rwMu := h.loadLock(eniID)
// To avoid multiple requests to manipulate same network resource
networkNSPath := taskMetadata.TaskNetworkConfig.NetworkNamespaces[0].Path
rwMu := h.loadLock(networkNSPath)
rwMu.Lock()
defer rwMu.Unlock()

Expand Down Expand Up @@ -492,11 +478,9 @@ func (h *FaultHandler) CheckNetworkPacketLoss() func(http.ResponseWriter, *http.
return
}

eniID := taskMetadata.TaskNetworkConfig.
NetworkNamespaces[0].
NetworkInterfaces[0].
ENIID
rwMu := h.loadLock(eniID)
// To avoid multiple requests to manipulate same network resource
networkNSPath := taskMetadata.TaskNetworkConfig.NetworkNamespaces[0].Path
rwMu := h.loadLock(networkNSPath)
rwMu.RLock()
defer rwMu.RUnlock()

Expand Down Expand Up @@ -706,10 +690,5 @@ func validateTaskNetworkConfig(taskNetworkConfig *state.TaskNetworkConfig) error
return errors.New("no ENI device name in the network namespace within task network config")
}

// ENIID is required to avoid race condition where multiple requests are manunipuating same ENI.
if taskNetworkConfig.NetworkNamespaces[0].NetworkInterfaces[0].ENIID == "" {
return errors.New("no ENI ID in the network namespace within task network config")
}

return nil
}
44 changes: 6 additions & 38 deletions ecs-agent/tmds/handlers/fault/v1/handlers/handlers_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,28 +50,18 @@ const (
awsvpcNetworkMode = "awsvpc"
deviceName = "eth0"
invalidNetworkMode = "invalid"
eniID = "eni-123"
)

var (
noDeviceNameInNetworkInterfaces = []*state.NetworkInterface{
{
DeviceName: "",
ENIID: eniID,
},
}

noENIIDInNetworkInterfaces = []*state.NetworkInterface{
{
DeviceName: deviceName,
ENIID: "",
},
}

happyNetworkInterfaces = []*state.NetworkInterface{
{
DeviceName: deviceName,
ENIID: eniID,
},
}

Expand Down Expand Up @@ -264,7 +254,7 @@ func generateNetworkBlackHolePortTestCases(name string, expectedHappyResponseBod
agentState.EXPECT().GetTaskMetadata(endpointId).Return(state.TaskResponse{
TaskResponse: &v2.TaskResponse{TaskARN: taskARN},
FaultInjectionEnabled: false,
}, nil)
}, nil).Times(1)
},
},
{
Expand All @@ -280,7 +270,7 @@ func generateNetworkBlackHolePortTestCases(name string, expectedHappyResponseBod
NetworkMode: invalidNetworkMode,
NetworkNamespaces: happyNetworkNamespaces,
},
}, nil)
}, nil).Times(1)
},
},
{
Expand All @@ -294,7 +284,7 @@ func generateNetworkBlackHolePortTestCases(name string, expectedHappyResponseBod
TaskResponse: &v2.TaskResponse{TaskARN: taskARN},
FaultInjectionEnabled: true,
TaskNetworkConfig: nil,
}, nil)
}, nil).Times(1)
},
},
{
Expand All @@ -311,7 +301,7 @@ func generateNetworkBlackHolePortTestCases(name string, expectedHappyResponseBod
NetworkMode: awsvpcNetworkMode,
NetworkNamespaces: nil,
},
}, nil)
}, nil).Times(1)
},
},
{
Expand All @@ -328,7 +318,7 @@ func generateNetworkBlackHolePortTestCases(name string, expectedHappyResponseBod
NetworkMode: awsvpcNetworkMode,
NetworkNamespaces: noPathInNetworkNamespaces,
},
}, nil)
}, nil).Times(1)
},
},
{
Expand All @@ -350,29 +340,7 @@ func generateNetworkBlackHolePortTestCases(name string, expectedHappyResponseBod
},
},
},
}, nil)
},
},
{
name: fmt.Sprintf("%s no ENI ID in task network config", name),
expectedStatusCode: 500,
requestBody: happyBlackHolePortReqBody,
expectedResponseBody: types.NewNetworkFaultInjectionErrorResponse(
fmt.Sprintf("failed to get task metadata due to internal server error for container: %s", endpointId)),
setAgentStateExpectations: func(agentState *mock_state.MockAgentState) {
agentState.EXPECT().GetTaskMetadata(endpointId).Return(state.TaskResponse{
TaskResponse: &v2.TaskResponse{TaskARN: taskARN},
FaultInjectionEnabled: true,
TaskNetworkConfig: &state.TaskNetworkConfig{
NetworkMode: awsvpcNetworkMode,
NetworkNamespaces: []*state.NetworkNamespace{
&state.NetworkNamespace{
Path: "/path",
NetworkInterfaces: noENIIDInNetworkInterfaces,
},
},
},
}, nil)
}, nil).Times(1)
},
},
}
Expand Down
Loading

0 comments on commit 0f2cd58

Please sign in to comment.