Add CLI for configuring synchronous mode #1094
Conversation
scripts/sonic-sync-mode
Outdated
| sudo needed for configuring synchronous mode | ||
| swss restart required to apply the configuration | ||
| Options to restart swss and apply the configuration: | ||
| 1. config save |
There was a problem hiding this comment.
config save [](start = 7, length = 11)
config save also has -y. Do you want using? #Closed
There was a problem hiding this comment.
Added -y in the comment
scripts/sonic-sync-mode
Outdated
| Options to restart swss and apply the configuration: | ||
| 1. config save | ||
| config reload -y | ||
| 2. systemctl restart swss |
There was a problem hiding this comment.
systemctl restart swss [](start = 7, length = 22)
Do you need config save before restart? #Closed
There was a problem hiding this comment.
This is not needed, updated the help message to clarify the usage.
scripts/sonic-sync-mode
Outdated
|
|
||
| def main(): | ||
| parser=argparse.ArgumentParser(description= | ||
| 'Configure enable or disable synchronous mode between orchagent and syncd', |
There was a problem hiding this comment.
Configure enable [](start = 11, length = 16)
Enable #Closed
scripts/sonic-sync-mode
Outdated
| table.set('localhost', fvs) | ||
| print('Wrote %s synchronous mode into CONFIG_DB, swss restart required to apply the configuration'%(args.mode)) | ||
| else: | ||
| print('Invalid argument: expect either enable or disable') |
There was a problem hiding this comment.
print [](start = 8, length = 5)
print to stderr, and return with non zero error code. #Closed
There was a problem hiding this comment.
Updated as prescribed.
scripts/sonic-sync-mode
Outdated
| parser.print_help() | ||
|
|
||
| if __name__ == '__main__': | ||
| main() |
There was a problem hiding this comment.
main [](start = 4, length = 4)
Do you have a plan to integrate into config CLI? Need a design doc. #Closed
There was a problem hiding this comment.
Integrated this into config CLI.
config/main.py
Outdated
| 1. Keep and apply configuration after config reload \n | ||
| config save -y \n | ||
| config reload -y \n | ||
| 2. Discard configuration after config reload \n |
There was a problem hiding this comment.
Discard configuration after config reload [](start = 15, length = 41)
This line has nothing to do with next line. #Closed
There was a problem hiding this comment.
Removed the irrelevant line and only kept the commands to restart swss.
|
retest this please |
|
need unit test |
config/main.py
Outdated
| config_db.mod_entry('DEVICE_METADATA' , 'localhost', {"synchronous_mode" : sync_mode}) | ||
| click.echo("Wrote %s synchronous mode into CONFIG_DB, swss restart required to apply the configuration" % sync_mode) | ||
| else: | ||
| click.echo("Error: Invalid argument %s, expect either enable or disable" % sync_mode) |
There was a problem hiding this comment.
Thanks for the suggestion. click does not seem to have a function "fail". Using "BadParameter" instead.
config/main.py
Outdated
| config_db = ConfigDBConnector() | ||
| config_db.connect() | ||
| config_db.mod_entry('DEVICE_METADATA' , 'localhost', {"synchronous_mode" : sync_mode}) | ||
| click.echo("Wrote %s synchronous mode into CONFIG_DB, swss restart required to apply the configuration" % sync_mode) |
There was a problem hiding this comment.
please be more specific about the message, "you need config save -y and reload -y"
There was a problem hiding this comment.
Added the options to restart swss in the message.
|
@lguohan Could you please let me know if this looks ok? |
- What I did
Add CLI to configure synchronous mode
config synchronous_mode <enable|disable>- How I did it
Add a CLI script that writes the configuration of synchronous mode into CONFIG_DB and informs users to restart swss to apply the configuration.
- How to verify it
I verified it by testing locally with a virtual switch.
- Previous command output (if the output of a command-line utility has changed)
N/A
- New command output (if the output of a command-line utility has changed)
Wrote enable/disable synchronous mode into CONFIG_DB, swss restart required to apply the configuration