Skip to content

Commit

Permalink
refactor(libsinsp): use smart pointers for cri_settings
Browse files Browse the repository at this point in the history
Signed-off-by: Roberto Scolaro <[email protected]>
  • Loading branch information
therealbobo authored and poiana committed Jan 16, 2024
1 parent 4ef81e1 commit cb309fa
Show file tree
Hide file tree
Showing 5 changed files with 52 additions and 53 deletions.
26 changes: 13 additions & 13 deletions userspace/libsinsp/container_engine/cri.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -57,15 +57,15 @@ constexpr const cgroup_layout CRI_CGROUP_LAYOUT[] = {

cri::cri(container_cache_interface &cache) : container_engine_base(cache)
{
libsinsp::cri::settings* cri_settings = libsinsp::cri::settings::instance();
if (cri_settings->get_cri_unix_socket_paths().empty())
libsinsp::cri::cri_settings& cri_settings = libsinsp::cri::cri_settings::get();
if (cri_settings.get_cri_unix_socket_paths().empty())
{
// containerd as primary default value when empty
cri_settings->add_cri_unix_socket_path("/run/containerd/containerd.sock");
cri_settings.add_cri_unix_socket_path("/run/containerd/containerd.sock");
// crio-o as secondary default value when empty
cri_settings->add_cri_unix_socket_path("/run/crio/crio.sock");
cri_settings.add_cri_unix_socket_path("/run/crio/crio.sock");
// k3s containerd as third option when empty
cri_settings->add_cri_unix_socket_path("/run/k3s/containerd/containerd.sock");
cri_settings.add_cri_unix_socket_path("/run/k3s/containerd/containerd.sock");
}


Expand All @@ -74,7 +74,7 @@ cri::cri(container_cache_interface &cache) : container_engine_base(cache)
// so we wouldn't make things complex to support that.
// On the other hand, specifying multiple unix socket paths (and using only the first match)
// will solve the "same config, multiple hosts" use case.
for (auto &p : cri_settings->get_cri_unix_socket_paths())
for (auto &p : cri_settings.get_cri_unix_socket_paths())
{
if(p.empty())
{
Expand All @@ -96,7 +96,7 @@ cri::cri(container_cache_interface &cache) : container_engine_base(cache)
else
{
// Store used unix_socket_path
cri_settings->set_cri_unix_socket_path(p);
cri_settings.set_cri_unix_socket_path(p);
break;
}

Expand All @@ -108,7 +108,7 @@ cri::cri(container_cache_interface &cache) : container_engine_base(cache)
else
{
// Store used unix_socket_path
cri_settings->set_cri_unix_socket_path(p);
cri_settings.set_cri_unix_socket_path(p);
break;
}
}
Expand All @@ -120,27 +120,27 @@ void cri::cleanup()
{
m_async_source->quiesce();
}
libsinsp::cri::settings::set_cri_extra_queries(true);
libsinsp::cri::cri_settings::set_cri_extra_queries(true);
}

void cri::set_cri_socket_path(const std::string& path)
{
libsinsp::cri::settings::clear_cri_unix_socket_paths();
libsinsp::cri::cri_settings::clear_cri_unix_socket_paths();
add_cri_socket_path(path);
}

void cri::add_cri_socket_path(const std::string& path)
{
libsinsp::cri::settings::add_cri_unix_socket_path(path);
libsinsp::cri::cri_settings::add_cri_unix_socket_path(path);
}

void cri::set_cri_timeout(int64_t timeout_ms)
{
libsinsp::cri::settings::set_cri_timeout(timeout_ms);
libsinsp::cri::cri_settings::set_cri_timeout(timeout_ms);
}

void cri::set_extra_queries(bool extra_queries) {
libsinsp::cri::settings::set_cri_extra_queries(extra_queries);
libsinsp::cri::cri_settings::set_cri_extra_queries(extra_queries);
}

void cri::set_async(bool async)
Expand Down
43 changes: 21 additions & 22 deletions userspace/libsinsp/cri.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,89 +41,88 @@ limitations under the License.
namespace libsinsp {
namespace cri {

class settings
class cri_settings
{
public:
static settings* instance();
cri_settings();
~cri_settings();
static cri_settings& get();

static const std::vector<std::string>& get_cri_unix_socket_paths()
{
return instance()->m_cri_unix_socket_paths;
return get().m_cri_unix_socket_paths;
}

static void set_cri_unix_socket_paths(const std::vector<std::string>& v)
{
instance()->m_cri_unix_socket_paths = v;
get().m_cri_unix_socket_paths = v;
}

static const int64_t& get_cri_timeout()
{
return instance()->m_cri_timeout;
return get().m_cri_timeout;
}

static void set_cri_timeout(const int64_t& v)
{
instance()->m_cri_timeout = v;
get().m_cri_timeout = v;
}

static const int64_t& get_cri_size_timeout()
{
return instance()->m_cri_size_timeout;
return get().m_cri_size_timeout;
}

static void set_cri_size_timeout(const int64_t& v)
{
instance()->m_cri_size_timeout = v;
get().m_cri_size_timeout = v;
}

static const sinsp_container_type& get_cri_runtime_type()
{
return instance()->m_cri_runtime_type;
return get().m_cri_runtime_type;
}

static void set_cri_runtime_type(const sinsp_container_type& v)
{
instance()->m_cri_runtime_type = v;
get().m_cri_runtime_type = v;
}

static const std::string& get_cri_unix_socket_path()
{
return instance()->m_cri_unix_socket_path;
return get().m_cri_unix_socket_path;
}

static void set_cri_unix_socket_path(const std::string& v)
{
instance()->m_cri_unix_socket_path = v;
get().m_cri_unix_socket_path = v;
}

static const bool& get_cri_extra_queries()
{
return instance()->m_cri_extra_queries;
return get().m_cri_extra_queries;
}

static void set_cri_extra_queries(const bool& v)
{
instance()->m_cri_extra_queries = v;
get().m_cri_extra_queries = v;
}

static void add_cri_unix_socket_path(const std::string& v)
{
instance()->m_cri_unix_socket_paths.emplace_back(v);
get().m_cri_unix_socket_paths.emplace_back(v);
}

static void clear_cri_unix_socket_paths()
{
instance()->m_cri_unix_socket_paths.clear();
get().m_cri_unix_socket_paths.clear();
}

private:
static settings* s_instance;
static std::unique_ptr<cri_settings> s_instance;

settings();
~settings();

settings(const settings&) = delete;
settings& operator=(const settings&) = delete;
cri_settings(const cri_settings&) = delete;
cri_settings& operator=(const cri_settings&) = delete;

std::vector<std::string> m_cri_unix_socket_paths;
int64_t m_cri_timeout;
Expand Down
16 changes: 8 additions & 8 deletions userspace/libsinsp/cri.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ inline cri_interface<api>::cri_interface(const std::string &cri_path)

vreq.set_version(api::version);
grpc::ClientContext context;
auto deadline = std::chrono::system_clock::now() + std::chrono::milliseconds(settings::get_cri_timeout());
auto deadline = std::chrono::system_clock::now() + std::chrono::milliseconds(cri_settings::get_cri_timeout());
context.set_deadline(deadline);
grpc::Status status = m_cri->Version(&context, vreq, &vresp);

Expand Down Expand Up @@ -85,7 +85,7 @@ inline cri_interface<api>::cri_interface(const std::string &cri_path)
{
m_cri_runtime_type = CT_CRI;
}
settings::set_cri_runtime_type(m_cri_runtime_type);
cri_settings::set_cri_runtime_type(m_cri_runtime_type);
}

template<typename api>
Expand All @@ -102,7 +102,7 @@ inline grpc::Status cri_interface<api>::get_container_status(const std::string &
req.set_container_id(container_id);
req.set_verbose(true);
grpc::ClientContext context;
auto deadline = std::chrono::system_clock::now() + std::chrono::milliseconds(settings::get_cri_timeout());
auto deadline = std::chrono::system_clock::now() + std::chrono::milliseconds(cri_settings::get_cri_timeout());
context.set_deadline(deadline);
return m_cri->ContainerStatus(&context, req, &resp);
}
Expand All @@ -114,7 +114,7 @@ inline grpc::Status cri_interface<api>::get_container_stats(const std::string &c
typename api::ContainerStatsRequest req;
req.set_container_id(container_id);
grpc::ClientContext context;
auto deadline = std::chrono::system_clock::now() + std::chrono::milliseconds(settings::get_cri_size_timeout());
auto deadline = std::chrono::system_clock::now() + std::chrono::milliseconds(cri_settings::get_cri_size_timeout());
context.set_deadline(deadline);
return m_cri->ContainerStats(&context, req, &resp);
}
Expand Down Expand Up @@ -540,7 +540,7 @@ inline void cri_interface<api>::get_pod_sandbox_resp(const std::string &pod_sand
req.set_pod_sandbox_id(pod_sandbox_id);
req.set_verbose(true);
grpc::ClientContext context;
auto deadline = std::chrono::system_clock::now() + std::chrono::milliseconds(settings::get_cri_timeout());
auto deadline = std::chrono::system_clock::now() + std::chrono::milliseconds(cri_settings::get_cri_timeout());
context.set_deadline(deadline);
status = m_cri->PodSandboxStatus(&context, req, &resp);
}
Expand Down Expand Up @@ -582,7 +582,7 @@ inline void cri_interface<api>::get_container_ip(const std::string &container_id
auto filter = req.mutable_filter();
filter->set_id(container_id);
grpc::ClientContext context;
auto deadline = std::chrono::system_clock::now() + std::chrono::milliseconds(settings::get_cri_timeout());
auto deadline = std::chrono::system_clock::now() + std::chrono::milliseconds(cri_settings::get_cri_timeout());
context.set_deadline(deadline);
grpc::Status lstatus = m_cri->ListContainers(&context, req, &resp);

Expand Down Expand Up @@ -622,7 +622,7 @@ inline std::string cri_interface<api>::get_container_image_id(const std::string
auto spec = filter->mutable_image();
spec->set_image(image_ref);
grpc::ClientContext context;
auto deadline = std::chrono::system_clock::now() + std::chrono::milliseconds(settings::get_cri_timeout());
auto deadline = std::chrono::system_clock::now() + std::chrono::milliseconds(cri_settings::get_cri_timeout());
context.set_deadline(deadline);
grpc::Status status = m_cri_image->ListImages(&context, req, &resp);

Expand Down Expand Up @@ -784,7 +784,7 @@ inline bool cri_interface<api>::parse(const libsinsp::cgroup_limits::cgroup_limi
container.m_id.c_str(), container.m_imagerepo.c_str(), container.m_imagetag.c_str(),
container.m_image.c_str(), container.m_imagedigest.c_str());

if(settings::get_cri_extra_queries())
if(cri_settings::get_cri_extra_queries())
{
if(!container.m_container_ip)
{
Expand Down
12 changes: 6 additions & 6 deletions userspace/libsinsp/cri_settings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace libsinsp
namespace cri
{

settings::settings():
cri_settings::cri_settings():
m_cri_unix_socket_paths(),
m_cri_timeout(1000),
m_cri_size_timeout(10000),
Expand All @@ -32,18 +32,18 @@ settings::settings():
m_cri_extra_queries(true)
{ }

settings::~settings()
cri_settings::~cri_settings()
{ }

settings* settings::s_instance = nullptr;
std::unique_ptr<cri_settings> cri_settings::s_instance = nullptr;

settings* settings::instance()
cri_settings& cri_settings::get()
{
if(s_instance == nullptr)
{
s_instance = new settings();
s_instance = std::make_unique<cri_settings>();
}
return s_instance;
return *s_instance;
}

} // namespace cri
Expand Down
8 changes: 4 additions & 4 deletions userspace/libsinsp/test/cri_settings.ut.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -24,17 +24,17 @@ limitations under the License.

TEST_F(sinsp_with_test_input, default_cri_socket_paths)
{
libsinsp::cri::settings* cri_settings = libsinsp::cri::settings::instance();
libsinsp::cri::cri_settings& cri_settings = libsinsp::cri::cri_settings::get();

if (!cri_settings->get_cri_unix_socket_paths().empty())
if (!cri_settings.get_cri_unix_socket_paths().empty())
{
cri_settings->clear_cri_unix_socket_paths();
cri_settings.clear_cri_unix_socket_paths();
}

add_default_init_thread();
open_inspector();

auto socket_paths = cri_settings->get_cri_unix_socket_paths();
auto socket_paths = cri_settings.get_cri_unix_socket_paths();

ASSERT_EQ(socket_paths.size(), 3);
ASSERT_TRUE("/run/containerd/containerd.sock"==socket_paths[0]);
Expand Down

0 comments on commit cb309fa

Please sign in to comment.