RDKEMW-10725:gstreamer-cleanup conditions when cdl_flashed_file_name is missing by vrenu2018 · Pull Request #511 · rdkcentral/sysint

vrenu2018 · 2026-04-13T15:53:06Z

Reason for change: gstreamer-cleanup is happening on every reboot when /opt/cdl_flashed_file_name is missing
Test Procedure: Boot the TV and check for gstreamer-cleanup metrics in rdk_milestones.log
Risks: low

…is missing Reason for change: gstreamer-cleanup is happening on every reboot when /opt/cdl_flashed_file_name is missing Test Procedure: Boot the TV and check for gstreamer-cleanup metrics in rdk_milestones.log Risks: low Signed-off-by: Renuka Varry <rvarry049@cable.comcast.com>

Copilot

Pull request overview

This PR aims to prevent gstreamer-cleanup from running on every reboot when /opt/cdl_flashed_file_name is missing by moving the cleanup decision logic into a dedicated script and refining the conditions.

Changes:

Refactors gstreamer-cleanup.service to call a standalone /lib/rdk/gstreamer-cleanup.sh script instead of embedding complex shell logic in the unit.
Adds lib/rdk/gstreamer-cleanup.sh to compute flashed-image state and decide whether to clear/regenerate the GStreamer registry.

Reviewed changes

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

File	Description
systemd_units/gstreamer-cleanup.service	Replaces inline cleanup logic with a call to `/lib/rdk/gstreamer-cleanup.sh` and keeps milestone logging.
lib/rdk/gstreamer-cleanup.sh	Implements the new conditional cleanup/regeneration logic when CDL marker files are missing or changed.

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

Copilot · 2026-04-13T15:56:38Z

+# Print the variables for debugging - can be removed once the logic is stable
+echo "DEBUG: CDLFILE=[${CDLFILE}]"
+echo "DEBUG: PREV_CDLFILE=[${PREV_CDLFILE}]"
+echo "DEBUG: CUR_IMAGE=[${CUR_IMAGE}]"


The script currently prints three DEBUG: lines on every boot. This will add noise to boot logs/metrics and may be undesirable long-term. Consider gating these behind an environment flag (e.g., GST_CLEANUP_DEBUG=1) or removing them once validation is complete.

Suggested change

# Print the variables for debugging - can be removed once the logic is stable

echo "DEBUG: CDLFILE=[${CDLFILE}]"

echo "DEBUG: PREV_CDLFILE=[${PREV_CDLFILE}]"

echo "DEBUG: CUR_IMAGE=[${CUR_IMAGE}]"

# Print the variables for debugging only when explicitly enabled

if [[ "${GST_CLEANUP_DEBUG}" == "1" ]]; then

echo "DEBUG: CDLFILE=[${CDLFILE}]"

echo "DEBUG: PREV_CDLFILE=[${PREV_CDLFILE}]"

echo "DEBUG: CUR_IMAGE=[${CUR_IMAGE}]"

fi

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

github-actions · 2026-04-13T17:27:14Z

All contributors have signed the CLA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

Copilot

Pull request overview

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

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

Copilot · 2026-04-13T17:30:16Z

+
+# Check all cleanup conditions in a single if statement using bash syntax
+if [[ ! -f /opt/previous_flashed_file_name || \
+      ( ! -f /opt/cdl_flashed_file_name && "${PREV_CDLFILE}" != *"${CUR_IMAGE}"* ) || \


Cleanup condition depends on CUR_IMAGE, but if /version.txt exists yet the grep doesn't match (or returns an empty value), the pattern "${PREV_CDLFILE}" != *"${CUR_IMAGE}"* will never be true (empty pattern matches everything). That can incorrectly skip registry cleanup when /opt/cdl_flashed_file_name is missing. Consider treating an empty CUR_IMAGE as "unknown" and forcing cleanup (or explicitly require CUR_IMAGE to be non-empty before using it in the match).

Suggested change

( ! -f /opt/cdl_flashed_file_name && "${PREV_CDLFILE}" != *"${CUR_IMAGE}"* ) || \

( ! -f /opt/cdl_flashed_file_name && ( -z "${CUR_IMAGE}" || "${PREV_CDLFILE}" != *"${CUR_IMAGE}"* ) ) || \

Copilot · 2026-04-13T17:30:17Z

+ExecStartPre=/bin/bash -c 'if [ -f /lib/rdk/logMilestone.sh ];then sh /lib/rdk/logMilestone.sh "GST_CLEANUP_START"; fi;'
+ExecStart=-/lib/rdk/gstreamer-cleanup.sh
+ExecStartPost=/bin/bash -c 'if [ -f /lib/rdk/logMilestone.sh ];then sh /lib/rdk/logMilestone.sh "GST_CLEANUP_COMPLETE"; fi;'


ExecStartPre/ExecStartPost were switched to /bin/bash -c, but the commands used are POSIX shell-compatible and the rest of the unit files in this repo consistently use /bin/sh for these hooks (e.g., systemd_units/dropbear.service:28, systemd_units/restart-parodus.service:25). Consider reverting these to /bin/sh -c for consistency and to avoid an unnecessary bash dependency at boot.

Suggested change

ExecStartPre=/bin/bash -c 'if [ -f /lib/rdk/logMilestone.sh ];then sh /lib/rdk/logMilestone.sh "GST_CLEANUP_START"; fi;'

ExecStart=-/lib/rdk/gstreamer-cleanup.sh

ExecStartPost=/bin/bash -c 'if [ -f /lib/rdk/logMilestone.sh ];then sh /lib/rdk/logMilestone.sh "GST_CLEANUP_COMPLETE"; fi;'

ExecStartPre=/bin/sh -c 'if [ -f /lib/rdk/logMilestone.sh ];then sh /lib/rdk/logMilestone.sh "GST_CLEANUP_START"; fi;'

ExecStart=-/lib/rdk/gstreamer-cleanup.sh

ExecStartPost=/bin/sh -c 'if [ -f /lib/rdk/logMilestone.sh ];then sh /lib/rdk/logMilestone.sh "GST_CLEANUP_COMPLETE"; fi;'

…is missing Reason for change: gstreamer-cleanup is happening on every reboot when /opt/cdl_flashed_file_name is missing Test Procedure: Boot the TV and check for gstreamer-cleanup metrics in rdk_milestones.log Risks: low Signed-off-by: Renuka Varry <rvarry049@cable.comcast.com>

Signed-off-by: Renuka Varry <rvarry049@cable.comcast.com>

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot

Pull request overview

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

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

Copilot · 2026-04-14T14:13:32Z

+ExecStartPre=/bin/bash -c 'if [ -f /lib/rdk/logMilestone.sh ];then sh /lib/rdk/logMilestone.sh "GST_CLEANUP_START"; fi;'
+ExecStart=-/lib/rdk/gstreamer-cleanup.sh
+ExecStartPost=/bin/bash -c 'if [ -f /lib/rdk/logMilestone.sh ];then sh /lib/rdk/logMilestone.sh "GST_CLEANUP_COMPLETE"; fi;'
 ExecStop=/bin/sh -c 'FW_UPDATE_STATE=$(cat /opt/fwdnldstatus.txt | grep FwUpdateState | cut -d "|" -f2); echo "FW_UPDATE_STATE: $FW_UPDATE_STATE"; if [ "$FW_UPDATE_STATE" == "Preparing to reboot" ]; then  echo "Removing gstreamer registry after firmware update"; rm -rf /opt/.gstreamer; fi;'


ExecStop runs under /bin/sh but uses == in the [ ... ] test, which is not POSIX and can fail on systems where /bin/sh is not bash. Use = for the string comparison (or switch the command to /bin/bash -c for consistency with the rest of the unit).

Suggested change

ExecStop=/bin/sh -c 'FW_UPDATE_STATE=$(cat /opt/fwdnldstatus.txt | grep FwUpdateState | cut -d "|" -f2); echo "FW_UPDATE_STATE: $FW_UPDATE_STATE"; if [ "$FW_UPDATE_STATE" == "Preparing to reboot" ]; then echo "Removing gstreamer registry after firmware update"; rm -rf /opt/.gstreamer; fi;'

ExecStop=/bin/sh -c 'FW_UPDATE_STATE=$(cat /opt/fwdnldstatus.txt | grep FwUpdateState | cut -d "|" -f2); echo "FW_UPDATE_STATE: $FW_UPDATE_STATE"; if [ "$FW_UPDATE_STATE" = "Preparing to reboot" ]; then echo "Removing gstreamer registry after firmware update"; rm -rf /opt/.gstreamer; fi;'

Copilot · 2026-04-14T14:13:33Z

+if [[ -f /version.txt ]]; then
+    CUR_IMAGE=$(grep "^imagename:" /version.txt | cut -d":" -f2)
+fi
+
+# Print the variables for debugging - can be removed once the logic is stable
+echo "DEBUG: CDLFILE=[${CDLFILE}]"
+echo "DEBUG: PREV_CDLFILE=[${PREV_CDLFILE}]"
+echo "DEBUG: CUR_IMAGE=[${CUR_IMAGE}]"
+
+# Check all cleanup conditions in a single if statement using bash syntax
+if [[ ! -f /opt/previous_flashed_file_name || \
+      ( ! -f /opt/cdl_flashed_file_name && "${PREV_CDLFILE}" != *"${CUR_IMAGE}"* ) || \
+      ( -f /opt/cdl_flashed_file_name && "${CDLFILE}" != *"${PREV_CDLFILE}"* ) ]]; then


CUR_IMAGE is parsed via grep "^imagename:" ... | cut -d":" -f2 without trimming, so it can contain leading whitespace/newlines; that will break the later substring match against PREV_CDLFILE. Also, if /version.txt is missing or imagename isn’t found, CUR_IMAGE stays empty and the condition "${PREV_CDLFILE}" != *"${CUR_IMAGE}"* becomes always false, meaning the cleanup will be skipped even though /opt/cdl_flashed_file_name is missing. Please trim CUR_IMAGE and add an explicit fallback for an empty CUR_IMAGE (e.g., treat it as a cleanup-required case or derive the current image name from another source).

vrenu2018 · 2026-04-14T14:32:49Z

I have read the CLA Document and I hereby sign the CLA

nhanasi · 2026-04-14T17:57:54Z

@vrenu2018 - plesae move the script/service to meta layer or Gstreamer module as these are not maintained by platform team
cc: @shibu-kv @vinupalackal @tdeva14

shibu-kv

Historically there was a tendency to add all scripts into sysint (System Integration) repository. With RDK-E model we have already switched to controlled component ownership model. In my opinion, we should move the gstreamer service and its supporting utilities to repos or meta layers maintained by gstreamer component owners.

vrenu2018 requested a review from a team as a code owner April 13, 2026 15:53

Copilot AI review requested due to automatic review settings April 13, 2026 15:53

Copilot started reviewing on behalf of vrenu2018 April 13, 2026 15:53 View session

vrenu2018 changed the title ~~RDKEMW-10725:gstreamer-cleanup conditions when cdl_flashed_file_name …~~ RDKEMW-10725:gstreamer-cleanup conditions when cdl_flashed_file_name is missing Apr 13, 2026

Copilot AI reviewed Apr 13, 2026

View reviewed changes

Apply suggestion from @Copilot

9c9690d

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings April 13, 2026 17:27

Copilot started reviewing on behalf of vrenu2018 April 13, 2026 17:27 View session

Copilot AI reviewed Apr 13, 2026

View reviewed changes

Renuka Varry and others added 3 commits April 13, 2026 17:46

Merge branch 'develop' into RDKEMW-10725

35fc9f5

Signed-off-by: Renuka Varry <rvarry049@cable.comcast.com>

License banner addition

e43c516

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

Copilot AI review requested due to automatic review settings April 14, 2026 14:07

Copilot started reviewing on behalf of vrenu2018 April 14, 2026 14:07 View session

rdkcentral deleted a comment from Copilot AI Apr 14, 2026

Copilot AI reviewed Apr 14, 2026

View reviewed changes

shibu-kv requested changes Apr 14, 2026

View reviewed changes

	( ! -f /opt/cdl_flashed_file_name && "${PREV_CDLFILE}" != "${CUR_IMAGE}" ) \|\| \
	( ! -f /opt/cdl_flashed_file_name && ( -z "${CUR_IMAGE}" \|\| "${PREV_CDLFILE}" != "${CUR_IMAGE}" ) ) \|\| \

	ExecStop=/bin/sh -c 'FW_UPDATE_STATE=$(cat /opt/fwdnldstatus.txt \| grep FwUpdateState \| cut -d "\|" -f2); echo "FW_UPDATE_STATE: $FW_UPDATE_STATE"; if [ "$FW_UPDATE_STATE" == "Preparing to reboot" ]; then echo "Removing gstreamer registry after firmware update"; rm -rf /opt/.gstreamer; fi;'
	ExecStop=/bin/sh -c 'FW_UPDATE_STATE=$(cat /opt/fwdnldstatus.txt \| grep FwUpdateState \| cut -d "\|" -f2); echo "FW_UPDATE_STATE: $FW_UPDATE_STATE"; if [ "$FW_UPDATE_STATE" = "Preparing to reboot" ]; then echo "Removing gstreamer registry after firmware update"; rm -rf /opt/.gstreamer; fi;'

Conversation

vrenu2018 commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

vrenu2018 commented Apr 14, 2026

Uh oh!

nhanasi commented Apr 14, 2026

Uh oh!

shibu-kv left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

vrenu2018 commented Apr 13, 2026 •

edited

Loading

github-actions bot commented Apr 13, 2026 •

edited

Loading