Update Start_MaintenanceTasks.sh #410
Conversation
There was a problem hiding this comment.
Pull request overview
This PR adds support for a new binary-based log upload tool (/usr/bin/logupload) with automatic fallback to the existing script-based implementation. The change enables the system to try the binary implementation first, and if it fails or is unavailable, fall back to the original shell script approach.
- Introduces a new
LOG_UPLOAD_BIN_PATHvariable to specify the binary location - Implements try-binary-first, fallback-to-script logic in both on-demand and regular log upload paths
- Adds detailed logging for binary execution success/failure and fallback behavior
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
lib/rdk/Start_MaintenanceTasks.sh
Outdated
| nice -n 19 "$LOG_UPLOAD_BIN_PATH" "$tftp_server" 1 1 "$uploadOnReboot" "$upload_protocol" "$upload_httplink" & >> /opt/logs/dcmscript.log | ||
| bg_pid=$! | ||
| wait $bg_pid | ||
| result=$? | ||
| if [ "$result" -eq 0 ]; then | ||
| logUploadLog "Binary execution succeeded" | ||
| return 0 | ||
| else | ||
| logUploadLog "Binary execution failed with result=$result; falling back to script" | ||
| nice -n 19 sh $LOGUPLOAD_SCRIPT "$tftp_server" 1 1 "$uploadOnReboot" "$upload_protocol" "$upload_httplink" & |
There was a problem hiding this comment.
The binary call is missing the trigger type argument that the script receives. Looking at line 181, the binary gets 7 arguments ending with "ondemand", but at line 209, the script gets 6 arguments (no trigger type). The regular execution path should pass a trigger type to maintain consistency, or omit it from the binary call on line 200 as well if it's not needed for regular execution.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| result=$? | ||
| if [ -x "$LOG_UPLOAD_BIN_PATH" ]; then | ||
| logUploadLog "Executing logupload binary: $LOG_UPLOAD_BIN_PATH" | ||
| "$LOG_UPLOAD_BIN_PATH" "$tftp_server" 1 1 "$uploadOnReboot" "$upload_protocol" "$upload_httplink" "ondemand" >> /opt/logs/dcmscript.log |
There was a problem hiding this comment.
The TriggerType parameter is being passed inconsistently. In the binary execution, it's passed as the string "ondemand" (line 181), but in the fallback script execution and the else branch, it's passed as the variable "$TriggerType" (lines 188, 193). This inconsistency could lead to different behavior between the binary and script executions.
No description provided.