BYOH agent service upgrades #46
Conversation
Changelist by BitoThis pull request implements the following key changes.
|
![bito-code-review[bot]](https://avatars.githubusercontent.com/in/1061978?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review Agent Run #79fd0b
Actionable Suggestions - 1
-
installer/internal/algo/ubuntu-templates/install.sh.tmpl - 1
- Inner trap handler overwrites outer handler · Line 89-100
Additional Suggestions - 2
-
installer/internal/algo/ubuntu-templates/install.sh.tmpl - 2
- Hardcoded image URL should be parameterized · Line 104-104
- Undefined variable reference in upgrade script · Line 189-189
Review Details
-
Files reviewed - 1 · Commit Range:
d96f12e..4bc4875- installer/internal/algo/ubuntu-templates/install.sh.tmpl
-
Files skipped - 0
-
Tools
- Whispers (Secret Scanner) - ✔︎ Successful
- Detect-secrets (Secret Scanner) - ✔︎ Successful
Bito Usage Guide
Commands
Type the following command in the pull request comment and save the comment.
-
/review- Manually triggers a full AI review. -
/pause- Pauses automatic reviews on this pull request. -
/resume- Resumes automatic reviews. -
/resolve- Marks all Bito-posted review comments as resolved. -
/abort- Cancels all in-progress reviews.
Refer to the documentation for additional commands.
Configuration
This repository uses Default Agent You can customize the agent settings here or contact your Bito workspace admin at [email protected].
Documentation & Help
AI Code Review powered by
| # Cleanup function for this scope | ||
| cleanup() { | ||
| local exit_code=$? | ||
| echo "[INFO] Starting cleanup of temporary files" | ||
| if [ -n "$AGENT_TEMP_DIR" ] && [ -d "$AGENT_TEMP_DIR" ]; then | ||
| rm -rf "$AGENT_TEMP_DIR" | ||
| echo "[INFO] Removed temporary directory: $AGENT_TEMP_DIR" | ||
| AGENT_TEMP_DIR="" | ||
| fi | ||
| exit $exit_code | ||
| } | ||
| trap cleanup EXIT |
There was a problem hiding this comment.
The inner cleanup function in upgrade_agent_if_needed overwrites the outer trap handler but doesn't restore it, potentially causing resource leaks if the outer handler isn't executed.
Code suggestion
Check the AI-generated fix before applying
| # Cleanup function for this scope | |
| cleanup() { | |
| local exit_code=$? | |
| echo "[INFO] Starting cleanup of temporary files" | |
| if [ -n "$AGENT_TEMP_DIR" ] && [ -d "$AGENT_TEMP_DIR" ]; then | |
| rm -rf "$AGENT_TEMP_DIR" | |
| echo "[INFO] Removed temporary directory: $AGENT_TEMP_DIR" | |
| AGENT_TEMP_DIR="" | |
| fi | |
| exit $exit_code | |
| } | |
| trap cleanup EXIT | |
| # Save the previous trap handler | |
| local previous_trap=$(trap -p EXIT) | |
| # Cleanup function for this scope | |
| cleanup() { | |
| local exit_code=$? | |
| echo "[INFO] Starting cleanup of temporary files" | |
| if [ -n "$AGENT_TEMP_DIR" ] && [ -d "$AGENT_TEMP_DIR" ]; then | |
| rm -rf "$AGENT_TEMP_DIR" | |
| echo "[INFO] Removed temporary directory: $AGENT_TEMP_DIR" | |
| AGENT_TEMP_DIR="" | |
| fi | |
| # Restore the previous trap handler if it exists | |
| if [ -n "$previous_trap" ]; then | |
| eval "$previous_trap" | |
| fi | |
| } | |
| trap cleanup EXIT |
Code Review Run #79fd0b
Should Bito avoid suggestions like this for future reviews? (Manage Rules)
- Yes, avoid them
1 participant
This resolves KAAP-634
This PR adds the pf9-byohost-agent service upgrade feature. So, we have a install.sh file which runs on the node everytime there is a change in the cluster MD. We were using this feature for BYOH cluster upgrades as well. Now, we added a logic to create a systemd service for agent service upgrade from the install.sh file itself. So, the process is:
systemd service for agent upgrade logic:
We have a UPGRADE_MARKER file in ~/.byoh/, this file is created when the upgrade happens for the first time.
So, the flow is, the upgrade service gets called which will run independent of the BYOH agent service and logs in
/var/log/pf9/agent-upgrade-*.log. This upgrade service, takes the back up of~/.byoh/configstops the BYOH agent service restores~/.byoh/configand installs the new bundle and create the UPGRADE_MARKER file and then checks if the BYOH agent service has come up running or not.Once, this upgrade service is successfully ran, the BYOH agent service restarts with the new bundle. In this iteration, the upgrade service doesn't get invoked because we are invoking the upgrade service only if the UPGRADE_MARKER is not present, if it is present, we continue only with the installation.
Testing:
Invoking the upgrade service in the foreground
Upgrade service logs :
Once, the upgrade is done, the installation script is continued in the next call
Summary by Bito
This pull request enhances the BYOH agent service upgrade process and containerd installation flow by implementing a systemd service for asynchronous upgrades with improved logging and error handling. It also refactors containerd installation to use tar extraction and dynamic configuration generation, resulting in more streamlined installation and upgrade processes on target nodes.