Skip to content

ENT-14434: Made reactor-plugin into its own forked process - #6335

Open
victormlg wants to merge 1 commit into
cfengine:masterfrom
victormlg:cf-reactor-move-around
Open

ENT-14434: Made reactor-plugin into its own forked process#6335
victormlg wants to merge 1 commit into
cfengine:masterfrom
victormlg:cf-reactor-move-around

Conversation

@victormlg

@victormlg victormlg commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Context

So originally, we wanted to have two daemons: one for cf-reactor and one for the agent driven cfengine code. Then it has been decided to merge these two binaries into a single one. For this to work I had to make the nova code for cf-reactor into a dynamic library. When I did that, I didn't change much of the structure of the code: cf-reactor was a binary in core, that would call the function ReactorEnterpriseMain, which starts a daemon for the reactor-plugin.

So this PR aims to move the daemon code to core, as it was originally planned, by making core cf-reactor into a daemon, and the reactor-plugin to start as a subprocess of it

Merge together: https://github.com/cfengine/enterprise/pull/996 https://github.com/cfengine/nova/pull/2696 NorthernTechHQ/libntech#299

@victormlg
victormlg marked this pull request as ready for review August 25, 2026 14:45
@victormlg
victormlg force-pushed the cf-reactor-move-around branch from 2e7ed0e to dfd91c6 Compare August 25, 2026 14:46
@victormlg
victormlg requested a review from larsewi August 26, 2026 09:54

@larsewi larsewi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please add some more context. E.g., explain why chose to make cf-reactor plugin its own process. Also I expected cf-reactor in nova to already fork out and make it a daemon, but I have not seen this code removed.

@victormlg
victormlg requested a review from larsewi August 28, 2026 07:47

@larsewi larsewi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

There one major issue here. If the nova fork dies, then core will continue running and no one will restart the nova reactor again. With this architecture, core must be responsible for restarting the nova fork if it dies.

Comment thread cf-reactor/cf-reactor.c Outdated
Comment thread cf-reactor/cf-reactor.c
Comment thread cf-reactor/cf-reactor.c Outdated
Comment thread cf-reactor/cf-reactor.c
@victormlg
victormlg force-pushed the cf-reactor-move-around branch 3 times, most recently from 553a07c to aa6d538 Compare August 28, 2026 12:44
@victormlg
victormlg requested a review from larsewi August 28, 2026 12:45
@victormlg
victormlg force-pushed the cf-reactor-move-around branch from aa6d538 to 7228cb8 Compare August 28, 2026 12:48
Ticket: ENT-14434
Signed-off-by: Victor Moene <victor.moene@northern.tech>
@victormlg
victormlg force-pushed the cf-reactor-move-around branch from 7228cb8 to be91f14 Compare August 28, 2026 12:50

@larsewi larsewi left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please write a test where you kill the nova cf-reactor and see that it comes back. Maybe also test reaching that limit that you set.

Comment thread cf-reactor/cf-reactor.c
break;
}

if (time(NULL) - child_started_at > REACTOR_MIN_UPTIME_SECS)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
if (time(NULL) - child_started_at > REACTOR_MIN_UPTIME_SECS)
if ((time(NULL) - child_started_at) > REACTOR_MIN_UPTIME_SECS)

Comment thread cf-reactor/cf-reactor.c
Comment on lines +322 to +323
/* Here cf-reactor tries to restart the reactor plugin if it exited. It retries
* 10 times before giving up. The counter is set back to 0 after some time without failure */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe the core process should die itself. Otherwise, systemd will think everything is fine. Not sure. We are basically implementing systemd ourself by now. Could be worth discussing with PM if this is the correct way to go.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants