isolated loops #20
isolated loops #20
Conversation
Rename the entrypoint script to `entrypoint.sh` to better reflect its purpose and to avoid confusion with the `runner.sh` script in the root directory. Rewrite the various READMEs and documentation files to reflect the change. Describe the additional level of directory isolation in the documentation.
WalkthroughThe recent updates focus on enhancing environment isolation and communication processes in an orchestration setup, specifically targeting the runner loops' operation. These improvements include creating isolated environments for each runner, refining the handshaking process for runner termination, and updating naming conventions for better consistency. Additionally, the documentation and scripts have been updated to clarify options separation, improve compatibility with container-based solutions, and adjust logging levels for better debugging. 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: 5
Configuration used: CodeRabbit UI
Files selected for processing (5)
- CONTRIBUTING.md (2 hunks)
- README.md (5 hunks)
- lib/common.sh (1 hunks)
- orchestrator.sh (3 hunks)
- runner.sh (1 hunks)
Additional comments: 6
lib/common.sh (1)
- 206-206: The change in log level from "verbose" to "debug" in the
sublogfunction aligns with the intention to provide more granular control over logging. Ensure that this change is reflected in the documentation or any logging configuration guidelines.orchestrator.sh (2)
- 143-153: The logic to create a separate environment for each runner loop enhances isolation and security. This change is well-implemented and follows best practices for handling environment variables and directory creation.
- 170-172: The use of
wait_pathto synchronize the start of the next runner loop based on the presence of a token file is a good practice for ensuring orderly execution. This approach improves the robustness of the runner orchestration.README.md (2)
- 42-49: The clarification on the separation of options between the orchestrator and runner scripts is helpful for users. It improves the documentation's clarity and helps prevent confusion when configuring the system.
- 71-75: The enhancements for compatibility with traditional container-based solutions, including the relaying of container daemon logs for better debugging, are valuable additions. These changes make the system more versatile and user-friendly.
runner.sh (1)
- 198-201: The update to use
entrypoint.shinstead ofrunner.shfor theRUNNER_ENTRYPOINTpath based on theRUNNER_DIRstatus is a clear improvement. It aligns with the PR's objective to rename the script for better clarity. Ensure that all references and documentation are updated to reflect this change.
| at `/_environment`. | ||
| common (temporary) directory created by the orchestrator and stored in the | ||
| variable `ORCHESTRATOR_ENVIRONMENT`. Each runner loop will be associated to a | ||
| separate sub-directory (the `RUNNER_ENVIRONMENT` variable) and that directory is |
There was a problem hiding this comment.
The phrase "associated to" is typically used in British English. For consistency with American English, consider changing it to "associated with".
- Each runner loop will be associated to a separate sub-directory
+ Each runner loop will be associated with a separate sub-directoryCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| separate sub-directory (the `RUNNER_ENVIRONMENT` variable) and that directory is | |
| Each runner loop will be associated with a separate sub-directory (the `RUNNER_ENVIRONMENT` variable) and that directory is |
There was a problem hiding this comment.
The entire file is written in British English!
There was a problem hiding this comment.
Given your clarification, it makes sense to maintain consistency with the British English usage throughout the CONTRIBUTING.md file. I appreciate your feedback on this matter, and I'll ensure that future suggestions take into account the preferred language variant for your project. Thank you for pointing this out!
[LEARN]
- For the
CONTRIBUTING.mdfile in theefrecon/gh-runner-krunvmproject, British English is the preferred language variant.
[/LEARN]
| created with the content of the secret. Once a microVM has ended, the | ||
| `runner.sh` loop script will detect if the `.brk` file exists and contains the | ||
| secret. If it does, it will abort the loop -- instead of creating yet another | ||
| runner. Using a random secret is for security and to avoid that workflows are |
There was a problem hiding this comment.
Consider using "can" instead of "are able to" for conciseness and readability.
- Using a random secret is for security and to avoid that workflows are able to actually force end the runner loop.
+ Using a random secret is for security and to avoid that workflows can actually force end the runner loop.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| runner. Using a random secret is for security and to avoid that workflows are | |
| runner. Using a random secret is for security and to avoid that workflows can actually force end the runner loop. |
| runner. Using a random secret is for security and to avoid that workflows are | ||
| able to actually force end the runner loop. Since the value of the secret is | ||
| passed through the `.env` file that is automatically removed as soon as the | ||
| microVM has booted and is running the `entrypoint.sh` script, workflows are not |
There was a problem hiding this comment.
Consider using "cannot" instead of "are not able to" for conciseness and readability.
- workflows are not able to break the external loop
+ workflows cannot break the external loopCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| microVM has booted and is running the `entrypoint.sh` script, workflows are not | |
| microVM has booted and is running the `entrypoint.sh` script, workflows cannot |
| able to actually force end the runner loop. Since the value of the secret is | ||
| passed through the `.env` file that is automatically removed as soon as the | ||
| microVM has booted and is running the `entrypoint.sh` script, workflows are not | ||
| able to break the external loop: they are able to create files in the |
There was a problem hiding this comment.
Consider using "can" instead of "are able to" for conciseness and readability.
- they are able to create files in the `/_environment` directory
+ they can create files in the `/_environment` directoryCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| able to break the external loop: they are able to create files in the | |
| can create files in the `/_environment` directory |
| microVM has booted and is running the `entrypoint.sh` script, workflows are not | ||
| able to break the external loop: they are able to create files in the | ||
| `/_environment` directory, but they cannot know the value of the secret to put | ||
| into the file to force the exiting handshake. | ||
|
|
||
| The same type of handshaking happens when the main runner loop is terminating, | ||
| for example after the life-time period provided with the command-line option |
There was a problem hiding this comment.
Consider adding a comma after "terminating" for better readability.
- The same type of handshaking happens when the main runner loop is terminating for example after the life-time period provided with the command-line option
+ The same type of handshaking happens when the main runner loop is terminating, for example after the life-time period provided with the command-line optionCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| for example after the life-time period provided with the command-line option | |
| for example, after the life-time period provided with the command-line option |
Isolate runner loops from one another through an additional sub-directory. This improves security since it means that an orchestrated runner cannot see the files from other runners running on the same host.
Rename
runner.shtoentrypoint.shfor improved clarity.Summary by CodeRabbit