Skip to content

Conversation

@psachdeva-ms
Copy link

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.

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]>
@psachdeva-ms
Copy link
Author

@microsoft-github-policy-service agree company="Microsoft"

Copy link
Contributor

Copilot AI left a 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 latest
  • redhat rhel 9_5 latest
  • debian debian-12 12 latest

@psachdeva-ms psachdeva-ms force-pushed the file_transfer_transformer branch 2 times, most recently from 7889948 to 5083939 Compare January 23, 2026 08:46
Copy link
Contributor

Copilot AI left a 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.

@LiliDeng
Copy link
Collaborator

@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]>
@psachdeva-ms psachdeva-ms force-pushed the file_transfer_transformer branch from b40cd66 to b9ab293 Compare January 29, 2026 14:54
@LiliDeng
Copy link
Collaborator

@psachdeva-ms CI check error

./lisa/tools/remote_copy.py:9:1: I001 isort found an import in the wrong position
./lisa/transformers/file_uploader.py:11:1: F401 'lisa.node.Node' imported but unused
./lisa/transformers/file_uploader.py:77:35: BLK100 Black would make changes.
nox > Command flake8  failed with exit code 1

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]>
from lisa.executable import Tool
from lisa.tools.chown import Chown
from lisa.tools.cp import Cp
from lisa.tools.chmod import Chmod
Copy link
Collaborator

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

Copy link
Author

Choose a reason for hiding this comment

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

Fixed :)

Copy link
Collaborator

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.

Copy link
Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants