CA-323523: Ensure that dhclients get restarted when gateway/DNS changes

Recent commit 75a4c5d changed the `options` argument to
`Dhclient.write_conf_file`, but failed to update one of its callers, in
`set_gateway_interface`. This meant that dhclient did not get restarted when
necessary as a result of configuration changes. Rather than fixing the bad
function call, this patch changes the code to be more robust.

When starting a `dhclient` process for an interface that requires configuration
by DHCP, the function `Dhclient.ensure_running` writes a configuration file to
disk and passes its path to `dhclient` as a command line argument. The purpose
of saving the configuration is to make `Dhclient.ensure_running` idempotent.
When called, the function checks if a `dhclient` process is already running for
the given interface, and if the last used configuration is identical to the
newly requested one. If so, it leaves the process alone to cause minimal
disturbance to the system.

An interesting use case is changing the default gateway to another interface,
and then back to the original interface (using `set_gateway_interface` and
`Interface.make_config` twice). The first change properly updates the gateway,
but the change back to the original interface doesn't end up restarting
`dhclient` to switch the gateway back. This was addressed by rewriting the conf
file of the gateway interface when changing it, in `set_gateway_interface` (see
637db58).

Now, is seems wrong that `set_dns_interface`, which is a function with a very
similar purpose to `set_gateway_interface`, does not rewrite conf files like
`set_gateway_interface` does. Fixing that, however, could lead to the two
functions interfering with each other. Therefore, rather than trying to update
the conf file, we just delete it so that it will definitely be correctly
regenerated later (and do this for both `set_*_interface` functions).

Signed-off-by: Rob Hoes <[email protected]>

Author: robhoes

lib/network_utils.ml

@@ -759,6 +759,12 @@ module Dhclient = struct
     let conf = generate_conf ~ipv6 interface options in
     Xapi_stdext_unix.Unixext.write_string_to_file (conf_file ~ipv6 interface) conf
 
+  let remove_conf_file ?(ipv6=false) interface =
+    let file = conf_file ~ipv6 interface in
+    try
+      Unix.unlink file
+    with _ -> ()
+
   let start ?(ipv6=false) interface options =
     (* If we have a gateway interface, pass it to dhclient-script via -e *)
     (* This prevents the default route being set erroneously on CentOS *)

networkd/network_server.ml

@@ -74,23 +74,28 @@ let reset_state () =
   config := Network_config.read_management_conf ()
 
 let set_gateway_interface _dbg name =
-  (* Update dhclient conf for interface on changing default gateway.
-     	 * If new default gateway is not same as gateway_interface from networkd.db then
-     	 * we need to remove gateway information from gateway_interface *)
+  (* Remove dhclient conf (if any) for the old and new gateway interfaces.
+   * This ensures that dhclient gets restarted with an updated conf file when
+   * necessary. *)
   begin match !config.gateway_interface with
-    | Some gateway_iface when name <> gateway_iface ->
-      let opts =
-        match !config.dns_interface with
-        | Some dns_iface when gateway_iface = dns_iface -> [`set_dns]
-        | _ -> []
-      in
-      Dhclient.write_conf_file gateway_iface opts
+    | Some old_iface when name <> old_iface ->
+      Dhclient.remove_conf_file name;
+      Dhclient.remove_conf_file old_iface
     | _ -> ()
   end;
   debug "Setting gateway interface to %s" name;
   config := {!config with gateway_interface = Some name}
 
 let set_dns_interface _dbg name =
+  (* Remove dhclient conf (if any) for the old and new DNS interfaces.
+   * This ensures that dhclient gets restarted with an updated conf file when
+   * necessary. *)
+  begin match !config.dns_interface with
+  | Some old_iface when name <> old_iface ->
+    Dhclient.remove_conf_file name;
+    Dhclient.remove_conf_file old_iface
+    | _ -> ()
+  end;
   debug "Setting DNS interface to %s" name;
   config := {!config with dns_interface = Some name}