Update task_health_monitor.sh #104
Conversation
|
I have read the CLA Document and I hereby sign the CLA You can retrigger this bot by commenting recheck in this Pull Request. Posted by the CLA Assistant Lite bot. |
Signed-off-by: Huy Nguyen <[email protected]> Signed-off-by: Mohammed Arif Mannoppilly Hassan <[email protected]> Co-authored-by: Uday Bhadauria <[email protected]> Co-authored-by: anoopchelakkode <[email protected]> Co-authored-by: Suganya-Sugumar <[email protected]> Co-authored-by: Michael Amal Anand <[email protected]> Co-authored-by: Shirish Shrivastava <[email protected]> Co-authored-by: Gowthami Kanthasamy <[email protected]> Co-authored-by: Mohammed Arif Mannoppilly Hassan <[email protected]>
There was a problem hiding this comment.
Pull request overview
This pull request updates health monitoring scripts and test diagnostics infrastructure to support additional hardware models and improve system resilience. The changes include:
Changes:
- Added support for fileUpload test functionality in the RDK automation test framework
- Enhanced self-healing daemon monitoring to support both RSA and ECC KDF types
- Added IPv6 address conflict detection and recovery mechanism in aggressive self-heal
- Extended device model support (CWA438TCOM) and refined conditional checks for bridge creation
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| source/dmltad/cosa_rdktest_dml.c | Adds fileUpload test handler following the same pattern as existing logUpload functionality |
| scripts/task_health_monitor.sh | Extends SE daemon monitoring for ECC support, adds CWA438TCOM model, and conditionally skips br403 checks for specific devices |
| scripts/selfheal_aggressive.sh | Introduces IPv6 DAD failure detection and DHCPv6 client restart recovery mechanism |
| CHANGELOG.md | Documents release 2.0.0 changes and associated pull requests |
Comments suppressed due to low confidence (1)
scripts/selfheal_aggressive.sh:1609
- The self_heal_sedaemon function in this file does not handle the ECC KDF type properly. When kdftype is "ECC", the function still uses hardcoded "startse05xd.service" for stopping and starting the daemon services (lines 1605 and 1608), instead of using the appropriate service name for ECC. This inconsistency with the task_health_monitor.sh implementation will prevent proper daemon recovery for ECC-based systems.
self_heal_sedaemon()
{
accessmgr=`pidof accessManager`
if [ "$kdftype" == "RSA" ]; then
ssadeamon=`pidof se05xd`
elif [ "$kdftype" == "ECC" ]; then
ssadeamon=`pidof rdkssaecckdf`
fi
if [[ -z "$ssadeamon" ]] || [[ -z "$accessmgr" ]]; then
echo_t "[RDKB_AGG_SELFHEAL] : Restarting accessmanager and se05xd"
t2CountNotify "SYS_SH_SERestart"
systemctl stop startse05xd.service
systemctl stop accessmanager.service
systemctl start accessmanager.service
systemctl start startse05xd.service
fi
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| self_heal_waninterface_ipv6_addressConflict() | ||
| { | ||
| if ip -6 addr show dev $WAN_INTERFACE scope global | grep -q "dadfailed"; then | ||
| echo_t "FAILURE: IPV6 address conflict found - IPV6 not usable - ! Bind & sockets failure expected !" | ||
| PROC="dibbler-client" | ||
| PID=$(ps | grep $PROC | grep -v grep | awk '{print $1}') | ||
|
|
||
| if [ -n "$PID" ]; then | ||
| Dhcpv6_Client_restart "dibbler-client" "Idle" | ||
| else | ||
| echo_t "$PROC Process not running" | ||
| fi | ||
| else | ||
| echo_t "SUCCESS: no IPV6 address conflict found - IPV6 is usable" | ||
| fi | ||
| } |
There was a problem hiding this comment.
The new function does not include any documentation or comments explaining its purpose, when it's called, or what the expected behavior is. Adding a comment block would improve maintainability and help other developers understand the IPv6 address conflict detection and recovery logic.
| fi | ||
| } | ||
|
|
||
| self_heal_waninterface_ipv6_addressConflict() |
There was a problem hiding this comment.
The function name "self_heal_waninterface_ipv6_addressConflict" doesn't follow consistent naming conventions. It should either use underscores throughout (e.g., "self_heal_wan_interface_ipv6_address_conflict") or camelCase, but not a mix. The current naming is inconsistent with other function names in the file like "self_heal_wan", "self_heal_sedaemon", etc.
| self_heal_waninterface_ipv6_addressConflict() | |
| self_heal_wan_interface_ipv6_address_conflict() |
| } | ||
| } else { | ||
| } | ||
| else if (strncasecmp(pString, "fileUpload|", 10) == 0) { |
There was a problem hiding this comment.
The prefix length check for "fileUpload|" is incorrect. The function uses strncasecmp with a length of 10, but "fileUpload|" has 11 characters. This will cause the prefix check to miss the pipe character and potentially match strings that start with "fileUpload" followed by any character at position 11.
| else if (strncasecmp(pString, "fileUpload|", 10) == 0) { | |
| else if (strncasecmp(pString, "fileUpload|", 11) == 0) { |
| } | ||
|
|
||
| self_heal_waninterface_ipv6_addressConflict() | ||
| { |
There was a problem hiding this comment.
The variable WAN_INTERFACE is used in the new function but may not be properly set if the function is called early in the loop iteration. The WAN_INTERFACE variable is set at line 1659, which is before this function call, so this should be safe. However, the function should validate that WAN_INTERFACE is not empty before attempting to use it in the ip command, as an undefined or empty interface name could cause the command to fail unexpectedly.
| { | |
| { | |
| if [ -z "$WAN_INTERFACE" ]; then | |
| echo_t "WARN: WAN_INTERFACE is not set; skipping IPv6 address conflict check" | |
| return 0 | |
| fi |
No description provided.