cleanup(userspace/libsinsp,test,build): drop container manager #2207

FedeDP · 2024-12-18T10:29:03Z

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:

drops all container related code
drops all container related tests
drops grpc,curl,openssl and cares deps that are not needed anymore
drops MINIMAL_BUILD whose only purpose was to disable gvisor; we already have a flag for that: BUILD_LIBSCAP_GVISOR
Allows e2e tests (not libsinsp_e2e ones) to run by downloading the plugin, since they test spawning containers

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:

reimplement ifinfo.cpp sinsp_network_interfaces::is_ipv4addr_in_local_machine(): https://github.com/falcosecurity/libs/blob/master/userspace/libsinsp/ifinfo.cpp#L326
reimplement sinsp_filtercheck_user.cpp sinsp_filter_check_user::extract_single : https://github.com/falcosecurity/libs/blob/master/userspace/libsinsp/sinsp_filtercheck_user.cpp#L93
move m_program_hash and m_program_hash_scripts as foreign key filled in by the container plugin and drop compute_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.
fix e2e-tests CI
- the asan failure is due to a small memleak in the podman library; i opened the upstream PR to fix it: fix(pkg/rootless): avoid memleak during init() contructor containers/podman#25049 -> the PR was merged; waiting for a new release from podman -> switched to podman v5.4.0-rc3
- other 2 tests are flaky; i am investigating some weird container.id=null lines in the sinsp output -> found out it was due to the container_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 #2250

Less important:

you'll note that the plugin is being downloaded by upstream releases: https://github.com/FedeDP/container_plugin/releases. Instead, we will download latest artifacts once we have a proper CI on top of it (ie: once the plugin is donated to the https://github.com/falcosecurity/plugins repo). -> opened the tracking issue: Donate container plugin to plugins repo plugins#636

Does this PR introduce a user-facing change?:

update(userspace/libsinsp,test,build)!: drop container manager
update(build)!: drop MINIMAL_BUILD

poiana · 2024-12-18T10:29:11Z

[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:

~~OWNERS~~ [FedeDP]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

FedeDP · 2024-12-18T10:29:30Z

cc @falcosecurity/libs-maintainers

github-actions · 2024-12-18T10:34:11Z

Perf diff from master - unit tests

     7.38%     -1.33%  [.] sinsp::next
    18.53%     +1.11%  [.] sinsp_threadinfo::get_main_thread
     7.11%     -0.71%  [.] sinsp_parser::reset
     8.35%     +0.47%  [.] std::_Sp_counted_base<(__gnu_cxx::_Lock_policy)2>::_M_release
     1.42%     +0.43%  [.] sinsp_parser::process_event
     0.71%     -0.36%  [.] std::__cxx11::basic_string<char, std::char_traits<char>, std::allocator<char> >::_M_construct<char const*>
     0.88%     +0.29%  [.] sinsp::fetch_next_event
     0.75%     +0.27%  [.] sinsp_evt_filter::sinsp_evt_filter
     9.73%     -0.26%  [.] sinsp_thread_manager::create_thread_dependencies
     1.85%     +0.25%  [.] sinsp_thread_manager::get_thread_ref

Heap diff from master - unit tests

peak heap memory consumption: 816.86K
peak RSS (including heaptrack overhead): 0B
total memory leaked: 143.85K

Heap diff from master - scap file

peak heap memory consumption: 279.76K
peak RSS (including heaptrack overhead): 0B
total memory leaked: 145.04K

Benchmarks diff from master

Comparing gbench_data.json to /root/actions-runner/_work/libs/libs/build/gbench_data.json
Benchmark                                                         Time             CPU      Time Old      Time New       CPU Old       CPU New
----------------------------------------------------------------------------------------------------------------------------------------------
BM_sinsp_split_mean                                            -0.0015         -0.0016           150           150           150           150
BM_sinsp_split_median                                          +0.0095         +0.0094           151           152           151           152
BM_sinsp_split_stddev                                          +2.3717         +2.3712             1             3             1             3
BM_sinsp_split_cv                                              +2.3768         +2.3768             0             0             0             0
BM_sinsp_concatenate_paths_relative_path_mean                  -0.0845         -0.0847            63            57            63            57
BM_sinsp_concatenate_paths_relative_path_median                -0.0967         -0.0968            63            57            63            57
BM_sinsp_concatenate_paths_relative_path_stddev                +0.3217         +0.3207             1             1             1             1
BM_sinsp_concatenate_paths_relative_path_cv                    +0.4438         +0.4428             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_mean                     +0.0002         +0.0001            24            24            24            24
BM_sinsp_concatenate_paths_empty_path_median                   +0.0005         +0.0003            24            24            24            24
BM_sinsp_concatenate_paths_empty_path_stddev                   -0.3736         -0.3690             0             0             0             0
BM_sinsp_concatenate_paths_empty_path_cv                       -0.3738         -0.3691             0             0             0             0
BM_sinsp_concatenate_paths_absolute_path_mean                  -0.1730         -0.1732            62            51            62            51
BM_sinsp_concatenate_paths_absolute_path_median                -0.1744         -0.1745            62            51            62            51
BM_sinsp_concatenate_paths_absolute_path_stddev                +0.2733         +0.2758             0             0             0             0
BM_sinsp_concatenate_paths_absolute_path_cv                    +0.5398         +0.5429             0             0             0             0

FedeDP · 2024-12-18T12:30:27Z

/milestone 0.21.0

codecov · 2024-12-18T12:39:10Z

Codecov Report

Attention: Patch coverage is 20.31250% with 51 lines in your changes missing coverage. Please review.

Project coverage is 77.01%. Comparing base (0d94d2b) to head (7a2bc04).
Report is 21 commits behind head on master.

Files with missing lines	Patch %	Lines
userspace/libsinsp/ifinfo.cpp	7.69%	24 Missing ⚠️
userspace/libsinsp/examples/test.cpp	0.00%	17 Missing ⚠️
userspace/libsinsp/sinsp_filtercheck_user.cpp	33.33%	6 Missing ⚠️
userspace/libsinsp/sinsp_filtercheck_fd.cpp	57.14%	3 Missing ⚠️
userspace/libsinsp/event.cpp	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2207      +/-   ##
==========================================
+ Coverage   75.32%   77.01%   +1.68%     
==========================================
  Files         280      220      -60     
  Lines       34556    30126    -4430     
  Branches     5902     4614    -1288     
==========================================
- Hits        26031    23203    -2828     
+ Misses       8525     6923    -1602

Flag	Coverage Δ
libsinsp	`77.01% <20.31%> (+1.68%)`	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

…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 <nierro92@gmail.com>

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

…gin. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

…re unused. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

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 <nierro92@gmail.com>

…sts. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

…xposed table. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

FedeDP · 2025-02-19T10:25:18Z

Force-pushed the requested changes @leogr !

FedeDP · 2025-02-26T09:27:00Z

/unhold
YAYYYY

Missed from falcosecurity#2207, the comment makes no sense now. Drop it. Signed-off-by: Angelo Puglisi <angelopuglisi86@gmail.com>

Missed from #2207, the comment makes no sense now. Drop it. Signed-off-by: Angelo Puglisi <angelopuglisi86@gmail.com>

poiana added kind/cleanup do-not-merge/work-in-progress release-note dco-signoff: yes area/build area/CI area/libsinsp labels Dec 18, 2024

poiana added area/tests approved size/XXL labels Dec 18, 2024

poiana requested review from hbrueckner and mstemm December 18, 2024 10:29

FedeDP force-pushed the cleanup/drop_container_manager branch 2 times, most recently from b28ea4c to dbfc736 Compare December 18, 2024 12:30

poiana added this to the 0.21.0 milestone Dec 18, 2024

FedeDP force-pushed the cleanup/drop_container_manager branch 9 times, most recently from 6fa963b to 079cef8 Compare December 19, 2024 15:41

FedeDP mentioned this pull request Dec 20, 2024

Reimplement containers metadata support as a plugin falcosecurity/falco#3403

Closed

FedeDP added 15 commits February 19, 2025 11:22

cleanup: dropped libcurl dependency.

6873670

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

chore(userspace/libsinsp): cleanup sinsp.h

6e78ae3

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

chore(userspace/libsinsp): allow sinsp-example to compile.

725e07b

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

fix(userspace/libsinsp/test): let libsinsp unit test build again.

43aa2cf

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

new(test/e2e,cmake): initial support for e2e tests with container plu…

7aa10ca

…gin. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

cleanup(cmake,userspace): drop {grpc,openssl,cares} deps since they a…

b956f02

…re unused. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

cleanup: drop MINIMAL_BUILD.

844e376

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 <nierro92@gmail.com>

cleanup(test/libsinsp_e2e): dropped container-related libsinsp e2e te…

b0e11c3

…sts. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

cleanup(ci): drop unused deps.

ea2dff4

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

chore(ci,build): introduce a ENABLE_E2E_TESTS cmake option.

2d4d8bf

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

update(cmake): bumped container plugin to latest release.

dfe4fd2

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

new(userspace/libsinsp): read user and ip from container plugin e…

1f85ce0

…xposed table. Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

cleanup: drop more references.

44efb12

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

cleanup(userspace/libsinsp): drop wrong ASSERTs.

7a2bc04

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>

FedeDP force-pushed the cleanup/drop_container_manager branch from a92a0c6 to 7a2bc04 Compare February 19, 2025 10:24

leogr approved these changes Feb 19, 2025

View reviewed changes

poiana assigned leogr Feb 19, 2025

poiana added the lgtm label Feb 19, 2025

LucaGuerra approved these changes Feb 26, 2025

View reviewed changes

poiana assigned LucaGuerra Feb 26, 2025

poiana removed the do-not-merge/hold label Feb 26, 2025

poiana merged commit 595bb73 into master Feb 26, 2025
46 of 47 checks passed

poiana deleted the cleanup/drop_container_manager branch February 26, 2025 09:27

bevanjkay mentioned this pull request May 20, 2025

falcosecurity-libs 0.21.0 Homebrew/homebrew-core#224095

Closed

deepskyblue86 mentioned this pull request Jul 8, 2025

chore(libsinsp): clean "detect container id" comments #2532

Merged

poiana pushed a commit that referenced this pull request Jul 9, 2025

chore(libsinsp): clean "detect container id" comments

4d572b2

Missed from #2207, the comment makes no sense now. Drop it. Signed-off-by: Angelo Puglisi <angelopuglisi86@gmail.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

cleanup(userspace/libsinsp,test,build): drop container manager #2207

cleanup(userspace/libsinsp,test,build): drop container manager #2207

Uh oh!

FedeDP commented Dec 18, 2024 •

edited

Loading

Uh oh!

poiana commented Dec 18, 2024

Uh oh!

FedeDP commented Dec 18, 2024

Uh oh!

github-actions bot commented Dec 18, 2024 •

edited

Loading

Uh oh!

FedeDP commented Dec 18, 2024

Uh oh!

codecov bot commented Dec 18, 2024 •

edited

Loading

Uh oh!

FedeDP commented Feb 19, 2025

Uh oh!

FedeDP commented Feb 26, 2025

Uh oh!

Uh oh!

Uh oh!

cleanup(userspace/libsinsp,test,build): drop container manager #2207

cleanup(userspace/libsinsp,test,build): drop container manager #2207

Uh oh!

Conversation

FedeDP commented Dec 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

poiana commented Dec 18, 2024

Uh oh!

FedeDP commented Dec 18, 2024

Uh oh!

github-actions bot commented Dec 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Perf diff from master - unit tests

Heap diff from master - unit tests

Heap diff from master - scap file

Benchmarks diff from master

Uh oh!

FedeDP commented Dec 18, 2024

Uh oh!

codecov bot commented Dec 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

FedeDP commented Feb 19, 2025

Uh oh!

FedeDP commented Feb 26, 2025

Uh oh!

Uh oh!

Uh oh!

FedeDP commented Dec 18, 2024 •

edited

Loading

github-actions bot commented Dec 18, 2024 •

edited

Loading

codecov bot commented Dec 18, 2024 •

edited

Loading