Validate config_db json file before config reload #3063
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Remove STR casting and if check for more than 1 config_db json files
spilkey-cisco
approved these changes
Dec 5, 2023
|
@abdosi: Can you please help review this? |
|
@mihirpat1, Can you please help review this? |
prgeor
reviewed
Dec 11, 2023
| command = [SONIC_CFGGEN_PATH, '-j', file] | ||
| clicommon.run_command(command) | ||
| for inst in range(0, num_cfg_file-1): | ||
| file = "/etc/sonic/config_db{}.json".format(inst) |
There was a problem hiding this comment.
@AnoopKamath is this tested on multi-asic platform?
prgeor
reviewed
Dec 11, 2023
| file = "/etc/sonic/config_db{}.json".format(inst) | ||
| if os.path.exists(file): | ||
| command = [SONIC_CFGGEN_PATH, '-j', file] | ||
| clicommon.run_command(command) |
There was a problem hiding this comment.
@AnoopKamath will this raise exception on failure?
|
@qiluo-msft could you review? |
qiluo-msft
reviewed
Dec 11, 2023
| @@ -1515,6 +1515,18 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form | |||
| click.echo("Input {} config file(s) separated by comma for multiple files ".format(num_cfg_file)) | |||
| return | |||
|
|
|||
| #Validate config_db.json file | |||
qiluo-msft
reviewed
Dec 11, 2023
| @@ -1515,6 +1515,18 @@ def reload(db, filename, yes, load_sysinfo, no_service_restart, force, file_form | |||
| click.echo("Input {} config file(s) separated by comma for multiple files ".format(num_cfg_file)) | |||
| return | |||
|
|
|||
| #Validate config_db.json file | |||
| if file_format == 'config_db': | |||
| file = DEFAULT_CONFIG_DB_FILE | |||
|
Please add positive and negative testcases. |
|
We do have json validator: https://github.com/sonic-net/sonic-utilities/blob/master/config/main.py#L1559 |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What I did
Check config_db json file before stopping the services. If there is an error, DB wont be able to connect to any services
How I did it
Run the SONIC_CFGGEN_PATH to all the one or more config_db.json file. If there is any error, it will come out of config reload loop before stopping all the services.
How to verify it
Create an error in config_db.json and we should see any error before all the services go down.
Previous command output (if the output of a command-line utility has changed)
Suppose there is an extra "," issue in one of the config_db.json file and when we issue 'config reload' there is no validation for config_db.json file. Only when DB services will restart and encounter issue it will flag it but it has brought all the services down. Now none of the services will start and most of the commands wont run and system will hang when we issue config reload or reboot.
New command output (if the output of a command-line utility has changed)