CP-51988: Fix functions not work for remote_pool repo

ocaml/idl/datamodel_errors.ml

@@ -1905,11 +1905,19 @@ let _ =
   error Api_errors.bundle_repo_not_enabled []
     ~doc:"Cannot sync bundle as the bundle repository is not enabled." () ;
   error Api_errors.can_not_sync_updates []
-    ~doc:"Cannot sync updates as the bundle repository is enabled." () ;
-  error Api_errors.bundle_repo_should_be_single_enabled []
     ~doc:
-      "If the bundle repository is enabled, it should be the only one enabled \
-       repository of the pool."
+      "The currently enabled repositories do not support synchronization of \
+       updates."
+    () ;
+  error Api_errors.can_not_periodic_sync_updates []
+    ~doc:
+      "The currently enabled repositories do not support periodic automatic \
+       updates."
+    () ;
+  error Api_errors.repo_should_be_single_one_enabled ["repo_types"]
+    ~doc:
+      "If the bundle repository or remote_pool repository is enabled, it \
+       should be the only one enabled repository of the pool."
     () ;
   error Api_errors.repository_is_in_use [] ~doc:"The repository is in use." () ;
   error Api_errors.repository_cleanup_failed []

ocaml/tests/test_pool_repository.ml

@@ -14,101 +14,138 @@
 
 module T = Test_common
 
-let test_set_remote_and_bundle_repos () =
+let on_repositories f =
   let __context = T.make_test_database () in
-  let name_label = "remote" in
-  let name_description = "remote" in
-  let binary_url = "https://repo.example.com" in
+  let pool = Helpers.get_pool ~__context in
+  let binary_url_1 = "https://repo.example.com" in
+  let binary_url_2 = "https://1.1.1.1/repository/enabled" in
   let source_url = "https://repo-src.example.com" in
-  let gpgkey_path = "" in
   let ref_remote =
-    Repository.introduce ~__context ~name_label ~name_description ~binary_url
-      ~source_url ~update:true ~gpgkey_path
+    Repository.introduce ~__context ~name_label:"remote"
+      ~name_description:"remote" ~binary_url:binary_url_1 ~source_url
+      ~update:true ~gpgkey_path:""
   in
   let ref_bundle =
     Repository.introduce_bundle ~__context ~name_label:"bundle"
       ~name_description:"bundle"
   in
-  let self = Helpers.get_pool ~__context in
-  Alcotest.check_raises "test_set_remote_and_bundle_repos"
-    Api_errors.(Server_error (bundle_repo_should_be_single_enabled, []))
-    (fun () ->
-      Xapi_pool.set_repositories ~__context ~self
-        ~value:[ref_remote; ref_bundle]
-    )
-
-let test_add_bundle_repo () =
-  let __context = T.make_test_database () in
-  let name_label = "remote" in
-  let name_description = "remote" in
-  let binary_url = "https://repo.example.com" in
-  let source_url = "https://repo-src.example.com" in
-  let gpgkey_path = "" in
-  let ref_remote =
-    Repository.introduce ~__context ~name_label ~name_description ~binary_url
-      ~source_url ~update:true ~gpgkey_path
+  let ref_remote_pool =
+    Repository.introduce_remote_pool ~__context ~name_label:"remote_pool"
+      ~binary_url:binary_url_2 ~name_description:"remote_pool" ~certificate:""
   in
-  let ref_bundle =
-    Repository.introduce_bundle ~__context ~name_label:"bundle"
-      ~name_description:"bundle"
-  in
-  let self = Helpers.get_pool ~__context in
-  Alcotest.check_raises "test_add_bundle_repo"
-    Api_errors.(Server_error (bundle_repo_should_be_single_enabled, []))
-    (fun () ->
-      Xapi_pool.set_repositories ~__context ~self ~value:[ref_remote] ;
-      Xapi_pool.add_repository ~__context ~self ~value:ref_bundle
-    )
+  f __context pool ref_remote ref_bundle ref_remote_pool
 
-let test_add_remote_repo () =
-  let __context = T.make_test_database () in
-  let name_label = "remote" in
-  let name_description = "remote" in
-  let binary_url = "https://repo.example.com" in
-  let source_url = "https://repo-src.example.com" in
-  let gpgkey_path = "" in
-  let ref_remote =
-    Repository.introduce ~__context ~name_label ~name_description ~binary_url
-      ~source_url ~update:true ~gpgkey_path
-  in
-  let ref_bundle =
-    Repository.introduce_bundle ~__context ~name_label:"bundle"
-      ~name_description:"bundle"
-  in
-  let self = Helpers.get_pool ~__context in
-  Alcotest.check_raises "test_add_remote_repo"
-    Api_errors.(Server_error (bundle_repo_should_be_single_enabled, []))
-    (fun () ->
+let test_set_repositories () =
+  on_repositories (fun __context self ref_remote ref_bundle ref_remote_pool ->
+      Xapi_pool.set_repositories ~__context ~self ~value:[ref_remote] ;
       Xapi_pool.set_repositories ~__context ~self ~value:[ref_bundle] ;
-      Xapi_pool.add_repository ~__context ~self ~value:ref_remote
-    )
+      Xapi_pool.set_repositories ~__context ~self ~value:[ref_remote_pool] ;
+      Alcotest.check_raises "test_set_repositories_1"
+        Api_errors.(
+          Server_error
+            ( repo_should_be_single_one_enabled
+            , [Record_util.origin_to_string `bundle]
+            )
+        )
+        (fun () ->
+          Xapi_pool.set_repositories ~__context ~self
+            ~value:[ref_remote; ref_bundle]
+        ) ;
+      Alcotest.check_raises "test_set_repositories_2"
+        Api_errors.(
+          Server_error
+            ( repo_should_be_single_one_enabled
+            , [`bundle; `remote_pool] |> List.map Record_util.origin_to_string
+            )
+        )
+        (fun () ->
+          Xapi_pool.set_repositories ~__context ~self
+            ~value:[ref_remote_pool; ref_bundle]
+        ) ;
+      Alcotest.check_raises "test_set_repositories_3"
+        Api_errors.(
+          Server_error
+            ( repo_should_be_single_one_enabled
+            , [Record_util.origin_to_string `remote_pool]
+            )
+        )
+        (fun () ->
+          Xapi_pool.set_repositories ~__context ~self
+            ~value:[ref_remote; ref_remote_pool]
+        )
+  )
 
-let test_can_not_enable_bundle_repo_auto_sync () =
-  let __context = T.make_test_database () in
-  let ref_bundle =
-    Repository.introduce_bundle ~__context ~name_label:"bundle"
-      ~name_description:"bundle"
-  in
-  let self = Helpers.get_pool ~__context in
-  Alcotest.check_raises "test_can_not_enable_bundle_repo_auto_sync"
-    Api_errors.(Server_error (can_not_sync_updates, []))
-    (fun () ->
-      Xapi_pool.set_repositories ~__context ~self ~value:[ref_bundle] ;
-      Xapi_pool.set_update_sync_enabled ~__context ~self ~value:true
-    )
+let test_add_repository () =
+  on_repositories (fun __context self ref_remote ref_bundle ref_remote_pool ->
+      Alcotest.check_raises "test_add_repository_1"
+        Api_errors.(
+          Server_error
+            ( repo_should_be_single_one_enabled
+            , [Record_util.origin_to_string `bundle]
+            )
+        )
+        (fun () ->
+          Xapi_pool.set_repositories ~__context ~self ~value:[ref_remote] ;
+          Xapi_pool.add_repository ~__context ~self ~value:ref_bundle
+        ) ;
+      Alcotest.check_raises "test_add_repository_2"
+        Api_errors.(
+          Server_error
+            ( repo_should_be_single_one_enabled
+            , [Record_util.origin_to_string `remote_pool]
+            )
+        )
+        (fun () ->
+          Xapi_pool.set_repositories ~__context ~self ~value:[ref_remote] ;
+          Xapi_pool.add_repository ~__context ~self ~value:ref_remote_pool
+        ) ;
+      Alcotest.check_raises "test_add_repository_3"
+        Api_errors.(
+          Server_error
+            ( repo_should_be_single_one_enabled
+            , [`remote_pool; `bundle] |> List.map Record_util.origin_to_string
+            )
+        )
+        (fun () ->
+          Xapi_pool.set_repositories ~__context ~self ~value:[ref_remote_pool] ;
+          Xapi_pool.add_repository ~__context ~self ~value:ref_bundle
+        ) ;
+      Alcotest.check_raises "test_add_repository_4"
+        Api_errors.(
+          Server_error
+            ( repo_should_be_single_one_enabled
+            , [`bundle; `remote_pool] |> List.map Record_util.origin_to_string
+            )
+        )
+        (fun () ->
+          Xapi_pool.set_repositories ~__context ~self ~value:[ref_bundle] ;
+          Xapi_pool.add_repository ~__context ~self ~value:ref_remote_pool
+        )
+  )
+
+let test_enable_periodic_repo_sync () =
+  on_repositories (fun __context self ref_remote ref_bundle ref_remote_pool ->
+      Xapi_pool.set_repositories ~__context ~self ~value:[ref_remote] ;
+      Xapi_pool.set_update_sync_enabled ~__context ~self ~value:true ;
+      Alcotest.check_raises "test_enable_periodic_repo_sync_1"
+        Api_errors.(Server_error (can_not_periodic_sync_updates, []))
+        (fun () ->
+          Xapi_pool.set_repositories ~__context ~self ~value:[ref_bundle] ;
+          Xapi_pool.set_update_sync_enabled ~__context ~self ~value:true
+        ) ;
+      Alcotest.check_raises "test_enable_periodic_repo_sync_2"
+        Api_errors.(Server_error (can_not_periodic_sync_updates, []))
+        (fun () ->
+          Xapi_pool.set_repositories ~__context ~self ~value:[ref_remote_pool] ;
+          Xapi_pool.set_update_sync_enabled ~__context ~self ~value:true
+        )
+  )
 
 let test =
   [
-    ( "test_set_remote_and_bundle_repos"
-    , `Quick
-    , test_set_remote_and_bundle_repos
-    )
-  ; ("test_add_bundle_repo", `Quick, test_add_bundle_repo)
-  ; ("test_add_remote_repo", `Quick, test_add_remote_repo)
-  ; ( "test_can_not_enable_bundle_repo_auto_sync"
-    , `Quick
-    , test_can_not_enable_bundle_repo_auto_sync
-    )
+    ("test_set_repositories", `Quick, test_set_repositories)
+  ; ("test_add_repository", `Quick, test_add_repository)
+  ; ("test_enable_periodic_repo_sync", `Quick, test_enable_periodic_repo_sync)
   ]
 
 let () =

ocaml/xapi-consts/api_errors.ml

@@ -1322,8 +1322,10 @@ let bundle_repo_not_enabled = add_error "BUNDLE_REPO_NOT_ENABLED"
 
 let can_not_sync_updates = add_error "CAN_NOT_SYNC_UPDATES"
 
-let bundle_repo_should_be_single_enabled =
-  add_error "BUNDLE_REPO_SHOULD_BE_SINGLE_ENABLED"
+let can_not_periodic_sync_updates = add_error "CAN_NOT_PERIODIC_SYNC_UPDATES"
+
+let repo_should_be_single_one_enabled =
+  add_error "REPO_SHOULD_BE_SINGLE_ONE_ENABLED"
 
 let repository_is_in_use = add_error "REPOSITORY_IS_IN_USE"
 

ocaml/xapi/xapi_pool.ml

@@ -3406,21 +3406,50 @@ let enable_tls_verification ~__context =
   | Some self ->
       Xapi_cluster_host.set_tls_config ~__context ~self ~verify:true
 
-let contains_bundle_repo ~__context ~repos =
-  List.exists
-    (fun repo -> Db.Repository.get_origin ~__context ~self:repo = `bundle)
+let assert_single_repo_can_be_enabled ~__context ~repos =
+  let origins =
     repos
+    |> List.filter_map (fun repo ->
+           match Db.Repository.get_origin ~__context ~self:repo with
+           | (`bundle | `remote_pool) as origin ->
+               Some origin
+           | `remote ->
+               None
+       )
+    |> List.fold_left
+         (fun acc origin -> if List.mem origin acc then acc else origin :: acc)
+         []
+  in
+  match (repos, origins) with
+  | _ :: _ :: _, _ :: _ ->
+      raise
+        Api_errors.(
+          Server_error
+            ( repo_should_be_single_one_enabled
+            , origins |> List.map Record_util.origin_to_string
+            )
+        )
+  | _, _ ->
+      ()
 
-let assert_single_bundle_repo_can_be_enabled ~__context ~repos =
-  if List.length repos > 1 && contains_bundle_repo ~__context ~repos then
-    raise Api_errors.(Server_error (bundle_repo_should_be_single_enabled, []))
+let assert_can_sync_updates ~__context ~repos =
+  List.iter
+    (fun repo ->
+      match Db.Repository.get_origin ~__context ~self:repo with
+      | `remote | `remote_pool ->
+          ()
+      | `bundle ->
+          raise Api_errors.(Server_error (can_not_sync_updates, []))
+    )
+    repos
 
-let assert_not_bundle_repo ~__context ~repos =
-  if contains_bundle_repo ~__context ~repos then
-    raise Api_errors.(Server_error (can_not_sync_updates, []))
+let can_periodic_sync_updates ~__context ~repos =
+  List.for_all
+    (fun repo -> Db.Repository.get_origin ~__context ~self:repo = `remote)
+    repos
 
-let disable_auto_update_sync_for_bundle_repo ~__context ~self ~repos =
-  if contains_bundle_repo ~__context ~repos then (
+let disable_unsupported_periodic_sync_updates ~__context ~self ~repos =
+  if not (can_periodic_sync_updates ~__context ~repos) then (
     Pool_periodic_update_sync.set_enabled ~__context ~value:false ;
     Db.Pool.set_update_sync_enabled ~__context ~self ~value:false
   )
@@ -3429,7 +3458,7 @@ let set_repositories ~__context ~self ~value =
   Xapi_pool_helpers.with_pool_operation ~__context ~self
     ~doc:"pool.set_repositories" ~op:`configure_repositories
   @@ fun () ->
-  assert_single_bundle_repo_can_be_enabled ~__context ~repos:value ;
+  assert_single_repo_can_be_enabled ~__context ~repos:value ;
   let existings = Db.Pool.get_repositories ~__context ~self in
   (* To be removed *)
   List.iter
@@ -3453,23 +3482,22 @@ let set_repositories ~__context ~self ~value =
   Db.Pool.set_repositories ~__context ~self ~value ;
   if Db.Pool.get_repositories ~__context ~self = [] then
     Db.Pool.set_last_update_sync ~__context ~self ~value:Date.epoch ;
-  disable_auto_update_sync_for_bundle_repo ~__context ~self ~repos:value
+  disable_unsupported_periodic_sync_updates ~__context ~self ~repos:value
 
 let add_repository ~__context ~self ~value =
   Xapi_pool_helpers.with_pool_operation ~__context ~self
     ~doc:"pool.add_repository" ~op:`configure_repositories
   @@ fun () ->
   let existings = Db.Pool.get_repositories ~__context ~self in
   if not (List.mem value existings) then (
-    assert_single_bundle_repo_can_be_enabled ~__context
-      ~repos:(value :: existings) ;
+    assert_single_repo_can_be_enabled ~__context ~repos:(value :: existings) ;
     Db.Pool.add_repositories ~__context ~self ~value ;
     Db.Repository.set_hash ~__context ~self:value ~value:"" ;
     Repository.reset_updates_in_cache () ;
-    Db.Pool.set_last_update_sync ~__context ~self ~value:Date.epoch
-  ) ;
-  disable_auto_update_sync_for_bundle_repo ~__context ~self
-    ~repos:(value :: existings)
+    Db.Pool.set_last_update_sync ~__context ~self ~value:Date.epoch ;
+    disable_unsupported_periodic_sync_updates ~__context ~self
+      ~repos:(value :: existings)
+  )
 
 let remove_repository ~__context ~self ~value =
   Xapi_pool_helpers.with_pool_operation ~__context ~self
@@ -3508,7 +3536,7 @@ let sync_updates ~__context ~self ~force ~token ~token_id =
     ~doc:"pool.sync_updates" ~op:`sync_updates
   @@ fun () ->
   let repos = Repository_helpers.get_enabled_repositories ~__context in
-  assert_not_bundle_repo ~__context ~repos ;
+  assert_can_sync_updates ~__context ~repos ;
   sync_repos ~__context ~self ~repos ~force ~token ~token_id
 
 let check_update_readiness ~__context ~self:_ ~requires_reboot =
@@ -3793,7 +3821,8 @@ let set_update_sync_enabled ~__context ~self ~value =
              repositories." ;
           raise Api_errors.(Server_error (no_repositories_configured, []))
       | repos ->
-          assert_not_bundle_repo ~__context ~repos
+          if not (can_periodic_sync_updates ~__context ~repos) then
+            raise Api_errors.(Server_error (can_not_periodic_sync_updates, []))
   ) ;
   Pool_periodic_update_sync.set_enabled ~__context ~value ;
   Db.Pool.set_update_sync_enabled ~__context ~self ~value