Skip to content

Commit

Permalink
CA-403620: Drop the usage of fuser in stunnel client proxy (#6197)
Browse files Browse the repository at this point in the history
Two changes in the PR:
**Drop the usage of fuser in stunnel client proxy**
The drawback of fuser is that it gets too many things involved. E.g. it
is observed that it got stuck on cifs kernel module.

This change uses a cleaner way to remember the stunnel client proxy.
Even when the xapi restarted unexpectedly, it can stop the remnant
stunnel proxy and start a new one.

**Make the stunnel proxy local port configurable**
Making it configurable can avoid the situation when the port conflicts
with others, e.g. an external program from users.
  • Loading branch information
minglumlu authored Jan 13, 2025
2 parents f9b5e52 + e7f2b70 commit fbaad2b
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 40 deletions.
58 changes: 22 additions & 36 deletions ocaml/libs/stunnel/stunnel.ml
Original file line number Diff line number Diff line change
Expand Up @@ -448,44 +448,30 @@ let with_connect ?unique_id ?use_fork_exec_helper ?write_to_log ~verify_cert
)
5
let with_client_proxy ~verify_cert ~remote_host ~remote_port ~local_host
~local_port f =
( try
D.debug "Clean up running stunnel client proxy if there is any ..." ;
let out, _ =
Forkhelpers.execute_command_get_output "/usr/sbin/fuser"
["-4k"; string_of_int local_port ^ "/tcp"]
in
D.debug "Killed running stunnel client proxy:%s" out
with
| Forkhelpers.Spawn_internal_error (stderr, stdout, process_status) -> (
match process_status with
| Unix.WEXITED 1 ->
D.debug "No running stunnel client proxy"
| _ ->
D.warn
"Cleaning up running stunnel client proxy returned unexpectedly: \
stdout=(%s); stderr=(%s)"
stdout stderr
)
) ;
retry
let with_client_proxy_systemd_service ~verify_cert ~remote_host ~remote_port
~local_host ~local_port ~service f =
let cmd_path = stunnel_path () in
let config =
config_file
~accept:(Some (local_host, local_port))
verify_cert remote_host remote_port
in
let stop () = ignore (Fe_systemctl.stop ~service) in
(* Try stopping anyway before starting it. *)
ignore_exn stop () ;
let conf_path, out = Filename.open_temp_file service ".conf" in
let finally = Xapi_stdext_pervasives.Pervasiveext.finally in
finally
(fun () ->
let pid, _ =
attempt_one_connect
(`Local_host_port (local_host, local_port))
verify_cert remote_host remote_port
in
D.debug "Started a client proxy (pid:%s): %s:%s -> %s:%s"
(string_of_int (getpid pid))
local_host (string_of_int local_port) remote_host
(string_of_int remote_port) ;
Xapi_stdext_pervasives.Pervasiveext.finally
(fun () -> f ())
(fun () -> disconnect_with_pid ~wait:false ~force:true pid)
finally (fun () -> output_string out config) (fun () -> close_out out) ;
finally
(fun () ->
Fe_systemctl.start_transient ~service cmd_path [conf_path] ;
f ()
)
(fun () -> ignore_exn stop ())
)
5
(fun () -> Unixext.unlink_safe conf_path)
let check_verify_error line =
let sub_after i s =
Expand Down
3 changes: 2 additions & 1 deletion ocaml/libs/stunnel/stunnel.mli
Original file line number Diff line number Diff line change
Expand Up @@ -88,11 +88,12 @@ val with_moved_exn : t -> (t -> 'd) -> 'd

val safe_release : t -> unit

val with_client_proxy :
val with_client_proxy_systemd_service :
verify_cert:verification_config option
-> remote_host:string
-> remote_port:int
-> local_host:string
-> local_port:int
-> service:string
-> (unit -> 'a)
-> 'a
7 changes: 4 additions & 3 deletions ocaml/xapi/repository_helpers.ml
Original file line number Diff line number Diff line change
Expand Up @@ -398,10 +398,11 @@ let with_local_repositories ~__context f =
with Pool_role.This_host_is_a_master ->
Option.get (Helpers.get_management_ip_addr ~__context)
in
Stunnel.with_client_proxy ~verify_cert:(Stunnel_client.pool ())
~remote_host:master_addr ~remote_port:Constants.default_ssl_port
~local_host:"127.0.0.1"
Stunnel.with_client_proxy_systemd_service
~verify_cert:(Stunnel_client.pool ()) ~remote_host:master_addr
~remote_port:Constants.default_ssl_port ~local_host:"127.0.0.1"
~local_port:!Xapi_globs.local_yum_repo_port
~service:"stunnel_proxy_for_update_client"
@@ fun () ->
let enabled = get_enabled_repositories ~__context in
Xapi_stdext_pervasives.Pervasiveext.finally
Expand Down
1 change: 1 addition & 0 deletions ocaml/xapi/xapi_globs.ml
Original file line number Diff line number Diff line change
Expand Up @@ -1143,6 +1143,7 @@ let xapi_globs_spec =
; ("max_traces", Int max_traces)
; ("max_observer_file_size", Int max_observer_file_size)
; ("test-open", Int test_open) (* for consistency with xenopsd *)
; ("local_yum_repo_port", Int local_yum_repo_port)
]

let xapi_globs_spec_with_descriptions = []
Expand Down

0 comments on commit fbaad2b

Please sign in to comment.