[PSU & system health] Support PSU power threshold checking by stephenxs · Pull Request #1060 · sonic-net/SONiC

stephenxs · 2022-08-18T09:18:11Z

PRs

PR title	state	context
[PSU] Add warning/critical thresholds for PSU power
[PSU daemon] Support PSU power threshold checking
[psushow & psuutil] Support PSU power threshold checking
[system health daemon] Support PSU power threshold checking
[Mellanox] Support PSU power threshold checking
[Mellanox] Support PSU power threshold checking

Support PSU power threshold checking

Two platform APIs are introduced to represent the warning and critical thresholds of a PSU's power
In the main loop
- Whenever a PSU becomes good, PSU daemon tries calling platform APIs to fetch both thresholds. If None is returned or NotImplemented is thrown by either API, the PSU power threshold checking will not be performed for the PSU
- the PSU daemon compares the power with thresholds, exposing status to the database, and logging messages.

Signed-off-by: Stephen Sun stephens@nvidia.com

Signed-off-by: Stephen Sun <stephens@nvidia.com>

doc/psud/PSU_daemon_design.md

Signed-off-by: Stephen Sun <stephens@nvidia.com>

liat-grozovik · 2022-09-28T07:18:59Z

@prgeor kinldy reminder to review and provide your feedback

stephenxs · 2022-10-04T01:05:57Z

Conflicts resolved.

Signed-off-by: Stephen Sun <stephens@nvidia.com>

zhangyanzhao · 2022-10-04T15:17:46Z

@stephenxs please update the https://github.com/sonic-net/sonic-utilities/blob/master/doc/Command-Reference.md with the new command line.

prgeor · 2022-11-10T22:47:36Z

@shyam77git can you review from chassis point of view?

zhangyanzhao · 2022-11-12T02:29:49Z

@prgeor the PR is approved and build passed, are you ok to merge? Thanks.

prgeor

@stephenxs as discussed in the community can we have an agreement that the "warning threshold" is confusing because no warning is raised at "warning" threshold? How about renaming it to "warning-suppress" level?

prgeor · 2022-11-12T00:51:46Z

doc/psud/PSU_daemon_design.md


 PSU number is stored in chassis table. Please refer to this [document](https://github.com/sonic-net/SONiC/blob/master/doc/pmon/pmon-enhancement-design.md), section 1.5.2.

 PSU information is stored in PSU table:


is this table for all PSUs in the system?

prgeor · 2022-11-12T00:52:25Z

doc/psud/PSU_daemon_design.md

+    power_overload            = "true" / "false"               ; whether the PSU's power exceeds the threshold
+    power_warning_threshold   = 1*4.3DIGIT                     ; The power warning threshold
+    power_critical_threshold  = 1*4.3DIGIT                     ; The power critical threshold


are these thresholds the same for all PSUs?

For us, yes. But it's possible for other vendors to have different thresholds among all PSUs.

prgeor · 2022-11-12T16:21:01Z

doc/psud/PSU_daemon_design.md

+- `Not OK` which can be caused by:
+  - power is not good, which means the PSU is present but no power (Eg. the power is down or power cable is unplugged)
+- `WARNING` which can be caused by:
+  - power exceeds the PSU's power threshold


which threshold? critical or warning?

The critical threshold. Will fix it.

prgeor · 2022-11-12T16:42:57Z

doc/psud/PSU_daemon_design.md

        raise NotImplementedError
-    ...

+    def get_psu_power_warning_threshold(self)


can we have a set API for user to override?

I do not suggest having an API to override the thresholds. As discussed before, it should be controlled by the platform vendor only because users do not have full knowledge to decide what the threshold should be. For example, in our system, the thresholds depend on temperatures as well.

Signed-off-by: Stephen Sun <stephens@nvidia.com>

prgeor · 2022-11-17T15:39:00Z

doc/system_health_monitoring/system-health-HLD.md

+	voltage_max_th           = INT                            ; max threshold of the output voltage
+	voltage_min_th           = INT                            ; min threshold of the output voltage
+	power_overload           = "true" / "false"               ; whether the PSU's power exceeds the threshold
+	power_warning_threshold  = 1*4.3DIGIT                     ; The power warning threshold


naming not changed to supress

Will fix it

prgeor · 2022-11-17T15:39:36Z

doc/system_health_monitoring/system-health-HLD.md

        orchagent is not running
 	Hardware           Fault
        PSU 1 temp 85C and threshold is 70C
+        PSU 1 power (66.32w) exceeds thresholds (warning: 60.00w critical: 70.00w)


'thresholds' which threshold?

it's hard to say which threshold is crossed because most times it exceeds the critical threshold but sometimes it can be in (warning_suppress_threshold, critical_threshold) because of hysteresis.
I think we can just put the value of warning_suppress_threshold here.

prgeor · 2022-11-17T15:40:42Z

doc/psud/PSU_daemon_design.md

+    power_warning_suppress_threshold   = 1*4.3DIGIT                     ; The power warning threshold
+    power_critical_threshold  = 1*4.3DIGIT                     ; The power critical threshold

 Now psud only collect and update "presence" and "status" field.


we need one field in the DB which indicates the critical threshold warning is ACTIVE. this is needed for telemetry

power_overload is the field to indicate whether it’s in the warning state

prgeor

@stephenxs its not clear how is the user notified? if syslog how many times syslog is raised?

Signed-off-by: Stephen Sun <stephens@nvidia.com>

Fix comments

stephenxs · 2022-11-18T00:07:06Z

@stephenxs its not clear how is the user notified? if syslog how many times syslog is raised?

syslog. it is printed only once when the alarm is raised and cleared.
PS. it's not relevant to the feature itself but is how it works in PSU daemon.

Signed-off-by: Stephen Sun <stephens@nvidia.com>

prgeor · 2022-11-18T00:46:47Z

doc/psud/PSU_daemon_design.md

+4. PSU's power was in range (warning threshold, critical threshold) and is greater than the critical threshold
+   1. if warning was raised, no action expected
+   2. if warning was not raised, a warning should be raised
+5. PSU's power was less than the warning threshold and is greater than the critical threshold: a warning should be raised


its confusing

It's to test the case that the PSU power jumps from a value below the warning suppress threshold to a value above critical threshold.

prgeor · 2022-11-18T00:56:08Z

doc/system_health_monitoring/system-health-HLD.md

+	voltage                           = INT                            ; output voltage of the PSU
+	voltage_max_th                    = INT                            ; max threshold of the output voltage
+	voltage_min_th                    = INT                            ; min threshold of the output voltage
+	power_overload                    = "true" / "false"               ; whether the PSU's power exceeds the threshold


let me check if this should be INT for telemetry

Signed-off-by: Stephen Sun <stephens@nvidia.com>

prgeor · 2022-11-18T19:52:59Z

doc/psud/PSU_daemon_design.md

+We use asymmetric thresholds between raising and clearing the alarm for the purpose of creating a hysteresis and avoiding alarm flapping.
+
+- an alarm will be raised when a PSU's power is rising accross the critical threshold
+- an alarm will be cleared when a PSU's power is dropping across the warning threshold


warning threshold -> 'warning suppress' threshold

prgeor · 2022-11-18T19:53:42Z

doc/psud/PSU_daemon_design.md

+
+1. Retrieve the current power
+2. If flag `PSU power exceeded threshold` is `true`, compare the current power against the warning threshold
+   - If `current power` < `warning threshold`


warning-suppress threshold

prgeor · 2022-11-18T19:54:06Z

doc/psud/PSU_daemon_design.md

+2. If flag `PSU power exceeded threshold` is `true`, compare the current power against the warning threshold
+   - If `current power` < `warning threshold`
+     - Set `PSU power exceeded threshold` to `false`
+     - Message in NOTICE level should be logged: `PSU <x>: current power <power> is below the warning threshold <threshold>` where


warning-suppress threshold

prgeor · 2022-11-18T19:56:35Z

doc/psud/PSU_daemon_design.md

+`psuutil` fetches the information via calling platform API directly. Both warning and critical thresholds will be exposed in the output of psuutil status.
+The "WARNING" state is not exposed because psuutil is a one-time command instead of a daemon, which means it does not store state information. It fetches information via calling platform API so it can not distinguish the following status:
+
+1. The power exceeded the critical threshold but is in the range between the warning and critical thresholds, which means the alarm should be raised
+2. The power didn't exceed the critical threshold and exceeds the warning threshold, which means the alarm should not be raised
+


warning -> 'warning-suppress'

Signed-off-by: Stephen Sun <stephens@nvidia.com>

PSU power exceeding check - initial revision

39aa11d

Signed-off-by: Stephen Sun <stephens@nvidia.com>

keboliu requested review from Junchao-Mellanox and prgeor August 23, 2022 02:20

Junchao-Mellanox previously approved these changes Aug 23, 2022

View reviewed changes

liat-grozovik reviewed Aug 24, 2022

View reviewed changes

Fix review comments

6afc6e7

Signed-off-by: Stephen Sun <stephens@nvidia.com>

stephenxs dismissed Junchao-Mellanox’s stale review via 6afc6e7 August 24, 2022 12:24

stephenxs requested review from Junchao-Mellanox and liat-grozovik August 25, 2022 11:43

stephenxs mentioned this pull request Aug 25, 2022

System health update - support checking PSU power exceeding threshold status #1061

Closed

Rename platform API

cb34e5f

Signed-off-by: Stephen Sun <stephens@nvidia.com>

Junchao-Mellanox previously approved these changes Aug 26, 2022

View reviewed changes

Update field name in state db

d8d9539

Signed-off-by: Stephen Sun <stephens@nvidia.com>

stephenxs dismissed Junchao-Mellanox’s stale review via d8d9539 August 26, 2022 12:55

stephenxs added 2 commits August 29, 2022 10:14

System health update

cb3615d

Signed-off-by: Stephen Sun <stephens@nvidia.com>

Update database field name

336c5b5

Signed-off-by: Stephen Sun <stephens@nvidia.com>

stephenxs changed the title ~~[PSU] Support PSU power threshold checking~~ [PSU & system health] Support PSU power threshold checking Aug 29, 2022

liat-grozovik previously approved these changes Aug 29, 2022

View reviewed changes

liat-grozovik added 3 commits August 29, 2022 11:16

Merge branch 'master' into psu-power-threshold-github

28e9913

Merge branch 'master' into psu-power-threshold-github

bd7e2e6

Merge branch 'master' into psu-power-threshold-github

e7398f7

Merge branch 'master' into psu-power-threshold-github

27ced8c

stephenxs dismissed liat-grozovik’s stale review via 27ced8c October 4, 2022 00:50

stephenxs requested review from Junchao-Mellanox and liat-grozovik October 4, 2022 00:53

stephenxs added 2 commits October 4, 2022 11:25

Rephrase and fix typo

97c1fae

Signed-off-by: Stephen Sun <stephens@nvidia.com>

Both critical and warning thresholds should be exposed

6f9bace

Signed-off-by: Stephen Sun <stephens@nvidia.com>

Merge branch 'master' into psu-power-threshold-github

74a5fbe

prgeor requested changes Nov 12, 2022

View reviewed changes

Fix Prince's comments

1ac1eff

Signed-off-by: Stephen Sun <stephens@nvidia.com>

stephenxs dismissed stale reviews from Junchao-Mellanox and jeffhtgt via 1ac1eff November 13, 2022 00:47

power_warning_threshold => power_warning_suppress_threshold

07a314f

Signed-off-by: Stephen Sun <stephens@nvidia.com>

prgeor reviewed Nov 17, 2022

View reviewed changes

stephenxs and others added 2 commits November 17, 2022 23:58

Update picture

9bcf22d

Signed-off-by: Stephen Sun <stephens@nvidia.com>

Fix comments

55dea13

Fix comments

Fix review comments

0993402

Signed-off-by: Stephen Sun <stephens@nvidia.com>

prgeor reviewed Nov 18, 2022

View reviewed changes

Update CLI

a0ae272

Signed-off-by: Stephen Sun <stephens@nvidia.com>

prgeor reviewed Nov 18, 2022

View reviewed changes

More warning => warning-suppress

05c5fea

Signed-off-by: Stephen Sun <stephens@nvidia.com>

prgeor approved these changes Nov 18, 2022

View reviewed changes

Merge branch 'master' into psu-power-threshold-github

0caacec

prgeor merged commit bf6fa10 into sonic-net:master Nov 18, 2022

prgeor added the PSU label Nov 18, 2022

prgeor self-assigned this Nov 18, 2022

stephenxs deleted the psu-power-threshold-github branch November 18, 2022 23:48


		PSU number is stored in chassis table. Please refer to this [document](https://github.com/sonic-net/SONiC/blob/master/doc/pmon/pmon-enhancement-design.md), section 1.5.2.

		PSU information is stored in PSU table:

Conversation

stephenxs commented Aug 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

liat-grozovik commented Sep 28, 2022

Uh oh!

stephenxs commented Oct 4, 2022

Uh oh!

zhangyanzhao commented Oct 4, 2022

Uh oh!

prgeor commented Nov 10, 2022

Uh oh!

zhangyanzhao commented Nov 12, 2022

Uh oh!

prgeor left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stephenxs Nov 13, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

stephenxs Nov 18, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

prgeor left a comment

Choose a reason for hiding this comment

Uh oh!

stephenxs commented Nov 18, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

stephenxs commented Aug 18, 2022 •

edited

Loading

stephenxs Nov 13, 2022 •

edited

Loading

stephenxs Nov 18, 2022 •

edited

Loading