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

Update override_config_table for multi-asic platforms #2620

Closed

Conversation

judyjoseph
Copy link
Contributor

@judyjoseph judyjoseph commented Jan 23, 2023

What I did

Update override_config_table for multi-asic platforms to apply the overriding config in golden_config_db.json to all namespaces in addition to the host namespace.

This will be removed later as we support the complete depreciation of minigraph and the golden_config_db.json file support the DB schema for multi-asic platforms.

How I did it

Check the db client list which contains the db connector to DB in multiple namespaces and do yang validation and config override.

How to verify it

Validated with MACSEC_PROFILE present in the golden_config_db.json file and checked that the MACSEC_PROFILE gets added/updated in the config_db's for host as well as the namespaces.

admin@str2--lc1-1:~$ sudo config load_minigraph -y -o
Disabling container monitoring ...
Stopping SONiC target ...
Running command: /usr/local/bin/sonic-cfggen -H -m -j /etc/sonic/init_cfg.json   --write-to-db
Running command: /usr/local/bin/sonic-cfggen -H -m -j /etc/sonic/init_cfg.json  -n asic0 --write-to-db
/usr/local/lib/python3.9/dist-packages/minigraph.py:796: FutureWarning: The behavior of this method will change in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
  if voqinbandintfs:
Running command: /usr/local/bin/sonic-cfggen -H -m -j /etc/sonic/init_cfg.json  -n asic1 --write-to-db
/usr/local/lib/python3.9/dist-packages/minigraph.py:796: FutureWarning: The behavior of this method will change in future versions. Use specific 'len(elem)' or 'elem is not None' test instead.
  if voqinbandintfs:
Running command: /usr/local/bin/sonic-cfggen -d -y /etc/sonic/sonic_version.yml -t /usr/share/sonic/templates/sonic-environment.j2,/etc/sonic/sonic-environment
Running command: config qos reload --no-dynamic-buffer --no-delay
Running command: /usr/local/bin/sonic-cfggen -n asic0 -d --write-to-db -t /usr/share/sonic/device/0/buffers.json.j2,config-db -t /usr/share/sonic/device/0/qos.json.j2,config-db -y /etc/sonic/sonic_version.yml
Running command: /usr/local/bin/sonic-cfggen -n asic1 -d --write-to-db -t /usr/share/sonic/device/1/buffers.json.j2,config-db -t /usr/share/sonic/device/1/qos.json.j2,config-db -y /etc/sonic/sonic_version.yml
Running command: pfcwd start_default
Running command: config override-config-table /etc/sonic/golden_config_db.json
Working in Host namespace :
  - Removing configDB overriden table first ...
  - Overriding input config to configDB ...
  - Overriding completed. No service is restarted.
Working in asic0 namespace :
  - Removing configDB overriden table first ...
  - Overriding input config to configDB ...
  - Overriding completed. No service is restarted.
Working in asic1 namespace :
  - Removing configDB overriden table first ...
  - Overriding input config to configDB ...
  - Overriding completed. No service is restarted.
Restarting SONiC target ...
Enabling container monitoring ...
Reloading Monit configuration ...
Reinitializing monit daemon
Please note setting loaded from minigraph will be lost after system reboot. To preserve setting, run `config save`.
admin@str2--lc1-1:~$ sudo config save -y
Running command: /usr/local/bin/sonic-cfggen -d --print-data > /etc/sonic/config_db.json
Running command: /usr/local/bin/sonic-cfggen -n asic0 -d --print-data > /etc/sonic/config_db0.json
Running command: /usr/local/bin/sonic-cfggen -n asic1 -d --print-data > /etc/sonic/config_db1.json

Previous command output (if the output of a command-line utility has changed)

New command output (if the output of a command-line utility has changed)

@judyjoseph judyjoseph marked this pull request as ready for review January 23, 2023 21:25
config/main.py Outdated
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

config/main.py Outdated
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.

config/main.py Outdated
@@ -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?

@judyjoseph
Copy link
Contributor Author

closing this in favor of #2738

@judyjoseph judyjoseph closed this Apr 20, 2023
@judyjoseph judyjoseph deleted the golden_config_multi_asic_fix branch April 20, 2023 23:00
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.

2 participants