-
Notifications
You must be signed in to change notification settings - Fork 224
File transfer transformer #4235
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
base: main
Are you sure you want to change the base?
File transfer transformer #4235
Conversation
Add a validate method to check if the given files exist on the the local node. Along with that enable the user to specify `files: ["*"]` in a runbook to upload all the files from the source directory. Signed-off-by: Piyush Sachdeva <[email protected]> Signed-off-by: Piyush Sachdeva <[email protected]>
Instead of checking the existence of the target directory on the destination directly from _internal_run method, do it in a new method: _check_dest_dir, and call this from _internal_run. Signed-off-by: Piyush Sachdeva <[email protected]> Signed-off-by: Piyush Sachdeva <[email protected]>
|
@microsoft-github-policy-service agree company="Microsoft" |
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.
Pull request overview
This PR introduces a new FileTransferTransformer that enables file copying between remote nodes using SCP, with automatic fallback to local download/upload if direct SCP fails. The PR also refactors the existing FileUploaderTransformer to share common validation and directory-checking logic.
Changes:
- Refactored FileUploaderTransformer to extract reusable methods (_check_dest_dir, _validate)
- Added FileTransferTransformer for remote-to-remote file transfers with SCP support
- Implemented copy_between_remotes method in RemoteCopy tool for direct node-to-node copying
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 17 comments.
| File | Description |
|---|---|
| lisa/transformers/file_uploader.py | Refactored existing FileUploaderTransformer and added new FileTransferTransformer class that extends it to support remote-to-remote file transfers with SCP fallback mechanism |
| lisa/tools/remote_copy.py | Added copy_between_remotes method to enable direct SCP-based file transfers between remote nodes, with SSH key management |
Test Suggestions (Required per LISA Guidelines):
Key Test Cases:
Since this PR introduces new transformer functionality for file transfers, the following integration tests should be run to validate the changes:
- Tests that verify basic file transfer operations between nodes
- Tests that validate SCP functionality and fallback mechanisms
- Tests that check transformer dependency resolution and execution order
Impacted LISA Features:
The changes primarily impact file transfer capabilities and transformer infrastructure, so no specific LISA feature classes are directly affected by this core functionality change.
Tested Azure Marketplace Images:
To validate the file transfer functionality across different operating systems and architectures, test with a minimal representative set:
canonical ubuntu-24_04-lts server latestredhat rhel 9_5 latestdebian debian-12 12 latest
7889948 to
5083939
Compare
5083939 to
b40cd66
Compare
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.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 17 comments.
|
@psachdeva-ms please fix the CI check error |
`copy_between_remotes` method does scp of a file[s] (recursively) from source node to destination node. The scp command is executed on the source node. If the destination node is an azure vm, make sure to add the IP address of the source node in the list of `source_address_prefixes` of the destination node for the command to succeed. Signed-off-by: Piyush Sachdeva <[email protected]> Signed-off-by: Piyush Sachdeva <[email protected]>
b40cd66 to
b9ab293
Compare
|
@psachdeva-ms CI check error |
The new file_transfer transformer uses `copy_between_remotes` method to scp file[s] from source to destination node. Its validate method verifies the existence of files on the source node and also has the functionality to transfer all files from source directory on source node using `files: ["*"]`. In case the `copy_between_remotes` fails, there is also a provision to first save files from source node to local, and then upload from the local node to destination. For this to work, the user should set the `local_path` variable. Signed-off-by: Piyush Sachdeva <[email protected]> Signed-off-by: Piyush Sachdeva <[email protected]>
b9ab293 to
15442ca
Compare
| from lisa.executable import Tool | ||
| from lisa.tools.chown import Chown | ||
| from lisa.tools.cp import Cp | ||
| from lisa.tools.chmod import Chmod |
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.
from lisa.tools.chmod import Chmod
from lisa.tools.chown import Chown
from lisa.tools.cp import Cp
from lisa.tools.ls import Ls
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.
Fixed :)
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.
./lisa/transformers/file_uploader.py:11:1: F401 'lisa.node.RemoteNode' imported but unused, remove this. I didn't see you push the changes.
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.
I kept that thinking if an object of a Type is used, it's good to import the type as well.
But will drop it.
This PR adds a new transformer: FileTransfer Transformer which is capable scp-ing files from one source environment vm to a target. Along with that there are minor changes to existing code for modularization and the implementation of core scp logic.