-
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
WIP NETOBSERV-1471 add tcp write stage #604
Conversation
Skipping CI for Draft Pull Request. |
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #604 +/- ##
==========================================
- Coverage 66.06% 65.77% -0.29%
==========================================
Files 103 104 +1
Lines 7509 7545 +36
==========================================
+ Hits 4961 4963 +2
- Misses 2255 2290 +35
+ Partials 293 292 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
@@ -0,0 +1,69 @@ | |||
/* | |||
* Copyright (C) 2021 IBM, Inc. |
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.
Fix (or remove) Copyright statement
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.
This is coming from both eBPF packets exporter and flp stdout write stage 😄
I just copied the code from there but I can remove the copyright if you want.
l, err := net.Listen("tcp", writeTCP.address) | ||
if err != nil { | ||
return nil, err | ||
} | ||
defer l.Close() | ||
clientConn, err := l.Accept() |
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.
It looks like you are expecting some other entity to initiate a TCP connection to FLP to which FLP will write its flow logs.
The code will block here until a single connection is completed. The rest of FLP will wait on this connection. Is this the desired behavior?
What is the use case you have in mind?
What if I have multiple targets to which I want to send my data via TCP? Can I set up multiple writeTCP stages? How will it work? or do we want to put the listen() operation into a separate thread and accumulate the connections that were attempted and send flow logs to all current connections?
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.
This is a temporary PR used in netobserv CLI: netobserv/network-observability-cli#2 (comment)
The CLI deploys eBPF agents using direct-flp
mode which embedd flowlogs-pipeline into the same pods.
Currently the connection between these pods and cli is made using TCP after calling oc port-forward commands.
Using gRPC instead is still in discussion.
If we keep that TCP implementation, we can indeed improve and makes the wait optionnal + allow multiple targets 👍
Closing this in favor of #621 |
Description
This PR adds TCP write stage to FLP.
We need to decide if we move to gRPC instead before merging.
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.