dyncfg/health: make stock alert override removable without restart (#22511)
* dyncfg: make stock/user alert removable immediately after dyncfg override dyncfg_add_low_level() sanitizes the command mask once at registration, stripping DYNCFG_CMD_REMOVE for non-dyncfg sources. When a successful update flips a job's ownership to dyncfg, the mask was not refreshed and the node kept reporting source_type=dyncfg with no `remove`. Required an agent restart for the override to become removable. Extract dyncfg_sanitize_cmds() and reapply it after a successful update transitions the current source type to dyncfg. Cover with a regression case in dyncfgtest. * dyncfg: add direct assertions for REMOVE mask after source_type update * dyncfg: sanitize command masks on file load to prevent stale overrides
Stelios Fragkakis committed
May 21, 2026 at 09:01 UTC
c3717756a526969dbd9f915306fb55a7c464b21e
5 files changed
+79
-26
src/daemon/dyncfg/dyncfg-files.c
+6
@@ -194,6 +194,12 @@ void dyncfg_file_load(const char *d_name) {
194
195
dyncfg_set_current_from_dyncfg(&tmp);
196
197
+ // Heal cmds on load: files written by older binaries can carry a stale
198
+ // mask (e.g. an overridden stock job persisted without REMOVE pre-fix).
199
+ // The invariant elsewhere is cmds := f(type, source_type, cmds); enforce
200
+ // it here too so the conflict_cb SWAP cannot reintroduce a stale mask.
201
+ tmp.cmds = dyncfg_sanitize_cmds(tmp.type, tmp.current.source_type, tmp.cmds);
202
+
203
dictionary_set(dyncfg_globals.nodes, id, &tmp, sizeof(tmp));
204
205
// check if we need to rename the file
src/daemon/dyncfg/dyncfg-intercept.c
+1
@@ -163,6 +163,7 @@ static void dyncfg_function_intercept_job_successfully_updated(DYNCFG *df, int c
163
dyncfg_set_dyncfg_source_from_txt(df, dc->source);
164
165
dyncfg_update_status_on_successful_add_or_update(df, code);
166
+ df->cmds = dyncfg_sanitize_cmds(df->type, df->current.source_type, df->cmds);
167
}
168
169
void dyncfg_function_intercept_result_cb(BUFFER *wb, int code, void *result_cb_data) {
src/daemon/dyncfg/dyncfg-internals.h
+7
@@ -58,6 +58,13 @@ void dyncfg_file_load(const char *filename);
58
void dyncfg_file_save(const char *id, DYNCFG *df);
59
void dyncfg_file_delete(const char *id);
60
61
+// Returns the canonical command mask a node should advertise given its type
62
+// and current source_type. Idempotent and order-independent: re-run after a
63
+// node's source_type changes (e.g. UPDATE flips an existing job from
64
+// USER/STOCK to DYNCFG-owned) to refresh the mask -- this is what lets
65
+// REMOVE appear on a freshly overridden alert without an Agent restart.
66
+// The full rule table lives in dyncfg.c.
67
+DYNCFG_CMDS dyncfg_sanitize_cmds(DYNCFG_TYPE type, DYNCFG_SOURCE_TYPE source_type, DYNCFG_CMDS cmds);
68
bool dyncfg_get_schema(const char *id, BUFFER *dst);
69
70
void dyncfg_echo_cb(BUFFER *wb, int code, void *result_cb_data);
src/daemon/dyncfg/dyncfg-unittest.c
+31
-1
@@ -446,8 +446,16 @@ static int dyncfg_unittest_run(const char *cmd, BUFFER *wb, const char *payload,
446
t->expected.enabled = false;
447
if(c == DYNCFG_CMD_ENABLE)
448
t->expected.enabled = true;
449
- if(c == DYNCFG_CMD_UPDATE)
449
+ if(c == DYNCFG_CMD_UPDATE) {
450
memset(&t->current.value, 0, sizeof(t->current.value));
451
+ if(t->type == DYNCFG_TYPE_JOB) {
452
+ // a successful update on a job flips ownership to DYNCFG, so the
453
+ // node must advertise REMOVE -- we hardcode the expected change
454
+ // here (rather than calling dyncfg_sanitize_cmds()) so a regression
455
+ // in the SUT cannot mask itself by also breaking the oracle.
456
+ t->cmds |= DYNCFG_CMD_REMOVE;
457
+ }
458
+ }
459
460
if(c & (DYNCFG_CMD_UPDATE) || (c & (DYNCFG_CMD_DISABLE|DYNCFG_CMD_ENABLE) && t->type != DYNCFG_TYPE_TEMPLATE)) {
461
freez((void *)t->source);
@@ -677,6 +685,28 @@ int dyncfg_unittest(void) {
685
dyncfg_unittest_run(PLUGINSD_FUNCTION_CONFIG " unittest:async:template2 add dyn3", wb, "{\"double\":3.14,\"boolean\":true}", LINE_FILE_STR);
686
dyncfg_unittest_run(PLUGINSD_FUNCTION_CONFIG " unittest:async:template2 add dyn4", wb, "{\"double\":3.14,\"boolean\":true}", LINE_FILE_STR);
687
688
+ // ------------------------------------------------------------------------
689
+ // updating an existing user/stock-style job makes it dyncfg-owned and removable
690
+
691
+ user1->expected.value.dbl = 3.14;
692
+ user1->expected.value.bln = true;
693
+ dyncfg_unittest_run(PLUGINSD_FUNCTION_CONFIG " unittest:sync:template1:user1 update", wb, "{\"double\":3.14,\"boolean\":true}", LINE_FILE_STR);
694
+
695
+ // direct, helper-free assertion that the production node really exposes
696
+ // REMOVE after the ownership flip -- catches regressions in dyncfg_sanitize_cmds()
697
+ // or the intercept-on-successful-update path independently of the harness oracle.
698
+ {
699
+ DYNCFG *df = dictionary_get(dyncfg_globals.nodes, user1->id);
700
+ if(!df)
701
+ dyncfg_unittest_register_error(user1->id, "node missing after update");
702
+ else {
703
+ if(df->current.source_type != DYNCFG_SOURCE_TYPE_DYNCFG)
704
+ dyncfg_unittest_register_error(user1->id, "after update, current.source_type should be DYNCFG");
705
+ if(!(df->cmds & DYNCFG_CMD_REMOVE))
706
+ dyncfg_unittest_register_error(user1->id, "after update flipping ownership to DYNCFG, cmds must include REMOVE");
707
+ }
708
+ }
709
+
710
// ------------------------------------------------------------------------
711
// saving of user_disabled
712
src/daemon/dyncfg/dyncfg.c
+34
-25
@@ -303,30 +303,7 @@ bool dyncfg_job_has_registered_template(const char *id) {
303
return ret;
304
}
305
306
-bool dyncfg_add_low_level(RRDHOST *host, const char *id, const char *path,
307
- DYNCFG_STATUS status, DYNCFG_TYPE type, DYNCFG_SOURCE_TYPE source_type, const char *source,
308
- DYNCFG_CMDS cmds, usec_t created_ut, usec_t modified_ut, bool sync,
309
- HTTP_ACCESS view_access, HTTP_ACCESS edit_access,
310
- rrd_function_execute_cb_t execute_cb, void *execute_cb_data) {
311
-
312
- if(view_access == HTTP_ACCESS_NONE)
313
- view_access = HTTP_ACCESS_SIGNED_ID | HTTP_ACCESS_SAME_SPACE | HTTP_ACCESS_VIEW_AGENT_CONFIG;
314
-
315
- if(edit_access == HTTP_ACCESS_NONE)
316
- edit_access = HTTP_ACCESS_SIGNED_ID | HTTP_ACCESS_SAME_SPACE | HTTP_ACCESS_EDIT_AGENT_CONFIG | HTTP_ACCESS_COMMERCIAL_SPACE;
317
-
318
- if(!dyncfg_is_valid_id(id)) {
319
- nd_log(NDLS_DAEMON, NDLP_ERR, "DYNCFG: id '%s' is invalid. Ignoring dynamic configuration for it.", id);
320
- return false;
321
- }
322
-
323
- if(type == DYNCFG_TYPE_JOB && !dyncfg_job_has_registered_template(id)) {
324
- nd_log(NDLS_DAEMON, NDLP_ERR, "DYNCFG: job id '%s' does not have a registered template. Ignoring dynamic configuration for it.", id);
325
- return false;
326
- }
327
-
328
- DYNCFG_CMDS old_cmds = cmds;
329
-
306
+DYNCFG_CMDS dyncfg_sanitize_cmds(DYNCFG_TYPE type, DYNCFG_SOURCE_TYPE source_type, DYNCFG_CMDS cmds) {
307
// all configurations support schema
308
cmds |= DYNCFG_CMD_SCHEMA;
309
@@ -345,7 +322,11 @@ bool dyncfg_add_low_level(RRDHOST *host, const char *id, const char *path,
322
}
323
324
// remove
348
- if(source_type != DYNCFG_SOURCE_TYPE_DYNCFG || type != DYNCFG_TYPE_JOB) {
325
+ if(source_type == DYNCFG_SOURCE_TYPE_DYNCFG && type == DYNCFG_TYPE_JOB) {
326
+ // dyncfg jobs must always be removable
327
+ cmds |= DYNCFG_CMD_REMOVE;
328
+ }
329
+ else {
330
// remove is only available for dyncfg jobs
331
cmds &= ~DYNCFG_CMD_REMOVE;
332
}
@@ -356,6 +337,34 @@ bool dyncfg_add_low_level(RRDHOST *host, const char *id, const char *path,
337
cmds &= ~(DYNCFG_CMD_GET | DYNCFG_CMD_UPDATE);
338
}
339
340
+ return cmds;
341
+}
342
+
343
+bool dyncfg_add_low_level(RRDHOST *host, const char *id, const char *path,
344
+ DYNCFG_STATUS status, DYNCFG_TYPE type, DYNCFG_SOURCE_TYPE source_type, const char *source,
345
+ DYNCFG_CMDS cmds, usec_t created_ut, usec_t modified_ut, bool sync,
346
+ HTTP_ACCESS view_access, HTTP_ACCESS edit_access,
347
+ rrd_function_execute_cb_t execute_cb, void *execute_cb_data) {
348
+
349
+ if(view_access == HTTP_ACCESS_NONE)
350
+ view_access = HTTP_ACCESS_SIGNED_ID | HTTP_ACCESS_SAME_SPACE | HTTP_ACCESS_VIEW_AGENT_CONFIG;
351
+
352
+ if(edit_access == HTTP_ACCESS_NONE)
353
+ edit_access = HTTP_ACCESS_SIGNED_ID | HTTP_ACCESS_SAME_SPACE | HTTP_ACCESS_EDIT_AGENT_CONFIG | HTTP_ACCESS_COMMERCIAL_SPACE;
354
+
355
+ if(!dyncfg_is_valid_id(id)) {
356
+ nd_log(NDLS_DAEMON, NDLP_ERR, "DYNCFG: id '%s' is invalid. Ignoring dynamic configuration for it.", id);
357
+ return false;
358
+ }
359
+
360
+ if(type == DYNCFG_TYPE_JOB && !dyncfg_job_has_registered_template(id)) {
361
+ nd_log(NDLS_DAEMON, NDLP_ERR, "DYNCFG: job id '%s' does not have a registered template. Ignoring dynamic configuration for it.", id);
362
+ return false;
363
+ }
364
+
365
+ DYNCFG_CMDS old_cmds = cmds;
366
+ cmds = dyncfg_sanitize_cmds(type, source_type, cmds);
367
+
368
if(cmds != old_cmds) {
369
CLEAN_BUFFER *t = buffer_create(1024, NULL);
370
buffer_sprintf(t, "DYNCFG: id '%s' was declared with cmds: ", id);