-
Notifications
You must be signed in to change notification settings - Fork 24
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
IPFIX Export support for Kubernetes transformations #338
Conversation
What is the purpose of the files added in the hack directory? testing? |
It was an example config for reference. Not intended for testing. I dint add a test, since I was unsure how ipfix export can be combined with the existing tests. |
I have removed the deployment manifest files |
Co-authored-by: Ronen Schaffer <ronen.schaffer@ibm.com>
Co-authored-by: Ronen Schaffer <ronen.schaffer@ibm.com>
Co-authored-by: Ronen Schaffer <ronen.schaffer@ibm.com>
@KalmanMeth @ronensc Thanks for the detailed reviews. I think I have now addressed all the comments |
This reverts commit 40bd9fc.
Co-authored-by: Ronen Schaffer <ronen.schaffer@ibm.com>
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.
LGTM
@praveingk Thank you for your contribution!
@ronensc @KalmanMeth @jotak Can this be merged if all tests are ok? |
Codecov Report
@@ Coverage Diff @@
## main #338 +/- ##
==========================================
- Coverage 69.23% 61.74% -7.49%
==========================================
Files 88 91 +3
Lines 5204 5835 +631
==========================================
Hits 3603 3603
- Misses 1382 2013 +631
Partials 219 219
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
Thanks @praveingk
Another thing, perhaps for a follow-up: I think the templates need to be sent again periodically. Not sure if this is part of the spec or not, but I think it's a common thing to do, for resiliency (e.g. if listener restarts, or if it starts after the emitter) |
Looking at the code, I think mocking the exporter would be a simpler approach (and the point is not to test the underlying go-ipfix lib). It seems like there's just 2 functions to mock, |
Thanks @jotak, IPFIX library by itself sends the templates periodically every 1 sec.
Sure @jotak, I will check the exporter for reference, Do you think it should be part of this PR, or a separate PR for that would be better? |
@praveingk cc: @jotak I think we can proceed to merge this PR and prepare a new PR with the tests. |
Thanks @KalmanMeth |
@KalmanMeth @praveingk sure, thanks for the PR! |
This PR adds IPFIX export support for FLP o/p:
Way to enable this in config, for e.g.:
enterpriseID is a custom registry ID for kubernetes transformations, and other non-standard entries.
Main Changes in the following files: