Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[Mellanox] Fix thermal control issue: use natural sort for fan status and thermal status #836

Merged
merged 3 commits into from
Mar 25, 2020
Merged

[Mellanox] Fix thermal control issue: use natural sort for fan status and thermal status #836

merged 3 commits into from
Mar 25, 2020

Conversation

Junchao-Mellanox
Copy link
Collaborator

- What I did

  1. Display command output "show platform fan" and "show platform temperature" in natural order
  2. Simplify the header of "show platform temperature"

- How I did it

  1. Use natsort.natsorted instead of python default sort
  2. Change header if "show platform temperature"

- How to verify it

Manually verified.

- Previous command output (if the output of a command-line utility has changed)

root@r-qa-sw-eth-2037:/home/admin# show platform temperature 
                  NAME    Temperature    High Threshold    Low Threshold    Critical High Threshold    Critical Low Threshold    Warning Status          Timestamp
----------------------  -------------  ----------------  ---------------  -------------------------  ------------------------  ----------------  -----------------
     Ambient ASIC Temp         39                                    N/A                                                  N/A             False  20200218 15:43:38
    Ambient COMEX Temp         26.25                                 N/A                                                  N/A             False  20200218 15:43:38
 Ambient Fan Side Temp         22.437                                N/A                                                  N/A             False  20200218 15:43:38
Ambient Port Side Temp         28.875                                N/A                                                  N/A             False  20200218 15:43:38
       CPU Core 0 Temp         33                    82              N/A                        104                       N/A             False  20200218 15:44:34
       CPU Core 1 Temp         33                    82              N/A                        104                       N/A             False  20200218 15:44:34
       CPU Core 2 Temp         33                    82              N/A                        104                       N/A             False  20200218 15:44:34
       CPU Core 3 Temp         33                    82              N/A                        104                       N/A             False  20200218 15:44:34
         CPU Pack Temp         33                    82              N/A                        104                       N/A             False  20200218 15:44:34
        Gearbox 1 Temp                                               N/A                                                  N/A             False  20200218 15:43:38
       Gearbox 10 Temp                                               N/A                                                  N/A             False  20200218 15:43:38
        Gearbox 2 Temp                                               N/A                                                  N/A             False  20200218 15:43:38
        Gearbox 3 Temp                                               N/A                                                  N/A             False  20200218 15:43:38
        Gearbox 4 Temp                                               N/A                                                  N/A             False  20200218 15:43:38
        Gearbox 5 Temp                                               N/A                                                  N/A             False  20200218 15:43:38

- New command output (if the output of a command-line utility has changed)

admin@r-tigris-06:~$ show platform temperature
                  NAME    Temperature    High Th    Low Th    Crit High Th    Crit Low Th    Warning          Timestamp
----------------------  -------------  ---------  --------  --------------  -------------  ---------  -----------------
     Ambient ASIC Temp           46.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
    Ambient COMEX Temp           29.5        N/A       N/A             N/A            N/A      False  20200221 11:54:47
 Ambient Fan Side Temp         27.062        N/A       N/A             N/A            N/A      False  20200221 11:54:47
Ambient Port Side Temp         32.062        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       CPU Core 0 Temp           38.0       82.0       N/A           104.0            N/A      False  20200221 11:54:47
       CPU Core 1 Temp           38.0       82.0       N/A           104.0            N/A      False  20200221 11:54:47
       CPU Core 2 Temp           38.0       82.0       N/A           104.0            N/A      False  20200221 11:54:47
       CPU Core 3 Temp           38.0       82.0       N/A           104.0            N/A      False  20200221 11:54:47
         CPU Pack Temp           38.0       82.0       N/A           104.0            N/A      False  20200221 11:54:47
        Gearbox 1 Temp           50.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
        Gearbox 2 Temp           49.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
        Gearbox 3 Temp           55.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
        Gearbox 4 Temp           50.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
        Gearbox 5 Temp           50.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
        Gearbox 6 Temp           47.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
        Gearbox 7 Temp           50.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
        Gearbox 8 Temp           55.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
        Gearbox 9 Temp           50.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 10 Temp           58.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 11 Temp           49.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 12 Temp           53.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 13 Temp           58.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 14 Temp           56.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 15 Temp           51.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 16 Temp           55.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 17 Temp           52.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 18 Temp           54.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 19 Temp           60.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 20 Temp           53.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 21 Temp           54.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 22 Temp           51.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 23 Temp           48.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 24 Temp           58.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 25 Temp           54.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 26 Temp           58.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 27 Temp           48.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 28 Temp           55.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 29 Temp           49.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 30 Temp           49.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 31 Temp           47.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
       Gearbox 32 Temp           45.0        N/A       N/A             N/A            N/A      False  20200221 11:54:47
            PSU-1 Temp            N/A        N/A       N/A             N/A            N/A      False  20200221 11:54:47
            PSU-2 Temp          28.25       60.0       N/A             N/A            N/A      False  20200221 11:54:47

@liat-grozovik
Copy link
Collaborator

@jleveque can you please check if the output looks ok to you?
i think

  • 'NAME' should be changed to 'Component Name'
  • align to left each column at least the Name column

@jleveque
Copy link
Contributor

@liat-grozovik: I agree that 'NAME' should be changed, but maybe it should be changed to 'Sensor' or 'Temperature' rather than 'Component Name'.

I am OK with everything being right-aligned.

However, I also think the column names should abbreviate 'threshold' in all caps ('TH') rather than 'Th'.

@liat-grozovik
Copy link
Collaborator

@liat-grozovik: I agree that 'NAME' should be changed, but maybe it should be changed to 'Sensor' or 'Temperature' rather than 'Component Name'.

I am OK with everything being right-aligned.

However, I also think the column names should abbreviate 'threshold' in all caps ('TH') rather than 'Th'.

I agree. @Junchao-Mellanox please align the above based on Joe comments.

@Junchao-Mellanox
Copy link
Collaborator Author

Fixed.

@jleveque
Copy link
Contributor

Retest this please

@Junchao-Mellanox
Copy link
Collaborator Author

@kebol could you please help review?

@jleveque jleveque merged commit 6f51428 into sonic-net:master Mar 25, 2020
@Junchao-Mellanox Junchao-Mellanox deleted the master-thermal-fix branch May 7, 2020 06:03
Junchao-Mellanox added a commit to Junchao-Mellanox/sonic-utilities that referenced this pull request Jun 1, 2020
@Junchao-Mellanox
[Mellanox] Fix thermal control issue: use natural sort for fan status…
d161073 
… and thermal status (sonic-net#836)

* [thermal fix] use natural sort for fan status and thermal status
* [thermal fix] set fan status to N/A when fan is removed
* Adjust header name for show platform temperature output
abdosi pushed a commit that referenced this pull request Jun 3, 2020
@Junchao-Mellanox @abdosi
[Mellanox] Fix thermal control issue: use natural sort for fan status…
6a0a3be 
… and thermal status (#836)

* [thermal fix] use natural sort for fan status and thermal status
* [thermal fix] set fan status to N/A when fan is removed
* Adjust header name for show platform temperature output
abdosi pushed a commit to abdosi/sonic-utilities that referenced this pull request Aug 4, 2020
@Junchao-Mellanox @abdosi
[Mellanox] Fix thermal control issue: use natural sort for fan status…
14269f8 
… and thermal status (sonic-net#836)

* [thermal fix] use natural sort for fan status and thermal status
* [thermal fix] set fan status to N/A when fan is removed
* Adjust header name for show platform temperature output
stepanblyschak pushed a commit to stepanblyschak/sonic-utilities that referenced this pull request Apr 28, 2022
@abdosi
[Submodule update] sonic-util
007eeec 
Make sure db_migrator is run after all config are loaded during (sonic-net#926)
Vnet alias mapping (sonic-net#924)
Changes to make lldp show command for multi-npu platforms. (sonic-net#914)
[Mellanox] Fix thermal control issue: use natural sort for fan
status and thermal status (sonic-net#836)
[Mellanox] add document for thermal control related cli (sonic-net#832)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jleveque jleveque jleveque approved these changes

@keboliu keboliu keboliu approved these changes

Assignees
No one assigned
Labels
Included in 201911 Branch Request for 201911 Branch
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Junchao-Mellanox @liat-grozovik @jleveque @keboliu @abdosi