Skip to content

Commit 4348580

Browse files
committed
clusterer/clusterer_controller: enforce use_controller vs module-load consistency
Two startup sanity checks for a previously silent misconfiguration: - clusterer_controller is only meaningful when the clusterer module has use_controller=1 (the global switch that pre-creates the controller-managed cluster stubs, sets each one's controller_managed flag so they never touch the DB, and arms the guard that stops the controller from hijacking a native cluster of the same id). If use_controller=0 there is no controller-managed cluster and those safety mechanisms are off, so mod_init now FAILS (refuse to start) rather than driving clusters clusterer never authorised. Hybrid setups keep use_controller=1 - only the per-cluster kind differs - so this never trips them. - The mirror case, use_controller=1 but no clusterer_controller module bound the controller API, logs an ERROR at clusterer child_init (the pre-created controller-managed stubs would never obtain an identity). clusterer does NOT abort here: its native/hybrid clusters still work; only the controller-stub clusters are dead. Plumbing: the clusterer_ctrl binds struct now carries clusterer's use_controller value, and load_clusterer_ctrl_binds() sets clusterer_ctrl_bound so clusterer can tell whether a controller registered. Verified on all permutations: pure controller, hybrid, native-only, and both mismatches.
1 parent 34f1042 commit 4348580

6 files changed

Lines changed: 102 additions & 8 deletions

File tree

‎modules/clusterer/clusterer_ctrl.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -372,12 +372,17 @@ int clusterer_ctrl_unset_shtag_managed(int cluster_id)
372372
return 0;
373373
}
374374

375+
/* 1 once a controller module has bound this API (see clusterer_ctrl.h). */
376+
int clusterer_ctrl_bound = 0;
377+
375378
int load_clusterer_ctrl_binds(clusterer_ctrl_binds_t *binds)
376379
{
377380
if (!binds) {
378381
LM_ERR("clusterer: load_clusterer_ctrl_binds: NULL binds\n");
379382
return -1;
380383
}
384+
clusterer_ctrl_bound = 1;
385+
binds->use_controller = use_controller;
381386
binds->set_my_identity = clusterer_ctrl_set_identity;
382387
binds->add_node = clusterer_ctrl_add_node;
383388
binds->remove_node = clusterer_ctrl_remove_node;

‎modules/clusterer/clusterer_ctrl.h‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,8 +157,18 @@ typedef struct clusterer_ctrl_binds {
157157
* @return 0 on success, -1 if cluster not found
158158
*/
159159
int (*unset_shtag_managed)(int cluster_id);
160+
161+
/* Introspection: the value of the clusterer 'use_controller' modparam at
162+
* bind time. Lets clusterer_controller detect the mirror misconfiguration
163+
* (controller loaded but clusterer has use_controller=0, so no stubs were
164+
* pre-created). */
165+
int use_controller;
160166
} clusterer_ctrl_binds_t;
161167

168+
/* Set to 1 by load_clusterer_ctrl_binds() when a controller module binds the
169+
* API, so clusterer can warn if use_controller=1 yet no controller registered. */
170+
extern int clusterer_ctrl_bound;
171+
162172
/**
163173
* load_clusterer_ctrl_binds() — fill a clusterer_ctrl_binds_t struct.
164174
*

‎modules/clusterer/clusterer_mod.c‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -657,6 +657,18 @@ static int child_init(int rank)
657657
return -1;
658658
}
659659
}
660+
661+
/* One-shot config sanity check (rank 1 fires once, and runs after every
662+
* module's mod_init, so a clusterer_controller would already have bound the
663+
* controller API): use_controller=1 is meaningless without that module -
664+
* the pre-created controller-managed cluster stubs would never obtain an
665+
* identity or form. */
666+
if (rank == 1 && use_controller && !clusterer_ctrl_bound)
667+
LM_ERR("clusterer: use_controller=1 but no clusterer_controller module "
668+
"registered the controller API - controller-managed cluster(s) "
669+
"will never obtain a node identity or form. Load the "
670+
"clusterer_controller module, or unset use_controller.\n");
671+
660672
return 0;
661673
}
662674

‎modules/clusterer_controller/README‎

Lines changed: 25 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -496,9 +496,15 @@ on-key")
496496
* proto_bin — required so that BIN listeners are registered
497497
and available for discovery when clusterer_controller
498498
initialises and scans the proto_bin listener list.
499-
* clusterer — required with use_controller=1 set, so that the
500-
clusterer internal API (load_clusterer_ctrl_binds) is
501-
exported and available at initialisation time.
499+
* clusterer — required, and it must have use_controller=1
500+
set. That global switch is what pre-creates the
501+
controller-managed cluster stubs, marks them so they never
502+
touch the database, and arms the guard that stops the
503+
controller from driving a native cluster of the same id. If
504+
clusterer_controller is loaded while clusterer has
505+
use_controller=0, it refuses to start with a clear error —
506+
there is no controller-managed cluster for it to drive and
507+
those safety mechanisms would be off.
502508

503509
Both dependencies are declared in the module's dep_export_t.
504510
OpenSIPS will refuse to start if either dependency is not
@@ -507,6 +513,22 @@ on-key")
507513
resolves the dependency at runtime and will initialize the
508514
required modules first regardless of loadmodule order.
509515

516+
The reverse is also checked: if use_controller=1 is set but the
517+
clusterer_controller module is not loaded, clusterer logs an
518+
error at startup, since the controller-managed cluster stubs
519+
would never obtain a node identity or form. clusterer itself
520+
keeps running, so any native or DB-backed clusters are
521+
unaffected.
522+
523+
Hybrid deployments are unaffected. use_controller is a single
524+
global switch; in a hybrid instance (native and
525+
controller-managed clusters side by side) it is always 1 and
526+
only the per-cluster kind differs (native via DB/static
527+
provisioning versus controller-managed via the cluster_id
528+
list). Both consistency checks above therefore only fire on a
529+
genuine module/config mismatch — never on a hybrid or
530+
pure-controller setup.
531+
510532
All other modules that use the clusterer interface (tm, dialog,
511533
dispatcher, usrloc etc.) may be loaded in any order relative to
512534
clusterer_controller. The clusterer module automatically

‎modules/clusterer_controller/clusterer_controller.c‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5804,6 +5804,25 @@ static int mod_init(void)
58045804
LM_INFO("clusterer_controller: clusterer API loaded - "
58055805
"topology will be driven dynamically\n");
58065806

5807+
/* The controller is only meaningful when the clusterer module is told
5808+
* to expect it (use_controller=1, a global switch): that is what
5809+
* pre-creates the controller-managed cluster stubs, sets each one's
5810+
* controller_managed flag (so they never touch the DB), and arms the
5811+
* guard that stops the controller from hijacking a native cluster of the
5812+
* same id. With use_controller=0 there is no controller-managed cluster
5813+
* at all and those safety mechanisms are off, so refuse to start rather
5814+
* than run the control plane against clusters clusterer never authorised
5815+
* us to drive. (Hybrid setups keep use_controller=1 - only the per-
5816+
* cluster kind differs - so this never trips them.) */
5817+
if (!clctl.use_controller) {
5818+
LM_ERR("clusterer_controller: loaded but the clusterer module has "
5819+
"use_controller=0 - there is no controller-managed cluster and "
5820+
"the controller's safety guards are disabled. Set "
5821+
"modparam(\"clusterer\", \"use_controller\", 1), or remove the "
5822+
"clusterer_controller module.\n");
5823+
return -1;
5824+
}
5825+
58075826
/* When we manage sharing tags, start every tag as BACKUP and
58085827
* lock out MI/script changes - the controller master decides
58095828
* who becomes active. (manage_shtags sentinels were already

‎modules/clusterer_controller/doc/clusterer_controller_admin.xml‎

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -502,11 +502,17 @@
502502
</listitem>
503503
<listitem>
504504
<para>
505-
<emphasis>clusterer</emphasis> — required with
506-
<varname>use_controller=1</varname> set, so that
507-
the clusterer internal API
508-
(<function>load_clusterer_ctrl_binds</function>) is
509-
exported and available at initialisation time.
505+
<emphasis>clusterer</emphasis> — required, and it
506+
<emphasis>must</emphasis> have
507+
<varname>use_controller=1</varname> set. That global switch
508+
is what pre-creates the controller-managed cluster stubs,
509+
marks them so they never touch the database, and arms the
510+
guard that stops the controller from driving a native cluster
511+
of the same id. If <emphasis>clusterer_controller</emphasis>
512+
is loaded while clusterer has
513+
<varname>use_controller=0</varname>, it refuses to start with
514+
a clear error — there is no controller-managed cluster for it
515+
to drive and those safety mechanisms would be off.
510516
</para>
511517
</listitem>
512518
</itemizedlist>
@@ -521,6 +527,26 @@
521527
required modules first regardless of
522528
<function>loadmodule</function> order.
523529
</para>
530+
<para>
531+
The reverse is also checked: if
532+
<varname>use_controller=1</varname> is set but the
533+
<emphasis>clusterer_controller</emphasis> module is not loaded,
534+
clusterer logs an error at startup, since the controller-managed
535+
cluster stubs would never obtain a node identity or form. clusterer
536+
itself keeps running, so any native or DB-backed clusters are
537+
unaffected.
538+
</para>
539+
<para>
540+
<emphasis role="bold">Hybrid deployments are unaffected.</emphasis>
541+
<varname>use_controller</varname> is a single global switch; in a
542+
hybrid instance (native and controller-managed clusters side by side)
543+
it is always <varname>1</varname> and only the
544+
<emphasis>per-cluster</emphasis> kind differs (native via DB/static
545+
provisioning versus controller-managed via the
546+
<varname>cluster_id</varname> list). Both consistency checks above
547+
therefore only fire on a genuine module/config mismatch — never on a
548+
hybrid or pure-controller setup.
549+
</para>
524550
<para>
525551
All other modules that use the clusterer interface
526552
(<emphasis>tm</emphasis>, <emphasis>dialog</emphasis>,

0 commit comments

Comments
 (0)