NETOBSERV-2243 Add IPsec support to netobserv CLI#286
NETOBSERV-2243 Add IPsec support to netobserv CLI#286jpinsonneau merged 5 commits intonetobserv:mainfrom
Conversation
jpinsonneau
left a comment
There was a problem hiding this comment.
Hey, nice to see you around @msherif1234 👋
This will require a config update, adding the feature in the columns and the graphs in the dashboard for metrics
it seems u have already taken care of the metrics bits Thanks!! |
|
I'm still getting n/a for now. I guess we need to bump FLP dependency since it's embedded in eBPF agent here |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #286 +/- ##
=======================================
Coverage 22.60% 22.60%
=======================================
Files 14 14
Lines 1451 1451
=======================================
Hits 328 328
Misses 1099 1099
Partials 24 24
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
|
/ok-to-test |
|
New image: It will expire after two weeks. To use this build, update your commands using: USER=netobserv VERSION=ae5d621 make commands |
8c4d73e to
fb2aaf7
Compare
|
/ok-to-test |
|
New image: It will expire after two weeks. To use this build, update your commands using: USER=netobserv VERSION=677b419 make commands |
|
/retest |
|
The e2e failure is related to #285 I have fixed it in #209 81b9def and cherry picked here |
|
/ok-to-test |
|
Hi @jpinsonneau can pls check that PR one more time and merge before any other conflicts, not sure if QE wanted to give it a try ? |
|
New image: It will expire after two weeks. To use this build, update your commands using: USER=netobserv VERSION=9017911 make commands |
jpinsonneau
left a comment
There was a problem hiding this comment.
Hey @msherif1234
That's good to me ! Thanks
Let's wait for @memodi / @Amoghrd tests unless we have the go for post merge testing 😉
|
@jpinsonneau I'll take a look this week if you don't mind waiting. |
memodi
left a comment
There was a problem hiding this comment.
I verified this, works well. Just minor comment about alphabetical order for the options. thanks @msherif1234
|
Hi @msherif1234 quick question from UX perspective, currently we report 2 fields/columns - IPSecRetCode an IPSecSuccess - since typically 0 return code translates to true IPSecSuccess - would we lose anything if we consolidate to single field/column say "IPSecSuccess" to have values of |
|
Hi @memodi IPsec return code is actually Linux errno values so can be anything but you are right when IPsec success is true the IPsec return code is 0 it's possible in UI to only show IPsec return code where 0 is good and != 0 is bad |
|
Hi @jpinsonneau sorry I mistakenly overwrote your commit that fix the e2e can pls readd it and if all good lets merge this Thanks!! |
Here we go: 7b58f63 |
memodi
left a comment
There was a problem hiding this comment.
thanks @msherif1234 @jpinsonneau
/label qe-approved
Signed-off-by: Mohamed S. Mahmoud <mmahmoud2201@gmail.com>
Signed-off-by: Mohamed S. Mahmoud <mmahmoud2201@gmail.com>
Signed-off-by: Mohamed S. Mahmoud <mmahmoud2201@gmail.com>
|
New changes are detected. LGTM label has been removed. |
|
[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 DetailsNeeds approval from an approver in each of these files:Approvers can indicate their approval by writing |
|
Konflux failing because of quay outage. Merging |
Description
Add IPsec support to CLI
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.