CP-54476 Merge ntp disable/enable to ntp mode

Signed-off-by: Changlei Li <changlei.li@cloud.com>

Author: changlei-li

doc/content/design/host-ntp-time.md

@@ -38,8 +38,7 @@ cache for the getter APIs.
 
 New fields:  
 `host.ntp_mode`, enum host_ntp_mode(DHCP, Custom, Factory, Disabled)  
-`host.ntp_custom_servers`, string set  
-`host.ntp_enabled`, bool  
+`host.ntp_custom_servers`, string set
 
 New APIs: `host.set_ntp_mode`, `host.set_ntp_custom_servers`, `host.get_ntp_mode`,
 `host.get_ntp_custom_servers`, `host.get_ntp_servers_status`
@@ -201,7 +200,6 @@ host.timezone
 host.set_timezone
 host.list_timezones
 host.get_timezone
-host.get_ntp_enabled
 host.get_ntp_synchronized
 host.set_ntp_mode
 host.get_ntp_mode

ocaml/idl/datamodel_host.ml

@@ -2579,11 +2579,10 @@ let host_ntp_mode =
   Enum
     ( "host_ntp_mode"
     , [
-        ("ntp_mode_dhcp", "Using NTP servers assigned by DHCP to sync time")
-      ; ( "ntp_mode_custom"
-        , "Using custom NTP servers configured by user to sync time"
-        )
-      ; ("ntp_mode_default", "Using default NTP servers to sync time")
+        ("DHCP", "Using NTP servers assigned by DHCP to sync time")
+      ; ("Custom", "Using custom NTP servers configured by user to sync time")
+      ; ("Factory", "Using built-in NTP servers to sync time")
+      ; ("Disabled", "NTP is disabled on the host")
       ]
     )
 
@@ -2606,16 +2605,6 @@ let set_ntp_custom_servers =
       ]
     ~allowed_roles:_R_POOL_OP ()
 
-let disable_ntp =
-  call ~name:"disable_ntp" ~lifecycle:[] ~doc:"Disable NTP on the host"
-    ~params:[(Ref _host, "self", "The host")]
-    ~allowed_roles:_R_POOL_OP ()
-
-let enable_ntp =
-  call ~name:"enable_ntp" ~lifecycle:[] ~doc:"Enable NTP on the host"
-    ~params:[(Ref _host, "self", "The host")]
-    ~allowed_roles:_R_POOL_OP ()
-
 let get_ntp_servers_status =
   call ~name:"get_ntp_servers_status" ~lifecycle:[]
     ~doc:"Get the NTP servers status on the host"
@@ -2822,8 +2811,6 @@ let t =
       ; set_max_cstate
       ; set_ntp_mode
       ; set_ntp_custom_servers
-      ; disable_ntp
-      ; enable_ntp
       ; get_ntp_servers_status
       ; set_timezone
       ; list_timezones
@@ -3297,15 +3284,12 @@ let t =
             ~default_value:(Some (VBool false)) "secure_boot"
             "Whether the host has booted in secure boot mode"
         ; field ~qualifier:DynamicRO ~lifecycle:[] ~ty:host_ntp_mode
-            ~default_value:(Some (VEnum "ntp_mode_dhcp")) "ntp_mode"
+            ~default_value:(Some (VEnum "DHCP")) "ntp_mode"
             "Indicates NTP servers are assigned by DHCP, or configured by \
-             user, or the default servers"
+             user, or the factory servers, or NTP is disabled"
         ; field ~qualifier:DynamicRO ~lifecycle:[] ~ty:(Set String)
             ~default_value:(Some (VSet [])) "ntp_custom_servers"
-            "The set of NTP servers configured for the host"
-        ; field ~qualifier:DynamicRO ~lifecycle:[] ~ty:Bool
-            ~default_value:(Some (VBool false)) "ntp_enabled"
-            "Reflects whether NTP is enabled on the host"
+            "Custom NTP servers configured by users, used in Custom NTP mode"
         ; field ~qualifier:DynamicRO ~lifecycle:[] ~ty:String
             ~default_value:(Some (VString "UTC")) "timezone"
             "The time zone identifier as defined in the IANA Time Zone Database"

ocaml/tests/common/test_common.ml

@@ -224,8 +224,8 @@ let make_host2 ~__context ?(ref = Ref.make ()) ?(uuid = make_uuid ())
     ~pending_guidances_recommended:[] ~pending_guidances_full:[]
     ~last_update_hash:"" ~ssh_enabled:true ~ssh_enabled_timeout:0L
     ~ssh_expiry:Date.epoch ~console_idle_timeout:0L ~ssh_auto_mode:false
-    ~max_cstate:"" ~secure_boot:false ~ntp_mode:`ntp_mode_dhcp
-    ~ntp_custom_servers:[] ~ntp_enabled:false ~timezone:"UTC" ;
+    ~max_cstate:"" ~secure_boot:false ~ntp_mode:`DHCP ~ntp_custom_servers:[]
+    ~timezone:"UTC" ;
   ref
 
 let make_pif ~__context ~network ~host ?(device = "eth0")

ocaml/xapi-cli-server/cli_frontend.ml

@@ -1098,24 +1098,6 @@ let rec cmdtable_data : (string * cmd_spec) list =
       ; flags= [Neverforward]
       }
     )
-  ; ( "host-enable-ntp"
-    , {
-        reqd= []
-      ; optn= []
-      ; help= "Enable ntp service on the host."
-      ; implementation= No_fd Cli_operations.host_enable_ntp
-      ; flags= [Host_selectors]
-      }
-    )
-  ; ( "host-disable-ntp"
-    , {
-        reqd= []
-      ; optn= []
-      ; help= "Disable ntp service on the host."
-      ; implementation= No_fd Cli_operations.host_disable_ntp
-      ; flags= [Host_selectors]
-      }
-    )
   ; ( "patch-upload"
     , {
         reqd= ["file-name"]

ocaml/xapi-cli-server/cli_operations.ml

@@ -7820,26 +7820,6 @@ let host_disable_ssh _printer rpc session_id params =
        params []
     )
 
-let host_enable_ntp _printer rpc session_id params =
-  ignore
-    (do_host_op rpc session_id
-       (fun _ host ->
-         let host = host.getref () in
-         Client.Host.enable_ntp ~rpc ~session_id ~self:host
-       )
-       params []
-    )
-
-let host_disable_ntp _printer rpc session_id params =
-  ignore
-    (do_host_op rpc session_id
-       (fun _ host ->
-         let host = host.getref () in
-         Client.Host.disable_ntp ~rpc ~session_id ~self:host
-       )
-       params []
-    )
-
 module SDN_controller = struct
   let introduce printer rpc session_id params =
     let port =

ocaml/xapi-cli-server/records.ml

@@ -3423,9 +3423,6 @@ let host_record rpc session_id host =
                 )
           )
           ()
-      ; make_field ~name:"ntp_enabled"
-          ~get:(fun () -> string_of_bool (x ()).API.host_ntp_enabled)
-          ()
       ; make_field ~name:"timezone"
           ~get:(fun () -> (x ()).API.host_timezone)
           ~set:(fun value ->

ocaml/xapi/message_forwarding.ml

@@ -4152,18 +4152,6 @@ functor
         let remote_fn = Client.Host.set_ntp_custom_servers ~self ~value in
         do_op_on ~local_fn ~__context ~host:self ~remote_fn
 
-      let enable_ntp ~__context ~self =
-        info "Host.enable_ntp: host='%s'" (host_uuid ~__context self) ;
-        let local_fn = Local.Host.enable_ntp ~self in
-        let remote_fn = Client.Host.enable_ntp ~self in
-        do_op_on ~local_fn ~__context ~host:self ~remote_fn
-
-      let disable_ntp ~__context ~self =
-        info "Host.disable_ntp: host='%s'" (host_uuid ~__context self) ;
-        let local_fn = Local.Host.disable_ntp ~self in
-        let remote_fn = Client.Host.disable_ntp ~self in
-        do_op_on ~local_fn ~__context ~host:self ~remote_fn
-
       let get_ntp_servers_status ~__context ~self =
         info "Host.get_ntp_servers_status: host = '%s'"
           (host_uuid ~__context self) ;
@@ -4195,7 +4183,7 @@ functor
         info "Host.set_servertime : host = '%s'; value = '%s'"
           (host_uuid ~__context self)
           (Clock.Date.to_rfc3339 value) ;
-        if Db.Host.get_ntp_enabled ~__context ~self then
+        if Db.Host.get_ntp_mode ~__context ~self <> `Disabled then
           raise
             (Api_errors.Server_error
                (Api_errors.not_allowed_when_ntp_is_enabled, [Ref.string_of self])

ocaml/xapi/xapi_globs.ml

@@ -1425,9 +1425,9 @@ let nvidia_gpumon_detach = ref false
 
 let failed_login_alert_freq = ref 3600
 
-let default_ntp_servers = ref []
+let factory_ntp_servers = ref []
 
-let legacy_default_ntp_servers = ref []
+let legacy_factory_ntp_servers = ref []
 
 let other_options =
   [
@@ -1916,16 +1916,16 @@ let other_options =
     , (fun () -> !timedatectl)
     , "Path to the timedatectl executable"
     )
-  ; gen_list_option "legacy-default-ntp-servers"
+  ; gen_list_option "legacy-factory-ntp-servers"
       "space-separated list of legacy default NTP servers"
       (fun s -> s)
       (fun s -> s)
-      legacy_default_ntp_servers
-  ; gen_list_option "default-ntp-servers"
+      legacy_factory_ntp_servers
+  ; gen_list_option "factory-ntp-servers"
       "space-separated list of default NTP servers"
       (fun s -> s)
       (fun s -> s)
-      default_ntp_servers
+      factory_ntp_servers
   ]
 
 (* The options can be set with the variable xapiflags in /etc/sysconfig/xapi.

ocaml/xapi/xapi_host.ml

@@ -1094,8 +1094,8 @@ let create ~__context ~uuid ~name_label ~name_description:_ ~hostname ~address
     ~recommended_guidances:[] ~latest_synced_updates_applied:`unknown
     ~pending_guidances_recommended:[] ~pending_guidances_full:[] ~ssh_enabled
     ~ssh_enabled_timeout ~ssh_expiry ~console_idle_timeout ~ssh_auto_mode
-    ~max_cstate:"" ~secure_boot ~ntp_mode:`ntp_mode_dhcp ~ntp_custom_servers:[]
-    ~ntp_enabled:false ~timezone:"UTC" ;
+    ~max_cstate:"" ~secure_boot ~ntp_mode:`DHCP ~ntp_custom_servers:[]
+    ~timezone:"UTC" ;
   (* If the host we're creating is us, make sure its set to live *)
   Db.Host_metrics.set_last_updated ~__context ~self:metrics ~value:(Date.now ()) ;
   Db.Host_metrics.set_live ~__context ~self:metrics ~value:host_is_us ;
@@ -3471,90 +3471,90 @@ let sync_max_cstate ~__context ~host =
 
 let set_ntp_mode ~__context ~self ~value =
   let current_mode = Db.Host.get_ntp_mode ~__context ~self in
-  let ntp_enabled = Db.Host.get_ntp_enabled ~__context ~self in
   if current_mode <> value then (
     let open Xapi_host_ntp in
-    let ensure_custom_servers_exist servers =
+    let ensure_servers_exist servers msg =
       if servers = [] then
-        raise
-          Api_errors.(
-            Server_error
-              ( invalid_ntp_config
-              , ["Can't set ntp_mode_custom when ntp_custom_servers is empty"]
-              )
-          )
+        raise Api_errors.(Server_error (invalid_ntp_config, [msg]))
+    in
+    let enter_mode = function
+      | `DHCP ->
+          add_dhcp_ntp_servers ()
+      | `Custom ->
+          let custom_servers =
+            Db.Host.get_ntp_custom_servers ~__context ~self
+          in
+          ensure_servers_exist custom_servers
+            "Can't set ntp_mode to Custom when ntp_custom_servers is empty" ;
+          set_servers_in_conf custom_servers
+      | `Factory ->
+          let factory_servers = !Xapi_globs.factory_ntp_servers in
+          ensure_servers_exist factory_servers
+            "Can't set ntp_mode to Factory when factory ntp servers is empty" ;
+          set_servers_in_conf factory_servers
+      | `Disabled ->
+          ()
+    in
+    let exit_mode = function
+      | `DHCP ->
+          remove_dhcp_ntp_servers ()
+      | `Custom | `Factory ->
+          clear_servers_in_conf ()
+      | `Disabled ->
+          ()
     in
-    let default_servers = !Xapi_globs.default_ntp_servers in
     ( match (current_mode, value) with
-    | `ntp_mode_dhcp, `ntp_mode_custom ->
-        let custom_servers = Db.Host.get_ntp_custom_servers ~__context ~self in
-        ensure_custom_servers_exist custom_servers ;
-        remove_dhcp_ntp_servers () ;
-        set_servers_in_conf custom_servers
-    | `ntp_mode_default, `ntp_mode_custom ->
-        let custom_servers = Db.Host.get_ntp_custom_servers ~__context ~self in
-        ensure_custom_servers_exist custom_servers ;
-        set_servers_in_conf custom_servers
-    | _, `ntp_mode_dhcp ->
-        clear_servers_in_conf () ; add_dhcp_ntp_servers ()
-    | `ntp_mode_dhcp, `ntp_mode_default ->
-        remove_dhcp_ntp_servers () ;
-        set_servers_in_conf default_servers
-    | `ntp_mode_custom, `ntp_mode_default ->
-        set_servers_in_conf default_servers
-    | _, _ ->
-        ()
+    | `Disabled, next ->
+        enter_mode next ; enable_ntp_service ()
+    | _, `Disabled ->
+        exit_mode current_mode ; disable_ntp_service ()
+    | current, next ->
+        exit_mode current ;
+        enter_mode next ;
+        Xapi_host_ntp.restart_ntp_service ()
     ) ;
-    if ntp_enabled then Xapi_host_ntp.restart_ntp_service () ;
     Db.Host.set_ntp_mode ~__context ~self ~value
   )
 
 let set_ntp_custom_servers ~__context ~self ~value =
   let current_mode = Db.Host.get_ntp_mode ~__context ~self in
   match (current_mode, value) with
-  | `ntp_mode_custom, [] ->
+  | `Custom, [] ->
       raise
         Api_errors.(
           Server_error
             ( invalid_ntp_config
-            , ["Can't set ntp_custom_servers empty when ntp_mode is custom"]
+            , ["Can't set ntp_custom_servers empty when ntp_mode is Custom"]
             )
         )
-  | `ntp_mode_custom, servers ->
+  | `Custom, servers ->
       Xapi_host_ntp.set_servers_in_conf servers ;
       Xapi_host_ntp.restart_ntp_service () ;
       Db.Host.set_ntp_custom_servers ~__context ~self ~value
   | _ ->
       Db.Host.set_ntp_custom_servers ~__context ~self ~value
 
-let enable_ntp ~__context ~self =
-  Xapi_host_ntp.enable_ntp_service () ;
-  Db.Host.set_ntp_enabled ~__context ~self ~value:true
-
-let disable_ntp ~__context ~self =
-  Xapi_host_ntp.disable_ntp_service () ;
-  Db.Host.set_ntp_enabled ~__context ~self ~value:false
-
 let sync_ntp_config ~__context ~host =
   Xapi_host_ntp.promote_legacy_default_servers () ;
   let servers = Xapi_host_ntp.get_servers_from_conf () in
   let is_ntp_dhcp_enabled = Xapi_host_ntp.is_ntp_dhcp_enabled () in
+  let ntp_enabled = Xapi_host_ntp.is_ntp_service_active () in
   let ntp_mode =
-    match (is_ntp_dhcp_enabled, servers) with
-    | true, _ ->
-        `ntp_mode_dhcp
-    | false, s
+    match (ntp_enabled, is_ntp_dhcp_enabled, servers) with
+    | false, _, _ ->
+        `Disabled
+    | _, true, _ ->
+        `DHCP
+    | _, false, s
       when Xapi_stdext_std.Listext.List.set_equiv s
-             !Xapi_globs.default_ntp_servers ->
-        `ntp_mode_default
-    | false, _ ->
-        `ntp_mode_custom
+             !Xapi_globs.factory_ntp_servers ->
+        `Factory
+    | _, false, _ ->
+        `Custom
   in
   Db.Host.set_ntp_mode ~__context ~self:host ~value:ntp_mode ;
-  if ntp_mode = `ntp_mode_custom then
-    Db.Host.set_ntp_custom_servers ~__context ~self:host ~value:servers ;
-  let ntp_enabled = Xapi_host_ntp.is_ntp_service_active () in
-  Db.Host.set_ntp_enabled ~__context ~self:host ~value:ntp_enabled
+  if ntp_mode = `Custom then
+    Db.Host.set_ntp_custom_servers ~__context ~self:host ~value:servers
 
 let get_ntp_servers_status ~__context ~self:_ =
   if Xapi_host_ntp.is_ntp_service_active () then

ocaml/xapi/xapi_host.mli

@@ -619,10 +619,6 @@ val set_ntp_mode :
 val set_ntp_custom_servers :
   __context:Context.t -> self:API.ref_host -> value:string list -> unit
 
-val enable_ntp : __context:Context.t -> self:API.ref_host -> unit
-
-val disable_ntp : __context:Context.t -> self:API.ref_host -> unit
-
 val sync_ntp_config : __context:Context.t -> host:API.ref_host -> unit
 
 val get_ntp_servers_status :