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

[snmp] Allow system with no ports in config db run without errors #221

Merged
merged 6 commits into from
Sep 17, 2021

Conversation

liorghub
Copy link
Contributor

@liorghub liorghub commented Jun 28, 2021

What I did
Allow system with no ports in config db run without errors.
This is needed for modular system which should boot properly without line cards.

How I did it
Remove snmpagent error exit if there are no ports in config DB or in counters DB.

How to verify it
Run snmpwalk on the root oid.

Important
This PR must be merged only after PR sonic-net/sonic-py-swsssdk#109 is merged.

@lgtm-com
Copy link

lgtm-com bot commented Jul 25, 2021

This pull request introduces 1 alert when merging 49188ec into 21d7d97 - view on LGTM.com

new alerts:

  • 1 for Wrong name for an argument in a call

@liorghub
Copy link
Contributor Author

liorghub commented Jul 25, 2021

  • Wrong name for an argument in a call

It is failing since in this commit we call function get_interface_oid_map in its new form.
The signature of function get_interface_oid_map was modified by me (hence the failure) in PR sonic-net/sonic-py-swsssdk#109.
Therefore PR sonic-net/sonic-py-swsssdk#109 should be merged before this PR.

@liorghub
Copy link
Contributor Author

Important
This PR should be merged only after PR sonic-net/sonic-py-swsssdk#109 is merged.

@liorghub
Copy link
Contributor Author

liorghub commented Aug 11, 2021

@qiluo-msft checkers are failing from the same reason I stated above.

It is failing since in this commit we call function get_interface_oid_map in its new form.
The signature of get_interface_oid_map was modified by me in PR sonic-net/sonic-py-swsssdk#109 (hence the failure).
Therefore PR sonic-net/sonic-py-swsssdk#109 must be merged before we can rerun the checkers for this PR.

@@ -30,3 +35,16 @@ def test_init_sync_d_lag_tables(self):

self.assertTrue("PortChannel_Temp" in lag_name_if_name_map)
self.assertTrue(lag_name_if_name_map["PortChannel_Temp"] == [])

@mock.patch('swsssdk.dbconnector.SonicV2Connector.get_all', mock.MagicMock(return_value=({})))
Copy link
Contributor

@qiluo-msft qiluo-msft Aug 17, 2021

Choose a reason for hiding this comment

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

swsssdk

swsssdk or swsscommon? #Closed

Copy link
Contributor

Choose a reason for hiding this comment

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

Did you run python3 -m pytest -s -v -k test_init_sync_d_interface_tables ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

swsssdk is the right one (method get_all is part of class SonicV2Connector which is implemented in sonic-buildimage/sonic-py-swsssdk/src/swsssdk/dbconnector.py).
Yes, I ran python3 -m pytest -s -v -k test_init_sync_d_interface_tables.

qiluo-msft
qiluo-msft previously approved these changes Aug 17, 2021
@qiluo-msft
Copy link
Contributor

Could you solve the conflicts?

@liorghub
Copy link
Contributor Author

Could you solve the conflicts?

@qiluo-msft
Hi Qi, there is no conflict.
In this commit, we call function get_interface_oid_map in its new form.
The signature of function get_interface_oid_map was modified by me in PR sonic-net/sonic-py-swsssdk#109.
Therefore PR sonic-net/sonic-py-swsssdk#109 must be merged before this PR.

@qiluo-msft
Copy link
Contributor

This is the conflict github reported:
image

qiluo-msft pushed a commit to sonic-net/sonic-py-swsssdk that referenced this pull request Sep 1, 2021
#109)

**What I did**
Allow system with no ports in config db to run without errors/warnings. 
This is needed for modular system which should boot and run properly without line cards.

**How I did it**
Check if table exists before calling get_all for it and return empty list if there are no ports in COUNTERS_PORT_NAME_MAP table in counters db.

**How to verify it**
Run snmpwalk on the root oid.

**Important**
This PR must be merged before PR sonic-net/sonic-snmpagent#221 is merged.
@liorghub
Copy link
Contributor Author

liorghub commented Sep 5, 2021

This is the conflict github reported:
image

@qiluo-msft I resolved the conflict, thanks for the heads up.

@liorghub
Copy link
Contributor Author

liorghub commented Sep 6, 2021

@qiluo-msft
Tests are failing due to the following issue:

if_name_map_util, if_id_map_util = port_util.get_interface_oid_map(db_conn, blocking=False)
TypeError: get_interface_oid_map() got an unexpected keyword argument 'blocking'

PR sonic-net/sonic-py-swsssdk#109 contains the required change in get_interface_oid_map() and it is merged, however tests ran with the old version of the function, do you know why?

@qiluo-msft
Copy link
Contributor

PR Azure/sonic-py-swsssdk#109 contains the required change in get_interface_oid_map() and it is merged, however tests ran with the old version of the function, do you know why?

You need to move submodule in sonic-buildimage repo.

@liorghub
Copy link
Contributor Author

liorghub commented Sep 7, 2021

PR Azure/sonic-py-swsssdk#109 contains the required change in get_interface_oid_map() and it is merged, however tests ran with the old version of the function, do you know why?

You need to move submodule in sonic-buildimage repo.

@qiluo-msft
Thanks for the clarification.
Submodule update PR - sonic-net/sonic-buildimage#8696

@liat-grozovik
Copy link
Collaborator

/azp run

@azure-pipelines
Copy link

Commenter does not have sufficient privileges for PR 221 in repo Azure/sonic-snmpagent

@liorghub
Copy link
Contributor Author

@liat-grozovik
We should rerun checkers only after I will update the hash of sonic-py-swsssdk submodule in sonic-buildimage.
I will update the hash right after sonic-net/sonic-py-swsssdk#114 is merged.

@qiluo-msft
Copy link
Contributor

Please update the hash of sonic-py-swsssdk submodule in sonic-buildimage.

@liorghub
Copy link
Contributor Author

liorghub commented Sep 16, 2021

@qiluo-msft sonic-buildimage is updated with the latest sonic-py-swsssdk hash (see sonic-net/sonic-buildimage#8757) can you please rerun the checkers?

@qiluo-msft
Copy link
Contributor

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@qiluo-msft
Copy link
Contributor

PR owner could retrigger checkers by "/azpw run"

@qiluo-msft qiluo-msft merged commit c2d4945 into sonic-net:master Sep 17, 2021
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.

3 participants