Skip to content

CP-53711: Apply SSH settings in joiner before update_non_vm_metadata #6408

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
merged 2 commits into from
Apr 8, 2025
Merged

CP-53711: Apply SSH settings in joiner before update_non_vm_metadata #6408

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
31c2b3c
Fixup
gangj Apr 7, 2025
5a533f5
CP-53711: Apply SSH settings in joiner before update_non_vm_metadata
gangj Apr 7, 2025
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
77 changes: 40 additions & 37 deletions ocaml/xapi/xapi_pool.ml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -943,38 +943,6 @@ let rec create_or_get_host_on_master __context rpc session_id (host_ref, host) :
create_or_get_sr_on_master __context rpc session_id
(my_local_cache_sr, my_local_cache_sr_rec)
in
let remote_coordinator = get_master ~rpc ~session_id in
let ssh_enabled =
Client.Host.get_ssh_enabled ~rpc ~session_id ~self:remote_coordinator
in
let ssh_enabled_timeout =
Client.Host.get_ssh_enabled_timeout ~rpc ~session_id
~self:remote_coordinator
in
let console_idle_timeout =
Client.Host.get_console_idle_timeout ~rpc ~session_id
~self:remote_coordinator
in
(* Configure SSH service on local host *)
Xapi_host.set_console_idle_timeout ~__context ~self:host_ref
~value:console_idle_timeout ;
Xapi_host.set_ssh_enabled_timeout ~__context ~self:host_ref
~value:ssh_enabled_timeout ;
( match ssh_enabled with
| true ->
Xapi_host.enable_ssh ~__context ~self:host_ref
| false ->
Xapi_host.disable_ssh ~__context ~self:host_ref
) ;
(* As ssh_expiry will be updated by host.enable_ssh and host.disable_ssh,
there is a corner case when the joiner's SSH state will not match SSH
service state in its new coordinator exactly: if the joiner joins when
SSH service has been enabled in the new coordinator, while not timed
out yet, the joiner will start SSH service with timeout
host.ssh_enabled_timeout, which means SSH service in the joiner will
be disabled later than in the new coordinator. *)
let ssh_expiry = Db.Host.get_ssh_expiry ~__context ~self:host_ref in

debug "Creating host object on master" ;
let ref =
Client.Host.create ~rpc ~session_id ~uuid:my_uuid
Expand All @@ -994,8 +962,11 @@ let rec create_or_get_host_on_master __context rpc session_id (host_ref, host) :
~local_cache_sr ~chipset_info:host.API.host_chipset_info
~ssl_legacy:false
~last_software_update:host.API.host_last_software_update
~last_update_hash:host.API.host_last_update_hash ~ssh_enabled
~ssh_enabled_timeout ~ssh_expiry ~console_idle_timeout
~last_update_hash:host.API.host_last_update_hash
~ssh_enabled:host.API.host_ssh_enabled
~ssh_enabled_timeout:host.API.host_ssh_enabled_timeout
~ssh_expiry:host.API.host_ssh_expiry
~console_idle_timeout:host.API.host_console_idle_timeout
in
(* Copy other-config into newly created host record: *)
no_exn
Expand Down Expand Up @@ -1588,6 +1559,7 @@ let join_common ~__context ~master_address ~master_username ~master_password
)
in

let remote_coordinator = get_master ~rpc ~session_id in
(* If management is on a VLAN, then get the Pool master
management network bridge before we logout the session *)
let pool_master_bridge, mgmt_pif =
Expand All @@ -1598,7 +1570,7 @@ let join_common ~__context ~master_address ~master_username ~master_password
if Db.PIF.get_VLAN_master_of ~__context ~self:my_pif <> Ref.null then
let pif =
Client.Host.get_management_interface ~rpc ~session_id
~host:(get_master ~rpc ~session_id)
~host:remote_coordinator
in
let network = Client.PIF.get_network ~rpc ~session_id ~self:pif in
(Some (Client.Network.get_bridge ~rpc ~session_id ~self:network), my_pif)
Expand Down Expand Up @@ -1688,8 +1660,39 @@ let join_common ~__context ~master_address ~master_username ~master_password
"Unable to set the write the new pool certificates to the disk : %s"
(ExnHelper.string_of_exn e)
) ;
Db.Host.set_latest_synced_updates_applied ~__context ~self:me
~value:`unknown ;
( try
let ssh_enabled_timeout =
Client.Host.get_ssh_enabled_timeout ~rpc ~session_id
~self:remote_coordinator
in
let console_idle_timeout =
Client.Host.get_console_idle_timeout ~rpc ~session_id
~self:remote_coordinator
in
Xapi_host.set_console_idle_timeout ~__context ~self:me
~value:console_idle_timeout ;
Xapi_host.set_ssh_enabled_timeout ~__context ~self:me
~value:ssh_enabled_timeout ;
let ssh_enabled =
Client.Host.get_ssh_enabled ~rpc ~session_id
~self:remote_coordinator
in
(* As ssh_expiry will be updated by host.enable_ssh and host.disable_ssh,
there is a corner case when the joiner's SSH state will not match SSH
service state in its new coordinator exactly: if the joiner joins when
SSH service has been enabled in the new coordinator, while not timed
out yet, the joiner will start SSH service with timeout
host.ssh_enabled_timeout, which means SSH service in the joiner will
be disabled later than in the new coordinator. *)
match ssh_enabled with
| true ->
Xapi_host.enable_ssh ~__context ~self:me
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this not highlighting that we have two API functions where one taking an argument would have been more convenient? I am not asking for a change but the API leads to verbose clients.

| false ->
Xapi_host.disable_ssh ~__context ~self:me
with e ->
error "Unable to configure SSH service on local host: %s"
(ExnHelper.string_of_exn e)
) ;
(* this is where we try and sync up as much state as we can
with the master. This is "best effort" rather than
critical; if we fail part way through this then we carry
Expand Down
Loading