Skip to content

Update 'get_crm_facts' helper function#3247

Merged
bingwang-ms merged 2 commits intosonic-net:masterfrom
bingwang-ms:update_crm_fact
Apr 1, 2021
Merged

Update 'get_crm_facts' helper function#3247
bingwang-ms merged 2 commits intosonic-net:masterfrom
bingwang-ms:update_crm_fact

Conversation

@bingwang-ms
Copy link
Copy Markdown
Collaborator

Signed-off-by: bingwang bingwang@microsoft.com

Description of PR

Summary:
The command crm show resource all is not available for some time after config reload.

admin@str2-7050cx3-acs-01:~$ crm show resources all

CRM counters are not ready. They would be populated after the polling interval.

Stage    Bind Point    Resource Name    Used Count    Available Count
-------  ------------  ---------------  ------------  -----------------


Table ID    Resource Name    Used Count    Available Count
----------  ---------------  ------------  -----------------

This PR adds a retry logic for handling this scenario.

Type of change

  • Bug fix
  • Testbed and Framework(new/improvement)
  • Test case(new/improvement)

Approach

What is the motivation for this PR?

This PR is to fix get_crm_facts for some corner cases.

How did you do it?

Add a retry logic.

How did you verify/test it?

Verified on 7050cx3-1.

Any platform specific information?

No.

Supported testbed topology if it's a new test case?

No.

Documentation

The command 'crm show resource all' is not available for some time after config reload.
This PR adds a retry logic for handling this scenario.

Signed-off-by: bingwang <bingwang@microsoft.com>
@bingwang-ms bingwang-ms requested a review from a team as a code owner March 31, 2021 08:31
@bingwang-ms bingwang-ms requested a review from wangxin March 31, 2021 08:32
logging.warning("CRM counters are not ready yet, will retry after 10 seconds")
time.sleep(10)
timeout -= 10
assert(timeout > 0)
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

assert(timeout>=0) ?

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Good catch. Thanks

Fix comment.
@bingwang-ms bingwang-ms merged commit 7e71818 into sonic-net:master Apr 1, 2021
nirmalya-keysight pushed a commit to nirmalya-keysight/sonic-mgmt that referenced this pull request Apr 5, 2021
* This PR updates 'get_crm_facts' function.

The command 'crm show resource all' is not available for some time after config reload.
This PR adds a retry logic for handling this scenario.

Signed-off-by: bingwang <bingwang@microsoft.com>
saravanansv pushed a commit to saravanansv/sonic-mgmt that referenced this pull request May 6, 2021
* This PR updates 'get_crm_facts' function.

The command 'crm show resource all' is not available for some time after config reload.
This PR adds a retry logic for handling this scenario.

Signed-off-by: bingwang <bingwang@microsoft.com>
vmittal-msft pushed a commit to vmittal-msft/sonic-mgmt that referenced this pull request Sep 28, 2021
* This PR updates 'get_crm_facts' function.

The command 'crm show resource all' is not available for some time after config reload.
This PR adds a retry logic for handling this scenario.

Signed-off-by: bingwang <bingwang@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants