Do not copy when running ephemeral runners #23
Conversation
When not running with an environment file, only pass the ephemeral option if it was set. This is a bugfix.
When the runner is known to be ephemeral, do not copy the content of the runner unpacked distribution to the work directory. Instead, run from there directly and set the RUNNER_BINROOT variable to point to the installation directory. Ensure that the content of the variable is used for deciding where to run the runner from and fix directory tree ownership prior to starting the runner. Rename the label to krun, instead of krunvm. This is to highlight the underlying technology used, rather than how the runners are created. This is a breaking change, but it is not expected to be used in production yet.
WalkthroughThe updates focus on enhancing the flexibility and configurability of a runner script and its entrypoint. Key improvements include the introduction of conditional Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 1
Configuration used: CodeRabbit UI
Files selected for processing (2)
- runner.sh (2 hunks)
- runner/entrypoint.sh (5 hunks)
Additional comments (3)
runner.sh (1)
- 264-266: The conditional addition of the
-eflag based onRUNNER_EPHEMERALis correctly implemented. Ensure to verify the integration withrun_krunvmand the actual behavior of ephemeral runners in practice.runner/entrypoint.sh (2)
- 103-105: The introduction of
RUNNER_DEFAULT_LABELSenhances the configurability of default runner labels, making the script more flexible. This is a positive change.- 409-416: Adjusting
RUNNER_PREFIXandRUNNER_LABELSbased onRUNNER_DISTROis a thoughtful improvement that enhances the runner's adaptability to different environments. This change increases operational efficiency and compatibility.
| if is_true "$RUNNER_EPHEMERAL"; then | ||
| debug "Ephemeral runner: no copy made, running from $RUNNER_INSTDIR" | ||
| RUNNER_BINROOT="$RUNNER_INSTDIR" | ||
| else | ||
| if ! [ -d "${RUNNER_WORKDIR%/}" ]; then | ||
| mkdir -p "${RUNNER_WORKDIR%/}" | ||
| verbose "Created runner directory ${RUNNER_WORKDIR%/}" | ||
| fi | ||
| RUNNER_BINROOT="${RUNNER_WORKDIR%/}/runner" | ||
| verbose "Copying runner installation to $RUNNER_BINROOT" | ||
| cp -rf "$RUNNER_INSTDIR" "$RUNNER_BINROOT" 2>/dev/null |
There was a problem hiding this comment.
The modifications in runner_install to handle ephemeral runners by avoiding unnecessary copying are well-implemented and align with the objective to optimize the setup process. Consider adding comments to clarify the behavior for future maintainers.
Would you like me to add explanatory comments to this section for clarity?
When running ephemeral runners, the default, do not copy the unpacked installation to the worker directory. Instead, run directly from there, after having changed ownership of all files recursively. Avoiding the copy speeds up the creation of new runners.
Summary by CodeRabbit
RUNNER_EPHEMERALflag.RUNNER_DEFAULT_LABELS.RUNNER_PREFIXandRUNNER_LABELSbased on the operating system distribution.