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

Support antctl command for packetcapture #6884

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

hangyan
Copy link
Member

@hangyan hangyan commented Dec 24, 2024

TODO items:

after #3659 , the CodeQL alert should be fixed and UT coverage will also be improved.

@hangyan hangyan force-pushed the topic/yhang/packetcapture-antctl branch from 4a616d6 to bf4cb68 Compare January 14, 2025 09:03
@hangyan hangyan changed the title [WIP] Support antctl command for packetcapture Support antctl command for packetcapture Jan 14, 2025
@hangyan
Copy link
Member Author

hangyan commented Feb 6, 2025

cc @antoninbas Can you help review this? also maybe merge #3659 first

cc @luolanzone

@hangyan hangyan force-pushed the topic/yhang/packetcapture-antctl branch 2 times, most recently from b27b72b to 43a9451 Compare February 7, 2025 10:54
@luolanzone luolanzone added this to the Antrea v2.4 release milestone Feb 8, 2025
@luolanzone
Copy link
Contributor

Please fix the antctl e2e test failures: https://github.com/antrea-io/antrea/actions/runs/13198383877?pr=6884

@luolanzone
Copy link
Contributor

@hangyan there are lots of failures in github checks, please check and fix them.

@hangyan hangyan force-pushed the topic/yhang/packetcapture-antctl branch from e1ba1b2 to 3436bfe Compare February 11, 2025 10:19
@hangyan
Copy link
Member Author

hangyan commented Feb 12, 2025

Please fix the antctl e2e test failures: https://github.com/antrea-io/antrea/actions/runs/13198383877?pr=6884

I disable the flag to run packetcapture sub-command in agentMode but controllerMode will still fail. One must be chosen, or this sub-command wont be registered to the rootCmd. However, seems packetcapture doens't make much sense in agentMode or controllerMode(controllerMode also contains outside of cluster case I think, but the e2e will check the command in controller pod).

The tests expect run antctl packetcapture to be successful. the first error comes from packet number should be larger than 0, that's the default value and it will cause error. Even we set with a valid default value, no src/dest endpoint ( i think Traceflow support this?), or packetcapture is not enabled , the command will still fail.

Can we make an exception in the test-case or we need to provide some sane defaults for it to working in the controllerMode?

cc @luolanzone @antoninbas

@hangyan hangyan force-pushed the topic/yhang/packetcapture-antctl branch from 5ac92d9 to e44de62 Compare February 12, 2025 07:15
@hangyan hangyan force-pushed the topic/yhang/packetcapture-antctl branch from 0c4cc8b to c4a4235 Compare March 19, 2025 10:18
@hangyan hangyan force-pushed the topic/yhang/packetcapture-antctl branch from fc46f32 to 86609ff Compare March 27, 2025 09:49
hangyan added 2 commits March 27, 2025 18:13
Signed-off-by: Hang Yan <[email protected]>
Signed-off-by: Hang Yan <[email protected]>
hangyan added 2 commits April 2, 2025 17:10
Signed-off-by: Hang Yan <[email protected]>
Signed-off-by: Hang Yan <[email protected]>
Signed-off-by: Hang Yan <[email protected]>
Copy link
Contributor

@antoninbas antoninbas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please rename option to options everywhere for consistency, otherwise it looks good to me

@antoninbas antoninbas added the action/release-note Indicates a PR that should be included in release notes. label Apr 3, 2025
@hangyan
Copy link
Member Author

hangyan commented Apr 5, 2025

Please rename option to options everywhere for consistency, otherwise it looks good to me

sure, will do.

I disable the flag to run packetcapture sub-command in agentMode but controllerMode will still fail. One must be chosen, or this sub-command wont be registered to the rootCmd. However, seems packetcapture doens't make much sense in agentMode or controllerMode(controllerMode also contains outside of cluster case I think, but the e2e will check the command in controller pod).

The tests expect run antctl packetcapture to be successful. the first error comes from packet number should be larger than 0, that's the default value and it will cause error. Even we set with a valid default value, no src/dest endpoint ( i think Traceflow support this?), or packetcapture is not enabled , the command will still fail.

Can we make an exception in the test-case or we need to provide some sane defaults for it to working in the controllerMode?

Any thoughts on this? cc @antoninbas

Signed-off-by: Hang Yan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
action/release-note Indicates a PR that should be included in release notes.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants