When dynamic agent or agentIP override is removed, revert to automati…

…c selection.
sflow · Dec 27, 2022 · 7cb2b83 · 7cb2b83
1 parent f867f90
commit 7cb2b83
Show file tree

Hide file tree

Showing 3 changed files with 50 additions and 50 deletions.
diff --git a/hsflowd.spec b/hsflowd.spec
@@ -1,6 +1,6 @@
 Summary: host sFlow daemon
 Name: hsflowd
-Version: 2.0.40
+Version: 2.0.41
 Release: 1
 License: http://sflow.net/license.html
 Group: Applications/Internet

diff --git a/src/Linux/hsflowconfig.c b/src/Linux/hsflowconfig.c
@@ -588,14 +588,15 @@ extern "C" {
     _________________   sFlowSettingsString     __________________
     -----------------___________________________------------------
    Only print the config fields that can be overridden dynamically
-   (e.g. via DNS-SD)
+   (e.g. via DNS-SD or from mod_sonic)
   */
 
   char *sFlowSettingsString(HSP *sp, HSPSFlowSettings *settings)
   {
     UTStrBuf *buf = UTStrBuf_new();
 
     if(settings) {
+      char ipbuf[51];
       UTStrBuf_printf(buf, "hostname=%s\n", sp->hostname);
       UTStrBuf_printf(buf, "sampling=%u\n", settings->samplingRate);
       UTStrBuf_printf(buf, "header=%u\n", settings->headerBytes);
@@ -610,29 +611,14 @@ extern "C" {
 	  UTStrBuf_printf(buf, "polling.%s=%u\n", appSettings->application, appSettings->polling_secs);
 	}
       }
-      // agentIP can override the config file here if set,  but otherwise print the address we selected
-      // TODO: should avoid writing in previously set agentIP or agentDevice. Should only write that in
-      // if it was an override. In installSFlowSettings we have to be careful to detect that a new config
-      // was submitted but still print the agentIP and agent lines to /etc/hsflowd.auto.  Before doing that
-      // however, we have to check for any change to agentIP, agent or agentCIDR,  and if so run the
-      // selectAgentAddress election again to pick an agentIP.
-      SFLAddress agIP = settings->agentIP.type ? settings->agentIP : sp->agentIP;
-      char ipbuf[51];
-      UTStrBuf_printf(buf, "agentIP=%s\n", SFLAddress_print(&agIP, ipbuf, 50));
-
-      // agentDevice can be overridden too,  otherwise print the one we selected
-      char *agDev = settings->agentDevice ?: sp->agentDevice;
-      if(agDev) {
-	UTStrBuf_printf(buf, "agent=%s\n", agDev);
-      }
-
-      UTStrBuf_printf(buf, "ds_index=%u\n", HSP_DEFAULT_PHYSICAL_DSINDEX);
 
-      // jsonPort always comes from local config file, but include it here so that
-      // others know where to send their JSON application/rtmetric/rtflow messages
-      if(sp->json.port) {
-	UTStrBuf_printf(buf, "jsonPort=%u\n", sp->json.port);
-      }
+      // agentIP and/or agentDevice can override the config file (and auto-selection) if set.
+      // If these overrides are removed again in another dynamic update then we simply leave them
+      // out here and it should trigger a return to the previous behavior.
+      if(settings->agentIP.type)
+	UTStrBuf_printf(buf, "agentIP=%s\n", SFLAddress_print(&settings->agentIP, ipbuf, 50));
+      if(settings->agentDevice)
+	UTStrBuf_printf(buf, "agent=%s\n", settings->agentDevice);
 
       // the DNS-SD responses seem to be reordering the collectors every time, so we have to take
       // another step here to make sure they are sorted.  Otherwise we think the config has changed

diff --git a/src/Linux/hsflowd.c b/src/Linux/hsflowd.c
@@ -522,19 +522,39 @@ extern "C" {
   */
 
   static void syncOutputFile(HSP *sp) {
+    // The current settings will usually be found in sp->sFlowSettings, but there
+    // is a case where those have not been determined yet and we start with the
+    // file settings:
+    HSPSFlowSettings *settings = sp->sFlowSettings ?: sp->sFlowSettings_file;
+
     myDebug(1, "syncOutputFile");
     rewind(sp->f_out);
     fprintf(sp->f_out, "# WARNING: Do not edit this file. It is generated automatically by hsflowd.\n");
 
     // revision appears both at the beginning and at the end
     fprintf(sp->f_out, "rev_start=%u\n", sp->revisionNo);
-    if(sp->sFlowSettings_str) fputs(sp->sFlowSettings_str, sp->f_out);
+    if(sp->sFlowSettings_str)
+      fputs(sp->sFlowSettings_str, sp->f_out);
+
+    // If agentIP or agentDevice was overridden then we just wrote that out with the sFlowSettings_str,
+    // but otherwise add the election-chosen ones here, so others can know what selection was made:
+    if(!settings->agentIP.type) {
+      char ipbuf[51];
+      fprintf(sp->f_out, "agentIP=%s\n", SFLAddress_print(&sp->agentIP, ipbuf, 50));
+    }
+    if(!settings->agentDevice)
+      fprintf(sp->f_out, "agent=%s\n", sp->agentDevice);
+
+    // jsonPort always comes from local config file, but include it here so that
+    // others know where to send their JSON application/rtmetric/rtflow messages
+    if(sp->json.port)
+      fprintf(sp->f_out, "jsonPort=%u\n", sp->json.port);
+
+    // Others may need to know our ds_index too
+    fprintf(sp->f_out, "ds_index=%u\n", HSP_DEFAULT_PHYSICAL_DSINDEX);
+
     // repeat the revision number. The reader knows that if the revison number
     // has not changed under his feet then he has a consistent config.
-
-    // TODO: if settings did not override agentIP, print here
-    // TODO: if settings did not override agent (device), print here
-
     fprintf(sp->f_out, "rev_end=%u\n", sp->revisionNo);
     fflush(sp->f_out);
     // chop off anything that may be lingering from before
@@ -1375,25 +1395,10 @@ extern "C" {
 
   static void evt_config_changed(EVMod *mod, EVEvent *evt, void *data, size_t dataLen) {
     myDebug(1, "main: evt_config_changed()");
-
-    HSP *sp = (HSP *)EVROOTDATA(mod);
-    if(sp->sFlowSettings
-       && sp->sFlowSettings != sp->sFlowSettings_file) {
-      // check for changes that we need to react to here:
-
-      // agent address might have been overridden (e.g. by mod_eapi)
-      if(sp->sFlowSettings->agentIP.type
-	 && !SFLAddress_equal(&sp->sFlowSettings->agentIP, &sp->agentIP)) {
-	myDebug(1, "evt_config_changed:  change sFlow agent address");
-	sp->agentIP = sp->sFlowSettings->agentIP;
-	if(sp->agent) {
-	  SEMLOCK_DO(sp->sync_agent) {
-	    sfl_agent_set_address(sp->agent, &sp->agentIP);
-	  }
-	}
-      }
-
-    }
+    // Used to check for agent-address override here and call sfl_agent_set_address(),
+    // but that is done in refreshAdaptorsAndAgentAddress() which is called from
+    // installSFlowSettings() during evt_config_end(), so any dynamic
+    // change to the agent address has already been handled before we get here.
   }
 
   /*_________________---------------------------__________________
@@ -2063,8 +2068,17 @@ extern "C" {
       // synchronization).  The main thread handles these HSPEVENT_CONFIG_*
       // events and assembles the new config.  When it is complete it sends
       // HSPEVENT_CONFIG_CHANGED to both the pollBus and the packetBus.
-      // That calls evt_config_changed() below, which updates polling and
-      // sampling settings.
+      // So a sequence of:
+      //   config_start
+      //     config_line
+      //     config_line
+      //      ...
+      //   config_end
+      // then triggers a sequence of:
+      //   config_first (if it was the first time)
+      //   config_changed
+      //   config_shake
+      //   config_done
     }
 
     if(sp->DNSSD.DNSSD