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.

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

PSUD-Delete or update CHASSIS_INFO table PSU/Modules data if added or… #351

Merged
merged 3 commits into from
May 5, 2023

Conversation

prem-nokia
Copy link
Contributor

@prem-nokia prem-nokia commented Apr 17, 2023

MSFT ADO: 24297786

Description

Code changes are implemented to update or delete entry in CHASSIS_INFO table if the PSU/Fan/Modules are added or removed.

Motivation and Context

Dynamically deletes or updates the tables and gives the updated data in real time values.

admin@chassis:~$ sonic-db-dump -n STATE_DB -y -k "*power*"

How Has This Been Tested?

Run the below commands
1.Initial state.

admin@chassis:~$ sonic-db-dump -n STATE_DB -y -k "*power*"
{
    "CHASSIS_INFO|chassis_power_budget 1": {
      "expireat": 1680208471.0072632,
      "ttl": -0.001,
      "type": "hash",
      "value": {
        "": "",
        "Consumed Power FABRIC-CARD3": "415.0",
        "Consumed Power FanTray0": "500.0",
        "Consumed Power FanTray1": "500.0",
        "Consumed Power FanTray2": "500.0",
        "Consumed Power FanTray3": "500.0",
        "Consumed Power LINE-CARD0": "1000.0",
        "Consumed Power LINE-CARD1": "1000.0",
        "Consumed Power SUPERVISOR0": "80.0",
        "Supplied Power PSU1": "3000.0",
        "Supplied Power PSU2": "3000.0",
        "Supplied Power PSU3": "3000.0",
        "Total Consumed Power": "4495.0",
        "Total Supplied Power": "9000.0"
      }
    }
}

2.After PSU, Fan and line card removed.

admin@chassis:~$ sonic-db-dump -n STATE_DB -y -k "*power*"
{
    "CHASSIS_INFO|chassis_power_budget 1": {
      "expireat": 1680208471.0072632,
      "ttl": -0.001,
      "type": "hash",
      "value": {
        "": "",
        "Consumed Power FABRIC-CARD3": "415.0",
        "Consumed Power FanTray1": "500.0",
        "Consumed Power FanTray2": "500.0",
        "Consumed Power FanTray3": "500.0",
        "Consumed Power LINE-CARD0": "1000.0",
        "Consumed Power SUPERVISOR0": "80.0",
        "Supplied Power PSU2": "3000.0",
        "Supplied Power PSU3": "3000.0",
        "Total Consumed Power": "2995.0",
        "Total Supplied Power": "6000.0"
      }
    }
}

Additional Information (Optional)

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Apr 17, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

… removed.

Signed-off-by: prem-nokia <premnath.saravanan@nokia.com>
@prem-nokia prem-nokia force-pushed the psud_chassis_info_table_update branch 2 times, most recently from 8e7c1f7 to d60990f Compare April 19, 2023 17:23
@mlok-nokia
Copy link
Contributor

@prgeor @abdosi @kenneth-arista @rlhui @arlakshm Hi, Friendly reminder. Please help to review this PR. Thanks.

@kenneth-arista
Copy link

The changes look reasonable to me. Do you have test cases to simulate removal of PSUs, fans and/or linecards?

@prem-nokia
Copy link
Contributor Author

The changes look reasonable to me. Do you have test cases to simulate removal of PSUs, fans and/or linecards?

No, but the new test implementation will always be executed during the build test, irrespective of additional or removal of PSUs, fans and/or linecards.

@prgeor prgeor added the psud label May 2, 2023
Comment on lines 208 to 214
name = try_get(psu.get_name, 'PSU {}'.format(index + 1))
chassis_tbl.hdel(CHASSIS_INFO_POWER_KEY_TEMPLATE.format(1), CHASSIS_INFO_POWER_SUPPLIER_FIELD.format(name))
continue

power_good = try_get(psu.get_powergood_status)
if not power_good:
name = try_get(psu.get_name, 'PSU {}'.format(index + 1))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we retrieve the name outside instead of repeating in the same block?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@prgeor, Hi Prince, This comment has been addressed. Please review it. Thanks

Comment on lines 229 to 233
name = try_get(power_consumer.get_name, 'FAN-DRAWER {}'.format(index))
chassis_tbl.hdel(CHASSIS_INFO_POWER_KEY_TEMPLATE.format(1), CHASSIS_INFO_POWER_CONSUMER_FIELD.format(name))
continue

name = try_get(power_consumer.get_name, 'FAN-DRAWER {}'.format(index))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we retrieve the name outside the block so that it can be reused ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes we could do that, it makes the script more understandable, but "name" is not reused in the for loop and either way it will be retrieved only once (either it is retrieved inside the if statements or outside).

@prgeor
Copy link
Collaborator

prgeor commented May 2, 2023

@prem-nokia please check comments

@kenneth-arista
Copy link

The changes look reasonable to me. Do you have test cases to simulate removal of PSUs, fans and/or linecards?

No, but the new test implementation will always be executed during the build test, irrespective of additional or removal of PSUs, fans and/or linecards.

Maybe I am misunderstanding the code changes to swsscommon.py in tests/mocked_libs/swsscommon. The test change in this PR adds the hdel mock method. I believe It would be good to have a test to exercise the exception logic added in this PR; e.g.

(a) when the PSU is missing
(b) when the power_good status is False
(c) when the fan is missing.

Signed-off-by: premsara <premnath.saravanan@nokia.com>
@prgeor prgeor merged commit f743d7c into sonic-net:master May 5, 2023
3 checks passed
@mlok-nokia
Copy link
Contributor

@prgeor This change is also required for branch 202205

@rlhui
Copy link

rlhui commented Jun 1, 2023

@prgeor would you please create an msft ado for this for cherry pick? thanks.

@prgeor
Copy link
Collaborator

prgeor commented Jun 8, 2023

@yxieca please help cherry-pick

@prgeor
Copy link
Collaborator

prgeor commented Jun 15, 2023

@abdosi @rlhui please create ADO for this if needed in 202205

@abdosi
Copy link
Contributor

abdosi commented Jun 15, 2023

@gechiang can you please help create Microsoft ADO for this so that we can pick into 202205.

@gechiang
Copy link
Contributor

@gechiang can you please help create Microsoft ADO for this so that we can pick into 202205.

@abdosi , @yxieca , ADO created.

@abdosi
Copy link
Contributor

abdosi commented Jun 23, 2023

@gechiang it is there in 202205 ?

@gechiang
Copy link
Contributor

@abdosi . no this is not yet picked up in 202205.
@yxieca , Saw you added label indicating it was picked up but later removed? This is needed for 202205. Thanks!

@rlhui rlhui added the P0 label Jun 23, 2023
@gechiang
Copy link
Contributor

@yxieca MSFT ADO: 24297786

yxieca pushed a commit that referenced this pull request Jun 23, 2023
#351)

* PSUD-Delete or update CHASSIS_INFO table PSU/Modules data if added or removed.

Signed-off-by: prem-nokia <premnath.saravanan@nokia.com>

* fix swsscommon.py hdel function.

* implemented code optimization.

Signed-off-by: premsara <premnath.saravanan@nokia.com>

---------

Signed-off-by: prem-nokia <premnath.saravanan@nokia.com>
Signed-off-by: premsara <premnath.saravanan@nokia.com>
@gechiang
Copy link
Contributor

@prem-nokia , can you clarify if this change was tested with an image that is 202205 based? How was it tested? Did you physically removed the LC/FAN and see your change takes place without seeing the change causing other issues?
We need this info to decide if it is safe to take into 202205 branch.

yxieca pushed a commit that referenced this pull request Jun 23, 2023
#351)

* PSUD-Delete or update CHASSIS_INFO table PSU/Modules data if added or removed.

Signed-off-by: prem-nokia <premnath.saravanan@nokia.com>

* fix swsscommon.py hdel function.

* implemented code optimization.

Signed-off-by: premsara <premnath.saravanan@nokia.com>

---------

Signed-off-by: prem-nokia <premnath.saravanan@nokia.com>
Signed-off-by: premsara <premnath.saravanan@nokia.com>
@mlok-nokia
Copy link
Contributor

@prem-nokia , can you clarify if this change was tested with an image that is 202205 based? How was it tested? Did you physically removed the LC/FAN and see your change takes place without seeing the change causing other issues? We need this info to decide if it is safe to take into 202205 branch.

This change has been tested on both 202205 and master with physically removing Linecard, Fan and Psu. Related issue was found on 202205 originally. Thanks for the following up.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

None yet

8 participants