-
Notifications
You must be signed in to change notification settings - Fork 1.6k
[Accton] Support system health monitoring on pddf implemented models #8198
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Accton] Support system health monitoring on pddf implemented models #8198
Conversation
1. Implement SysLED on chassis. 2. Add config to system health monitor. Signed-off-by: Sean Wu <[email protected]>
1. Workaround to PddfFan.get_target_speed(), where always returns 0 for PSU's fan. NOTE: No fans get enumerated by thermalctld because PDDF hasn't implemented FanDrawer yet. Signed-off-by: Sean Wu <[email protected]>
Enable thermalctld for more HW checks. Signed-off-by: Sean Wu <[email protected]>
Enable thermalctld for more HW checks Signed-off-by: Sean Wu <[email protected]>
1. Enable thermalctld for more HW checks 2. Correct LED attr_name in pddf-device.json Signed-off-by: Sean Wu <[email protected]>
Models: AS7326-56X, AS7726-32X, AS9716-32D Signed-off-by: Sean Wu <[email protected]>
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
/azpw run |
/AzurePipelines run |
Azure Pipelines successfully started running 1 pipeline(s). |
@@ -46,3 +46,9 @@ def get_direction(self): | |||
|
|||
return direction | |||
|
|||
def get_target_speed(self): | |||
if self.is_psu_fan: | |||
raise NotImplementedError # Target speed not supported for PSU fans |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@seanwu-ec
Do you think its better to implement this is the common pddf_fan.py ?
# git diff platform/pddf/platform-api-pddf-base/sonic_platform_pddf_base/pddf_fan.py
diff --git a/platform/pddf/platform-api-pddf-base/sonic_platform_pddf_base/pddf_fan.py b/platform/pddf/platform-api-pddf-base/sonic_platform_pddf_base/pddf_fan.py
index 30be0dd74..d272d40e6 100755
--- a/platform/pddf/platform-api-pddf-base/sonic_platform_pddf_base/pddf_fan.py
+++ b/platform/pddf/platform-api-pddf-base/sonic_platform_pddf_base/pddf_fan.py
@@ -224,7 +224,7 @@ class PddfFan(FanBase):
target_speed = 0
if self.is_psu_fan:
# Target speed not usually supported for PSU fans
- target_speed = 0
+ raise NotImplementedError
else:
idx = (self.fantray_index-1)*self.platform['num_fans_pertray'] + self.fan_index
attr = "fan" + str(idx) + "_pwm"
#
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, because PSU doesn't reveal target fan speed. So I think it's better pddf_fan.py implement this.
I've raised an issue for that sometime before: #8129
In order to pass the fan status check at https://github.com/Azure/sonic-platform-daemons/blob/master/sonic-thermalctld/scripts/thermalctld#L341, according to the try_get() in thermalctld, there are two ways of doing this. One is raise NotImplementedError
another one is return None
. You can go whichever you like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @seanwu-ec
I added the fix in common PDDF APIs as part of PR (#7834)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can remove those changes from individual platform's fan.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done. 😊 Thanks, @FuzailBrcm
* Have moved that implementation to PR#7834 Signed-off-by: Sean Wu <[email protected]>
some codes were merged by PR#10053, PR#9982 and PR#8164. so close this PR. |
Why I did it
To support SystemHealthMonitoring feature on Accton's models (which have implemented PDDF APIs).
How I did it
How to verify it
Use
show system-health detail
and check if all desired checks pass, and confirm if systemLED works reasonably.Which release branch to backport (provide reason below if selected)
Description for the changelog
[Accton] Support system health monitoring on pddf implemented models
Models: AS9716-32D, AS7326-56X, AS7726-32X, AS7816-64X
A picture of a cute animal (not mandatory but encouraged)