-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[HLD] Add SYSTEM_DEFAULTS
table in config_db
to control the behavior of SONiC
#982
Conversation
Signed-off-by: bingwang <bingwang@microsoft.com>
|
||
### 6.3 Code change | ||
1. Update `db_migrator.py` to migrate flags from `DEVICE_METADATA|localhost` table to `FLAGS` table. | ||
2. Update the code of component to subscribe `FLAGS` table to watch the update. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
db migrator change would be required for warm reboot upgrade.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please clarify why do we need the change for warm reboot upgrade? db_migrator.py
script will be called in database.sh
after redis startup, is it good enough?
For the flags that can be changed by reconfiguration, we can update entries in `minigraph.xml`, and parse the new values in to config_db with minigraph parser at reloading minigraph. | ||
|
||
#### 5.2.3 Update value directly in db memory | ||
For some behavior change, we may don't have to interrupt dataplane. To support controlling SONiC behavior on-the-fly, we can update the value of flags in memory with tools like `sonic-cfggen`, `configlet` or `config apply-patch`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does init_cfg overwrites the one that is already saved and restored from config_db by the user? How is it handled? Can you provide some details for this case?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The value in config_db.json
will not be rewritten by init_cfg.json
as config_db.json
is loaded after init_cfg.json
. I added clarification in the doc, please help have a review. Thanks
8498931
to
8837dc2
Compare
Signed-off-by: bingwang <bingwang@microsoft.com>
|
||
### 6.3 Code change | ||
1. Update `db_migrator.py` to migrate flags from `DEVICE_METADATA|localhost` table to `FLAGS` table. | ||
2. Update the code of component to subscribe `FLAGS` table to watch the update. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are the current flags and what components need to be updated to subscribe to this new table? Can you provide more details
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated. Thanks
Signed-off-by: bingwang <bingwang@microsoft.com>
Below is a sample of `FLAGS` table | ||
|
||
``` | ||
"FLAGS": { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FLAG seems to be too generic. May I suggest a rename to SYSTEM_DEFAULTS
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, it's not SYSTEM_DEFAULTS
sometimes. For example, the FLAGS for pcbb is enable
by default, but somehow we may need to change the value to disable
for some reason, then the table name SYSTEM_DEFAULTS
seems wired.
Any more suggestions for a more beautiful table name? I also think the name FLAGS
is too generic.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why PCBB is enabled by default? In any case, this is loaded during the bootup and probably changed via feature or CLI etc. I see this as an extension of init_cfg.json and DEVICE_METADATA. Prefer to convey the meaning as SYSTEM_DEFAULT.
FLAGS
table in config_db
to control the behavior of SONiCSYSTEM_DEFAULTS
table in config_db
to control the behavior of SONiC
Signed-off-by: bingwang <wang.bing@microsoft.com>
Why I did it This PR is to backport PR #11117 into 202205 branch. This PR is to define Yang model for SYSTEM_DEFAULTS table. The table was introduced in PR sonic-net/SONiC#982 The table will be like "SYSTEM_DEFAULTS": { "tunnel_qos_remap": { "status": "enabled" } } Work item tracking Microsoft ADO (https://msazure.visualstudio.com/One/_workitems/edit/23037078) How I did it Add a new yang file sonic-system-defaults. Yang. How to verify it Verified by UT
This PR is to introduce a new table
FLAGS
intoconfig_db
to control the behavior of SONiC.Signed-off-by: bingwang bingwang@microsoft.com