Skip to content

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

Merged
jleveque merged 3 commits intosonic-net:masterfrom
Junchao-Mellanox:master-thermal-fix
Mar 25, 2020
Merged

[Mellanox] Fix thermal control issue: use natural sort for fan status and thermal status#836
jleveque merged 3 commits intosonic-net:masterfrom
Junchao-Mellanox:master-thermal-fix

Conversation

@Junchao-Mellanox
Copy link
Copy Markdown
Collaborator

@Junchao-Mellanox Junchao-Mellanox commented Mar 11, 2020

- 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
Copy Markdown
Collaborator

liat-grozovik commented Mar 16, 2020

@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
Copy Markdown
Contributor

jleveque commented Mar 16, 2020

@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
Copy Markdown
Collaborator

liat-grozovik commented Mar 17, 2020

@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
Copy Markdown
Collaborator Author

Junchao-Mellanox commented Mar 18, 2020

Fixed.

@jleveque
Copy link
Copy Markdown
Contributor

jleveque commented Mar 18, 2020

Retest this please

@Junchao-Mellanox
Copy link
Copy Markdown
Collaborator Author

Junchao-Mellanox commented Mar 25, 2020

@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

@keboliu keboliu keboliu approved these changes

+1 more reviewer

@jleveque jleveque jleveque approved these changes

Reviewers whose approvals may not affect merge requirements

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