From f535e225ad05d323d3aa6fb5d56edf9e6fdc43c0 Mon Sep 17 00:00:00 2001 From: Roberto Scolaro Date: Tue, 7 Jan 2025 15:33:48 +0000 Subject: [PATCH] fix(libsinsp): address review comments Signed-off-by: Roberto Scolaro --- userspace/libsinsp/container.h | 10 ++-------- userspace/libsinsp/container_engine/bpm.cpp | 2 +- .../container_engine/container_cache_interface.h | 6 +++--- userspace/libsinsp/container_engine/containerd.cpp | 8 ++++---- userspace/libsinsp/container_engine/cri.cpp | 4 ++-- userspace/libsinsp/container_engine/docker/base.cpp | 3 +-- userspace/libsinsp/container_engine/libvirt_lxc.cpp | 2 +- userspace/libsinsp/container_engine/lxc.cpp | 2 +- userspace/libsinsp/container_engine/mesos.cpp | 2 +- userspace/libsinsp/container_engine/rkt.cpp | 2 +- 10 files changed, 17 insertions(+), 24 deletions(-) diff --git a/userspace/libsinsp/container.h b/userspace/libsinsp/container.h index 77ae3c3e26..260ef2985e 100644 --- a/userspace/libsinsp/container.h +++ b/userspace/libsinsp/container.h @@ -211,17 +211,11 @@ class sinsp_container_manager : public libsinsp::container_engine::container_cac */ void set_lookup_status(const std::string& container_id, sinsp_container_type ctype, - size_t engine_index, - sinsp_container_lookup::state state) override { + sinsp_container_lookup::state state, + size_t engine_index = 0) override { m_lookups[container_id][ctype][engine_index] = state; } - void set_lookup_status(const std::string& container_id, - sinsp_container_type ctype, - sinsp_container_lookup::state state) { - m_lookups[container_id][ctype][0] = state; - } - /** * \brief do we want to start a new lookup for container metadata? * @param container_id the container id we want to look up diff --git a/userspace/libsinsp/container_engine/bpm.cpp b/userspace/libsinsp/container_engine/bpm.cpp index 9f27e91a8a..e51acb45a1 100644 --- a/userspace/libsinsp/container_engine/bpm.cpp +++ b/userspace/libsinsp/container_engine/bpm.cpp @@ -57,7 +57,7 @@ bool bpm::resolve(sinsp_threadinfo* tinfo, bool query_os_for_missing_info) { } tinfo->m_container_id = container_info.m_id; - if(container_cache().should_lookup(container_info.m_id, CT_BPM, 0)) { + if(container_cache().should_lookup(container_info.m_id, CT_BPM)) { container_info.m_name = container_info.m_id; container_info.set_lookup_status(sinsp_container_lookup::state::SUCCESSFUL); container_cache().add_container(std::make_shared(container_info), diff --git a/userspace/libsinsp/container_engine/container_cache_interface.h b/userspace/libsinsp/container_engine/container_cache_interface.h index eb40c8f048..11f57b7858 100644 --- a/userspace/libsinsp/container_engine/container_cache_interface.h +++ b/userspace/libsinsp/container_engine/container_cache_interface.h @@ -35,12 +35,12 @@ class container_cache_interface { virtual bool should_lookup(const std::string& container_id, sinsp_container_type ctype, - size_t engine_index) = 0; + size_t engine_index = 0) = 0; virtual void set_lookup_status(const std::string& container_id, sinsp_container_type ctype, - size_t engine_index, - sinsp_container_lookup::state state) = 0; + sinsp_container_lookup::state state, + size_t engine_index = 0) = 0; /** * Get a container from the cache. diff --git a/userspace/libsinsp/container_engine/containerd.cpp b/userspace/libsinsp/container_engine/containerd.cpp index c6f6b9f2bc..32df3e470f 100644 --- a/userspace/libsinsp/container_engine/containerd.cpp +++ b/userspace/libsinsp/container_engine/containerd.cpp @@ -339,7 +339,7 @@ bool libsinsp::container_engine::containerd::resolve(sinsp_threadinfo *tinfo, return true; } - if(cache->should_lookup(request.container_id, request.container_type, 0)) { + if(cache->should_lookup(request.container_id, request.container_type)) { libsinsp_logger()->format(sinsp_logger::SEV_DEBUG, "containerd_async (%s): No existing container info", request.container_id.c_str()); @@ -347,8 +347,8 @@ bool libsinsp::container_engine::containerd::resolve(sinsp_threadinfo *tinfo, // give containerd a chance to return metadata for this container cache->set_lookup_status(request.container_id, request.container_type, - 0, - sinsp_container_lookup::state::STARTED); + sinsp_container_lookup::state::STARTED, + 0); parse_containerd(request, cache); } return false; @@ -375,7 +375,7 @@ bool libsinsp::container_engine::containerd::resolve(sinsp_threadinfo *tinfo, container.m_cpu_period = limits.m_cpu_period; container.m_cpuset_cpu_count = limits.m_cpuset_cpu_count; - if(container_cache().should_lookup(container.m_id, CT_CONTAINERD, 0)) { + if(container_cache().should_lookup(container.m_id, CT_CONTAINERD)) { container.m_name = container.m_id; container.set_lookup_status(sinsp_container_lookup::state::SUCCESSFUL); container_cache().add_container(std::make_shared(container), tinfo); diff --git a/userspace/libsinsp/container_engine/cri.cpp b/userspace/libsinsp/container_engine/cri.cpp index 56203ef587..a14ad48405 100644 --- a/userspace/libsinsp/container_engine/cri.cpp +++ b/userspace/libsinsp/container_engine/cri.cpp @@ -173,8 +173,8 @@ bool cri::resolve(sinsp_threadinfo *tinfo, bool query_os_for_missing_info) { cache->set_lookup_status(container_id, get_cri_runtime_type(), - m_engine_index, - sinsp_container_lookup::state::STARTED); + sinsp_container_lookup::state::STARTED, + m_engine_index); // sinsp_container_lookup is set-up to perform 5 retries at most, with // an exponential backoff with 2000 ms of maximum wait time. diff --git a/userspace/libsinsp/container_engine/docker/base.cpp b/userspace/libsinsp/container_engine/docker/base.cpp index 132937b7f4..1f3f8351a0 100644 --- a/userspace/libsinsp/container_engine/docker/base.cpp +++ b/userspace/libsinsp/container_engine/docker/base.cpp @@ -34,7 +34,7 @@ bool docker_base::resolve_impl(sinsp_threadinfo *tinfo, return true; } - if(cache->should_lookup(request.container_id, request.container_type, 0)) { + if(cache->should_lookup(request.container_id, request.container_type)) { libsinsp_logger()->format(sinsp_logger::SEV_DEBUG, "docker_async (%s): No existing container info", request.container_id.c_str()); @@ -42,7 +42,6 @@ bool docker_base::resolve_impl(sinsp_threadinfo *tinfo, // give docker a chance to return metadata for this container cache->set_lookup_status(request.container_id, request.container_type, - 0, sinsp_container_lookup::state::STARTED); parse_docker(request, cache); } diff --git a/userspace/libsinsp/container_engine/libvirt_lxc.cpp b/userspace/libsinsp/container_engine/libvirt_lxc.cpp index 8e7b2c2cf2..06e5d6ccd3 100644 --- a/userspace/libsinsp/container_engine/libvirt_lxc.cpp +++ b/userspace/libsinsp/container_engine/libvirt_lxc.cpp @@ -78,7 +78,7 @@ bool libvirt_lxc::resolve(sinsp_threadinfo* tinfo, bool query_os_for_missing_inf } tinfo->m_container_id = container.m_id; - if(container_cache().should_lookup(container.m_id, CT_LIBVIRT_LXC, 0)) { + if(container_cache().should_lookup(container.m_id, CT_LIBVIRT_LXC)) { container.m_name = container.m_id; container.set_lookup_status(sinsp_container_lookup::state::SUCCESSFUL); container_cache().add_container(std::make_shared(container), tinfo); diff --git a/userspace/libsinsp/container_engine/lxc.cpp b/userspace/libsinsp/container_engine/lxc.cpp index f28d1af766..1b4fa2d079 100644 --- a/userspace/libsinsp/container_engine/lxc.cpp +++ b/userspace/libsinsp/container_engine/lxc.cpp @@ -55,7 +55,7 @@ bool lxc::resolve(sinsp_threadinfo *tinfo, bool query_os_for_missing_info) { } tinfo->m_container_id = container.m_id; - if(container_cache().should_lookup(container.m_id, CT_LXC, 0)) { + if(container_cache().should_lookup(container.m_id, CT_LXC)) { container.m_name = container.m_id; container.set_lookup_status(sinsp_container_lookup::state::SUCCESSFUL); container_cache().add_container(std::make_shared(container), tinfo); diff --git a/userspace/libsinsp/container_engine/mesos.cpp b/userspace/libsinsp/container_engine/mesos.cpp index 00ae2d03b5..c4943d5cd3 100644 --- a/userspace/libsinsp/container_engine/mesos.cpp +++ b/userspace/libsinsp/container_engine/mesos.cpp @@ -57,7 +57,7 @@ bool libsinsp::container_engine::mesos::resolve(sinsp_threadinfo* tinfo, return false; tinfo->m_container_id = container.m_id; - if(container_cache().should_lookup(container.m_id, CT_MESOS, 0)) { + if(container_cache().should_lookup(container.m_id, CT_MESOS)) { container.m_name = container.m_id; container.set_lookup_status(sinsp_container_lookup::state::SUCCESSFUL); container_cache().add_container(std::make_shared(container), tinfo); diff --git a/userspace/libsinsp/container_engine/rkt.cpp b/userspace/libsinsp/container_engine/rkt.cpp index 97c76ebfb5..796e379286 100644 --- a/userspace/libsinsp/container_engine/rkt.cpp +++ b/userspace/libsinsp/container_engine/rkt.cpp @@ -167,7 +167,7 @@ bool rkt::rkt::resolve(sinsp_threadinfo* tinfo, bool query_os_for_missing_info) } tinfo->m_container_id = container.m_id; - if(!query_os_for_missing_info || !cache->should_lookup(container.m_id, CT_RKT, 0)) { + if(!query_os_for_missing_info || !cache->should_lookup(container.m_id, CT_RKT)) { return true; }