-
Notifications
You must be signed in to change notification settings - Fork 169
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: cleanup(userspace/libsinsp,test,build): drop container manager #2207
base: master
Are you sure you want to change the base?
Conversation
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: FedeDP 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 |
cc @falcosecurity/libs-maintainers |
Perf diff from master - unit tests
Heap diff from master - unit tests
Heap diff from master - scap file
Benchmarks diff from master
|
b28ea4c
to
dbfc736
Compare
/milestone 0.21.0 |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2207 +/- ##
==========================================
+ Coverage 75.16% 76.94% +1.77%
==========================================
Files 278 219 -59
Lines 34478 30208 -4270
Branches 5922 4653 -1269
==========================================
- Hits 25916 23244 -2672
+ Misses 8562 6964 -1598
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
6fa963b
to
079cef8
Compare
Will rebase on master once #2195 is merged too. |
079cef8
to
fee7a8a
Compare
Rebased on top of latest master. |
b91ae13
to
ca705a6
Compare
…container_id from threadinfo. Signed-off-by: Federico Di Pierro <[email protected]>
… couple of unused methods in sinsp. Signed-off-by: Federico Di Pierro <[email protected]>
…ger, container info and dependent classes. Signed-off-by: Federico Di Pierro <[email protected]>
… `TYPE_IS_CONTAINER_LIVENESS_PROBE`, `TYPE_IS_CONTAINER_READINESS_PROBE` extractors. They are now implemented by the plugin. Also, dropped threadinfo::m_category, unused. Finally, dropped `sinsp_observer::on_resolve_container`. Signed-off-by: Federico Di Pierro <[email protected]>
Signed-off-by: Federico Di Pierro <[email protected]>
Signed-off-by: Federico Di Pierro <[email protected]>
…n threadinfo. It leverages sinsp state table API to retrieve "container_id" field written by the plugin. Use it where needed. Moreover, user_group_manager cannot subscribe to container changes anymore, since container changes are no more in sinsp. Instead, parse ASYNC event "container_removed" to cleanup user_group tables. Signed-off-by: Federico Di Pierro <[email protected]>
Signed-off-by: Federico Di Pierro <[email protected]>
Signed-off-by: Federico Di Pierro <[email protected]>
…gin. Signed-off-by: Federico Di Pierro <[email protected]>
…re unused. Signed-off-by: Federico Di Pierro <[email protected]>
Nowadays, it was only filtering out gvisor (and thus protobuf), but we already have the `BUILD_LIBSCAP_GVISOR` flag for that. Signed-off-by: Federico Di Pierro <[email protected]>
…sts. Signed-off-by: Federico Di Pierro <[email protected]>
Signed-off-by: Federico Di Pierro <[email protected]>
Signed-off-by: Federico Di Pierro <[email protected]>
Signed-off-by: Federico Di Pierro <[email protected]>
…is in the future. Signed-off-by: Federico Di Pierro <[email protected]>
14f504d
to
4656cef
Compare
What type of PR is this?
/kind cleanup
Any specific area of the project related to this PR?
/area build
/area CI
/area libsinsp
/area tests
What this PR does / why we need it:
Following falcosecurity/falco#3403, this PR does multiple things:
BUILD_LIBSCAP_GVISOR
Which issue(s) this PR fixes:
Fixes #
Special notes for your reviewer:
Opening this just to let other people know about the cleanup; also, this will be wip until all TODOs below are done.
TODO:
sinsp_network_interfaces::is_ipv4addr_in_local_machine()
: https://github.com/falcosecurity/libs/blob/master/userspace/libsinsp/ifinfo.cpp#L326sinsp_filter_check_user::extract_single
: https://github.com/falcosecurity/libs/blob/master/userspace/libsinsp/sinsp_filtercheck_user.cpp#L93m_program_hash
andm_program_hash_scripts
as foreign key filled in by the container plugin and dropcompute_program_hash
. OR, use a RAII object just like we did for users/groups (chore(userspace/libsinsp): move user group manager on container_id changed refresh to a RAII object #2194) -> after a discussion with @jasondellaluce we decided to go the cleanup(userspace/libsinsp): call sinsp_observer methods after an event has been processed by all parsers #2222 route: since sinsp (and Falco) are not using those fields, we offload their implementation to the libs consumers, if needed.container.id=null
lines in the sinsp output -> found out it was due to thecontainer_removed
event being processed WHILE we were still dequeuing from the ring buffer syscall events coming from the container :O fix(userspace/libsinsp): do not immediately process async events whose timestamp is in the future in case a SCAP_TIMEOUT is received #2250Less important:
Does this PR introduce a user-facing change?: