[sysName]: Implement sysName OID #185

SuvarnaMeenakshi · 2020-12-17T23:00:44Z

[sysName]: Add sysName OID implementation in snmpagent.
sysName OID is currently supported by snmpd. Override this
implementation to make sure the latest hostname is taken
from config db.
Signed-off-by: Suvarna Meenakshi [email protected]

- What I did
Add sysName OID implementation in snmpagent as mentioned in #170

- How I did it

Add a new MIB class to support retrieving sysName from config_db
Add unit-test to test sysName OID.

- How to verify it
Bring up snmp docker with the changes.
Configure a new hostname using "config hostname <>" command.
Execute SNMP query to get 1.3.6.1.2.1.1.5.0, ensure new configured hostname is retrieved.

Testing result on sonic-vs platform:

admin@vlab-01:~$ sudo config hostname test
Running command: service hostname-config restart
Reloading Monit configuration ...
Reinitializing monit daemon
Please note loaded setting will be lost after system reboot. To preserve setting, run `config save`.
admin@vlab-01:~$ date
Thu 24 Dec 2020 02:30:04 AM UTC

admin@vlab-01:~$ docker exec -it snmp snmpwalk -v2c -c public 127.0.0.1 1.3.6.1.2.1.1.5.0
iso.3.6.1.2.1.1.5.0 = STRING: "vlab-01"
admin@vlab-01:~$ docker exec -it snmp snmpwalk -v2c -c public 127.0.0.1 1.3.6.1.2.1.1.5.0
iso.3.6.1.2.1.1.5.0 = STRING: "test"
admin@vlab-01:~$ date
Thu 24 Dec 2020 02:30:48 AM UTC

- Description for the changelog

sysName OID is currently supported by snmpd. Override this implementation to make sure the latest hostname is taken from config db. Signed-off-by: Suvarna Meenakshi <[email protected]>

qiluo-msft · 2020-12-18T03:25:21Z

src/sonic_ax_impl/mibs/ietf/rfc1213.py

+        """
+        self.db_conn.connect(mibs.CONFIG_DB)
+
+        device_metadata = self.db_conn.get_all(self.db_conn.CONFIG_DB, "DEVICE_METADATA|localhost")


device_metadata [](start = 8, length = 15)

~~I notice PhysicalTableMIBUpdater.reinit_data() fetch DEVICE_METADATA periodically. Could you reuse that?~~
For device name changing case, I think the latency is good enough. No need to query Redis on each query. #Closed

That data is coming from STATE_DB and not CONFIG_DB.

Modified implementation to get hostname in reinit_data() function which gets invoked every minute.
Also, using hostname from system as default hostname during intialization.

qiluo-msft

As comments

in reinit_data. Update unit-test cases accordingly. Signed-off-by: Suvarna Meenakshi <[email protected]>

there is an error when reading from config_db the system hostname is used. Signed-off-by: Suvarna Meenakshi <[email protected]>

qiluo-msft · 2020-12-23T23:27:51Z

src/sonic_ax_impl/mibs/ietf/rfc1213.py

+
+    def reinit_data(self):
+        self.db_conn.connect(self.db_conn.CONFIG_DB)
+        device_metadata = self.db_conn.get_all(self.db_conn.CONFIG_DB, "DEVICE_METADATA|localhost")


get_all [](start = 39, length = 7)

Did you test the empty table on DUT? I think device_metadata will be an empty dict instead of None.
Suggest check device_metadata instead of device_metadata is not None below. #Closed

Alternatively, you can use get function to get hostname directly.

In reply to: 548318135 [](ancestors = 548318135)

If DEVICE_METADATA table is not present, then get_all returns None.
Updated code as per suggestion and tested on sonic-vs image.
#Resolved

I am okay with the code. However I am confused by your comment "If DEVICE_METADATA table is not present, then get_all returns None".
Could you double check?

In reply to: 548356652 [](ancestors = 548356652)

On a device, to check this behavior, I tried the below commands; here "DEVICE_METADATA" exists, without DEVICE_METADATA in config_db, hwsku will not be known so dockers will not come up. So instead of checking directly for "DEVICE_METADATA", I checked for some modified fields that will not exist.

device_metadata = db_conn.get_all(db_conn.CONFIG_DB, "DEVICE_METADATA|loca")
print(device_metadata)
None
device_metadata = db_conn.get_all(db_conn.CONFIG_DB, "DEVICE_METADAT")
print(device_metadata)
None
...
hostname=device_metadata.get(b'hos')
print(hostname)
None

The above comment does not hold good for SNMP docker with master branch with latest changes.
If a specific key does not exist in the table, A SWIG object is returned and None is not returned.
If key does not exist:

>>> from sonic_ax_impl import mibs db_conn.connect(db_conn.CONFIG_DB) >>> device_metadata = db_conn.get_all(db_conn.CONFIG_DB, "DEVICE_METADATA|loca") >>> print(device_metadata) <swsscommon.swsscommon.FieldValueMap; proxy of <Swig Object of type 'swss::TableMap *' at 0x7f291e85a450> >

If DEVICE_METADATA exists, but key within DEVICE_METADATA table does not exist:

>>> device_metadata = db_conn.get_all(db_conn.CONFIG_DB, "DEVICE_METADATA|localhost") >>> print(device_metadata) <swsscommon.swsscommon.FieldValueMap; proxy of <Swig Object of type 'swss::TableMap *' at 0x7f291e85ab70> > >>> print(device_metadata.get('foo')) None

Signed-off-by: Suvarna Meenakshi <[email protected]>

qiluo-msft · 2020-12-30T02:32:26Z

src/sonic_ax_impl/mibs/ietf/rfc1213.py

+class SysNameMIB(metaclass=MIBMeta, prefix='.1.3.6.1.2.1.1.5'):
+    """
+
+    """


Remove the unused lines? or add some useful comment? #Closed

Removed lines.

qiluo-msft · 2020-12-30T02:33:53Z

tests/mock_tables/global_db/config_db.json

@@ -14,5 +14,9 @@
    "admin_status": "up",
    "alias": "mgmt2",
    "speed": 1000
+  },
+ "DEVICE_METADATA|localhost": {


[](start = 0, length = 1)

wrong indentation. #Closed

Fixed indendation.

qiluo-msft

Notice minor issues.

Signed-off-by: Suvarna Meenakshi <[email protected]>

[sysName]: Add sysName OID implementation in snmpagent.

4d5d900

sysName OID is currently supported by snmpd. Override this implementation to make sure the latest hostname is taken from config db. Signed-off-by: Suvarna Meenakshi <[email protected]>

SuvarnaMeenakshi requested a review from qiluo-msft December 17, 2020 23:00

qiluo-msft reviewed Dec 18, 2020

View reviewed changes

SuvarnaMeenakshi added 2 commits December 23, 2020 03:10

Update sysNameMIB implementation to get hostname periodically

6762a72

in reinit_data. Update unit-test cases accordingly. Signed-off-by: Suvarna Meenakshi <[email protected]>

Set default hostname to system hostname so that in case

5ef3567

there is an error when reading from config_db the system hostname is used. Signed-off-by: Suvarna Meenakshi <[email protected]>

qiluo-msft reviewed Dec 23, 2020

View reviewed changes

Minor comments as per review comments.

1258c9b

Signed-off-by: Suvarna Meenakshi <[email protected]>

qiluo-msft previously approved these changes Dec 24, 2020

View reviewed changes

sonic-net deleted a comment from svc-acs Dec 26, 2020

qiluo-msft reviewed Dec 30, 2020

View reviewed changes

Minor fix as per review comment.

28a644d

Signed-off-by: Suvarna Meenakshi <[email protected]>

SuvarnaMeenakshi dismissed qiluo-msft’s stale review via 28a644d December 30, 2020 02:49

qiluo-msft approved these changes Dec 30, 2020

View reviewed changes

SuvarnaMeenakshi merged commit 4aad821 into sonic-net:master Dec 30, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[sysName]: Implement sysName OID #185

[sysName]: Implement sysName OID #185

SuvarnaMeenakshi commented Dec 17, 2020 •

edited

Loading

qiluo-msft Dec 18, 2020 •

edited

Loading

SuvarnaMeenakshi Dec 18, 2020

SuvarnaMeenakshi Dec 23, 2020

qiluo-msft left a comment

qiluo-msft Dec 23, 2020 •

edited

Loading

qiluo-msft Dec 23, 2020

SuvarnaMeenakshi Dec 24, 2020 •

edited by qiluo-msft

Loading

qiluo-msft Dec 26, 2020

SuvarnaMeenakshi Dec 28, 2020

SuvarnaMeenakshi Dec 29, 2020

qiluo-msft Dec 30, 2020 •

edited

Loading

SuvarnaMeenakshi Dec 30, 2020

qiluo-msft Dec 30, 2020 •

edited

Loading

SuvarnaMeenakshi Dec 30, 2020

qiluo-msft left a comment

[sysName]: Implement sysName OID #185

[sysName]: Implement sysName OID #185

Conversation

SuvarnaMeenakshi commented Dec 17, 2020 • edited Loading

qiluo-msft Dec 18, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft left a comment

Choose a reason for hiding this comment

qiluo-msft Dec 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SuvarnaMeenakshi Dec 24, 2020 • edited by qiluo-msft Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Dec 30, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft Dec 30, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qiluo-msft left a comment

Choose a reason for hiding this comment

SuvarnaMeenakshi commented Dec 17, 2020 •

edited

Loading

qiluo-msft Dec 18, 2020 •

edited

Loading

qiluo-msft Dec 23, 2020 •

edited

Loading

SuvarnaMeenakshi Dec 24, 2020 •

edited by qiluo-msft

Loading

qiluo-msft Dec 30, 2020 •

edited

Loading

qiluo-msft Dec 30, 2020 •

edited

Loading