Feature/load new lifecycle config by NicolasFussberger · Pull Request #208 · eclipse-score/lifecycle

NicolasFussberger · 2026-05-19T12:44:30Z

Introduces an interface in src/launch_manager_daemon/config for loading a launch manager configuration file
Introduce an implementation of this interface using flatbuffer
Introduce unit tests for the new code

Note: The code is not actually in use yet. The python script which does the translation from the user-facing json configuration to the flatbuffer json format is not part of the PR and will be added in a follow up PR.
Furthermore, the launch manager code needs to be adapted to use the new structure.

Background:

In the last months we had defined a new user-facing configuration format (see src/launch_manager_daemon/config/config_schema/launch_manager.schema.json).
However, this new format is currently still mapped to the old configuration files (to processes / process groups). So the code is not actually reading the new configuration file

Relates To: #209

github-actions · 2026-05-19T12:45:30Z

License Check Results

🚀 The license check job ran with the Bazel command:

bazel run --lockfile_mode=error //:license-check

Status: ⚠️ Needs Review

Click to expand output

[License Check Output]
Extracting Bazel installation...
Starting local Bazel server (8.4.2) and connecting to it...
INFO: Invocation ID: 18448b84-b23d-42bb-90f0-11b5cebf5262
Computing main repo mapping: 
Computing main repo mapping: 
Loading: 
Loading: 0 packages loaded
Loading: 0 packages loaded
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Loading: 0 packages loaded
    currently loading: 
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)
Analyzing: target //:license-check (1 packages loaded, 0 targets configured)

Analyzing: target //:license-check (30 packages loaded, 10 targets configured)

Analyzing: target //:license-check (85 packages loaded, 10 targets configured)

Analyzing: target //:license-check (87 packages loaded, 13 targets configured)

Analyzing: target //:license-check (141 packages loaded, 2727 targets configured)

Analyzing: target //:license-check (150 packages loaded, 5404 targets configured)

Analyzing: target //:license-check (155 packages loaded, 5453 targets configured)

Analyzing: target //:license-check (158 packages loaded, 7525 targets configured)

Analyzing: target //:license-check (162 packages loaded, 10142 targets configured)

Analyzing: target //:license-check (162 packages loaded, 10142 targets configured)

INFO: Analyzed target //:license-check (163 packages loaded, 10268 targets configured).
[12 / 16] JavaToolchainCompileClasses external/rules_java+/toolchains/platformclasspath_classes; 0s disk-cache, processwrapper-sandbox ... (2 actions running)
[14 / 16] JavaToolchainCompileBootClasspath external/rules_java+/toolchains/platformclasspath.jar; 0s disk-cache, processwrapper-sandbox
[15 / 16] Building license.check.license_check.jar (); 0s disk-cache, multiplex-worker
INFO: Found 1 target...
Target //:license.check.license_check up-to-date:
  bazel-bin/license.check.license_check
  bazel-bin/license.check.license_check.jar
INFO: Elapsed time: 23.691s, Critical Path: 2.69s
INFO: 16 processes: 12 internal, 3 processwrapper-sandbox, 1 worker.
INFO: Build completed successfully, 16 total actions
INFO: Running command line: bazel-bin/license.check.license_check ./formatted.txt <args omitted>
usage: org.eclipse.dash.licenses.cli.Main [-batch <int>] [-cd <url>]
       [-confidence <int>] [-ef <url>] [-excludeSources <sources>] [-help] [-lic
       <url>] [-project <shortname>] [-repo <url>] [-review] [-summary <file>]
       [-timeout <seconds>] [-token <token>]

github-actions · 2026-05-19T12:51:29Z

The created documentation from the pull request is available at: docu-html

NicolasFussberger · 2026-05-19T13:28:41Z

+    module_name = "score_baselibs",
+    commit = "498a4b256c9073602140243d30c33b106e279f75",
+    remote = "https://github.com/eclipse-score/baselibs.git",
+)


@paulquiring Any idea when a new baselibs release will be available containing the new flatbuffer functionality?

Next release of baselibs: planed on 21.05 or 22.05. See https://github.com/orgs/eclipse-score/discussions/2390?sort=new#discussioncomment-16987333

paulquiring · 2026-05-20T08:02:15Z

+    }
+
+  private:
+    friend class Builder;


A comment would have been helpful to mention that this saves the need to write getters in the builder class.

Discussed in meeting. The friend is needed to access the private constructor of the Config class.
This is in line with typical builder pattern implementation

Moving the Builder class to the outer scope so that it is not a nested class anymore.
Nested class is not required and may be discouraged by some SCA rules.

Done in f70d64b

paulquiring · 2026-05-20T08:45:20Z

+        {
+            if (ev != nullptr && ev->key() != nullptr)
+            {
+                result.emplace(ev->key()->str(), safeString(ev->value()));


I guess the key cannot be nullptr?

In the flatbuffer schema I have defined it as

// A key-value pair representing an environment variable. table EnvironmentalVariable { key:string (required); value:string (required); }

So I think if an environment variable is there, the key and value must be available.

Consider adding asserts for values that are never expected to be nullptr.
safestring can be removed for value() because it is required in the schema

Asserts added in 99cdb9c

paulquiring · 2026-05-20T08:51:29Z

+
+SwitchRunTargetAction convertRequiredSwitchRunTargetAction(const fb::SwitchRunTargetAction* sa)
+{
+    return convertSwitchRunTargetAction(sa).value();


value can throw.

.value_or(std::string{}) as alternative?

It should never happen because the SwitchRunTargetRecoveryAction is defined as required in this context:

// Configuration for a Run Target representing an operational mode of the system. table RunTarget { .... // Recovery action when a component in this Run Target fails. recovery_action:SwitchRunTargetAction (required); }

However to be defensive against potential future changes on the schema, we could:

Add a try/catch within parseFlatbuffer and return score::cpp::make_unexpected(IConfigLoader::Error::InvalidFormat); in case there is any exception.

And/or put in an assertion

auto opt=convertSwitchRunTargetAction(sa) assert(opt.has_value() && "SwitchRunTargetAction is null although it is expected to be a mandatory configuration"); return opt.value();

What do you think?

Discussed: We shall add asserts

Done in 99cdb9c

paulquiring · 2026-05-20T12:26:09Z

+    module_name = "score_baselibs",
+    commit = "498a4b256c9073602140243d30c33b106e279f75",
+    remote = "https://github.com/eclipse-score/baselibs.git",
+)


Next release of baselibs: planed on 21.05 or 22.05. See https://github.com/orgs/eclipse-score/discussions/2390?sort=new#discussioncomment-16987333

paulquiring · 2026-05-20T12:37:17Z

+constexpr int32_t kExpectedSchemaVersion = 1;
+constexpr double kSecondsToMilliseconds = 1000.0;
+
+uint32_t secondsToMs(double seconds)


#include <iostream> #include <cstdint> #include <string_view> #include <cassert> constexpr double kSecondsToMilliseconds = 1000.0; uint32_t secondsToMs(double seconds) { return static_cast<uint32_t>(seconds * kSecondsToMilliseconds); } int main () { assert(secondsToMs(0.000500) == 0);// holds is this a problem ? assert(secondsToMs(0.001000) == 1); }

if this is a problem we might need to change the fbs

I guess this is not an issue.

The user is configuring all time values in seconds

Though we should reconsider whether the want to switch the configuration to ms.

How to safeguard that the user does not configure anything < 1ms?

Adding an assertion for now, to be moved to schema validation later

Assertion added in a1f2860

Assertion fails if configured seconds > 0 but is rounded down to 0ms.

paulquiring · 2026-05-20T17:22:14Z

review from my side is finished.

SimonKozik · 2026-05-20T14:12:44Z

+
+uint32_t secondsToMs(double seconds)
+{
+    return static_cast<uint32_t>(seconds * kSecondsToMilliseconds);


Could this result in UB if seconds < 0 or > UINT32_MAX?

Schema validator should take care about negative input, but static analysis or compiler warnings may not know about this. However we don't have cap on max timeout in schema.

Maybe we can log warning if value is too small or too big and apply some boundary values

SimonKozik · 2026-05-20T14:17:50Z

+    Config::Builder builder;
+
+    // initial_run_target is a required field, guaranteed non-null by the schema and verifier.
+    builder.setInitialRunTarget(config->initial_run_target()->str());


I have mixed feelings about defensive coding, but if somebody prepares crazy config (bypassing verifier), we will crash instead of printing error message and exiting.

However if somebody is doing this, then maybe, they should not expect much support from launch manager.

I had similar discussion with Paul. For fields that are marked as required in the flatbuffer schema, flatbuffer will ensure that this is not nullptr.

Nevertheless, we decided for adding additional asserts to check that those are not nullptr.
Adding further nullptr checks seemed redundant and would make covering this code via tests almost impossible.

Is that approach fine for you?

paulquiring · 2026-05-21T13:27:56Z

+
+struct Sandbox
+{
+    uint32_t uid{};


uid_t

Suggested change

uint32_t uid{};

uid_t uid{};

done here efe69a0

paulquiring · 2026-05-21T13:28:13Z

+struct Sandbox
+{
+    uint32_t uid{};
+    uint32_t gid{};


Suggested change

uint32_t gid{};

gid_t gid{};

done here efe69a0

paulquiring · 2026-05-21T13:31:59Z

+{
+    uint32_t ready_timeout_ms{};
+    uint32_t shutdown_timeout_ms{};
+    std::unordered_map<std::string, std::string> environmental_variables;


from: https://linux.die.net/man/3/execve
we need a format like this:

char *env[] = { "HOME=/usr/home", "LOGNAME=home", (char *)0 };

this will do the job

std::vector<const char*> environmental_variables = {"HOME=/usr/home", "LOGNAME=home" , nullptr };

execve(... , const_cast<char* const*>(environmental_variables .data()));

With std::vector<const char*> I think there is the question where do the strings live in case they are not constant char sequences. We would require a combination of

std::vector<const char*> pointers_; std::vector<std::string> strings_;

Please tell me if this to too overcomplicated. But I see the use case that we might want to add an environment variable before starting the process that was not configured by the user. We had this use case already in the past.

This could be achieved rather cleanly with a dedicated class, that stores both std::vector<const char*> and std::vector<std::string>

class EnvironmentVariable { public: EnvironmentVariable(std::string_view key, std::string_view value); std::string_view key() const; std::string_view value() const; const char* c_str() const; private: std::string entry_; // internal storage in "key=value" format. std::size_t key_length_; }; class Environment { public: using const_iterator = std::vector<EnvironmentVariable>::const_iterator; Environment() = default; Environment(const Environment&) = delete; Environment& operator=(const Environment&) = delete; Environment(Environment&& other) noexcept; Environment& operator=(Environment&& other) noexcept; ~Environment() = default; void reserve(std::size_t count); void add(std::string_view key, std::string_view value); const_iterator begin() const; const_iterator end() const; std::size_t size() const; // Access format expected by execve systemcall const std::vector<const char*>& envp() const; private: void rebuildPointers(); std::vector<EnvironmentVariable> entries_; std::vector<const char*> pointers_; };

The alternative that I see is to keep doing what we do know:
char* envp_[static_cast<std::size_t>(score::lcm::internal::kEnvArraySize)];

However, than we have fixed size and need to deal with the scenario this size may be exceeded by the configuration. Still there is the problem of string ownership for which I believe we will need to wrap it in a dedicated class.

paulquiring · 2026-05-21T13:35:58Z

+{
+    uint32_t uid{};
+    uint32_t gid{};
+    std::vector<uint32_t> supplementary_group_ids;


Suggested change

std::vector<uint32_t> supplementary_group_ids;

std::vector<gid_t > supplementary_group_ids;

done here efe69a0

NicolasFussberger · 2026-05-22T08:10:44Z

+    gid_t gid{};
+    std::vector<gid_t> supplementary_group_ids;
+    std::optional<std::string> security_policy;
+    std::string scheduling_policy;


Do we want to translate scheduling_policy already to an integer?

I think for logging it might be nice to have the string available, so you can log "SCHED_OTHER" instead of "scheduling policy: 1".

Though I don't have a string opinion. Storing the same string over and over again is also not nice.

Probably makes sense to already translate it to int

NicolasFussberger added 10 commits May 18, 2026 15:37

Provide new configuration interfaces

73fd84e

Upgrade baselibs & flatbuffer version

164b832

Use baselibs flatbuffer helper

2325ae7

Config refactoring

5090054

Use bazel rule to generate fb file

d73fa63

Extend documentation

dcaa534

Narrow RecoveryAction options

5203bbc

Remove optional from required fields

c9367b1

Remove unrelated test output

fb24424

Run formater on new code

b5f777f

NicolasFussberger had a problem deploying to workflow-approval May 19, 2026 12:44 — with GitHub Actions Failure

NicolasFussberger had a problem deploying to workflow-approval May 19, 2026 12:44 — with GitHub Actions Error

NicolasFussberger requested a review from paulquiring May 19, 2026 12:45

NicolasFussberger commented May 19, 2026

View reviewed changes

paulquiring reviewed May 20, 2026

View reviewed changes

Comment thread src/launch_manager_daemon/config/src/new_lm_flatcfg.fbs

NicolasFussberger added 3 commits May 21, 2026 08:21

Change method names to CamelCase

eb8a044

Move accessor method impl to cpp file

d523966

Add #endif comments

24d6e5f

NicolasFussberger requested a deployment to workflow-approval May 21, 2026 06:46 — with GitHub Actions Waiting

ConfigBuilder is no longer a nested class

f70d64b

NicolasFussberger requested a deployment to workflow-approval May 21, 2026 09:25 — with GitHub Actions Waiting

Add assertions for required fields

99cdb9c

NicolasFussberger requested a deployment to workflow-approval May 21, 2026 09:34 — with GitHub Actions Waiting

Add assertion for sub-ms precision

a1f2860

NicolasFussberger requested a deployment to workflow-approval May 21, 2026 09:37 — with GitHub Actions Waiting

SimonKozik reviewed May 21, 2026

View reviewed changes

Merge branch 'main' into feature/load-new-lifecycle-config

89b2eb7

paulquiring requested a deployment to workflow-approval May 21, 2026 11:53 — with GitHub Actions Waiting

paulquiring suggested changes May 21, 2026

View reviewed changes

Use uid_t and gid_t types

efe69a0

NicolasFussberger requested a deployment to workflow-approval May 22, 2026 06:23 — with GitHub Actions Waiting

NicolasFussberger commented May 22, 2026

View reviewed changes

	std::vector<uint32_t> supplementary_group_ids;
	std::vector<gid_t > supplementary_group_ids;

Conversation

NicolasFussberger commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions Bot commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

License Check Results

Uh oh!

github-actions Bot commented May 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NicolasFussberger May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NicolasFussberger May 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

paulquiring commented May 20, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NicolasFussberger May 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NicolasFussberger commented May 19, 2026 •

edited

Loading

github-actions Bot commented May 19, 2026 •

edited

Loading

NicolasFussberger May 21, 2026 •

edited

Loading

NicolasFussberger May 21, 2026 •

edited

Loading

NicolasFussberger May 22, 2026 •

edited

Loading

NicolasFussberger May 22, 2026 •

edited

Loading

NicolasFussberger May 22, 2026 •

edited

Loading