Adding start and stop black hole port fault implementation #4355
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
mye956
force-pushed
the
bhp-startstop
branch
4 times, most recently
from
9fe4569 to
da7e8d8
Compare
mye956
changed the title
xxx0624
reviewed
Sep 20, 2024
| cmdOutput, cmdErr := h.stopNetworkBlackHolePort(ctxWithTimeout, aws.StringValue(request.Protocol), port, chainName, | ||
| taskMetadata.TaskNetworkConfig.NetworkMode, taskMetadata.TaskNetworkConfig.NetworkNamespaces[0].Path, insertTable) | ||
|
|
||
| if err := ctx.Err(); err == context.DeadlineExceeded { |
There was a problem hiding this comment.
How about err != DeadlineExceeded
There was a problem hiding this comment.
Hmmm, was hoping to check for context timeout has been reached here. All other errors will still be an internal error but will include the specific error in the error response.
mye956
force-pushed
the
bhp-startstop
branch
2 times, most recently
from
b2326b7 to
e9709f0
Compare
tshan2001
previously approved these changes
Sep 23, 2024
mye956
force-pushed
the
bhp-startstop
branch
2 times, most recently
from
5470641 to
cff31c8
Compare
mye956
force-pushed
the
bhp-startstop
branch
from
cff31c8 to
9027b49
Compare
tshan2001
approved these changes
Sep 24, 2024
xxx0624
approved these changes
Sep 24, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
This PR will introduce both start and stop network black hole port fault injection into the
ecs-agentdirectory. It does so by makingiptablescommands viaos/exec.Implementation details
We will be adding two new functions,
startNetworkBlackholePort()andstopNetworkBlackHolePort(), into theecs-agent/tmds/handlers/fault/v1/handlers/handlers.gofile.startNetworkBlackholePort(): This function is responsible for starting/injecting a new network black hole port with the specified traffic type, protocol, and port number that's passed from the request body. It is called inStartNetworkBlackholePort(). The general workflow of this function is as followed:iptables -N <chain>(the chain name is in the form of "--")iptables -A <chain> -p <protocol> --dport <port> -j DROPstopNetworkBlackHolePort(): This function is responsible for stopping a specific network black hole port with the specified traffic type, protocol, and port number that's passed from the request body. It is called inStopNetworkBlackHolePort(). The general workflow of this function is as followed:iptables -F <chain>iptables -D <INPUT/OUTPUT> -j <chain>iptables -X <chain>Similar to
CheckNetworkBlackHolePort(), bothStartNetworkBlackholePort()andStopNetworkBlackHolePort()handler functions will also have the following checks before responding back to the request.:startNetworkBlackholePort()andstopNetworkBlackHolePort()takes too long to finish then we will respond back with a 500 + "request timed out" error message.iptablescommands instartNetworkBlackholePort()andstopNetworkBlackHolePort()then we will respond back with a 500 + whatever the standard error was from theiptablescommandsTesting
generateStartBlackHolePortFaultTestCasesandgenerateStopBlackHolePortFaultTestCaseswith mock exec expectation calls. Existing test cases now also have the correct mock exec expectation calls.generateNetworkBlackHolePortTestCasestogenerateCommonNetworkBlackHolePortTestCasesagent/handlers/task_server_setup_test.goto just test whether or not we can make successful requests to each of the fault injection TMDS endpoints. The deleted tests are also already tested inecs-agent/tmds/handlers/fault/v1/handlers/handlers_test.goalready.Manual Testing:
Hooked up the fault injection handlers to also register upon TMDS server start up, ran a AWSVPC task that calls all three BHP endpoints (start -> check status -> stop BHP fault)
Corresponding iptables output in task ENI/network namespace
Same test but using Host mode task
Corresponding iptables on host network namespace
New tests cover the changes: yes
Description for the changelog
Feature: Adding start and stop network black hole port fault implementation
Additional Information
Does this PR include breaking model changes? If so, Have you added transformation functions?
Does this PR include the addition of new environment variables in the README?
Licensing
By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.