Skip to content

Commit

Permalink
CP-51988: Fix functions not work for remote_pool repo (#6095)
Browse files Browse the repository at this point in the history 
1. `remote_pool` repo doesn't support periodic sync updates.
2. Periodic sync updates should be auto-disabled when calling
`set_repositories` and `add_repository` for `remote_pool` repo.
4. Update UT.
  • Loading branch information
@BengangY
BengangY authored Nov 8, 2024
2 parents dbe6a0c + 29e1fe4 commit deb25d2
Show file tree
Hide file tree
Showing 4 changed files with 183 additions and 107 deletions.
16 changes: 12 additions & 4 deletions ocaml/idl/datamodel_errors.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 []
Expand Down
197 changes: 117 additions & 80 deletions ocaml/tests/test_pool_repository.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 () =
Expand Down
6 changes: 4 additions & 2 deletions ocaml/xapi-consts/api_errors.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down
71 changes: 50 additions & 21 deletions ocaml/xapi/xapi_pool.ml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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 =
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit deb25d2

Please sign in to comment.