Skip to content

Commit

Permalink
[db_migrator] Remove hardcoded config and migrate config from minigra…
Browse files Browse the repository at this point in the history
…ph (sonic-net#2887)

Microsoft ADO: 18217044

This PR is follow up to an old PR: sonic-net#2515

This PR addresses two issues:

Not migrating RESTAPI, TELEMETRY, DEVICE_METADATA entries if they are not supported in target image's minigraph.py.
Not migrating these entries if minigraph.xml file is missing.
Not maintaining the config for these tables in two different places. Currently db migrator has its own constants, and minigraph.py maintains its own.
How I did it
This change removes hardcoding config in migrator code, and migrating the config for RESTAPI, TELEMETRY, DEVICE_METADATA from minigraph generator.

How to verify it
Tested on a physical device.
  • Loading branch information
vaibhavhd authored and pdhruv-marvell committed Aug 23, 2023
1 parent 26df5d2 commit 85d326d
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 45 deletions.
40 changes: 32 additions & 8 deletions scripts/db_migrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@

from sonic_py_common import device_info, logger
from swsscommon.swsscommon import SonicV2Connector, ConfigDBConnector, SonicDBConfig
from db_migrator_constants import RESTAPI, TELEMETRY, CONSOLE_SWITCH
from minigraph import parse_xml

INIT_CFG_FILE = '/etc/sonic/init_cfg.json'
MINIGRAPH_FILE = '/etc/sonic/minigraph.xml'

# mock the redis for unit test purposes #
try:
Expand All @@ -22,6 +23,7 @@
sys.path.insert(0, modules_path)
sys.path.insert(0, tests_path)
INIT_CFG_FILE = os.path.join(mocked_db_path, "init_cfg.json")
MINIGRAPH_FILE = os.path.join(mocked_db_path, "minigraph.xml")
except KeyError:
pass

Expand Down Expand Up @@ -51,6 +53,16 @@ def __init__(self, namespace, socket=None):
self.TABLE_KEY = 'DATABASE'
self.TABLE_FIELD = 'VERSION'

# load config data from minigraph to get the default/hardcoded values from minigraph.py
# this is to avoid duplicating the hardcoded these values in db_migrator
self.minigraph_data = None
try:
if os.path.isfile(MINIGRAPH_FILE):
self.minigraph_data = parse_xml(MINIGRAPH_FILE)
except Exception as e:
log.log_error('Caught exception while trying to parse minigraph: ' + str(e))
pass

db_kwargs = {}
if socket:
db_kwargs['unix_socket_path'] = socket
Expand Down Expand Up @@ -527,38 +539,50 @@ def migrate_vxlan_config(self):

def migrate_restapi(self):
# RESTAPI - add missing key
if not self.minigraph_data or 'RESTAPI' not in self.minigraph_data:
return
restapi_data = self.minigraph_data['RESTAPI']
log.log_notice('Migrate RESTAPI configuration')
config = self.configDB.get_entry('RESTAPI', 'config')
if not config:
self.configDB.set_entry("RESTAPI", "config", RESTAPI.get("config"))
self.configDB.set_entry("RESTAPI", "config", restapi_data.get("config"))
certs = self.configDB.get_entry('RESTAPI', 'certs')
if not certs:
self.configDB.set_entry("RESTAPI", "certs", RESTAPI.get("certs"))
self.configDB.set_entry("RESTAPI", "certs", restapi_data.get("certs"))

def migrate_telemetry(self):
# TELEMETRY - add missing key
if not self.minigraph_data or 'TELEMETRY' not in self.minigraph_data:
return
telemetry_data = self.minigraph_data['TELEMETRY']
log.log_notice('Migrate TELEMETRY configuration')
gnmi = self.configDB.get_entry('TELEMETRY', 'gnmi')
if not gnmi:
self.configDB.set_entry("TELEMETRY", "gnmi", TELEMETRY.get("gnmi"))
self.configDB.set_entry("TELEMETRY", "gnmi", telemetry_data.get("gnmi"))
certs = self.configDB.get_entry('TELEMETRY', 'certs')
if not certs:
self.configDB.set_entry("TELEMETRY", "certs", TELEMETRY.get("certs"))
self.configDB.set_entry("TELEMETRY", "certs", telemetry_data.get("certs"))

def migrate_console_switch(self):
# CONSOLE_SWITCH - add missing key
if not self.minigraph_data or 'CONSOLE_SWITCH' not in self.minigraph_data:
return
console_switch_data = self.minigraph_data['CONSOLE_SWITCH']
log.log_notice('Migrate CONSOLE_SWITCH configuration')
console_mgmt = self.configDB.get_entry('CONSOLE_SWITCH', 'console_mgmt')
if not console_mgmt:
self.configDB.set_entry("CONSOLE_SWITCH", "console_mgmt",
CONSOLE_SWITCH.get("console_mgmt"))
console_switch_data.get("console_mgmt"))

def migrate_device_metadata(self):
# DEVICE_METADATA - synchronous_mode entry
log.log_notice('Migrate DEVICE_METADATA missing configuration (synchronous_mode=enable)')
if not self.minigraph_data or 'DEVICE_METADATA' not in self.minigraph_data:
return
log.log_notice('Migrate DEVICE_METADATA missing configuration')
metadata = self.configDB.get_entry('DEVICE_METADATA', 'localhost')
device_metadata_data = self.minigraph_data["DEVICE_METADATA"]["localhost"]
if 'synchronous_mode' not in metadata:
metadata['synchronous_mode'] = 'enable'
metadata['synchronous_mode'] = device_metadata_data.get("synchronous_mode")
self.configDB.set_entry('DEVICE_METADATA', 'localhost', metadata)

def migrate_port_qos_map_global(self):
Expand Down
32 changes: 0 additions & 32 deletions scripts/db_migrator_constants.py

This file was deleted.

1 change: 0 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@
'scripts/coredump-compress',
'scripts/configlet',
'scripts/db_migrator.py',
'scripts/db_migrator_constants.py',
'scripts/decode-syseeprom',
'scripts/dropcheck',
'scripts/disk_check.py',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
},
"RESTAPI|certs": {
"server_key": "/etc/sonic/credentials/restapiserver.key",
"ca_crt": "/etc/sonic/credentials/AME_ROOT_CERTIFICATE.pem",
"server_crt": "/etc/sonic/credentials/restapiserver.crt",
"client_crt_cname": "client.restapi.sonic.gbl"
"ca_crt": "/etc/sonic/credentials/restapica.crt",
"server_crt": "/etc/sonic/credentials/restapiserver.crt",
"client_crt_cname": "client.restapi.sonic"
},
"TELEMETRY|gnmi": {
"client_auth": "true",
Expand Down
29 changes: 29 additions & 0 deletions tests/db_migrator_input/minigraph.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<DeviceMiniGraph xmlns="Microsoft.Search.Autopilot.Evolution" xmlns:i="http://www.w3.org/2001/XMLSchema-instance">
<CpgDec/>
<DpgDec>
<DeviceDataPlaneInfo>
<LoopbackIPInterfaces/>
<ManagementIPInterfaces/>
<Hostname>SONiC-Dummy</Hostname>
<PortChannelInterfaces/>
<VlanInterfaces/>
<IPInterfaces/>
<AclInterfaces/>
</DeviceDataPlaneInfo>
</DpgDec>
<PngDec>
<Devices>
<Device i:type="TorRouter">
<Hostname>SONiC-Dummy</Hostname>
</Device>
</Devices>
</PngDec>
<MetadataDeclaration>
<Devices xmlns:a="http://schemas">
<a:DeviceMetadata>
<a:Properties/>
</a:DeviceMetadata>
</Devices>
</MetadataDeclaration>
<Hostname>SONiC-Dummy</Hostname>
</DeviceMiniGraph>
19 changes: 18 additions & 1 deletion tests/db_migrator_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -501,14 +501,31 @@ def test_warm_upgrade_to_2_0_2(self):
dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'cross_branch_upgrade_to_version_2_0_2_expected')
expected_db = Db()

expected_db
new_tables = ["RESTAPI", "TELEMETRY", "CONSOLE_SWITCH"]
for table in new_tables:
resulting_table = dbmgtr.configDB.get_table(table)
expected_table = expected_db.cfgdb.get_table(table)
diff = DeepDiff(resulting_table, expected_table, ignore_order=True)
assert not diff

def test_warm_upgrade__without_mg_to_2_0_2(self):
dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'cross_branch_upgrade_to_version_2_0_2_input')
import db_migrator
dbmgtr = db_migrator.DBMigrator(None)
# set minigraph_data to None to mimic the missing minigraph.xml scenario
dbmgtr.minigraph_data = None
dbmgtr.migrate()
dbconnector.dedicated_dbs['CONFIG_DB'] = os.path.join(mock_db_path, 'config_db', 'cross_branch_upgrade_without_mg_2_0_2_expected.json')
expected_db = Db()

new_tables = ["RESTAPI", "TELEMETRY", "CONSOLE_SWITCH"]
for table in new_tables:
resulting_table = dbmgtr.configDB.get_table(table)
expected_table = expected_db.cfgdb.get_table(table)
print(resulting_table)
diff = DeepDiff(resulting_table, expected_table, ignore_order=True)
assert not diff

class Test_Migrate_Loopback(object):
@classmethod
def setup_class(cls):
Expand Down

0 comments on commit 85d326d

Please sign in to comment.