Skip to content
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

feat(tests): new e2e tests [6/N] #1776

Merged
merged 13 commits into from
Apr 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 27 additions & 1 deletion .github/workflows/e2e_ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,11 @@ jobs:
driver: [ {name: kmod, option: -k}, {name: bpf, option: -b}, {name: modern-bpf, option: -m} ]
fail-fast: false
steps:
- name: Checkout Libs ⤵️
uses: actions/checkout@b4ffde65f46336ab88eb53be808477a3936bae11 # v4.1.1
with:
fetch-depth: 0

- name: Restore build
id: cache
uses: actions/cache/restore@v3
Expand All @@ -129,13 +134,34 @@ jobs:
# Asan in llvm 14 provided in ubuntu 22.04 is incompatible with
# high-entropy ASLR in much newer kernels that GitHub runners are
# using leading to random crashes: https://reviews.llvm.org/D148280
run: sudo sysctl vm.mmap_rnd_bits=28
run: sudo sysctl vm.mmap_rnd_bits=28

- name: Install multilib
if: matrix.arch == 'amd64'
run: |
sudo apt install -y --no-install-recommends gcc-multilib g++-multilib

- name: Install deps
run: |
sudo apt install -y --no-install-recommends clang gcc llvm build-essential cmake

- name: Install kernel headers (actuated)
uses: self-actuated/get-kernel-sources@master
if: matrix.arch == 'arm64'

- name: Install kernel headers and gcc
if: matrix.arch == 'amd64'
run: |
sudo apt install -y --no-install-recommends linux-headers-$(uname -r) gcc-multilib g++-multilib

# We have no guarantees that the kernel version is the same for the
# different workers, so we rebuild the drivers.
- name: Rebuild drivers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be great to avoid building them at all in the build step.
Also, i am not sure whether this would always cleanly rebuild the drivers (since it should already find them there, from the restored build).

Copy link
Contributor Author

@therealbobo therealbobo Apr 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll see if the cmake setup makes me do that! :)

Also, i am not sure whether this would always cleanly rebuild the drivers (since it should already find them there, from the restored build).

I was also thinking that but it turns out that it could happen that the worker where the build step is performed is different from the one in which the tests are run; therefore the kernel can be slightly different and the tests will fail :/

run: |
cd build
make -B driver bpf
cd ..

- name: Run e2e tests with ${{ matrix.driver.name }} 🏎️
if: matrix.arch == 'amd64'
env:
Expand Down
2 changes: 1 addition & 1 deletion driver/SCHEMA_VERSION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
2.19.5
2.19.6
59 changes: 30 additions & 29 deletions driver/bpf/fillers.h
Original file line number Diff line number Diff line change
Expand Up @@ -2381,44 +2381,45 @@ FILLER(proc_startupdate, true)
args_len = 0;
}

if (args_len > 0) {
int exe_len;
int exe_len;
therealbobo marked this conversation as resolved.
Show resolved Hide resolved

exe_len = bpf_probe_read_kernel_str(&data->buf[data->state->tail_ctx.curoff & SCRATCH_SIZE_HALF],
SCRATCH_SIZE_HALF,
&data->buf[data->state->tail_ctx.curoff & SCRATCH_SIZE_HALF]);
exe_len = bpf_probe_read_kernel_str(&data->buf[data->state->tail_ctx.curoff & SCRATCH_SIZE_HALF],
SCRATCH_SIZE_HALF,
&data->buf[data->state->tail_ctx.curoff & SCRATCH_SIZE_HALF]);

if (exe_len < 0)
return PPM_FAILURE_INVALID_USER_MEMORY;
if (exe_len < 0)
{
return PPM_FAILURE_INVALID_USER_MEMORY;
}

/*
* exe
*/
/*
* exe
*/
if (exe_len == 0)
{
res = bpf_push_empty_param(data);
}
else
{
data->curarg_already_on_frame = true;
res = __bpf_val_to_ring(data, 0, exe_len, PT_CHARBUF, -1, false, KERNEL);
CHECK_RES(res);
}
CHECK_RES(res);

args_len -= exe_len;
if (args_len < 0)
return PPM_FAILURE_INVALID_USER_MEMORY;
args_len -= exe_len;

/*
* Args
*/
data->curarg_already_on_frame = true;
res = __bpf_val_to_ring(data, 0, args_len, PT_BYTEBUF, -1, false, KERNEL);
CHECK_RES(res);
} else {
/*
* exe
*/
/*
* Args
*/
if(args_len <= 0)
{
res = bpf_push_empty_param(data);
CHECK_RES(res);

/*
* Args
*/
res = bpf_push_empty_param(data);
}
else
{
data->curarg_already_on_frame = true;
res = __bpf_val_to_ring(data, 0, args_len, PT_BYTEBUF, -1, false, KERNEL);
CHECK_RES(res);
}

Expand Down
4 changes: 3 additions & 1 deletion test/libsinsp_e2e/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ add_executable(libsinsp_e2e_tests
paths.cpp
process.cpp
subprocess.cpp
suppress_events.cpp
sys_call_test.cpp
tcp_client_server.cpp
tcp_client_server_ipv4_mapped.cpp
Expand Down Expand Up @@ -93,7 +94,8 @@ if("${CMAKE_SIZEOF_VOID_P}" EQUAL "8")
endif("${CMAKE_SIZEOF_VOID_P}" EQUAL "8")

configure_file(
${CMAKE_CURRENT_SOURCE_DIR}/test_helper.sh ${CMAKE_CURRENT_BINARY_DIR}/test_helper.sh COPYONLY
${CMAKE_CURRENT_SOURCE_DIR}/test_helper.sh.in
${CMAKE_CURRENT_BINARY_DIR}/test_helper.sh
)

file(COPY
Expand Down
11 changes: 9 additions & 2 deletions test/libsinsp_e2e/event_capture.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ void event_capture::init_inspector()
"couldn't open inspector (maybe driver hasn't been loaded yet?) err=" +
get_inspector()->getlasterr() + " exception=" + e.what();
{
//std::unique_lock<std::mutex> lock(m_mutex);
m_capture_started = true;
m_condition_started.notify_one();
}
Expand All @@ -88,7 +87,15 @@ void event_capture::capture()
if(!inspector_ok)
{
init_inspector();
inspector_ok = true;
if(!m_start_failed)
{
inspector_ok = true;
}
else
{
std::cerr << m_start_failure_message << std::endl;
return;
}
}

m_param.m_inspector = get_inspector();
Expand Down
11 changes: 9 additions & 2 deletions test/libsinsp_e2e/forking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -336,6 +336,7 @@ static int clone_callback_1(void* arg)
// using a weird clone() here something goes wrong with
// recent version of glibc
ctid = syscall(SYS_getpid);
fsync(cp->fd);
therealbobo marked this conversation as resolved.
Show resolved Hide resolved
close(cp->fd);
return 0;
}
Expand All @@ -346,6 +347,7 @@ TEST_F(sys_call_test, forking_clone_fs)
char bcwd[1024];
int prfd;
int ptid; // parent tid
pid_t clone_tid;
int child_tid;
int parent_res;
int flags = CLONE_FILES | CLONE_FS | CLONE_VM | CLONE_PARENT_SETTID;
Expand Down Expand Up @@ -394,7 +396,7 @@ TEST_F(sys_call_test, forking_clone_fs)

/* Create child; child commences execution in childFunc() */

pid_t clone_tid = clone(clone_callback_1, stackTop, flags, &cp,
clone_tid = clone(clone_callback_1, stackTop, flags, &cp,
&child_tid);
if (clone_tid == -1)
FAIL();
Expand Down Expand Up @@ -477,7 +479,12 @@ TEST_F(sys_call_test, forking_clone_fs)
sinsp_fdinfo* fdi = ti->get_fd(prfd);
if(fdi && fdi->tostring_clean().find(FILENAME) != std::string::npos)
{
EXPECT_EQ(parent_res, res);
EXPECT_EQ(parent_res, res) << "filename: "
<< fdi->tostring_clean() << std::endl
<< "res: " << res << std::endl
<< "parent tid: " << ptid << std::endl
<< "child tid: " << child_tid << std::endl
<< "clone tid: " << clone_tid << std::endl;
}
}
else if (ti->m_tid == child_tid)
Expand Down
Loading
Loading