RDKB-63303, RDKB-63414 : Move RdkLogger Scripts to Cron

[ksures101]

Reason for change: Running the rdklogger scripts as cron jobs instead of background processes when the cron is enabled and running as normal background process when the cron is disabled.
Test Procedure: Check scripts are running in crontab when the cron is enabled.
Priority: P1
Risks: None

Signed-off-by: KavyaChowdahalli_Suresh@comcast.com

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

[Copilot]

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 21 comments.

Comments suppressed due to low confidence (1)

rdkbLogMonitor.sh:585

• bootup_upload invokes uploadRDKBLogs.sh with the protocol argument set to HTTP, which strongly suggests logs are uploaded over unencrypted HTTP. Anyone on the network path (or with access to upstream infrastructure) can sniff or tamper with these log uploads, potentially exposing sensitive operational data. Please switch these uploads to a TLS-protected channel (e.g., HTTPS with certificate validation) and avoid using plain HTTP for log transfer in this function.

	   	    random_sleep
			$RDK_LOGGER_PATH/uploadRDKBLogs.sh $SERVER "HTTP" $URL "true" "" $TMP_LOG_UPLOAD_PATH "true"
	   else 
	      echo_t "No log file found in logbackupreboot folder"
	   fi
	   
        #Venu
	logBackupEnable=`syscfg get log_backup_enable` 
        if [ "$logBackupEnable" = "true" ];then
          if [ -d $PRESERVE_LOG_PATH ] ; then
            cd $PRESERVE_LOG_PATH
            fileToUpload=`ls | grep tgz`
            if [ "$fileToUpload" != "" ]
            then
              sleep 60
              echo_t "Uploading backup logs found in $PRESERVE_LOG_PATH"
              $RDK_LOGGER_PATH/uploadRDKBLogs.sh $SERVER "HTTP" $URL "true" "" $PRESERVE_LOG_PATH "true"

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

In cron mode, the first run after boot detection at lines 736-738 only touches the BOOT_PROCESSED_FLAG but doesn't prevent the threshold check. On the very first run after boot, the flag doesn't exist, so the code touches it and then continues. However, there's no explicit flow control after line 738 - execution continues to line 746 without any condition. The logic should either: 1) Continue to run bootup processing after touching the flag (current implicit behavior), or 2) Have an explicit flow path that makes it clear what happens after the first boot. The current structure is ambiguous because the else block at lines 739-746 only runs when the flag already exists, but the code after line 746 runs in both cases.

Suggested change

	if [ ! -f "$BOOT_PROCESSED_FLAG" ]; then
	echo_t "First run after boot: running bootup_tarlogs and bootup_upload"
	touch "$BOOT_PROCESSED_FLAG"
	else
	getLogfileSize "$LOG_PATH"
	if [ "$totalSize" -lt "$MAXSIZE" ]; then
	echo_t "Log size below threshold ($totalSize < $MAXSIZE). Exiting early (cron will retry in 1 min)."
	exit 0
	fi
	echo_t "THRESHOLD HIT! Log size: $totalSize >= $MAXSIZE. Starting full processing..."
	if [ -f "$BOOT_PROCESSED_FLAG" ]; then
	getLogfileSize "$LOG_PATH"
	if [ "$totalSize" -lt "$MAXSIZE" ]; then
	echo_t "Log size below threshold ($totalSize < $MAXSIZE). Exiting early (cron will retry in 1 min)."
	exit 0
	fi
	echo_t "THRESHOLD HIT! Log size: $totalSize >= $MAXSIZE. Starting full processing..."
	else
	echo_t "First run after boot detected: creating BOOT_PROCESSED_FLAG and skipping threshold check for initial full processing."
	touch "$BOOT_PROCESSED_FLAG"

[Copilot]

The check_maintenance_window_upload function sets upload_logfile but never uses it outside of the function (lines 297, 308, 310, 315). The function either calls calcRandTimeandUpload (which performs the upload) or does nothing. When the current time is not in the maintenance window (lines 314-316), upload_logfile is set to 1 but no action is taken - the upload doesn't happen. This appears to be a logic error. The function should either perform the upload when outside the maintenance window, or the variable should be returned/used by the caller to determine whether to proceed with the upload.

[Copilot]

The code at lines 1123-1128 is unreachable. The while loop at lines 1116-1122 runs indefinitely (while [ 1 ]), so execution never reaches the cleanup code that removes the lock directory. This code should either be removed, or the loop should have a proper exit condition with the cleanup code in a trap handler similar to what's done in cron mode.

[Copilot]

In service mode, CRON_MODE is set to 0 inside the while loop (line 346), but this is unnecessary since CRON_MODE is already set to 0 at line 374 before calling service_mode. Setting it repeatedly inside the loop is redundant and could be misleading. Consider removing this line or moving it outside the loop.

Suggested change

CRON_MODE=0

[Copilot]

In service_mode, CRON_MODE is being set to 0 at line 1118 inside the loop. However, CRON_MODE should already be 0 when entering service_mode (it's set at line 1151 before calling service_mode). Setting it repeatedly inside the loop is unnecessary and could be misleading. Consider moving this assignment outside the loop or removing it entirely if it's already set before calling the function.

Suggested change

CRON_MODE=0

[Copilot]

The tick counter increments from 0 to 4 and wraps around (line 178), but the countdown only happens when current_tick is 0 (line 156). This means the delay countdown only progresses once every 5 minutes (when tick is 0), but the script runs every minute. This creates a timing issue: if the random delay is 1800 seconds (30 minutes), it would take 1800/300 * 5 = 30 minutes of wall clock time instead of the expected 30 minutes. The countdown logic appears to subtract 300 seconds per iteration, but with the tick mechanism, this happens every 5 minutes, making the total delay 5x longer than intended.

[Copilot]

Inconsistent indentation with tabs. Lines 160-165 and 175 use tabs while the surrounding code uses spaces.

[Copilot]

The cron entry check uses a simple grep which could match partial strings. For example, if there's a comment in the crontab containing "rdkbLogMonitor.sh", it would incorrectly report the cron entry as present. Consider using a more precise pattern match, such as grep -F (fixed string) with the full cron line pattern, or anchor the grep pattern to avoid false positives.

Suggested change

	if crontab -l 2>/dev/null | grep -q "rdkbLogMonitor.sh"; then
	if crontab -l 2>/dev/null | grep -F -x -q "$CRON_LINE"; then

[Copilot]

The cron entry check uses a simple grep which could match partial strings. Consider using a more precise pattern match to avoid false positives.

Suggested change

	if crontab -l 2>/dev/null | grep -q "fileUploadRandom.sh"; then
	if crontab -l 2>/dev/null | grep -qE '^[^#]*[[:space:]]/rdklogger/fileUpload\.sh([[:space:]]|$)'; then

[Copilot]

Within regular_upload_state, uploadRDKBLogs.sh is called with HTTP as the protocol, meaning regular log uploads are likely sent over plaintext HTTP. This allows a network-path attacker to observe or modify the uploaded log data, undermining both confidentiality and integrity of potentially sensitive diagnostics information. Use a TLS-secured protocol (e.g., HTTPS) for all such uploads in this function and ensure server certificates are validated to prevent man-in-the-middle attacks.