mgr/cephadm: replace execnet and remoto with asyncssh #42051

melissa-kun-li · 2021-06-28T14:38:05Z

This PR will replace execnet and remoto with asyncssh implemented in the ssh module. The asyncssh debug log will be redirected to the raised OrchestratorError if an ssh connection error occurs with the remote host. remotes.py is also removed and replaced with _write_remote_file in ssh.py

A thread is started from the EventLoopThread class in ssh.py, which also starts the event loop for running the async functions _remote_connection, _execute_command, _check_execute_command, and _write_remote_file

Fixes: https://tracker.ceph.com/issues/44676
Signed-off-by: Melissa Li li.melissa.kun@gmail.com

Checklist

References tracker ticket
Updates documentation if necessary
Includes tests for new functionality or reproducer for bug

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox

src/pybind/mgr/cephadm/module.py

src/pybind/mgr/cephadm/ssh.py

src/pybind/mgr/cephadm/module.py

src/pybind/mgr/cephadm/serve.py

src/pybind/mgr/cephadm/ssh.py

sebastian-philipp · 2021-07-01T14:51:44Z

Ok, form my POV, this looks ok as it is. If we can get all the various checks to pass and get it through teuthology, this is ready.

dsavineau · 2021-07-15T14:55:22Z

I think we need someone to start working on the asyncssh packaging for EL8 as this package doesn't exist at the moment.

Debian and Ubuntu already have it [1][2]

[1] https://packages.debian.org/search?keywords=python3-asyncssh
[2] https://packages.ubuntu.com/search?keywords=python3-asyncssh

tchaikov · 2021-07-20T05:27:37Z

I think we need someone to start working on the asyncssh packaging for EL8 as this package doesn't exist at the moment.

Debian and Ubuntu already have it [1][2]

[1] https://packages.debian.org/search?keywords=python3-asyncssh
[2] https://packages.ubuntu.com/search?keywords=python3-asyncssh

i just created a copr repo at https://copr.fedorainfracloud.org/coprs/tchaikov/python3-asyncssh/, which offers python3-libnacl and python3-asyncssh. python3-libnacl is a build-time and run-time dependency of python3-asyncssh. to install python3-asyncssh, we need to

sudo dnf -y copr enable tchaikov/python3-asyncssh epel-8-$(arch)
sudo dnf -y install python3-asyncssh

to integrate this package into the ceph image, we might need to create a change like ceph/ceph-container#1821.

in the long run, probably a better way to address this dependency is probably to maintain these packages in EPEL8.

melissa-kun-li · 2021-07-21T13:37:53Z

jenkins test make check

mgfritch · 2021-07-21T15:26:48Z

make check failures appear related to #42425

../src/test/rgw/test_rgw_dmclock_scheduler.cc:425: Failure
Value of: context.stopped()
  Actual: false
Expected: true

melissa-kun-li · 2021-07-21T16:38:04Z

jenkins test make check

dsavineau · 2021-07-22T16:00:39Z

i just created a copr repo at https://copr.fedorainfracloud.org/coprs/tchaikov/python3-asyncssh/, which offers python3-libnacl and python3-asyncssh. python3-libnacl is a build-time and run-time dependency of python3-asyncssh. to install python3-asyncssh, we need to

sudo dnf -y copr enable tchaikov/python3-asyncssh epel-8-$(arch)

sudo dnf -y install python3-asyncssh

to integrate this package into the ceph image, we might need to create a change like ceph/ceph-container#1821.

in the long run, probably a better way to address this dependency is probably to maintain these packages in EPEL8.

Thanks for taking care of this @tchaikov

I'm preparing a PR for ceph-container.

I just have a question about the python3-libnacl dependency. Currently the dependency isn't installed automatically when installing python3-asyncssh. Is this expected ?

# dnf install python3-asyncssh
Failed to set locale, defaulting to C.UTF-8
Last metadata expiration check: 0:00:09 ago on Thu Jul 22 15:44:49 2021.
Dependencies resolved.
==============================================================================================================================================================================================================================================
 Package                                               Architecture                            Version                                        Repository                                                                                 Size
==============================================================================================================================================================================================================================================
Installing:
 python3-asyncssh                                      noarch                                  2.7.0-1.el8                                    copr:copr.fedorainfracloud.org:tchaikov:python3-asyncssh                                  476 k
Installing dependencies:
 python3-cffi                                          x86_64                                  1.11.5-5.el8                                   baseos                                                                                    237 k
 python3-cryptography                                  x86_64                                  3.2.1-4.el8                                    baseos                                                                                    559 k
 python3-ply                                           noarch                                  3.9-9.el8                                      baseos                                                                                    111 k
 python3-pycparser                                     noarch                                  2.14-14.el8                                    baseos                                                                                    109 k

Transaction Summary
==============================================================================================================================================================================================================================================
Install  5 Packages

Total download size: 1.5 M
Installed size: 7.2 M
Is this ok [y/N]: n
Operation aborted.

I can see that libnacl is optional and only required for some scenario [1] which I think we're not using. Am I wrong ?

@melissa-kun-li are we using anything specific from [1] here ?

If we need to install this dependency explicitly then this should be done at package level (spec file and debian/control) and we need to update the requirements.txt file with something like asynssh[libnacl]

[1] https://asyncssh.readthedocs.io/en/latest/#optional-extras

tchaikov · 2021-07-22T16:19:21Z

i just created a copr repo at https://copr.fedorainfracloud.org/coprs/tchaikov/python3-asyncssh/, which offers python3-libnacl and python3-asyncssh. python3-libnacl is a build-time and run-time dependency of python3-asyncssh. to install python3-asyncssh, we need to

sudo dnf -y copr enable tchaikov/python3-asyncssh epel-8-$(arch)

sudo dnf -y install python3-asyncssh

to integrate this package into the ceph image, we might need to create a change like ceph/ceph-container#1821.
in the long run, probably a better way to address this dependency is probably to maintain these packages in EPEL8.

Thanks for taking care of this @tchaikov

I'm preparing a PR for ceph-container.

I just have a question about the python3-libnacl dependency. Currently the dependency isn't installed automatically when installing python3-asyncssh. Is this expected ?

yes, it is expected. see https://src.fedoraproject.org/rpms/python-asyncssh/blob/rawhide/f/python-asyncssh.spec#_35 .

it depends on if we want to support the Ed25519 key signing algorithm. although it's a recommended key signing algorithm nowadays, it might not be that popular. as ssh-keygen generates an RSA key by default.

but, if user uses Ed25519 for SSH key signing, python3-asyncssh alone does not work. if we want to be safe, i'd suggest install python3-libnacl as well.

I can see that libnacl is optional and only required for some scenario [1] which I think we're not using. Am I wrong ?

@melissa-kun-li are we using anything specific from [1] here ?

If we need to install this dependency explicitly then this should be done at package level (spec file and debian/control) and we need to update the requirements.txt file with something like asynssh[libnacl]

[1] https://asyncssh.readthedocs.io/en/latest/#optional-extras

dsavineau · 2021-07-22T16:36:30Z

it depends on if we want to support the Ed25519 key signing algorithm. although it's a recommended key signing algorithm nowadays, it might not be that popular. as ssh-keygen generates an RSA key by default.

but, if user uses Ed25519 for SSH key signing, python3-asyncssh alone does not work. if we want to be safe, i'd suggest install python3-libnacl as well.

I might be wrong, but it seems openssl 1.1.1 already support ed25519 keys and libnacl is only required (as per the asyncssh doc) when using openssl prior 1.1.1b. CentOS 8 uses 1.1.1g

https://asyncssh.readthedocs.io/en/stable/api.html#key-exchange-algorithms

tchaikov · 2021-07-22T16:43:47Z

@dsavineau thank you for reading the document! indeed. if we have the luxury of using 1.1.1b and up, seems we are able to have access to the support of all the pub key types. so in our use case, where CentOS8 is the target platform, python3-asyncssh alone would suffice.

melissa-kun-li · 2021-07-22T17:49:11Z

thanks @tchaikov and @dsavineau !

@melissa-kun-li are we using anything specific from [1] here ?

If we need to install this dependency explicitly then this should be done at package level (spec file and debian/control) and we need to update the requirements.txt file with something like asynssh[libnacl]

[1] https://asyncssh.readthedocs.io/en/latest/#optional-extras

i don't think we need anything from the asyncssh optional extras. i think python3-bcrypt is already installed in the ceph container

ceph.spec.in

…Centos8 Disable GSSAPIAuthentication on the RHEL8 and Centos8 testnodes for this PR: ceph/ceph#42051 Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

… asyncssh connection errors Create asyncssh connection object in async `_remote_connection` function and cache in `self.cons` Create a handler for asyncssh log redirection and output ssh log if a connection error occurs Disable asyncssh logger from propagating because the asyncssh info messages are verbose Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

_execute_command will run commands over ssh using the asyncssh `run` method: https://asyncssh.readthedocs.io/en/latest/api.html#asyncssh.SSHClientConnection.run _check_execute_command will check the output of _execute_command and raise OrchestratorError if command fails on the remote host. All commands run over ssh are prepended with sudo in `_execute_command` and shell-escaped with shlex quote. If the cached ssh connection is closed or broken, the connection object will be removed from the cache, added to the `offline_hosts`, and an OrchestratorError will be raised. On the next call, the connection object will attempt to be recreated. Exceptions involving asyncssh methods should be handled otherwise errors like TypeError: __init__() missing 1 required positional argument: 'reason' could occur due to the asyncssh error interacting with `raise_if_exception` Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

…on remote host _write_remote_file uses _check_execute_command in ssh.py which calls _execute_command which uses shlex quote. Thus, any commands with an int will need to be transformed into a str because shlex quote does not take int objects Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

… results of the async functions with get_result The EventLoopThread class starts a thread and an event loop which runs forever. Coroutines are scheduled on the event loop by the `get_result` method which uses `run_coroutine_threadsafe` to return a concurrent.futures.Future, and ultimately the result with .result() Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

…s in ssh.py Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

…ve.py with write_remote_file in ssh.py remove remotes.py because it is specific to execnet/remoto. _write_remote_file in ssh.py now fulfills the function of write_file in remotes.py and the old _write_remote_file in serve.py Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

…k_execute_command in _run_cephadm remove _get_connection from module.py and _remote_connection in serve.py, replacing with _remote_connection in ssh.py. also, replace remoto.process.check with _execute_command and _check_execute_command in ssh.py Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

…binary_conn _deploy_cephadm_binary can be simplified to call write_remote_file in ssh.py, and _deploy_cephadm_binary_conn is no longer needed Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

add `event_loop` and `tkey` object to with_cephadm_module, and create MockEventLoopThread in fixtures.py to test async functions of ssh.py. rewrite test_offline to be compatible with asyncssh Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

remove test_stale_connections because it applies to remoto Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

src/pybind/mgr/cephadm/ssh.py

melissa-kun-li · 2021-08-21T02:01:24Z

https://pulpito.ceph.com/adking-2021-08-20_22:03:22-rados:cephadm-wip-adk-testing-2021-08-20-1531-distro-basic-smithi/

Looks like asyncssh was attempting GSSAPI auth and failing somewhere, causing the two CancelledErrors for GSSAPI keyex and GSSAPI-with-mic : https://gist.github.com/melissa-kun-li/fd62b90c8c15e153bf7d9fed20d69f50. So I disabled it from attempting GSSAPI auth in the asyncssh.connect() call ( preferred_auth=['publickey'] ) and now it looks like tests are passing for Centos 8.3. In this case, users can still have GSSAPIAuthentication yes in their sshd_config or ssh_config and use GSSAPI for other things on their machine, asyncssh / cephadm will just not attempt it. We don't need GSSAPI auth because we're using publickey

review was stale. Thank you Kefu!

ktdreyer · 2021-09-03T15:05:01Z

asyncssh is headed to EPEL 8 in https://bodhi.fedoraproject.org/updates/FEDORA-EPEL-2021-9e9ea1117b

melissa-kun-li requested a review from a team as a code owner June 28, 2021 14:38

github-actions bot added cephadm pybind labels Jun 28, 2021

sebastian-philipp reviewed Jun 29, 2021

View reviewed changes

github-actions bot added mgr build/ops labels Jun 30, 2021

tchaikov self-assigned this Jul 19, 2021

melissa-kun-li force-pushed the asyncssh branch 2 times, most recently from 30e19ce to ca2b23a Compare July 21, 2021 05:11

tchaikov removed their assignment Jul 21, 2021

melissa-kun-li force-pushed the asyncssh branch from ca2b23a to d58529a Compare July 21, 2021 19:36

melissa-kun-li changed the title ~~[WIP] mgr/cephadm: replace execnet and remoto with asyncssh~~ mgr/cephadm: replace execnet and remoto with asyncssh Jul 21, 2021

sebastian-philipp added the needs-review label Jul 22, 2021

sebastian-philipp approved these changes Jul 22, 2021

View reviewed changes

dsavineau mentioned this pull request Jul 22, 2021

[skip ci] centos: add el8 copr repo for python3-asyncssh ceph/ceph-container#1914

Merged

melissa-kun-li force-pushed the asyncssh branch from d58529a to 8ce451d Compare July 22, 2021 18:25

tchaikov added needs-qa wip-kefu-testing labels Jul 24, 2021

tchaikov previously requested changes Jul 25, 2021

View reviewed changes

ceph.spec.in Show resolved Hide resolved

melissa-kun-li mentioned this pull request Aug 18, 2021

roles/testnode/templates/ssh: temporarily set RHEL8 sshd loglevel to debug ceph/ceph-cm-ansible#645

Closed

melissa-kun-li force-pushed the asyncssh branch from 0c1e9e8 to 028d165 Compare August 19, 2021 14:12

melissa-kun-li mentioned this pull request Aug 19, 2021

roles/testnode/templates/ssh: disable GSSAPIAuthentication for RHEL8 and Centos8 ceph/ceph-cm-ansible#646

Closed

melissa-kun-li added 14 commits August 20, 2021 14:27

mgr/cephadm: move _reconfig_ssh, _reset_con, and _reset_cons to ssh.py

4762492

Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

mgr/cephadm: make synchronous wrapper functions of the async function…

2873aee

…s in ssh.py Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

mgr/cephadm: remove _executable_path from module.py

b55eb2a

Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

mgr/cephadm: remove test_stale_connections

368c60d

remove test_stale_connections because it applies to remoto Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

mgr/cephadm: rewrite test_etc_ceph to apply to asyncssh

696dfbc

Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

mgr/cephadm: replace instances of remoto/execnet with asyncssh

791d8be

Fixes: https://tracker.ceph.com/issues/44676 Signed-off-by: Melissa Li <li.melissa.kun@gmail.com>

melissa-kun-li force-pushed the asyncssh branch from 028d165 to 791d8be Compare August 20, 2021 18:30

melissa-kun-li added needs-qa needs-review and removed needs-review wip-swagner3-testing labels Aug 20, 2021

adk3798 added the wip-adk-testing label Aug 20, 2021

tchaikov reviewed Aug 21, 2021

View reviewed changes

src/pybind/mgr/cephadm/ssh.py Show resolved Hide resolved

sebastian-philipp approved these changes Aug 23, 2021

View reviewed changes

sebastian-philipp merged commit fb7cb4b into ceph:master Aug 23, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mgr/cephadm: replace execnet and remoto with asyncssh #42051

mgr/cephadm: replace execnet and remoto with asyncssh #42051

melissa-kun-li commented Jun 28, 2021 •

edited

sebastian-philipp commented Jul 1, 2021

dsavineau commented Jul 15, 2021

tchaikov commented Jul 20, 2021

melissa-kun-li commented Jul 21, 2021

mgfritch commented Jul 21, 2021

melissa-kun-li commented Jul 21, 2021

dsavineau commented Jul 22, 2021 •

edited

tchaikov commented Jul 22, 2021

dsavineau commented Jul 22, 2021

tchaikov commented Jul 22, 2021

melissa-kun-li commented Jul 22, 2021

melissa-kun-li commented Aug 21, 2021

ktdreyer commented Sep 3, 2021

mgr/cephadm: replace execnet and remoto with asyncssh #42051

mgr/cephadm: replace execnet and remoto with asyncssh #42051

Conversation

melissa-kun-li commented Jun 28, 2021 • edited

Checklist

sebastian-philipp commented Jul 1, 2021

dsavineau commented Jul 15, 2021

tchaikov commented Jul 20, 2021

melissa-kun-li commented Jul 21, 2021

mgfritch commented Jul 21, 2021

melissa-kun-li commented Jul 21, 2021

dsavineau commented Jul 22, 2021 • edited

tchaikov commented Jul 22, 2021

dsavineau commented Jul 22, 2021

tchaikov commented Jul 22, 2021

melissa-kun-li commented Jul 22, 2021

melissa-kun-li commented Aug 21, 2021

ktdreyer commented Sep 3, 2021

melissa-kun-li commented Jun 28, 2021 •

edited

dsavineau commented Jul 22, 2021 •

edited