diff --git a/ocaml/idl/datamodel_errors.ml b/ocaml/idl/datamodel_errors.ml index aead3e0abc..0258785caf 100644 --- a/ocaml/idl/datamodel_errors.ml +++ b/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 [] diff --git a/ocaml/tests/test_pool_repository.ml b/ocaml/tests/test_pool_repository.ml index bdfcc314e2..4d0bdb45ee 100644 --- a/ocaml/tests/test_pool_repository.ml +++ b/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 () = diff --git a/ocaml/xapi-consts/api_errors.ml b/ocaml/xapi-consts/api_errors.ml index 97880cde57..bcd0f18b3e 100644 --- a/ocaml/xapi-consts/api_errors.ml +++ b/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" diff --git a/ocaml/xapi/xapi_pool.ml b/ocaml/xapi/xapi_pool.ml index 7a6aaa2101..12d817c187 100644 --- a/ocaml/xapi/xapi_pool.ml +++ b/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,7 +3482,7 @@ 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 @@ -3461,15 +3490,14 @@ let add_repository ~__context ~self ~value = @@ 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