xapi/nm: Send non-empty dns to networkd when using IPv6 autoconf (#6586)

Because Autoconf is not DHCP, networkd uses the dns value to write to
resolv.conf. This is done on ocaml/networkd/bin/network_server.ml line
745
This allows to have non-empty resolv.conf when using IPv6 autoconf.

This mismatch was caused byt technical debt that cause this check to be
duplicated, the second removes it so
xapi decides the instances where these values are written and networkd
follows that decision.

I've tested new isntallations using IPv4 and couldn't see any
regression, two users have tested the patch and now the DNS entries
don't get overriden when using IPv6 in Autoconf mode:
xcp-ng/xcp#641 (comment)

I'd like Rob to review this approach in any case

Author: psafont

ocaml/networkd/bin/network_server.ml

@@ -554,7 +554,8 @@ module Interface = struct
   let set_dns _ dbg ~name ~nameservers ~domains =
     Debug.with_thread_associated dbg
       (fun () ->
-        update_config name {(get_config name) with dns= (nameservers, domains)} ;
+        update_config name
+          {(get_config name) with dns= Some (nameservers, domains)} ;
         debug "Configuring DNS for %s: nameservers: [%s]; domains: [%s]" name
           (String.concat ", " (List.map Unix.string_of_inet_addr nameservers))
           (String.concat ", " domains) ;
@@ -727,7 +728,7 @@ module Interface = struct
                   ; ipv6_conf
                   ; ipv6_gateway
                   ; ipv4_routes
-                  ; dns= nameservers, domains
+                  ; dns
                   ; mtu
                   ; ethtool_settings
                   ; ethtool_offload
@@ -736,15 +737,10 @@ module Interface = struct
                 ) ) ->
                 update_config name c ;
                 exec (fun () ->
-                    (* We only apply the DNS settings when not in a DHCP mode
-                       to avoid conflicts. The `dns` field
-                       should really be an option type so that we don't have to
-                       derive the intention of the caller by looking at other
-                       fields. *)
-                    match (ipv4_conf, ipv6_conf) with
-                    | Static4 _, _ | _, Static6 _ | _, Autoconf6 ->
+                    match dns with
+                    | Some (nameservers, domains) ->
                         set_dns () dbg ~name ~nameservers ~domains
-                    | _ ->
+                    | None ->
                         ()
                 ) ;
                 exec (fun () -> set_ipv4_conf dbg name ipv4_conf) ;

ocaml/networkd/bin_db/networkd_db.ml

@@ -74,20 +74,25 @@ let _ =
                     [("gateway", Unix.string_of_inet_addr addr)]
               in
               let dns =
-                let dns' =
-                  List.map Unix.string_of_inet_addr (fst interface_config.dns)
-                in
-                if dns' = [] then
-                  []
-                else
-                  [("dns", String.concat "," dns')]
+                interface_config.dns
+                |> Option.map fst
+                |> Option.map (List.map Unix.string_of_inet_addr)
+                |> Option.fold ~none:[] ~some:(function
+                     | [] ->
+                         []
+                     | dns' ->
+                         [("dns", String.concat "," dns')]
+                     )
               in
               let domains =
-                let domains' = snd interface_config.dns in
-                if domains' = [] then
-                  []
-                else
-                  [("domain", String.concat "," domains')]
+                interface_config.dns
+                |> Option.map snd
+                |> Option.fold ~none:[] ~some:(function
+                     | [] ->
+                         []
+                     | domains' ->
+                         [("domain", String.concat "," domains')]
+                     )
               in
               mode @ addrs @ gateway @ dns @ domains
           | None4 ->

ocaml/networkd/lib/network_config.ml

@@ -37,7 +37,6 @@ let bridge_naming_convention (device : string) =
 let get_list_from ~sep ~key args =
   List.assoc_opt key args
   |> Option.map (fun v -> Astring.String.cuts ~empty:false ~sep v)
-  |> Option.value ~default:[]
 
 let parse_ipv4_config args = function
   | Some "static" ->
@@ -73,11 +72,13 @@ let parse_ipv6_config args = function
       (None6, None)
 
 let parse_dns_config args =
-  let nameservers =
-    get_list_from ~sep:"," ~key:"DNS" args |> List.map Unix.inet_addr_of_string
+  let ( let* ) = Option.bind in
+  let* nameservers =
+    get_list_from ~sep:"," ~key:"DNS" args
+    |> Option.map (List.map Unix.inet_addr_of_string)
   in
-  let domains = get_list_from ~sep:" " ~key:"DOMAIN" args in
-  (nameservers, domains)
+  let* domains = get_list_from ~sep:" " ~key:"DOMAIN" args in
+  Some (nameservers, domains)
 
 let read_management_conf () =
   try
@@ -103,15 +104,15 @@ let read_management_conf () =
     let device =
       (* Take 1st member of bond *)
       match (bond_mode, bond_members) with
-      | None, _ | _, [] -> (
+      | None, _ | _, (None | Some []) -> (
         match List.assoc_opt "LABEL" args with
         | Some x ->
             x
         | None ->
             error "%s: missing LABEL in %s" __FUNCTION__ management_conf ;
             raise Read_error
       )
-      | _, hd :: _ ->
+      | _, Some (hd :: _) ->
           hd
     in
     Inventory.reread_inventory () ;

ocaml/xapi-idl/network/network_interface.ml

@@ -158,7 +158,10 @@ type interface_config_t = {
   ; ipv6_conf: ipv6 [@default None6]
   ; ipv6_gateway: Unix.inet_addr option [@default None]
   ; ipv4_routes: ipv4_route_t list [@default []]
-  ; dns: Unix.inet_addr list * string list [@default [], []]
+  ; dns: (Unix.inet_addr list * string list) option [@default None]
+        (** the list
+  of nameservers and domains to persist in /etc/resolv.conf. Must be None when
+  using a DHCP mode *)
   ; mtu: int [@default 1500]
   ; ethtool_settings: (string * string) list [@default []]
   ; ethtool_offload: (string * string) list [@default [("lro", "off")]]
@@ -200,7 +203,7 @@ let default_interface =
   ; ipv6_conf= None6
   ; ipv6_gateway= None
   ; ipv4_routes= []
-  ; dns= ([], [])
+  ; dns= None
   ; mtu= 1500
   ; ethtool_settings= []
   ; ethtool_offload= [("lro", "off")]

ocaml/xapi/nm.ml

@@ -634,28 +634,25 @@ let bring_pif_up ~__context ?(management_interface = false) (pif : API.ref_PIF)
                     rc.API.pIF_ip_configuration_mode = `Static
                 | `IPv6 ->
                     rc.API.pIF_ipv6_configuration_mode = `Static
+                    || rc.API.pIF_ipv6_configuration_mode = `Autoconf
               in
               let dns =
                 match (static, rc.API.pIF_DNS) with
                 | false, _ | true, "" ->
-                    ([], [])
+                    None
                 | true, pif_dns ->
                     let nameservers =
                       List.map Unix.inet_addr_of_string
-                        (String.split ',' pif_dns)
+                        (String.split_on_char ',' pif_dns)
                     in
                     let domains =
                       match List.assoc_opt "domain" rc.API.pIF_other_config with
-                      | None ->
+                      | None | Some "" ->
                           []
-                      | Some domains -> (
-                        try String.split ',' domains
-                        with _ ->
-                          warn "Invalid DNS search domains: %s" domains ;
-                          []
-                      )
+                      | Some domains ->
+                          String.split_on_char ',' domains
                     in
-                    (nameservers, domains)
+                    Some (nameservers, domains)
               in
               let mtu = determine_mtu rc net_rc in
               let ethtool_settings, ethtool_offload =