Skip to content

Update override_config_table for multi-asic platforms #2620

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.

Sign up for GitHub

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

Closed
wants to merge 3 commits into from
Closed

Update override_config_table for multi-asic platforms #2620

Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
f2a5d79
Update override_config_table fr multi-asic platforms
judyjoseph Jan 23, 2023
1afd692
Fix the missing parameter to function override_config_db
judyjoseph Jan 23, 2023
1bca842
Fix comments
judyjoseph Feb 10, 2023
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
67 changes: 34 additions & 33 deletions config/main.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1893,36 +1893,37 @@ 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)

updated_config = update_config(current_config, 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()
cm.validateConfigData()
except Exception as ex:
click.secho("Failed to validate running config. Error: {}".format(ex), fg="magenta")
sys.exit(1)
# Do the yang validation and config override for host namespace and
# other namespaces in case of multi-asic platform
for ns, config_db in db.cfgdb_clients.items():
Copy link
Contributor

Choose a reason for hiding this comment

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

for

I understand that MACSEC_PROFILE will be in all namespaces in addition to the host namespace. However this is right in general.

Can we limit the loop only for MACSEC_PROFILE table?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have updated the logic to have a list of tables which is there per namespace, if any of the tables in the config_input json is present in that list, we do override config for all namespaces. Else only for host. Currently MACSEC_PROFILE is there in that table list.

this logic can be generalized to any table depending on how we plan to go with Virtual connector, to do this action based on table, and have it give back which connector to use, or which namespace the config needs to be overridden.

# 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)

updated_config = update_config(current_config, 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()
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, 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(ns, config_db, config_input)
Copy link
Contributor

Choose a reason for hiding this comment

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

ns

Seems the new parameter is only used for echo. In this case, you do not need an extra parameter, you can just echo before the function call "override_config_db in namespace {}"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated not to add an additional "ns" parameter



def validate_config_by_cm(cm, config_json, jname):
Expand All @@ -1943,18 +1944,18 @@ def update_config(current_config, config_input):
return updated_config


def override_config_db(config_db, config_input):
def override_config_db(ns, config_db, config_input):
Copy link
Contributor

Choose a reason for hiding this comment

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

override_config_db

Could you turn "How to verify it" into a unit testcase?

namespace = "Host" if ns is DEFAULT_NAMESPACE else ns
# Deserialized golden config to DB recognized format
sonic_cfggen.FormatConverter.to_deserialized(config_input)
# Delete table from DB then mod_config to apply golden config
click.echo("Removing configDB overriden table first ...")
click.echo("{} namespace - Removing configDB overriden table first ...".format(namespace))
for table in config_input:
config_db.delete_table(table)
click.echo("Overriding input config to configDB ...")
click.echo("{} namespace - Overriding input config to configDB ...".format(namespace))
data = sonic_cfggen.FormatConverter.output_to_db(config_input)
config_db.mod_config(data)
click.echo("Overriding completed. No service is restarted.")

click.echo("{} namespace - Overriding completed. No service is restarted.".format(namespace))

#
# 'hostname' command
Expand Down