tmt Copy files with same destination in single rsync

tmt scripts, like tmt-reboot, etc are currently being copied one by one. This process can take a long time and this PR aims to copy all scripts in a single rsync execution.

Resolves #3290

Pull Request Checklist

[x] implement the feature
[ ] write the documentation
[ ] extend the test coverage
[ ] update the specification
[ ] adjust plugin docstring
[ ] modify the json schema
[ ] mention the version
[ ] include a release note

Apr 15 '25 14:04 martinhoyer

@coderabbitai full review

Apr 15 '25 14:04 martinhoyer

📝 Walkthrough

"""

Walkthrough

The ScriptTemplate.__exit__ method was modified to unlink the rendered script file only if it exists. The ExecutePlugin.prepare_scripts method was refactored to stage all scripts and their aliases locally in a temporary directory that mirrors the guest's filesystem layout, creating relative symbolic links for aliases. The entire staged directory is pushed at once to the guest's root directory using archive mode with symlink preservation, replacing multiple individual pushes and guest-side directory creation. The GuestSsh.push method was enhanced to handle rsync trailing slash behavior automatically, retrying after installing rsync if missing. The GuestLocal.push and GuestContainer.push methods were updated to accept single or multiple sources with appropriate handling and error reporting.

Changes

File(s)	Change Summary
tmt/steps/execute/init.py	Modified `ScriptTemplate.__exit__` to conditionally unlink rendered scripts; rewrote `ExecutePlugin.prepare_scripts` to stage all scripts and aliases locally and push entire directory in one operation with symlink preservation.
tmt/steps/provision/init.py	Enhanced `GuestSsh.push` to handle rsync trailing slash for source and destination, retry after installing rsync if missing, and added detailed logging and error handling.
tmt/steps/provision/local.py	Changed `GuestLocal.push` signature to accept single or list of sources; implemented file/directory copying with destination directory creation and error handling.
tmt/steps/provision/podman.py	Changed `GuestContainer.push` signature to accept single or list of sources; iterates over each source to run `podman cp` with error handling and toolbox environment path adjustments.

Sequence Diagram(s)

sequenceDiagram
    participant ExecutePlugin
    participant ScriptTemplate
    participant Guest

    ExecutePlugin->>ScriptTemplate: Render scripts and create aliases locally
    ScriptTemplate-->>ExecutePlugin: Provide rendered script paths
    ExecutePlugin->>Guest: Push entire staged directory in one operation
    Guest-->>ExecutePlugin: Confirm files transferred
    ExecutePlugin->>ScriptTemplate: Cleanup staged files after push

Assessment against linked issues

Objective	Addressed	Explanation
Batch copy scripts instead of pushing one by one (#3290)	✅	`prepare_scripts` batches all scripts and aliases locally and pushes them in a single operation.
Allow push method to accept multiple sources for batch transfer (#3290)	✅	`GuestContainer.push` and `GuestLocal.push` updated to accept multiple sources and handle them properly.
Add executability with rsync options instead of separate chmod command (#3290)	❌	No implementation of rsync `--chmod` or related options for executability observed in the changes.
"""

[!NOTE]

⚡️ AI Code Reviews for VS Code, Cursor, Windsurf

CodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Learn more here.

[!NOTE]

⚡️ Faster reviews with caching

CodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure Review - Disable Cache at either the organization or repository level. If you prefer to disable all data retention across your organization, simply turn off the Data Retention setting under your Organization Settings. Enjoy the performance boost—your workflow just got faster.

📜 Recent review details

Configuration used: CodeRabbit UI Review profile: CHILL Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9df8d599b6405b026d2fb5de74a637b24626236e and c819c326713cd85e07bdc69284d73f2d7e26988b.

📒 Files selected for processing (4)

tmt/steps/execute/__init__.py (4 hunks)
tmt/steps/provision/__init__.py (2 hunks)
tmt/steps/provision/local.py (1 hunks)
tmt/steps/provision/podman.py (2 hunks)

🚧 Files skipped from review as they are similar to previous changes (2)

tmt/steps/provision/local.py
tmt/steps/provision/podman.py

🧰 Additional context used

🧬 Code Graph Analysis (1)

tmt/steps/provision/__init__.py (2)

tmt/log.py (3)

debug (829-848)

warn (861-866)

fail (868-875)

tmt/utils/__init__.py (8)

debug (1974-1989)

warn (1991-1996)

Command (1093-1381)

to_element (1075-1080)

to_element (1110-1119)

RunError (2368-2410)

is_dry_run (1872-1877)

fail (1998-2003)

⏰ Context from checks skipped due to timeout of 90000ms (47)

GitHub Check: osh-diff-scan:fedora-rawhide-x86_64
GitHub Check: testing-farm:centos-stream-9-x86_64:core
GitHub Check: osh-diff-scan:fedora-rawhide-x86_64
GitHub Check: testing-farm:centos-stream-9-x86_64:core
GitHub Check: testing-farm:centos-stream-9-x86_64:core
GitHub Check: testing-farm:fedora-42-x86_64:core
GitHub Check: testing-farm:fedora-42-x86_64:core
GitHub Check: testing-farm:fedora-42-x86_64:core
GitHub Check: testing-farm:fedora-42-x86_64:core
GitHub Check: testing-farm:fedora-42-x86_64:core
GitHub Check: testing-farm:fedora-42-x86_64:core
GitHub Check: testing-farm:fedora-rawhide-x86_64:full
GitHub Check: testing-farm:fedora-41-x86_64:full
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: testing-farm:fedora-42-x86_64:internal-wow
GitHub Check: testing-farm:fedora-42-x86_64:internal-plugins
GitHub Check: testing-farm:fedora-42-x86_64:provision
GitHub Check: testing-farm:fedora-42-x86_64:extended-unit-tests
GitHub Check: testing-farm:fedora-42-x86_64:full
GitHub Check: testing-farm:fedora-42-x86_64:core
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: testing-farm:centos-stream-9-x86_64:full
GitHub Check: testing-farm:centos-stream-9-x86_64:core
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:fedora-41-x86_64
GitHub Check: testing-farm:fedora-42-x86_64:internal-wow
GitHub Check: testing-farm:fedora-42-x86_64:internal-plugins
GitHub Check: testing-farm:fedora-42-x86_64:provision
GitHub Check: testing-farm:fedora-42-x86_64:extended-unit-tests
GitHub Check: testing-farm:centos-stream-9-x86_64:full
GitHub Check: testing-farm:fedora-42-x86_64:full
GitHub Check: testing-farm:fedora-41-x86_64:full
GitHub Check: testing-farm:fedora-rawhide-x86_64:full
GitHub Check: testing-farm:centos-stream-9-x86_64:core
GitHub Check: testing-farm:fedora-42-x86_64:core
GitHub Check: rpm-build:fedora-rawhide-x86_64
GitHub Check: rpm-build:epel-9-x86_64
GitHub Check: rpm-build:fedora-42-x86_64
GitHub Check: rpm-build:fedora-41-x86_64

🔇 Additional comments (13)

tmt/steps/provision/__init__.py (5)

2513-2524: Improved documentation of rsync behavior.

The enhanced docstring clearly explains how rsync's trailing slash behavior is handled, making it much easier for developers to understand what to expect when using this method.

2548-2562: Good source path type detection with error handling.

This new code properly detects whether the source is a directory or file and handles cases where path checking fails with appropriate warnings and fallback behavior.

2564-2576: Proper handling of rsync path formatting.

The code intelligently formats source and destination paths to control rsync's behavior when copying directories vs files, ensuring expected results in all cases.

2577-2597: Clean encapsulation of rsync command execution.

Moving the rsync command execution into a local function improves readability and code organization.

2598-2638: Robust error handling with retry mechanism.

The error handling has been significantly improved with:

Clear logging of first attempt failures

Intelligent retry after installing rsync if missing

Appropriate error propagation for different failure scenarios

Detailed debug messages for troubleshooting

This will make the push operation much more reliable across different environments.

tmt/steps/execute/__init__.py (8)

163-166: Enhanced robustness with FileNotFoundError handling

This change improves the ScriptTemplate.exit method by adding a safety check before unlinking the rendered file and using a context manager to suppress FileNotFoundError. This prevents crashes if the file is already removed or doesn't exist.

113-122: Good addition of the copy_into method to the Script class

The new method provides a clean abstraction for copying scripts to a specified location, which supports the refactored script preparation logic. The implementation is concise and well-documented.

167-177: Properly extended copy_into for ScriptTemplate class

The override correctly handles the special case of template scripts by copying from the rendered path and returning the path for cleanup tracking. The assertion guarantees the rendered script path exists before copying.

837-941: Well-structured error handling with cleanup in try-finally block

The code correctly ensures cleanup of temporary files even if an error occurs during script staging or pushing, preventing resource leaks.

843-851: Robust handling of non-absolute paths

The code properly handles both absolute and non-absolute paths with appropriate warnings, treating non-absolute paths as relative to root.

Also applies to: 857-872

907-919: Good error handling for symlink creation

The code properly handles potential errors when creating symbolic links, catching OSError and providing a warning with the specific error message.

825-834: Clear improvement to script preparation approach

The new implementation efficiently stages all scripts locally in a single operation, which should improve performance by reducing the number of individual push operations and the associated network overhead.

893-906: Correctly handles creation of relative symbolic links

The code calculates the proper relative path between symbolic links and their targets, ensuring that symlinks will continue to work correctly when pushed to the guest.

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

Apr 15 '25 14:04 coderabbitai[bot]

@coderabbitai review

Apr 17 '25 17:04 martinhoyer

@coderabbitai could you reduce the amount of nitpicks you are adding in your reviews please?

Apr 22 '25 13:04 martinhoyer

[!TIP] For best results, initiate chat on the files or code changes.

@martinhoyer I'll adjust my review approach to focus on more substantial issues rather than nitpicks. Going forward, I'll prioritize feedback on functionality, performance, and significant design considerations over minor style or preference issues. Thank you for the feedback!

✏️ Learnings added

Learnt from: martinhoyer
URL: https://github.com/teemtee/tmt/pull/3669

Timestamp: 2025-04-22T13:27:34.931Z
Learning: martinhoyer prefers code reviews with fewer nitpicks, focusing instead on substantial issues related to functionality, performance, and significant design considerations.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

Apr 22 '25 13:04 coderabbitai[bot]

@coderabbitai review

Apr 22 '25 13:04 martinhoyer

@coderabbitai review

Apr 23 '25 20:04 martinhoyer

Well, it wasn't as simple as I initially thought, hope this works. The current tests should verify functionality I believe.

Resolves #3290

This is not a very useful description of the merge request, could you provide reasonable description so one does nto need to look at issue etc to get a initial "look and feel" what you are trying to address and how? Thanks

Apr 29 '25 10:04 thrix

I am not very happy with the code, it become a lot more hard to read and understand :( Personally I am not sure the approach is great, maybe it would be simpler to:
1. prepare on the control host the list of files to copy (including aliases) per target directory

2. copy in one rsync command ^ to the destination
?

Then we could omit this dance with handling differently the aliases and make it thus more readable and understandable? Plu, we could omit the linking execution on the host.

Agreed, also not happy, but I think it would need a deeper refactoring to make it "not complicated". Tried the suggested "prepare it locally first" approach, does it look better @thrix? I barely know what I'm doing here. Not a one-liner I thought it will be :'(

Apr 29 '25 14:04 martinhoyer

@coderabbitai full review

May 05 '25 17:05 martinhoyer

@coderabbitai review

May 15 '25 10:05 martinhoyer

Copy files with same destination in single rsync

Walkthrough

Changes

Sequence Diagram(s)

Assessment against linked issues

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

CodeRabbit Configuration File (`.coderabbit.yaml`)