RDKBACCL-1938 : Log Rotation and DAC are failing - #10
Merged
Conversation
RDKBACCL-1938: Log Rotation not working with Configruable WAN Reason for change:Log rotation not working when we have customized wan name instead of erouter0 Test Procedure: Build and flash the image ,Validate Log rotation and DAC functionality Risks: None
There was a problem hiding this comment.
Pull request overview
This PR introduces/updates the DCM (XConf config fetch), DCA telemetry processing, and STB log upload scripts intended to address failures in log rotation and DAC behavior when WAN interface naming is configurable (vs hard-coded erouter0).
Changes:
- Adds
DCMscript.shto fetch/process XConf configuration and coordinate telemetry/log behavior. - Adds
dca_utility.shto generate telemetry output and trigger log upload based on DCM settings. - Adds
uploadSTBLogs.shto package and upload logs via HTTP/TFTP, including handling previous logs.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 17 comments.
| File | Description |
|---|---|
| scripts/uploadSTBLogs.sh | New log packaging/upload script (HTTP/TFTP) plus previous-log handling and rotated-log renaming. |
| scripts/DCMscript.sh | New DCM/XConf fetch + response processing + telemetry coordination logic. |
| scripts/dca_utility.sh | New DCA telemetry generation + cron scheduling + log upload trigger logic. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+30
to
+32
| if [ $# -ne 6 ]; then | ||
| echo "USAGE: $0 <TFTP Server IP> <Flag (STB delay or not)> <SCP_SERVER> <UploadOnReboot> <UploadProtocol> <UploadHttpLink>" | ||
| fi |
Comment on lines
+142
to
+147
| srcLogPath=$1 | ||
| ret=`ls $srcLogPath/*.txt | wc -l` | ||
| if [ ! $ret ]; then | ||
| ret=`ls $srcLogPath/*.log | wc -l` | ||
| if [ ! $ret ]; then exit 1; fi | ||
| fi |
|
|
||
| CURL_CMD="curl -w '%{http_code}\n' -F \"filename=@$1\" -o \"$FILENAME\" \"$CLOUD_URL\" --connect-timeout 10 -m 10" | ||
| echo URL_CMD: $CURL_CMD | ||
| ret= eval $CURL_CMD > $HTTP_CODE |
|
|
||
| CURL_CMD="curl -w '%{http_code}\n' -o \"$FILENAME\" \"$NewUrl\" --connect-timeout 10 -m 10" | ||
| echo URL_CMD: $CURL_CMD | ||
| result= eval $CURL_CMD > $HTTP_CODE |
| ret= eval $CURL_CMD > $HTTP_CODE | ||
| http_code=$(awk -F\" '{print $1}' $HTTP_CODE) | ||
| if [ $http_code -eq 200 ];then | ||
| echo "`/timestamp` Done Uploading Logs through HTTP" >> $LOG_PATH/dcmscript.log |
Comment on lines
+35
to
+36
| EROUTER_IF=erouter0 | ||
| DCMRESPONSE="$PERSISTENT_PATH/DCMresponse.txt" |
Comment on lines
+223
to
+227
| estbMac=`/usr/bin/rpcclient $ARM_ARPING_IP "ifconfig erouter0" | grep 'Link encap:' | cut -d ' ' -f7` | ||
| echo "$estbMac" | ||
| else | ||
| estbMac=`ifconfig erouter0 | grep 'Link encap:' | cut -d ' ' -f7` | ||
| echo "$estbMac" |
| CURL_CMD="curl '$DCM_LOG_SERVER_URL?estbMacAddress=$JSONSTR&model=$MODEL_NAME' -o $DCMRESPONSE > /tmp/httpcode.txt" | ||
|
|
||
| # Execute curl command | ||
| result= eval $CURL_CMD |
Comment on lines
+644
to
+646
| isPeriodicFWCheckEnabled=`syscfg get PeriodicFWCheck_Enable` | ||
| if [ "$isPeriodicFirmwareEnabled" == "true" ]; then | ||
| echo "XCONF SCRIPT : Calling XCONF Client firmwareSched for the updated time" |
Comment on lines
+711
to
+713
| isPeriodicFWCheckEnabled=`syscfg get PeriodicFWCheck_Enable` | ||
| if [ "$isPeriodicFirmwareEnabled" == "true" ]; then | ||
| echo "XCONF SCRIPT : Calling XCONF Client firmwareSched for the updated time" |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (8)
scripts/uploadSTBLogs.sh:32
- The script prints a usage message when the argument count is wrong, but it continues executing with empty/invalid positional parameters. Also, utils.sh is sourced twice.
. $RDK_PATH/utils.sh
. $RDK_PATH/interfaceCalls.sh
. $RDK_PATH/utils.sh
. $RDK_PATH/logfiles.sh
. $RDK_PATH/commonUtils.sh
if [ $# -ne 6 ]; then
echo "USAGE: $0 <TFTP Server IP> <Flag (STB delay or not)> <SCP_SERVER> <UploadOnReboot> <UploadProtocol> <UploadHttpLink>"
fi
scripts/DCMscript.sh:126
last_charextraction uses an invalid awk command (FS=with no value), which will fail and can break URL normalization for ConfigurationServiceURL.
URL=`grep 'urn:settings:ConfigurationServiceURL' /tmp/DCMSettings.conf | cut -d '=' -f2 | head -n 1`
#last_char=`echo $URL | sed -e 's/\(^.*\)\(.$\)/\2/'`
last_char=`echo $URL | awk '$0=$NF' FS=`
if [ "$last_char" != "?" ]
scripts/DCMscript.sh:434
- This code attempts to kill StartDCM.sh, but
startdcmEndstores the fullpsoutput line (not a PID).kill -9will fail (or may target an unintended token). Extract and kill the PID explicitly.
echo "No xconf comm ,exiting script"
startdcmEnd=`ps -ef | grep -i "StartDCM.sh" | head -n 1`
kill -9 $startdcmEnd
exit 0
scripts/DCMscript.sh:485
- This conditional uses curly quotes and an incorrect variable name (
T2_enableinstead ofT2_ENABLE), which will cause a shell syntax error and prevent the script from running.
T2_ENABLE=`syscfg get T2Enable`
# Safe wait for IP acquisition
if [ “x$T2_enable” == “xfalse” ]; then
loop=1
counter=0
scripts/DCMscript.sh:648
isPeriodicFWCheckEnabledis set, but the condition checksisPeriodicFirmwareEnabled(different variable). This makes the firmware schedule trigger conditional effectively always false.
isPeriodicFWCheckEnabled=`syscfg get PeriodicFWCheck_Enable`
if [ "$isPeriodicFirmwareEnabled" == "true" ]; then
echo "XCONF SCRIPT : Calling XCONF Client firmwareSched for the updated time"
sh /etc/firmwareSched.sh &
fi
scripts/uploadSTBLogs.sh:216
- HttpLogUpload has invalid command execution (
ret= eval ...) and uses/timestamp(missing/bin/), which will fail at runtime and can prevent log uploads.
CURL_CMD="curl -w '%{http_code}\n' -F \"filename=@$1\" -o \"$FILENAME\" \"$CLOUD_URL\" --connect-timeout 10 -m 10"
echo URL_CMD: $CURL_CMD
ret= eval $CURL_CMD > $HTTP_CODE
http_code=$(awk -F\" '{print $1}' $HTTP_CODE)
if [ $http_code -eq 200 ];then
echo "`/timestamp` Done Uploading Logs through HTTP" >> $LOG_PATH/dcmscript.log
scripts/uploadSTBLogs.sh:258
rm -rf $DCM_LOG_PATH/removes the entire /upload directory, which is risky (especially if it is a mount point). If the intent is to clean up log artifacts, only delete the contents.
rm -rf $DCM_LOG_PATH/
scripts/DCMscript.sh:391
- The curl command construction in sendHttpRequestToServer is syntactically broken due to nested double-quotes around
%{http_code}, andresult= eval ...doesn't actually capture a result. As written, this function will fail to run and DCM settings fetch will break.
CURL_CMD="curl -w "%{http_code}" '$URL?estbMacAddress=$JSONSTR&model=$MODEL_NAME' -o $DCMRESPONSE >> /tmp/telehttpcode.txt "
echo "------CURL_CMD:"$CURL_CMD
# Execute curl command
result= eval $CURL_CMD > $TELE_HTTP_CODE
Comment on lines
+263
to
+266
| echo "Sleeping for seven minutes " | ||
| if [ "true" != "$RDK_EMULATOR" ]; then | ||
| sleep 12 | ||
| fi |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (8)
scripts/DCMscript.sh:129
- The awk invocation used to compute the last character of URL is malformed (
FS=), which will error and prevent the script from appending the required?suffix.
last_char=`echo $URL | awk '$0=$NF' FS=`
if [ "$last_char" != "?" ]
then
URL="$URL?"
fi
scripts/DCMscript.sh:391
- This curl command string has broken quoting (
-w "%{http_code}"inside a double-quoted string) and the subsequentevalline won’t reliably capture the HTTP code. As written, this block is a shell parse error.
CURL_CMD="curl -w "%{http_code}" '$URL?estbMacAddress=$JSONSTR&model=$MODEL_NAME' -o $DCMRESPONSE >> /tmp/telehttpcode.txt "
echo "------CURL_CMD:"$CURL_CMD
# Execute curl command
result= eval $CURL_CMD > $TELE_HTTP_CODE
scripts/DCMscript.sh:434
ps -ef | ... | head -n 1returns an entire process line, butkill -9 $startdcmEndexpects a PID. This will fail (or behave unexpectedly) when the retry logic hits this path.
startdcmEnd=`ps -ef | grep -i "StartDCM.sh" | head -n 1`
kill -9 $startdcmEnd
exit 0
scripts/DCMscript.sh:487
- This condition uses smart quotes and a misspelled variable (
T2_enablevsT2_ENABLE), which will cause a syntax error and prevent the script from running past this point.
if [ “x$T2_enable” == “xfalse” ]; then
loop=1
counter=0
while [ $loop -eq 1 ]
do
scripts/uploadSTBLogs.sh:147
- The
retchecks in this function are incorrect ([ ! $ret ]is always false for non-empty strings like "0"), so the.logfallback / exit path is never taken. This can leave files un-timestamped and break upload packaging expectations.
ret=`ls $srcLogPath/*.txt | wc -l`
if [ ! $ret ]; then
ret=`ls $srcLogPath/*.log | wc -l`
if [ ! $ret ]; then exit 1; fi
fi
scripts/uploadSTBLogs.sh:217
- This block attempts to log using
`/timestamp`(likely a typo for/bin/timestamp), and theret= eval ...form doesn’t capture the curl exit code. Fix the timestamp path and run the eval with proper quoting.
ret= eval $CURL_CMD > $HTTP_CODE
http_code=$(awk -F\" '{print $1}' $HTTP_CODE)
if [ $http_code -eq 200 ];then
echo "`/timestamp` Done Uploading Logs through HTTP" >> $LOG_PATH/dcmscript.log
result=0
scripts/DCMscript.sh:413
- In the retry path, the curl URL is also single-quoted, preventing
$URL,$JSONSTR, and$MODEL_NAMEexpansion; plus theevalusage makes it hard to reliably capture the HTTP code. This retry will never actually call the intended endpoint.
CURL_CMD="curl -w "%{http_code}" '$URL?estbMacAddress=$JSONSTR&model=$MODEL_NAME' -o $DCMRESPONSE >> /tmp/telehttpcode.txt "
result= eval $CURL_CMD
http_code_retry=$(awk -F\" '{print $1}' /tmp/telehttpcode.txt)
scripts/uploadSTBLogs.sh:231
- These log lines use
`/timestamp`which is almost certainly a typo and will fail command substitution. Use/bin/timestamp(consistent with other logging in this script) so upload success/failure is recorded correctly.
echo "`/timestamp` Done Uploading Logs through - HTTP" >> $LOG_PATH/dcmscript.log
result=0
else
echo "`/timestamp` Failed Uploading Logs through - HTTP" >> $LOG_PATH/dcmscript.log
Comment on lines
+489
to
+493
| if [ "X$estbIp" == "X" ]; then | ||
| echo_t "waiting for IP" >> $DCM_LOG_FILE | ||
| sleep 2 | ||
| let counter++ | ||
| else |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (11)
scripts/uploadSTBLogs.sh:32
- The script prints usage when the argument count is wrong, but then continues executing with empty parameters. That can trigger errors in numeric tests (e.g., "[ $FLAG -eq 0 ]") and unintended uploads. Exit after printing usage.
if [ $# -ne 6 ]; then
echo "USAGE: $0 <TFTP Server IP> <Flag (STB delay or not)> <SCP_SERVER> <UploadOnReboot> <UploadProtocol> <UploadHttpLink>"
fi
scripts/uploadSTBLogs.sh:147
- modifyFileWithTimestamp() uses "if [ ! $ret ]" which is always false for non-empty strings like "0", so it won’t detect the "no matching files" case and can end up iterating over the literal glob ".". Also, the function relies on callers to cd into srcLogPath, which makes it fragile.
srcLogPath=$1
ret=`ls $srcLogPath/*.txt | wc -l`
if [ ! $ret ]; then
ret=`ls $srcLogPath/*.log | wc -l`
if [ ! $ret ]; then exit 1; fi
fi
scripts/uploadSTBLogs.sh:216
- In HttpLogUpload(), "ret= eval $CURL_CMD > $HTTP_CODE" does not capture curl’s exit status and is parsed as an environment assignment + command; it also leaves ret empty. Use eval as a standalone command and then parse the http code. Also, "/timestamp" is not the same as "/bin/timestamp" and will fail unless that path exists.
ret= eval $CURL_CMD > $HTTP_CODE
http_code=$(awk -F\" '{print $1}' $HTTP_CODE)
if [ $http_code -eq 200 ];then
echo "`/timestamp` Done Uploading Logs through HTTP" >> $LOG_PATH/dcmscript.log
scripts/uploadSTBLogs.sh:259
- "rm -rf $DCM_LOG_PATH/" deletes the upload directory itself (currently hardcoded to /upload). This can break subsequent runs and any other process expecting /upload to exist. Prefer clearing contents instead of removing the directory.
rm -rf $DCM_LOG_PATH/
}
scripts/DCMscript.sh:126
- This command substitution is syntactically invalid ("awk '$0=$NF' FS=") and will cause the script to fail to parse. If the intent is to get the last character of the URL, use a simple/valid method.
URL=`grep 'urn:settings:ConfigurationServiceURL' /tmp/DCMSettings.conf | cut -d '=' -f2 | head -n 1`
#last_char=`echo $URL | sed -e 's/\(^.*\)\(.$\)/\2/'`
last_char=`echo $URL | awk '$0=$NF' FS=`
if [ "$last_char" != "?" ]
scripts/DCMscript.sh:391
- The CURL_CMD assignment has broken quoting (nested double-quotes), which will make the script fail to parse. Also, "result= eval ..." is not a reliable way to run curl. Build the URL in a variable and run curl directly (or eval a properly quoted string).
estbMacAddress=`dmcli eRT getv Device.DeviceInfo.X_COMCAST-COM_WAN_MAC | grep type: | awk '{print $5}' | tr '[:upper:]' '[:lower:]'`
JSONSTR=$estbMacAddress
CURL_CMD="curl -w "%{http_code}" '$URL?estbMacAddress=$JSONSTR&model=$MODEL_NAME' -o $DCMRESPONSE >> /tmp/telehttpcode.txt "
echo "------CURL_CMD:"$CURL_CMD
# Execute curl command
result= eval $CURL_CMD > $TELE_HTTP_CODE
scripts/DCMscript.sh:412
- The retry loop repeats the same broken CURL_CMD quoting and eval usage. This block will never run correctly unless the curl invocation is fixed here as well.
CURL_CMD="curl -w "%{http_code}" '$URL?estbMacAddress=$JSONSTR&model=$MODEL_NAME' -o $DCMRESPONSE >> /tmp/telehttpcode.txt "
result= eval $CURL_CMD
scripts/DCMscript.sh:434
- startdcmEnd captures an entire ps output line, not a PID, so "kill -9 $startdcmEnd" will fail (and may try to kill unintended PIDs if fields happen to parse). Extract the PID before killing, and guard against empty.
startdcmEnd=`ps -ef | grep -i "StartDCM.sh" | head -n 1`
kill -9 $startdcmEnd
exit 0
scripts/DCMscript.sh:484
- This condition uses curly quotes (non-ASCII) and refers to T2_enable (lowercase) even though the variable set above is T2_ENABLE. As written it will fail in /bin/sh and the IP wait logic won’t run as intended.
T2_ENABLE=`syscfg get T2Enable`
# Safe wait for IP acquisition
if [ “x$T2_enable” == “xfalse” ]; then
loop=1
scripts/dca_utility.sh:532
- "result= eval $CURL_CMD" is parsed as an env assignment + command and doesn’t give you a usable status/result. Since the curl command is already simple, invoke curl directly and avoid eval entirely.
CURL_CMD="curl '$DCM_LOG_SERVER_URL?estbMacAddress=$JSONSTR&model=$MODEL_NAME' -o $DCMRESPONSE > /tmp/httpcode.txt"
# Execute curl command
result= eval $CURL_CMD
sleep 5
scripts/uploadSTBLogs.sh:225
- In the 302-redirect branch, the upload repeats the same "eval" misuse and uses "/timestamp" (likely a typo for /bin/timestamp). This can prevent HTTP uploads from ever reporting success/failure correctly.
result= eval $CURL_CMD > $HTTP_CODE
http_code=$(awk -F\" '{print $1}' $HTTP_CODE)
Comment on lines
32
to
35
| if [ "$WAN_INTERFACE" = "" ] | ||
| then | ||
| WAN_INTERFACE="erouter0" | ||
| WAN_INTERFACE=`syscfg get wan_physical_ifname` | ||
| fi |
Comment on lines
+180
to
+181
| wan_name=`syscfg get wan_physical_ifname` | ||
| Box_IPv6_addr=`ifconfig $wan_name | grep inet6 | grep Global | awk '{print $(NF-1)}' | cut -f1 -d\/` |
Comment on lines
39
to
42
| if [ "$CM_INTERFACE" = "" ] | ||
| then | ||
| CM_INTERFACE=erouter0 | ||
| CM_INTERFACE=`syscfg get wan_physical_ifname` | ||
| fi |
Comment on lines
+120
to
+121
| wan_name=`syscfg get wan_physical_ifname` | ||
| CMMac=`ifconfig $wan_name | grep HWaddr | cut -d" " -f7` |
Comment on lines
+322
to
+325
| HTTP_CODE=`result= eval $CURL_CMD` | ||
| http_code=$(echo $HTTP_CODE | cut -d "." -f 2) | ||
| echo "http_code is :"$http_code | ||
| if [ $http_code -eq 200 ] ; then |
Comment on lines
+332
to
+336
| CURL_CMD="curl -w '%{http_code}\n' -F filename=@$PREV_LOG_PATH/$RpiMacAddress-Logs-$LOG_FILE '$HTTPLOGUPLOADURL' --connect-timeout 100 -m 100" | ||
| HTTP_CODE=`result= eval $CURL_CMD` | ||
| http_code_la=$(echo $HTTP_CODE | cut -d "." -f 2) | ||
| echo "http_code is :"$http_code_la | ||
| if [ "$http_code_la" != "200" ]; then |
cpokuru
approved these changes
Aug 3, 2026
Comment on lines
+35
to
+36
| EROUTER_IF=erouter0 | ||
| DCMRESPONSE="$PERSISTENT_PATH/DCMresponse.txt" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
RDKBACCL-1938: Log Rotation not working with Configruable WAN
Reason for change:Log rotation not working when we have customized wan name instead of erouter0 Test Procedure: Build and flash the image ,Validate Log rotation and DAC functionality Risks: None