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

iox-#2330 add notify systemd #2334

Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
fe5015f
iox-#2330 Add libsystemd-dev to install dependencies
khromenokroman Aug 22, 2024
25155df
iox-#2330 Add systemd build option
khromenokroman Aug 22, 2024
e09617a
iox-#2330 Add conditional compilation for systemd support
khromenokroman Aug 22, 2024
76dd33f
iox-#2330 Rename listen_thread_watchdog to m_listen_thread_watchdog
khromenokroman Aug 22, 2024
485bfa4
iox-#2330 Improve thread management and error logging
khromenokroman Aug 23, 2024
d72b763
iox-#2330 Add early returns for watchdog error handling
khromenokroman Aug 23, 2024
07c89d9
iox-#2330 Refactor environment variable handling for 'INVOCATION_ID'
khromenokroman Aug 23, 2024
425db5e
iox-#2330 Refine systemd configuration for iceoryx_posh_roudi
khromenokroman Aug 23, 2024
952e91f
iox-#2330 Remove unnecessary dependency from GitHub action
khromenokroman Aug 23, 2024
7abf543
iox-#2330 Refactor systemd handling into a dedicated class
khromenokroman Aug 23, 2024
bc13c4b
iox-#2330 Add shutdown method to Systemd_service_handler
khromenokroman Aug 23, 2024
4aee645
iox-#2330 Refactor Systemd include with conditional compilation
khromenokroman Aug 24, 2024
864d7b1
iox-#2330 Refactor class names and method names for consistency
khromenokroman Aug 24, 2024
a5f4bb4
iox-#2330 Refactor systemd handler variable names
khromenokroman Aug 24, 2024
4b4651c
iox-#2330 Update changelog (text, reference)
khromenokroman Aug 24, 2024
3c334a6
iox-#2330 Add systemd support example to installation guide
khromenokroman Aug 24, 2024
8173252
iox-#2330 Add condition_variable to watchdog loop
khromenokroman Aug 24, 2024
efe3917
iox-#2330 Refactor environment variable retrieval
khromenokroman Aug 24, 2024
4ba2e9e
iox-#2330 Refactor SystemdServiceHandler for better readability
khromenokroman Aug 24, 2024
03d6f0d
iox-#2330 Refactor systemd notification handler
khromenokroman Aug 25, 2024
62ba7b0
iox-#2330 Refactor SDNotify signal handling
khromenokroman Aug 25, 2024
f72cc56
iox-#2330 Refactor systemd include for conditional compilation
khromenokroman Aug 25, 2024
92fa972
iox-#2330 Refactor service management class names
khromenokroman Aug 28, 2024
2defac8
iox-#2330 Add Ubuntu Systemd build job to CI
khromenokroman Aug 28, 2024
19f09c5
iox-#2330 test ci
khromenokroman Aug 28, 2024
ad57509
iox-#2330 test ci
khromenokroman Aug 28, 2024
b4e3db2
iox-#2330 Add dependency installation to build workflow
khromenokroman Aug 28, 2024
8b50b7b
iox-#2330 Add step to show systemd unit file in workflow
khromenokroman Aug 28, 2024
5abaede
iox-#2330 Update method to create systemd unit file
khromenokroman Aug 28, 2024
1d1b73b
iox-#2330 Add steps to manage test_iox with systemd
khromenokroman Aug 28, 2024
93a1ac9
iox-#2330 Update ExecStart path in systemd unit file
khromenokroman Aug 28, 2024
022d7fd
iox-#2330 Add systemctl status check in build workflow
khromenokroman Aug 28, 2024
5f5afa1
iox-#2330 Refactor build workflow to use systemd container
khromenokroman Aug 28, 2024
ef4bffd
iox-#2330 Refactor workflow to update package list before install
khromenokroman Aug 28, 2024
f73c9c6
iox-#2330 Refactor workflow to update package list before install
khromenokroman Aug 28, 2024
41b10a2
iox-#2330 Update systemd container image in workflow
khromenokroman Aug 28, 2024
ee418c3
iox-#2330 Add git to Build Systemd workflow
khromenokroman Aug 28, 2024
2914009
iox-#2330 Add dbus to Build Systemd workflow
khromenokroman Aug 28, 2024
402a626
iox-#2330 Refactor workflow to remove container usage
khromenokroman Aug 28, 2024
250b4e4
iox-#2330 Add sudo to commands in build_systemd.yaml
khromenokroman Aug 28, 2024
7d6d7e2
iox-#2330 Fix incorrect path in systemd unit file display step
khromenokroman Aug 28, 2024
d84998f
iox-#2330 Update ExecStart path and improve service start feedback
khromenokroman Aug 28, 2024
7ab593c
iox-#2330 Update systemd unit file to remove specific user
khromenokroman Aug 28, 2024
ab28d4f
iox-#2330 Add journal log display steps in build workflow
khromenokroman Aug 28, 2024
58c5feb
iox-#2330 Update build workflow with roudi start failure handling
khromenokroman Aug 28, 2024
f4cdef7
iox-#2330 Add systemd run checks to build workflow
khromenokroman Aug 29, 2024
e96b661
iox-#2330 Remove systemd build workflow and fix namespace typo
khromenokroman Aug 29, 2024
20793f5
iox-#2330 Refactor call to `iox_getenv_s` for type safety
khromenokroman Aug 29, 2024
ef310cb
iox-#2330 Refactor call to iox_getenv_s for type safety (rev.2)
khromenokroman Aug 29, 2024
a5f4534
Merge branch 'eclipse-iceoryx:main' into iox-#2330-add_notify_systemd
khromenokroman Sep 1, 2024
c9236aa
iox-#2330 Refactor ci
khromenokroman Sep 1, 2024
9775883
iox-#2330 Edit CMakeLists.txt
khromenokroman Sep 9, 2024
a60d701
iox-#2330 Edit CMakeLists.txt
khromenokroman Sep 9, 2024
ddce6b5
iox-#2330 Edit note-release
khromenokroman Sep 9, 2024
0921e43
iox-#2330 Sort bazel
khromenokroman Sep 9, 2024
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
2 changes: 1 addition & 1 deletion .github/actions/install-iceoryx-deps-and-clang/action.yml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ runs:
sudo wget -O - https://apt.llvm.org/llvm-snapshot.gpg.key | sudo apt-key add -
sudo add-apt-repository "deb http://apt.llvm.org/focal/ llvm-toolchain-focal-15 main"
sudo apt-get update
sudo apt-get install -y libacl1-dev libncurses5-dev
sudo apt-get install -y libacl1-dev libncurses5-dev libsystemd-dev
sudo apt-get install -y clang-format-15 clang-tidy-15 clang-tools-15 clang-15 lld
sudo rm /usr/bin/clang
sudo rm /usr/bin/clang++
Expand Down
2 changes: 2 additions & 0 deletions iceoryx_meta/build_options.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ option(TOML_CONFIG "TOML support for RouDi with dynamic configuration" ON)
option(IOX_EXPERIMENTAL_POSH "Export experimental posh features (no guarantees)" OFF)
option(IOX_IGNORE_32_BIT_CHECK "Ignores the check for 32 bit systems! It is not recommended to turn this on in production systems" OFF)
option(IOX_REPRODUCIBLE_BUILD "Create reproducible builds by omit setting the build timestamp in the version header" ON)
option(USE_SYSTEMD "Build with systemd support" OFF)

set(CMAKE_EXPORT_COMPILE_COMMANDS ON) # "Create compile_commands.json file"

Expand Down Expand Up @@ -99,4 +100,5 @@ function(show_config_options)
message(" IOX_EXPERIMENTAL_POSH................: " ${IOX_EXPERIMENTAL_POSH})
message(" IOX_IGNORE_32_BIT_CHECK..............: " ${IOX_IGNORE_32_BIT_CHECK})
message(" IOX_REPRODUCIBLE_BUILD...............: " ${IOX_REPRODUCIBLE_BUILD})
message(" USE_SYSTEMD..........................: " ${USE_SYSTEMD})
endfunction()
2 changes: 1 addition & 1 deletion iceoryx_platform/cmake/IceoryxPackageHelper.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ Macro(iox_add_executable)
if ( QNX )
target_link_libraries(${IOX_TARGET} ${IOX_LIBS_QNX})
elseif ( LINUX )
target_link_libraries(${IOX_TARGET} ${IOX_LIBS_LINUX})
target_link_libraries(${IOX_TARGET} ${IOX_LIBS_LINUX} systemd)
Copy link
Member

Choose a reason for hiding this comment

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

Please add this only to the iox-roudi target

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@elBoberido fixed

elseif ( APPLE )
target_link_libraries(${IOX_TARGET} ${IOX_LIBS_APPLE})
elseif ( WIN32 )
Expand Down
4 changes: 4 additions & 0 deletions iceoryx_posh/include/iceoryx_posh/internal/roudi/roudi.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@
#include "iox/smart_lock.hpp"

#include <cstdint>
#include <systemd/sd-daemon.h>
#include <thread>

namespace iox
Expand All @@ -51,6 +52,8 @@ class RouDi
PortManager& portManager,
const config::RouDiConfig& roudiConfig) noexcept;

static constexpr uint16_t SIZE_ERROR_MESSAGE = 4096;

virtual ~RouDi() noexcept;

/// @brief Triggers the discovery loop to run immediately instead of waiting for the next tick interval
Expand Down Expand Up @@ -129,6 +132,7 @@ class RouDi
private:
std::thread m_monitoringAndDiscoveryThread;
std::thread m_handleRuntimeMessageThread;
std::thread listen_thread_watchdog; // 8
Copy link
Member

Choose a reason for hiding this comment

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

Please prefix members with m_

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@elBoberido I'm sorry for my mistake, it's been fixed

Copy link
Member

Choose a reason for hiding this comment

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

No Problem. But do not use ifdefs in the code. Please write an abstraction, similar to what we did IpcChannelType and use that abstraction. For platforms which do not support systemd, write an alternative which basically does nothing when the methods are called. Having ifdefs all over the place makes it hard to maintain and therefore we choose the other approach.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@elBoberido I'm not sure if I understood you correctly, please look at the roudi.hpp file did you talk about this? The announcement and the definition will share this is a test for now, thank you :)


protected:
ProcessIntrospectionType m_processIntrospection;
Expand Down
48 changes: 48 additions & 0 deletions iceoryx_posh/source/roudi/roudi.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -161,6 +161,15 @@ void RouDi::shutdown() noexcept
// Postpone the IpcChannelThread in order to receive TERMINATION
m_runHandleRuntimeMessageThread = false;

/*
* This is necessary to prevent the main thread from exiting before
* the 'listen_thread_watchdog' has finished, hence ensuring a
* proper termination of the entire application.
*/
if (listen_thread_watchdog.joinable()) {
listen_thread_watchdog.join();
}

if (m_handleRuntimeMessageThread.joinable())
{
IOX_LOG(DEBUG, "Joining 'IPC-msg-process' thread...");
Expand Down Expand Up @@ -254,6 +263,45 @@ void RouDi::processRuntimeMessages(runtime::IpcInterfaceCreator&& roudiIpcInterf
IOX_LOG(INFO, "RouDi is ready for clients");
fflush(stdout); // explicitly flush 'stdout' for 'launch_testing'

/*
* We get information about how they are running. If as a unit, then we launch
* watchdog and send a notification about the launch, otherwise we do nothing
*/
const char* invocation_id = std::getenv("INVOCATION_ID");
Copy link
Member

Choose a reason for hiding this comment

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

Please use iox_getenv_s

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please use iox_getenv_s

@elBoberido fixed

if (invocation_id != nullptr)
{
IOX_LOG(WARN, "Run APP in unit(systemd)");
listen_thread_watchdog = std::thread([this] {
if (auto wdres = sd_notify(0, "READY=1") < 0)
{
std::array<char, SIZE_ERROR_MESSAGE> buf{};
strerror_r(-static_cast<int>(wdres), buf.data(), buf.size());
IOX_LOG(ERROR, "WatchDogError: " << std::string(buf.data()));
return;
}
IOX_LOG(DEBUG, "WatchDog READY=1");

IOX_LOG(INFO, "Start watchdog");
while (m_runHandleRuntimeMessageThread.load())
{
if (auto wdres = sd_notify(0, "WATCHDOG=1") < 0)
Copy link
Member

Choose a reason for hiding this comment

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

Please use the IOX_POSIX_CALL. It takes care of the error handling

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please use the IOX_POSIX_CALL. It takes care of the error handling

@elBoberido fixed

{
std::array<char, SIZE_ERROR_MESSAGE> buf{};
strerror_r(-static_cast<int>(wdres), buf.data(), buf.size());
IOX_LOG(ERROR, "WatchDogError: " << std::string(buf.data()));
return;
}
std::this_thread::sleep_for(std::chrono::seconds(1));
Copy link
Member

Choose a reason for hiding this comment

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

This needs to be replaced with a blocking timed_wait on a semaphore or something else which can be interupted, else the test time will be massively increased.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@elBoberido I'm not very good at multithreaded programming, but in my opinion it's not a very elegant solution :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@elBoberido fixed

}
});
if (pthread_setname_np(listen_thread_watchdog.native_handle(), "watchdog") != 0)
{
std::array<char, SIZE_ERROR_MESSAGE> buf{};
strerror_r(errno, buf.data(), buf.size());
IOX_LOG(ERROR, "Can not set name for thread watchdog: " << std::string(buf.data()));
}
Copy link
Member

Choose a reason for hiding this comment

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

Please use setThreadName

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Please use setThreadName

@elBoberido fixed

}

while (m_runHandleRuntimeMessageThread)
{
// read RouDi's IPC channel
Expand Down