ENT-14195: Event handling file deletion by victormlg · Pull Request #6346 · cfengine/core

victormlg · 2026-09-03T14:18:52Z

The function WatcherRegister is what allows to add watchers to the reactor loop. Its definition looks like:

void WatcherRegister(const char *key, EventType type, void *payload,
  Bundle *bundle, time_t interval);

Where key is the name of the promise, type is the attribute in the when body, payload is some data used specifically for the event type, for example in our case the name of the file watched. bundle is the bundle to be run, for now only a single one, and finally interval is how often do we want to check for the file.

Exemple:

WatcherRegister("a.txt deleted", FILE_DELETED, 
  FileWatcherPayloadNew("/tmp/a.txt"), create_file_bundle, 1);

This maps to events promises:

body when file_deleted(filename)
{
  file_deleted => "$(filename)";
}

bundle reactor A
{
  vars:
    "filename" string => "/tmp/a.txt";

  events:
    "a.txt deleted"
      when => file_deleted("$(filename)"),
      then => create_file("$(filename)");
}

Implementation

We have a thread iterating through the Watcher list. When an event is registered, it pushes the key on a queue and signals the daemon that there is something on the queue. Then, inside ReactorContextHandleEvents, the bundle that should be run is looked up from the key in a hashmap.

Ticket: ENT-14195 Signed-off-by: Victor Moene <victor.moene@northern.tech>

olehermanse · 2026-09-04T17:54:30Z

@cf-bottom jenkins, please

cf-bottom · 2026-09-04T18:02:37Z

Alright, I triggered a build:

Jenkins: https://ci.cfengine.com/job/pr-pipeline/14636/

Packages: http://buildcache.cfengine.com/packages/testing-pr/jenkins-pr-pipeline-14636/

olehermanse

I think this PR changes too many things in one PR and one commit; I would split it up like this:

1. README

Add cf-reactor/README.md with an explanation of how this should work. This could explain the different sections I've listed below.

2. Moving stuff around

Looks like you are moving some stuff around for having the "Nova" functions in the right place. This could be its own commit.

3. Empty main loop

You could set up an empty infinite main loop, with just a sleep statement and TODO comments for what we're going to implement below. At this point, cf-reactor only does the nova reactor-plugin stuff, and runs forever (sleeping most of the time). This is the same as we expect in the future as well (as long as user didn't write any new event promises).

4. Tracking spec

Policy needs to be converted to a data structure of what we want to track.
Events promises may have conditions (class guards), so those should be included.
Policy may also be changed, so how to reload should be considered, or there should be a TODO comment to implement that later.

5. Polling

Implement the part where you actually check if the file exists, and record that in a current state, and do this again and again. At this point there should be TODO comments for actually creating events. Also some TODO comments about how this would be implemented for the non-polling way might be appropriate.

6. Events

Implement the part where you generate events from changes in the current state between polling. I would even go so far as to just creating the events and sending them to a queue / handler that does nothing, just deletes them with a TODO comment to actually handle the events.

7. Actually running the bundles

Implement the last step of actually running the bundle.

Why / how

I would start by opening a PR with only the README (1), to get that reviewed as soon as possible, and to be able to incorporate any feedback you get into future commits / PRs.

After that / while you wait for review, I would start the second PR with commit 2 (maybe 3 and maybe 4 as well). Finally, you can keep working on a third PR with the remaining commits. When PR 1 and 2 are merged, you can consider whether PR 3 is ready to be reviewed and merged, or if you should split it up further.

I think this would make it easier for us to review, and easier for you to incorporate our feedback on the early steps into the later steps.

olehermanse · 2026-09-04T19:31:32Z

+ * @brief Shared state for the cf-reactor daemon's single select(2) loop.
+ *
+ * `all_fds` is a single flat array shared by every event source the daemon
+ * watches. Nova's fds always occupy the first `num_nova_fds` slots (Nova


Instead of referring to Nova, I think the thing you should refer to is the reactor-plugin. At least to me that is more clear.

olehermanse · 2026-09-04T19:36:16Z

+typedef struct
+{
+    char *path;
+    bool existed_last_check;
+} FileWatcherPayload;


To me, Watcher and Payload are a bit vague / ambiguous.

I would suggest simply:

Suggested change

typedef struct

{

char *path;

bool existed_last_check;

} FileWatcherPayload;

typedef struct

{

char *path;

bool exists;

} FileState;

olehermanse · 2026-09-04T19:36:51Z

+    int *all_fds;
+    size_t all_fds_capacity;
+    size_t num_nova_fds;
+    size_t num_fds;
+
+    fd_set readfds;


Clearly indicate which fields are related to reactor-plugin and which are not.

olehermanse · 2026-09-04T19:40:44Z

+#ifndef CFENGINE_WATCHER_H
+#define CFENGINE_WATCHER_H
+
+#include <cf-reactor.h>
+#include <cf3.defs.h>   /* Bundle */
+
+typedef enum
+{
+  EVENT_FILE_DELETED,
+} EventType;
+
+typedef bool (*WatcherCheckFn)(void *payload);
+typedef void (*WatcherPayloadDestroyFn)(void *payload);
+
+void WatcherRegistryInitialize(void);
+void WatcherRegistryFinalize(void);


IMO it is not completely obvious what Watcher means.

Is this polling only, or does it include future implementations which are not based on polling?

Please add a comment to the top of this file explaining this.

olehermanse · 2026-09-04T19:46:51Z

+/**
+ * @brief Register a specific watcher instance.
+ * 
+ * @param key the events promise identifier
+ * @param type the type of watcher, defined in when bodies
+ * @param payload the data used for by the watcher, depending on the type
+ * @param bundle the bundle to run on event
+ * @param interval interval between runs
+ */
+void WatcherRegister(const char *key, EventType type, void *payload, Bundle *bundle, time_t interval);


Looks like interval does not need to be a parameter (yet)?

If watcher is supposed to cover non-polling implementations, it looks out of place.

If it is always a hard coded value, it makes more sense to just use that macro name.

If interval is configurable, or varies, maybe we should mention how / where. I.e. you can set it in policy, you can set it in env var, it varies based on event type, etc.

interval is useful for debugging and it could be added in the future, so I think it makes sense to have it.

olehermanse · 2026-09-04T19:49:43Z

+    assert(ctx != NULL);
+
+    WatcherRegistryInitialize();
+    // TODO: Register watchers with `WatcherRegister()`


What does this mean? Looking at the PR and commit title and description it's not very clear to me what is implemented and what is missing and why this is still a TODO.

This just indicates where one should manually register events promise "watchers", and will be removed when we will create them from parsing the policy

github-advanced-security Bot found potential problems Sep 3, 2026

View reviewed changes

Comment thread cf-reactor/reactor_context.c Fixed

Comment thread cf-reactor/reactor_context.c Fixed

Comment thread cf-reactor/reactor_context.c Fixed

Comment thread cf-reactor/reactor_context.c Fixed

Comment thread cf-reactor/watcher.c Fixed

Comment thread cf-reactor/watcher.c Fixed

victormlg force-pushed the event-handling-file-deletion branch 2 times, most recently from 976ccce to 91ad653 Compare September 4, 2026 08:25

victormlg marked this pull request as ready for review September 4, 2026 08:26

victormlg force-pushed the event-handling-file-deletion branch 2 times, most recently from ecc967c to 5b8844b Compare September 4, 2026 09:25

github-advanced-security Bot found potential problems Sep 4, 2026

View reviewed changes

Comment thread cf-reactor/watcher.c Dismissed

Event handling for file deletion

6537ff4

Ticket: ENT-14195 Signed-off-by: Victor Moene <victor.moene@northern.tech>

victormlg force-pushed the event-handling-file-deletion branch from 5b8844b to 6537ff4 Compare September 4, 2026 10:36

victormlg requested review from craigcomstock and larsewi September 4, 2026 11:14

craigcomstock approved these changes Sep 4, 2026

View reviewed changes

olehermanse requested changes Sep 4, 2026

View reviewed changes

olehermanse reviewed Sep 4, 2026

View reviewed changes

Uh oh!

Conversation

victormlg commented Sep 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

olehermanse commented Sep 4, 2026

Uh oh!

cf-bottom commented Sep 4, 2026

Uh oh!

olehermanse left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

1. README

2. Moving stuff around

3. Empty main loop

4. Tracking spec

5. Polling

6. Events

7. Actually running the bundles

Why / how

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!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

5 participants

victormlg commented Sep 3, 2026 •

edited

Loading

olehermanse left a comment •

edited

Loading