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

[config]Support multi-asic Golden Config override #2738

Merged
merged 7 commits into from
Apr 20, 2023
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 10 additions & 5 deletions config/config_mgmt.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ class ConfigMgmt():
to verify config for the commands which are capable of change in config DB.
'''

def __init__(self, source="configDB", debug=False, allowTablesWithoutYang=True, sonicYangOptions=0):
def __init__(self, source="configDB", debug=False, allowTablesWithoutYang=True,
sonicYangOptions=0, configdb=None):
'''
Initialise the class, --read the config, --load in data tree.

Expand All @@ -44,6 +45,7 @@ def __init__(self, source="configDB", debug=False, allowTablesWithoutYang=True,
debug (bool): verbose mode.
allowTablesWithoutYang (bool): allow tables without yang model in
config or not.
configdb: configdb to work on.

Returns:
void
Expand All @@ -54,6 +56,11 @@ def __init__(self, source="configDB", debug=False, allowTablesWithoutYang=True,
self.source = source
self.allowTablesWithoutYang = allowTablesWithoutYang
self.sonicYangOptions = sonicYangOptions
if configdb is None:
self.configdb = ConfigDBConnector()
self.configdb.connect()
else:
self.configdb = configdb

# logging vars
self.SYSLOG_IDENTIFIER = "ConfigMgmt"
Expand Down Expand Up @@ -194,8 +201,7 @@ def readConfigDB(self):
self.sysLog(doPrint=True, msg='Reading data from Redis configDb')
# Read from config DB on sonic switch
data = dict()
configdb = ConfigDBConnector()
configdb.connect()
configdb = self.configdb
sonic_cfggen.deep_update(data, sonic_cfggen.FormatConverter.db_to_output(configdb.get_config()))
self.configdbJsonIn = sonic_cfggen.FormatConverter.to_serialized(data)
self.sysLog(syslog.LOG_DEBUG, 'Reading Input from ConfigDB {}'.\
Expand All @@ -215,8 +221,7 @@ def writeConfigDB(self, jDiff):
'''
self.sysLog(doPrint=True, msg='Writing in Config DB')
data = dict()
configdb = ConfigDBConnector()
configdb.connect(False)
configdb = self.configdb
sonic_cfggen.deep_update(data, sonic_cfggen.FormatConverter.to_deserialized(jDiff))
self.sysLog(msg="Write in DB: {}".format(data))
configdb.mod_config(sonic_cfggen.FormatConverter.output_to_db(data))
Expand Down
86 changes: 60 additions & 26 deletions config/main.py
Original file line number Diff line number Diff line change
Expand Up @@ -1875,6 +1875,33 @@ def override_config_by(golden_config_path):
return


def generate_sysinfo(config_input, ns=None):
# Generate required sysinfo for Golden Config.
device_metadata = config_input.get('DEVICE_METADATA')

if not device_metadata or 'localhost' not in device_metadata:
return

if ns:
asic_role = device_metadata.get('localhost', {}).get('sub_role')
switch_type = device_metadata.get('localhost', {}).get('switch_type')

if ((switch_type is not None and switch_type.lower() == "chassis-packet") or
(asic_role is not None and asic_role.lower() == "backend")):
mac = device_info.get_system_mac(namespace=ns)
else:
mac = device_info.get_system_mac()
else:
mac = device_info.get_system_mac()
Copy link
Contributor

@qiluo-msft qiluo-msft Apr 11, 2023

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Could you remove code dup in next PR?


platform = device_info.get_platform()

device_metadata['localhost']['mac'] = mac
device_metadata['localhost']['platform'] = platform

return


#
# 'override-config-table' command ('config override-config-table ...')
#
Expand Down Expand Up @@ -1902,36 +1929,43 @@ def override_config_table(db, input_config_db, dry_run):
fg='magenta')
sys.exit(1)

config_db = db.cfgdb

# Read config from configDB
current_config = config_db.get_config()
# Serialize to the same format as json input
sonic_cfggen.FormatConverter.to_serialized(current_config)
cfgdb_clients = db.cfgdb_clients

updated_config = update_config(current_config, config_input)
for ns, config_db in cfgdb_clients.items():
# Read config from configDB
current_config = config_db.get_config()
# Serialize to the same format as json input
sonic_cfggen.FormatConverter.to_serialized(current_config)

yang_enabled = device_info.is_yang_config_validation_enabled(config_db)
if yang_enabled:
# The ConfigMgmt will load YANG and running
# config during initialization.
try:
cm = ConfigMgmt()
cm.validateConfigData()
except Exception as ex:
click.secho("Failed to validate running config. Error: {}".format(ex), fg="magenta")
sys.exit(1)
if multi_asic.is_multi_asic():
ns_config_input = config_input[ns]
generate_sysinfo(ns_config_input, ns)
else:
ns_config_input = config_input
generate_sysinfo(ns_config_input)
Copy link
Contributor

@qiluo-msft qiluo-msft Apr 11, 2023

Choose a reason for hiding this comment

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

generate_sysinfo

Is it better to fill missing mac/platform during override_config_db() ?

If the original ConfigDB already has mac/platform, we should keep them even we override other fields. #Pending

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean the generated sysinfo should have less priority than existing one?
Though I think they should be the same.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will submit another PR for sysinfo generation about override.

updated_config = update_config(current_config, ns_config_input)

yang_enabled = device_info.is_yang_config_validation_enabled(config_db)
if yang_enabled:
# The ConfigMgmt will load YANG and running
# config during initialization.
try:
cm = ConfigMgmt(configdb=config_db)
cm.validateConfigData()
except Exception as ex:
click.secho("Failed to validate running config. Error: {}".format(ex), fg="magenta")
sys.exit(1)

# Validate input config
validate_config_by_cm(cm, config_input, "config_input")
# Validate updated whole config
validate_config_by_cm(cm, updated_config, "updated_config")
# Validate input config
validate_config_by_cm(cm, ns_config_input, "config_input")
# Validate updated whole config
validate_config_by_cm(cm, updated_config, "updated_config")

if dry_run:
print(json.dumps(updated_config, sort_keys=True,
indent=4, cls=minigraph_encoder))
else:
override_config_db(config_db, config_input)
if dry_run:
print(json.dumps(updated_config, sort_keys=True,
indent=4, cls=minigraph_encoder))
else:
override_config_db(config_db, ns_config_input)


def validate_config_by_cm(cm, config_json, jname):
Expand Down
55 changes: 55 additions & 0 deletions tests/config_override_input/multi_asic_dm_gen_sysinfo.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
{
"": {
"DEVICE_METADATA": {
"localhost": {
"default_bgp_status": "down",
"default_pfcwd_status": "enable",
"deployment_id": "1",
"docker_routing_config_mode": "separated",
"hostname": "sonic-switch",
"hwsku": "Mellanox-SN3800-D112C8",
"peer_switch": "sonic-switch",
"type": "ToRRouter",
"suppress-fib-pending": "enabled"
}
}
},
"asic0": {
"DEVICE_METADATA": {
"localhost": {
"asic_id": "01.00.0",
"asic_name": "asic0",
"bgp_asn": "65100",
"cloudtype": "None",
"default_bgp_status": "down",
"default_pfcwd_status": "enable",
"deployment_id": "None",
"docker_routing_config_mode": "separated",
"hostname": "sonic",
"hwsku": "multi_asic",
"region": "None",
"sub_role": "FrontEnd",
"type": "LeafRouter"
}
}
},
"asic1": {
"DEVICE_METADATA": {
"localhost": {
"asic_id": "08:00.0",
"asic_name": "asic1",
"bgp_asn": "65100",
"cloudtype": "None",
"default_bgp_status": "down",
"default_pfcwd_status": "enable",
"deployment_id": "None",
"docker_routing_config_mode": "separated",
"hostname": "sonic",
"hwsku": "multi_asic",
"region": "None",
"sub_role": "BackEnd",
"type": "LeafRouter"
}
}
}
}
11 changes: 11 additions & 0 deletions tests/config_override_input/multi_asic_dm_rm.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
{
"": {
"DEVICE_METADATA": {}
},
"asic0": {
"DEVICE_METADATA": {}
},
"asic1": {
"DEVICE_METADATA": {}
}
}
23 changes: 23 additions & 0 deletions tests/config_override_input/multi_asic_macsec_ov.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
{
"": {
"MACSEC_PROFILE": {
"profile": {
"key": "value"
}
}
},
"asic0": {
"MACSEC_PROFILE": {
"profile": {
"key": "value"
}
}
},
"asic1": {
"MACSEC_PROFILE": {
"profile": {
"key": "value"
}
}
}
}
103 changes: 101 additions & 2 deletions tests/config_override_test.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import os
import json
import filecmp
import importlib
import config.main as config

from click.testing import CliRunner
Expand All @@ -20,6 +21,9 @@
RUNNING_CONFIG_YANG_FAILURE = os.path.join(DATA_DIR, "running_config_yang_failure.json")
GOLDEN_INPUT_YANG_FAILURE = os.path.join(DATA_DIR, "golden_input_yang_failure.json")
FINAL_CONFIG_YANG_FAILURE = os.path.join(DATA_DIR, "final_config_yang_failure.json")
MULTI_ASIC_MACSEC_OV = os.path.join(DATA_DIR, "multi_asic_macsec_ov.json")
MULTI_ASIC_DEVICE_METADATA_RM = os.path.join(DATA_DIR, "multi_asic_dm_rm.json")
MULTI_ASIC_DEVICE_METADATA_GEN_SYSINFO = os.path.join(DATA_DIR, "multi_asic_dm_gen_sysinfo.json")

# Load sonic-cfggen from source since /usr/local/bin/sonic-cfggen does not have .py extension.
sonic_cfggen = load_module_from_source('sonic_cfggen', '/usr/local/bin/sonic-cfggen')
Expand Down Expand Up @@ -173,7 +177,7 @@ def test_yang_verification_enabled(self):
def is_yang_config_validation_enabled_side_effect(filename):
return True

def config_mgmt_side_effect():
def config_mgmt_side_effect(configdb):
return config_mgmt.ConfigMgmt(source=CONFIG_DB_JSON_FILE)

db = Db()
Expand Down Expand Up @@ -232,7 +236,7 @@ def check_yang_verification_failure(self, db, config, running_config,
def read_json_file_side_effect(filename):
return golden_config

def config_mgmt_side_effect():
def config_mgmt_side_effect(configdb):
return config_mgmt.ConfigMgmt(source=CONFIG_DB_JSON_FILE)

# ConfigMgmt will call ConfigDBConnector to load default config_db.json.
Expand All @@ -257,3 +261,98 @@ def teardown_class(cls):
print("TEARDOWN")
os.environ["UTILITIES_UNIT_TESTING"] = "0"
return


class TestConfigOverrideMultiasic(object):
@classmethod
def setup_class(cls):
print("SETUP")
os.environ["UTILITIES_UNIT_TESTING"] = "1"
os.environ["UTILITIES_UNIT_TESTING_TOPOLOGY"] = "multi_asic"
# change to multi asic config
from .mock_tables import dbconnector
from .mock_tables import mock_multi_asic
importlib.reload(mock_multi_asic)
dbconnector.load_namespace_config()
return

def test_macsec_override(self):
def read_json_file_side_effect(filename):
with open(MULTI_ASIC_MACSEC_OV, "r") as f:
macsec_profile = json.load(f)
return macsec_profile
db = Db()
cfgdb_clients = db.cfgdb_clients

# The profile_content was copied from MULTI_ASIC_MACSEC_OV, where all
# ns sharing the same content: {"profile": {"key": "value"}}
profile_content = {"profile": {"key": "value"}}

with mock.patch('config.main.read_json_file',
mock.MagicMock(side_effect=read_json_file_side_effect)):
runner = CliRunner()
result = runner.invoke(config.config.commands["override-config-table"],
['golden_config_db.json'], obj=db)
assert result.exit_code == 0

for ns, config_db in cfgdb_clients.items():
assert config_db.get_config()['MACSEC_PROFILE'] == profile_content

def test_device_metadata_table_rm(self):
def read_json_file_side_effect(filename):
with open(MULTI_ASIC_DEVICE_METADATA_RM, "r") as f:
device_metadata = json.load(f)
return device_metadata
db = Db()
cfgdb_clients = db.cfgdb_clients

for ns, config_db in cfgdb_clients.items():
assert 'DEVICE_METADATA' in config_db.get_config()

with mock.patch('config.main.read_json_file',
mock.MagicMock(side_effect=read_json_file_side_effect)):
runner = CliRunner()
result = runner.invoke(config.config.commands["override-config-table"],
['golden_config_db.json'], obj=db)
assert result.exit_code == 0

for ns, config_db in cfgdb_clients.items():
assert 'DEVICE_METADATA' not in config_db.get_config()

def test_device_metadata_gen_sysinfo(self):
def read_json_file_side_effect(filename):
with open(MULTI_ASIC_DEVICE_METADATA_GEN_SYSINFO, "r") as f:
device_metadata = json.load(f)
return device_metadata
db = Db()
cfgdb_clients = db.cfgdb_clients

with mock.patch('config.main.read_json_file',
mock.MagicMock(side_effect=read_json_file_side_effect)),\
mock.patch('sonic_py_common.device_info.get_platform',
return_value="multi_asic"),\
mock.patch('sonic_py_common.device_info.get_system_mac',
return_value="11:22:33:44:55:66"):
runner = CliRunner()
result = runner.invoke(config.config.commands["override-config-table"],
['golden_config_db.json'], obj=db)
assert result.exit_code == 0

for ns, config_db in cfgdb_clients.items():
platform = config_db.get_config()['DEVICE_METADATA']['localhost'].get('platform')
mac = config_db.get_config()['DEVICE_METADATA']['localhost'].get('mac')
assert platform == "multi_asic"
assert mac == "11:22:33:44:55:66"


@classmethod
def teardown_class(cls):
print("TEARDOWN")
os.environ["UTILITIES_UNIT_TESTING"] = "0"
os.environ["UTILITIES_UNIT_TESTING_TOPOLOGY"] = ""
# change back to single asic config
from .mock_tables import dbconnector
from .mock_tables import mock_single_asic
importlib.reload(mock_single_asic)
dbconnector.load_namespace_config()
return