-
Notifications
You must be signed in to change notification settings - Fork 6
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
NETOBSERV-1684 Auto teardown after some time / number of flows / packets #59
Conversation
Skipping CI for Draft Pull Request. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #59 +/- ##
==========================================
- Coverage 28.99% 28.53% -0.47%
==========================================
Files 8 8
Lines 1045 1076 +31
==========================================
+ Hits 303 307 +4
- Misses 720 747 +27
Partials 22 22
Flags with carried forward coverage won't be shown. Click here to find out more.
|
pls update the flows doc with the new commands |
FYI the example u have in PR description is missing |
|
||
// terminate capture if max bytes reached | ||
totalBytes = totalBytes + int64(bytes) | ||
if totalBytes > maxBytes { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do u need to handle cases where user pass only either maxBytes
or maxTime
but not both ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
if user pass only one of these, the default is used for the other one:
rootCmd.PersistentFlags().DurationVarP(&maxTime, "maxtime", "", 5*time.Minute, "Maximum capture time")
rootCmd.PersistentFlags().Int64VarP(&maxBytes, "maxbytes", "", 50000000, "Maximum capture bytes")
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was more of thinking I wanted to skip one and use the other for example if I know I am sampling large packets so I wanted to skip maxBytes
check and just use maxTime
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hm that's right ... maybe I should allow -1
value as no limit ?
It would feel strange that the behavior of one of these two changes according to the other so I feel it's the best option here. WDYT ?
New image: It will expire after two weeks. To use this build, update your commands using: USER=netobserv VERSION=679bb17 make commands |
/label qe-approved |
@Amoghrd: The label(s) In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
done in 8ac4491 |
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED Approval requirements bypassed by manually added approval. This pull-request has been approved by: The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Description
Auto teardown after some time / number of flows / packets
Also took the opportunity to add log level argument and improve display
Dependencies
n/a
Checklist
If you are not familiar with our processes or don't know what to answer in the list below, let us know in a comment: the maintainers will take care of that.