-
Notifications
You must be signed in to change notification settings - Fork 3
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.
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
[Testing] Test SSH file transfer with images. #208
Open
JoeZiminski
wants to merge
48
commits into
main
Choose a base branch
from
test_ssh_with_image
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
48 commits
Select commit
Hold shift + click to select a range
b5d54ed
lots of changes, sort out.
JoeZiminski 81bfb10
still working, needs a bit more tidying up.
JoeZiminski 30e7e6f
Continue working.
JoeZiminski 8b08531
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 3bf4a1b
Tidy ups.
JoeZiminski 93573c3
Fix linting.
JoeZiminski 83d05d9
Tidy ups and documentation.
JoeZiminski 581a68b
Add documentation.
JoeZiminski 5cf71a6
Try different Dockerfile for linux.
JoeZiminski 694db51
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] 4824ba6
Fix issue after rebase.
JoeZiminski f9a53c1
Remove breakpoint.
JoeZiminski 64bb9cc
Don't use singularity, docker or nothing.
JoeZiminski 26a8d11
Rework ssh setup.
JoeZiminski 86cad6c
Fix connection refused issue.
JoeZiminski d0494b3
Use run not POpen for actions.
JoeZiminski d48aa38
try in detachted state.
JoeZiminski 203b1d9
Add error messages on docker setup.
JoeZiminski b4bb0a4
run linting.
JoeZiminski 9aeed97
update connection failed error message.
JoeZiminski 1e7b9f8
Test version working locally on linux but only can connect with sudo.
JoeZiminski 200c360
Try freeing up and using the free port.
JoeZiminski de9f5c8
Test sudo service only on linux.
JoeZiminski 02125f5
Add sudo to docker setup commands.
JoeZiminski 5e00442
test with auto add policy.
JoeZiminski 89d40a8
Really restrict to the connect call.
JoeZiminski b28937c
restrict to tests of interest temporarily.
JoeZiminski 3e09136
Fix to port 3306 in tests and for paramiko.
JoeZiminski acbc58b
Extend port to rclone.
JoeZiminski c901052
Use environment variable to set port.
JoeZiminski 689db1d
Add all OS back.
JoeZiminski 085ef63
try remove tag for windows.
JoeZiminski e402c98
Update docker commands for windows.
JoeZiminski 77dfa09
Fix nonsense docker build command.
JoeZiminski 8f4d341
Only run when docker running and on ubuntu on runners.
JoeZiminski 75d3f43
Try build and run docker only once per session.
JoeZiminski ddb81d5
Teardown image at end of ssh tests, factor out ssh tests.
JoeZiminski 7692819
SPlit ssh tests.
JoeZiminski 1216e01
Add sudo to the docker teardown commands for Linux.
JoeZiminski 1b9d105
try class scope of setup ssh container.
JoeZiminski e86aa27
Try move ssh fixture to classes.
JoeZiminski 995175d
Try a different command to shutdown on linux.
JoeZiminski 566e88c
Extend to macOS.
JoeZiminski 2abe5fa
Tidy ups and some docs.
JoeZiminski f9eab00
Merge branch 'test_ssh_with_image' of github.com:neuroinformatics-uni…
JoeZiminski 8da74aa
Finish tidying up docstrings.
JoeZiminski b26163b
Change ssh test image name and fix docstring.
JoeZiminski 50ae318
Small tidy ups.
JoeZiminski File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
# Use a base image with the desired OS (e.g., Ubuntu, Debian, etc.) | ||
FROM ubuntu:latest | ||
|
||
# Install SSH server | ||
RUN apt-get update && \ | ||
apt-get upgrade -y | ||
RUN apt-get install openssh-server -y supervisor | ||
RUN apt-get install nano | ||
|
||
# Create an SSH user | ||
RUN useradd -rm -d /home/sshuser -s /bin/bash -g root -G sudo -u 1000 sshuser | ||
|
||
# Set the SSH user's password (replace "password" with your desired password) | ||
RUN echo "sshuser:password" | chpasswd | ||
|
||
# Allow SSH access | ||
RUN mkdir /var/run/sshd | ||
|
||
RUN /usr/bin/ssh-keygen -A | ||
|
||
# Expose the SSH port | ||
EXPOSE 22 | ||
|
||
# Start SSH server on container startup | ||
CMD ["/usr/sbin/sshd", "-D"] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,55 +1,206 @@ | ||
import builtins | ||
import copy | ||
import os | ||
import platform | ||
import stat | ||
import subprocess | ||
import sys | ||
import warnings | ||
from pathlib import Path | ||
|
||
import paramiko | ||
|
||
from datashuttle.utils import rclone, ssh | ||
|
||
# Choose port 3306 for running on GH actions | ||
# suggested in https://github.com/orgs/community/discussions/25550 | ||
PORT = 3306 | ||
os.environ["DS_SSH_PORT"] = str(PORT) | ||
|
||
|
||
def setup_project_for_ssh( | ||
project, central_path, central_host_id, central_host_username | ||
project, | ||
): | ||
""" | ||
Set up the project configs to use SSH connection | ||
to central | ||
Set up the project configs to use | ||
SSH connection to central. The settings | ||
set up a connection to the Dockerfile image | ||
found in /ssh_test_images. | ||
""" | ||
project.update_config_file( | ||
central_path=central_path, | ||
connection_method="ssh", | ||
central_path=f"/home/sshuser/datashuttle/{project.project_name}", | ||
central_host_id="localhost", | ||
central_host_username="sshuser", | ||
) | ||
project.update_config_file(central_host_id=central_host_id) | ||
project.update_config_file(central_host_username=central_host_username) | ||
project.update_config_file(connection_method="ssh") | ||
|
||
rclone.setup_rclone_config_for_ssh( | ||
project.cfg, | ||
project.cfg.get_rclone_config_name("ssh"), | ||
project.cfg.ssh_key_path, | ||
) | ||
|
||
|
||
def setup_mock_input(input_): | ||
def setup_ssh_connection(project, setup_ssh_key_pair=True): | ||
""" | ||
This is very similar to pytest monkeypatch but | ||
using that was giving me very strange output, | ||
monkeypatch.setattr('builtins.input', lambda _: "n") | ||
i.e. pdb went deep into some unrelated code stack | ||
Convenience function to verify the server hostkey and ssh | ||
key pairs to the Dockerfile image for ssh tests. | ||
|
||
This requires monkeypatching a number of functions involved | ||
in the SSH setup process. `input()` is patched to always | ||
return the required hostkey confirmation "y". `getpass()` is | ||
patched to always return the password for the container in which | ||
SSH tests are run. `isatty()` is patched because when running this | ||
for some reason it appears to be in a TTY - this might be a | ||
container thing. | ||
""" | ||
# Monkeypatch | ||
orig_builtin = copy.deepcopy(builtins.input) | ||
builtins.input = lambda _: input_ # type: ignore | ||
return orig_builtin | ||
builtins.input = lambda _: "y" # type: ignore | ||
|
||
orig_getpass = copy.deepcopy(ssh.getpass.getpass) | ||
ssh.getpass.getpass = lambda _: "password" # type: ignore | ||
|
||
orig_isatty = copy.deepcopy(sys.stdin.isatty) | ||
sys.stdin.isatty = lambda: True | ||
|
||
# Run setup | ||
verified = ssh.verify_ssh_central_host( | ||
project.cfg["central_host_id"], project.cfg.hostkeys_path, log=True | ||
) | ||
|
||
if setup_ssh_key_pair: | ||
ssh.setup_ssh_key(project.cfg, log=False) | ||
|
||
# Restore functions | ||
builtins.input = orig_builtin | ||
ssh.getpass.getpass = orig_getpass | ||
sys.stdin.isatty = orig_isatty | ||
|
||
return verified | ||
|
||
|
||
def restore_mock_input(orig_builtin): | ||
def setup_ssh_container(container_name): | ||
""" | ||
orig_builtin: the copied, original builtins.input | ||
Build and run the docker container used for | ||
ssh tests. | ||
""" | ||
builtins.input = orig_builtin | ||
assert docker_is_running(), ( | ||
"docker is not running, " | ||
"this should be checked at the top of test script" | ||
) | ||
|
||
image_path = Path(__file__).parent / "ssh_test_images" | ||
os.chdir(image_path) | ||
|
||
if platform.system() != "Windows": | ||
build_command = "sudo docker build -t ssh_server ." | ||
run_command = ( | ||
f"sudo docker run -d -p {PORT}:22 " | ||
f"--name {container_name} ssh_server" | ||
) | ||
else: | ||
build_command = "docker build ." | ||
run_command = ( | ||
f"docker run -d -p {PORT}:22 --name {container_name} ssh_server" | ||
) | ||
|
||
build_output = subprocess.run( | ||
build_command, | ||
shell=True, | ||
capture_output=True, | ||
) | ||
assert build_output.returncode == 0, ( | ||
f"docker build failed with: STDOUT-{build_output.stdout} " | ||
f"STDERR-{build_output.stderr}" | ||
) | ||
|
||
run_output = subprocess.run( | ||
run_command, | ||
shell=True, | ||
capture_output=True, | ||
) | ||
|
||
assert run_output.returncode == 0, ( | ||
f"docker run failed with: STDOUT-{run_output.stdout} " | ||
f"STDERR-{run_output.stderr}" | ||
) | ||
|
||
|
||
def setup_hostkeys(project): | ||
def recursive_search_central(project): | ||
""" | ||
Convenience function to verify the server hostkey. | ||
A convenience function to recursively search a | ||
project for files through SSH, used during testing | ||
across an SSH connection to collected names of | ||
files that were transferred. | ||
""" | ||
orig_builtin = setup_mock_input(input_="y") | ||
ssh.verify_ssh_central_host( | ||
project.cfg["central_host_id"], project.cfg.hostkeys_path, log=True | ||
with paramiko.SSHClient() as client: | ||
ssh.connect_client_core(client, project.cfg) | ||
|
||
sftp = client.open_sftp() | ||
|
||
all_filenames = [] | ||
|
||
sftp_recursive_file_search( | ||
sftp, | ||
(project.cfg["central_path"] / "rawdata").as_posix(), | ||
all_filenames, | ||
) | ||
return all_filenames | ||
|
||
|
||
def sftp_recursive_file_search(sftp, path_, all_filenames): | ||
""" | ||
Append all filenames found within a folder, | ||
when searching over a sftp connection. | ||
""" | ||
try: | ||
sftp.stat(path_) | ||
except FileNotFoundError: | ||
return | ||
|
||
for file_or_folder in sftp.listdir_attr(path_): | ||
if stat.S_ISDIR(file_or_folder.st_mode): | ||
sftp_recursive_file_search( | ||
sftp, | ||
path_ + "/" + file_or_folder.filename, | ||
all_filenames, | ||
) | ||
else: | ||
all_filenames.append(path_ + "/" + file_or_folder.filename) | ||
|
||
|
||
def get_test_ssh(): | ||
""" | ||
Return bool indicating whether Docker is installed and running, | ||
which is required for ssh tests. | ||
""" | ||
docker_installed = docker_is_running() | ||
if not docker_installed: | ||
warnings.warn( | ||
"SSH tests are not run as docker either not installed or running." | ||
) | ||
return docker_installed | ||
|
||
|
||
def docker_is_running(): | ||
if not is_docker_installed(): | ||
return False | ||
|
||
is_running = check_sys_command_returns_0("docker stats --no-stream") | ||
return is_running | ||
|
||
|
||
def is_docker_installed(): | ||
return check_sys_command_returns_0("docker -v") | ||
|
||
|
||
def check_sys_command_returns_0(command): | ||
return ( | ||
subprocess.run( | ||
command, | ||
shell=True, | ||
stdout=subprocess.DEVNULL, | ||
stderr=subprocess.DEVNULL, | ||
).returncode | ||
== 0 | ||
) | ||
restore_mock_input(orig_builtin) |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this mean that on other OSes pytest will run all tests except the ones specified here? That's what we'd want/expect right?