Skip to content

CA-422713: XSI-2105: Pool.join failed due to AD status corrupt #6832

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
liulinC merged 1 commit into from
Jan 21, 2026
Merged

CA-422713: XSI-2105: Pool.join failed due to AD status corrupt #6832

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions ocaml/idl/datamodel_host.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2033,6 +2033,13 @@ let disable_external_auth =
; param_release= george_release
; param_default= Some (VMap [])
}
; {
param_type= Bool
; param_name= "force"
; param_doc= "Disable external auth even when not enabled"
; param_release= numbered_release "26.2.0-next"
; param_default= Some (VBool false)
}
]
~doc:"This call disables external authentication on the local host"
~allowed_roles:_R_POOL_ADMIN ()
Expand Down
2 changes: 1 addition & 1 deletion ocaml/xapi-cli-server/cli_operations.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7089,7 +7089,7 @@ let host_disable_external_auth _printer rpc session_id params =
let host_uuid = List.assoc "host-uuid" params in
let host = Client.Host.get_by_uuid ~rpc ~session_id ~uuid:host_uuid in
let config = read_map_params "config" params in
Client.Host.disable_external_auth ~rpc ~session_id ~host ~config
Client.Host.disable_external_auth ~rpc ~session_id ~host ~config ~force:true
Copy link
Contributor

@changlei-li changlei-li Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not let the user pass the force value like xe host-disable-external-auth --force=true

Copy link
Collaborator Author

@liulinC liulinC Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is let use use --force=true per my test.
note: force=true|--force is forced to reach here.

Copy link
Contributor

@changlei-li changlei-li Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see the command requires --force to make the user aware of it is a recover command only. So the user can't select force:false. It is intended, Right?

Copy link
Collaborator Author

@liulinC liulinC Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, This API is hidden, only used for specific purpose with --force for awareness.


let host_refresh_pack_info _printer rpc session_id params =
let host_uuid = List.assoc "host-uuid" params in
Expand Down
8 changes: 5 additions & 3 deletions ocaml/xapi/message_forwarding.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3730,11 +3730,13 @@ functor
in
do_op_on ~local_fn ~__context ~host ~remote_fn

let disable_external_auth ~__context ~host ~config =
let disable_external_auth ~__context ~host ~config ~force =
info "Host.disable_external_auth: host = '%s'"
(host_uuid ~__context host) ;
let local_fn = Local.Host.disable_external_auth ~host ~config in
let remote_fn = Client.Host.disable_external_auth ~host ~config in
let local_fn = Local.Host.disable_external_auth ~host ~config ~force in
let remote_fn =
Client.Host.disable_external_auth ~host ~config ~force
in
do_op_on ~local_fn ~__context ~host ~remote_fn

let install_ca_certificate ~__context ~host ~name ~cert =
Expand Down
34 changes: 11 additions & 23 deletions ocaml/xapi/xapi_host.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1889,13 +1889,13 @@ let enable_external_auth ~__context ~host ~config ~service_name ~auth_type =

(* CP-718: Disables external auth/directory service for host *)
let disable_external_auth_common ?(during_pool_eject = false) ~__context ~host
~config () =
~config ~force () =
(* CP-825: Serialize execution of host-enable-extauth and host-disable-extauth *)
(* we need to protect against concurrent access to the host.external_auth_type variable *)
with_lock serialize_host_enable_disable_extauth (fun () ->
let host_name_label = Db.Host.get_name_label ~__context ~self:host in
let auth_type = Db.Host.get_external_auth_type ~__context ~self:host in
if auth_type = "" then
if auth_type = "" && not force then
(* nothing to do, external authentication is already disabled *)
let msg = "external authentication service is already disabled" in
debug "Failed to disable external authentication in host %s: %s"
Expand Down Expand Up @@ -1936,6 +1936,8 @@ let disable_external_auth_common ?(during_pool_eject = false) ~__context ~host
, [msg]
)
)
| Extauth_is_disabled ->
Some Extauth_is_disabled
| e ->
(*absorb any exception*)
debug
Expand All @@ -1957,19 +1959,6 @@ let disable_external_auth_common ?(during_pool_eject = false) ~__context ~host
Xapi_globs.event_hook_auth_on_xapi_initialize_succeeded := true ;

(* succeeds because there's no need to initialize anymore *)

(* If any cache is present, clear it in order to ensure cached
logins don't persist after disabling external
authentication. *)
Xapi_session.clear_external_auth_cache () ;

(* 3. CP-703: we always revalidate all sessions after the external authentication has been disabled *)
(* so that all sessions that were externally authenticated will be destroyed *)
debug
"calling revalidate_all_sessions after disabling external auth for \
host %s"
host_name_label ;
Xapi_session.revalidate_all_sessions ~__context ;
if not during_pool_eject then
(* CA-28168 *)
(* CA-24856: detect non-homogeneous external-authentication config in this host *)
Expand All @@ -1978,19 +1967,18 @@ let disable_external_auth_common ?(during_pool_eject = false) ~__context ~host
if auth_type = Xapi_globs.auth_type_AD then
Extauth_ad.stop_backend_daemon ~wait_until_success:false ;
match plugin_disable_failure with
| None ->
(* we do not want to stop pool_eject and permit Extauth_is_disabled during force *)
| Some e when during_pool_eject || (e = Extauth_is_disabled && force) ->
Copy link
Contributor

@lindig lindig Jan 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the use of when

()
| Some e ->
if not during_pool_eject then
raise e (* bubble up plugin's on_disable exception *)
else
()
(* we do not want to stop pool_eject *)
raise e
| None ->
()
)

let disable_external_auth ~__context ~host ~config =
let disable_external_auth ~__context ~host ~config ~force =
disable_external_auth_common ~during_pool_eject:false ~__context ~host ~config
()
~force ()

module Static_vdis_list = Xapi_database.Static_vdis_list

Expand Down
2 changes: 2 additions & 0 deletions ocaml/xapi/xapi_host.mli
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -361,13 +361,15 @@ val disable_external_auth_common :
-> __context:Context.t
-> host:API.ref_host
-> config:(string * string) list
-> force:bool
-> unit
-> unit

val disable_external_auth :
__context:Context.t
-> host:API.ref_host
-> config:(string * string) list
-> force:bool
-> unit

(** {2 Static VDIs} *)
Expand Down
18 changes: 15 additions & 3 deletions ocaml/xapi/xapi_pool.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2037,7 +2037,7 @@ let eject_self ~__context ~host =
(* disable the external authentication of this slave being ejected *)
(* this call will return an exception if something goes wrong *)
Xapi_host.disable_external_auth_common ~during_pool_eject:true ~__context
~host ~config:[] () ;
~host ~config:[] ~force:false () ;
Copy link
Contributor

@changlei-li changlei-li Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not clear for what cases we need force:false? What's the benefit of it?

Copy link
Collaborator Author

@liulinC liulinC Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we do not use force by default, until user want to use it manually.


(* FIXME: in the future, we should send the windows AD admin/pass here *)
(* in order to remove the slave from the AD database during pool-eject *)
Expand Down Expand Up @@ -2973,7 +2973,7 @@ let enable_external_auth ~__context ~pool:_ ~config ~service_name ~auth_type =
(* best-effort attempt to disable all enabled hosts, swallowing any exceptions *)
try
call_fn_on_host ~__context
(Client.Host.disable_external_auth ~config)
(Client.Host.disable_external_auth ~config ~force:false)
host
with e ->
debug
Expand Down Expand Up @@ -3041,7 +3041,7 @@ let disable_external_auth ~__context ~pool:_ ~config =
(* forward the call to the host in the pool *)
try
call_fn_on_host ~__context
(Client.Host.disable_external_auth ~config)
(Client.Host.disable_external_auth ~config ~force:false)
host ;
(* no failed host to add to the filtered list, just visit next host *)
(host, "", "")
Expand Down Expand Up @@ -3100,9 +3100,21 @@ let disable_external_auth ~__context ~pool:_ ~config =
)
)
) else (* OK *)
(
(* If any cache is present, clear it in order to ensure cached
logins don't persist after disabling external
authentication. *)
Xapi_session.clear_external_auth_cache () ;

(* CP-703: we always revalidate all sessions after the external authentication has been disabled *)
(* so that all sessions that were externally authenticated will be destroyed *)
debug "calling revalidate_all_sessions after disabling external auth" ;
Xapi_session.revalidate_all_sessions ~__context ;

debug
"The external authentication of all hosts in the pool was disabled \
successfully"
)
Copy link
Contributor

@changlei-li changlei-li Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it change the behavior? If the pool disable partially fails, some hosts have been disabled. Does it need to clear the session?

Copy link
Collaborator Author

@liulinC liulinC Jan 21, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, we should not clear the session.
That is also the problem of the old code.

)

(* CA-24856: detect non-homogeneous external-authentication config in pool *)
Expand Down
Loading