From ea9449bc96f9cb3ce3467eb771f65c00650b3367 Mon Sep 17 00:00:00 2001 From: Leonardo Grasso Date: Wed, 16 Oct 2024 15:22:37 +0200 Subject: [PATCH 1/3] chore(libsinsp): remove unused container manager method Signed-off-by: Leonardo Grasso --- userspace/libsinsp/container.cpp | 26 -------------------------- userspace/libsinsp/container.h | 9 --------- 2 files changed, 35 deletions(-) diff --git a/userspace/libsinsp/container.cpp b/userspace/libsinsp/container.cpp index f6f74a3c09..c1128bac18 100644 --- a/userspace/libsinsp/container.cpp +++ b/userspace/libsinsp/container.cpp @@ -551,7 +551,6 @@ void sinsp_container_manager::create_engines() { m_static_name, m_static_image); m_container_engines.push_back(engine); - m_container_engine_by_type[CT_STATIC] = engine; return; } #if !defined(MINIMAL_BUILD) && !defined(__EMSCRIPTEN__) @@ -559,65 +558,40 @@ void sinsp_container_manager::create_engines() { if(m_container_engine_mask & (1 << CT_PODMAN)) { auto podman_engine = std::make_shared(*this); m_container_engines.push_back(podman_engine); - m_container_engine_by_type[CT_PODMAN] = podman_engine; } if(m_container_engine_mask & (1 << CT_DOCKER)) { auto docker_engine = std::make_shared(*this); m_container_engines.push_back(docker_engine); - m_container_engine_by_type[CT_DOCKER] = docker_engine; } if(m_container_engine_mask & ((1 << CT_CRI) | (1 << CT_CRIO) | (1 << CT_CONTAINERD))) { auto cri_engine = std::make_shared(*this); m_container_engines.push_back(cri_engine); - m_container_engine_by_type[CT_CRI] = cri_engine; - m_container_engine_by_type[CT_CRIO] = cri_engine; - m_container_engine_by_type[CT_CONTAINERD] = cri_engine; } if(m_container_engine_mask & (1 << CT_LXC)) { auto lxc_engine = std::make_shared(*this); m_container_engines.push_back(lxc_engine); - m_container_engine_by_type[CT_LXC] = lxc_engine; } if(m_container_engine_mask & (1 << CT_LIBVIRT_LXC)) { auto libvirt_lxc_engine = std::make_shared(*this); m_container_engines.push_back(libvirt_lxc_engine); - m_container_engine_by_type[CT_LIBVIRT_LXC] = libvirt_lxc_engine; } if(m_container_engine_mask & (1 << CT_MESOS)) { auto mesos_engine = std::make_shared(*this); m_container_engines.push_back(mesos_engine); - m_container_engine_by_type[CT_MESOS] = mesos_engine; } if(m_container_engine_mask & (1 << CT_RKT)) { auto rkt_engine = std::make_shared(*this); m_container_engines.push_back(rkt_engine); - m_container_engine_by_type[CT_RKT] = rkt_engine; } if(m_container_engine_mask & (1 << CT_BPM)) { auto bpm_engine = std::make_shared(*this); m_container_engines.push_back(bpm_engine); - m_container_engine_by_type[CT_BPM] = bpm_engine; } #endif // _WIN32 #endif // MINIMAL_BUILD } -void sinsp_container_manager::update_container_with_size(sinsp_container_type type, - const std::string& container_id) { - auto found = m_container_engine_by_type.find(type); - if(found == m_container_engine_by_type.end()) { - libsinsp_logger()->format(sinsp_logger::SEV_ERROR, - "Container type %d not found when requesting size for %s", - type, - container_id.c_str()); - return; - } - - libsinsp_logger()->format(sinsp_logger::SEV_DEBUG, "Request size for %s", container_id.c_str()); - found->second->update_with_size(container_id); -} - void sinsp_container_manager::cleanup() { for(auto& eng : m_container_engines) { eng->cleanup(); diff --git a/userspace/libsinsp/container.h b/userspace/libsinsp/container.h index 17305c96d3..fdabe6591a 100644 --- a/userspace/libsinsp/container.h +++ b/userspace/libsinsp/container.h @@ -179,12 +179,6 @@ class sinsp_container_manager : public libsinsp::container_engine::container_cac void create_engines(); - /** - * Update the container_info associated with the given type and container_id - * to include the size of the container layer. This is not filled in the - * initial request because it can easily take seconds. - */ - void update_container_with_size(sinsp_container_type type, const std::string& container_id); void cleanup(); void set_docker_socket_path(std::string socket_path); @@ -254,9 +248,6 @@ class sinsp_container_manager : public libsinsp::container_engine::container_cac std::list> m_container_engines; - std::map> - m_container_engine_by_type; sinsp* m_inspector; std::shared_ptr m_sinsp_stats_v2; From f2a81a98fd23ffb4373093b9259a1972e6f96728 Mon Sep 17 00:00:00 2001 From: Leonardo Grasso Date: Mon, 21 Oct 2024 13:30:28 +0200 Subject: [PATCH 2/3] chore(libsinsp): remove unused `update_with_size` method from `container_engine` Signed-off-by: Leonardo Grasso --- .../container_engine_base.cpp | 4 --- .../container_engine/container_engine_base.h | 7 ----- userspace/libsinsp/container_engine/cri.cpp | 28 ------------------- userspace/libsinsp/container_engine/cri.h | 1 - .../container_engine/docker/docker_linux.cpp | 24 ---------------- .../container_engine/docker/docker_linux.h | 2 -- 6 files changed, 66 deletions(-) diff --git a/userspace/libsinsp/container_engine/container_engine_base.cpp b/userspace/libsinsp/container_engine/container_engine_base.cpp index d174b94066..66c39a4968 100644 --- a/userspace/libsinsp/container_engine/container_engine_base.cpp +++ b/userspace/libsinsp/container_engine/container_engine_base.cpp @@ -25,10 +25,6 @@ namespace container_engine { container_engine_base::container_engine_base(container_cache_interface &cache): m_cache(cache) {} -void container_engine_base::update_with_size(const std::string &container_id) { - SINSP_DEBUG("Updating container size not supported for this container type."); -} - void container_engine_base::cleanup() {} } // namespace container_engine diff --git a/userspace/libsinsp/container_engine/container_engine_base.h b/userspace/libsinsp/container_engine/container_engine_base.h index 5c5fea1919..a0b0084cb4 100644 --- a/userspace/libsinsp/container_engine/container_engine_base.h +++ b/userspace/libsinsp/container_engine/container_engine_base.h @@ -43,13 +43,6 @@ class container_engine_base { */ virtual bool resolve(sinsp_threadinfo* tinfo, bool query_os_for_missing_info) = 0; - /** - * Update an existing container with the size of the container layer. - * The size is not requested as the part of the initial request (in resolve) - * because determining the size can take seconds. - */ - virtual void update_with_size(const std::string& container_id); - virtual void cleanup(); protected: diff --git a/userspace/libsinsp/container_engine/cri.cpp b/userspace/libsinsp/container_engine/cri.cpp index 6a85d2d398..1a01d1b008 100644 --- a/userspace/libsinsp/container_engine/cri.cpp +++ b/userspace/libsinsp/container_engine/cri.cpp @@ -272,34 +272,6 @@ bool cri::resolve(sinsp_threadinfo *tinfo, bool query_os_for_missing_info) { return true; } -void cri::update_with_size(const std::string &container_id) { - sinsp_container_info::ptr_t existing = container_cache().get_container(container_id); - if(!existing) { - libsinsp_logger()->format(sinsp_logger::SEV_ERROR, - "cri (%s): Failed to locate existing container data", - container_id.c_str()); - ASSERT(false); - return; - } - - std::optional writable_layer_size = get_writable_layer_size(existing->m_full_id); - - if(!writable_layer_size.has_value()) { - return; - } - - // Make a mutable copy of the existing container_info - shared_ptr updated(std::make_shared(*existing)); - updated->m_size_rw_bytes = *writable_layer_size; - - if(existing->m_size_rw_bytes == updated->m_size_rw_bytes) { - // no data has changed - return; - } - - container_cache().replace_container(updated); -} - sinsp_container_type cri::get_cri_runtime_type() const { if(m_cri_v1) { return m_cri_v1->get_cri_runtime_type(); diff --git a/userspace/libsinsp/container_engine/cri.h b/userspace/libsinsp/container_engine/cri.h index e00af883ad..987f3fdd6f 100644 --- a/userspace/libsinsp/container_engine/cri.h +++ b/userspace/libsinsp/container_engine/cri.h @@ -80,7 +80,6 @@ class cri : public container_engine_base { public: cri(container_cache_interface& cache); bool resolve(sinsp_threadinfo* tinfo, bool query_os_for_missing_info) override; - void update_with_size(const std::string& container_id) override; void cleanup() override; static void set_cri_socket_path(const std::string& path); static void add_cri_socket_path(const std::string& path); diff --git a/userspace/libsinsp/container_engine/docker/docker_linux.cpp b/userspace/libsinsp/container_engine/docker/docker_linux.cpp index 34e2f8ba1d..460dbede39 100644 --- a/userspace/libsinsp/container_engine/docker/docker_linux.cpp +++ b/userspace/libsinsp/container_engine/docker/docker_linux.cpp @@ -43,27 +43,3 @@ bool docker_linux::resolve(sinsp_threadinfo* tinfo, bool query_os_for_missing_in docker_lookup_request(container_id, s_docker_sock, CT_DOCKER, 0, false), query_os_for_missing_info); } - -void docker_linux::update_with_size(const std::string& container_id) { - auto cb = [this](const docker_lookup_request& instruction, const sinsp_container_info& res) { - libsinsp_logger()->format(sinsp_logger::SEV_DEBUG, - "docker_async (%s): with size callback result=%d", - instruction.container_id.c_str(), - res.get_lookup_status()); - - sinsp_container_info::ptr_t updated = std::make_shared(res); - container_cache().replace_container(updated); - }; - - libsinsp_logger()->format(sinsp_logger::SEV_DEBUG, - "docker_async size request (%s)", - container_id.c_str()); - - sinsp_container_info result; - docker_lookup_request instruction(container_id, - s_docker_sock, - CT_DOCKER, - 0, - true /*request rw size*/); - (void)m_docker_info_source->lookup(instruction, result, cb); -} diff --git a/userspace/libsinsp/container_engine/docker/docker_linux.h b/userspace/libsinsp/container_engine/docker/docker_linux.h index 393f488db5..6f62aedeca 100644 --- a/userspace/libsinsp/container_engine/docker/docker_linux.h +++ b/userspace/libsinsp/container_engine/docker/docker_linux.h @@ -15,8 +15,6 @@ class docker_linux : public docker_base { // implement container_engine_base bool resolve(sinsp_threadinfo* tinfo, bool query_os_for_missing_info) override; - void update_with_size(const std::string& container_id) override; - private: static std::string s_docker_sock; }; From 04fce8b04ebb1d33ae97e4d0da1f348a9986d6d1 Mon Sep 17 00:00:00 2001 From: Leonardo Grasso Date: Tue, 22 Oct 2024 12:51:25 +0200 Subject: [PATCH 3/3] chore(libsinsp): remove `get_writable_layer_size` impl from CRI Signed-off-by: Leonardo Grasso --- userspace/libsinsp/container_engine/cri.cpp | 12 +---- userspace/libsinsp/container_engine/cri.h | 4 +- userspace/libsinsp/cri.h | 10 +---- userspace/libsinsp/cri.hpp | 49 +-------------------- 4 files changed, 4 insertions(+), 71 deletions(-) diff --git a/userspace/libsinsp/container_engine/cri.cpp b/userspace/libsinsp/container_engine/cri.cpp index 1a01d1b008..9a93603611 100644 --- a/userspace/libsinsp/container_engine/cri.cpp +++ b/userspace/libsinsp/container_engine/cri.cpp @@ -1,6 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 /* -Copyright (C) 2023 The Falco Authors. +Copyright (C) 2024 The Falco Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -282,16 +282,6 @@ sinsp_container_type cri::get_cri_runtime_type() const { } } -std::optional cri::get_writable_layer_size(const string &container_id) { - if(m_cri_v1) { - return m_cri_v1->get_writable_layer_size(container_id); - } else if(m_cri_v1alpha2) { - return m_cri_v1alpha2->get_writable_layer_size(container_id); - } else { - return std::nullopt; - } -} - bool cri_async_source::parse(const cri_async_source::key_type &key, sinsp_container_info &container) { if(m_cri_v1) { diff --git a/userspace/libsinsp/container_engine/cri.h b/userspace/libsinsp/container_engine/cri.h index 987f3fdd6f..946e45b291 100644 --- a/userspace/libsinsp/container_engine/cri.h +++ b/userspace/libsinsp/container_engine/cri.h @@ -1,6 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 /* -Copyright (C) 2023 The Falco Authors. +Copyright (C) 2024 The Falco Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -90,8 +90,6 @@ class cri : public container_engine_base { private: [[nodiscard]] sinsp_container_type get_cri_runtime_type() const; - std::optional get_writable_layer_size(const std::string& container_id); - std::unique_ptr m_async_source; std::unique_ptr<::libsinsp::cri::cri_interface_v1alpha2> m_cri_v1alpha2; std::unique_ptr<::libsinsp::cri::cri_interface_v1> m_cri_v1; diff --git a/userspace/libsinsp/cri.h b/userspace/libsinsp/cri.h index 6ee41949f6..b78650b59c 100644 --- a/userspace/libsinsp/cri.h +++ b/userspace/libsinsp/cri.h @@ -1,6 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 /* -Copyright (C) 2023 The Falco Authors. +Copyright (C) 2024 The Falco Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -216,14 +216,6 @@ class cri_interface { */ std::string get_container_image_id(const std::string &image_ref); - /** - * @brief get the size of the container's writable layer via ContainerStat API calls - * @param container_id container ID - * @note currently unused - * @return the size of the writable layer in bytes. Returns an empty option on error - */ - std::optional get_writable_layer_size(const std::string &container_id); - /////////////////////////////////////////////////////////// // CRI response (ContainerStatusResponse) parsers helpers /////////////////////////////////////////////////////////// diff --git a/userspace/libsinsp/cri.hpp b/userspace/libsinsp/cri.hpp index 7f742204b2..e5b7fbc3a4 100644 --- a/userspace/libsinsp/cri.hpp +++ b/userspace/libsinsp/cri.hpp @@ -1,6 +1,6 @@ // SPDX-License-Identifier: Apache-2.0 /* -Copyright (C) 2023 The Falco Authors. +Copyright (C) 2024 The Falco Authors. Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. @@ -163,53 +163,6 @@ inline std::string cri_interface::get_container_image_id(const std::string return ""; } -template -inline std::optional cri_interface::get_writable_layer_size( - const std::string &container_id) { - // Synchronously get the stats response and update the container table. - // Note that this needs to use the full id. - typename api::ContainerStatsResponse resp; - grpc::Status status = get_container_stats_resp(container_id, resp); - - libsinsp_logger()->format( - sinsp_logger::SEV_DEBUG, - "cri (%s): Status from ContainerStats: (%s)", - container_id.c_str(), - status.error_message().empty() ? "SUCCESS" : status.error_message().c_str()); - - if(!status.ok()) { - return std::nullopt; - } - - if(!resp.has_stats()) { - libsinsp_logger()->format(sinsp_logger::SEV_DEBUG, - "cri (%s): Failed to update size: stats() not found", - container_id.c_str()); - ASSERT(false); - return std::nullopt; - } - - const auto &resp_stats = resp.stats(); - - if(!resp_stats.has_writable_layer()) { - libsinsp_logger()->format(sinsp_logger::SEV_DEBUG, - "cri (%s): Failed to update size: writable_layer() not found", - container_id.c_str()); - ASSERT(false); - return std::nullopt; - } - - if(!resp_stats.writable_layer().has_used_bytes()) { - libsinsp_logger()->format(sinsp_logger::SEV_DEBUG, - "cri (%s): Failed to update size: used_bytes() not found", - container_id.c_str()); - ASSERT(false); - return std::nullopt; - } - - return resp_stats.writable_layer().used_bytes().value(); -} - ///////////////////////////// // Generic parsers helpers /////////////////////////////