-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,129 @@ | ||
# Control SONiC behaviors with FLAGS table | ||
|
||
## 1 Table of Content ### | ||
|
||
- [Revision](#11-revision) | ||
- [Scope](#2-scope) | ||
- [Definitions/Abbreviations](#3-definitionsabbreviations) | ||
- [Overview](#4-overview) | ||
- [Design](#5-design) | ||
- [Change required](#6-change-required) | ||
- [Test requirement](#7-test-requirement) | ||
|
||
|
||
### 1.1 Revision ### | ||
| Rev | Date | Author | Change Description | | ||
|:---:|:-----------:|:------------------:|-----------------------------------| | ||
| 0.1 | | Bing Wang | Initial version | | ||
|
||
|
||
## 2 Scope ## | ||
|
||
This document covers high level design of `FLAGS` table in SONiC. | ||
|
||
## 3 Definitions/Abbreviations ## | ||
|
||
|
||
| Term | Meaning | | ||
|:--------:|:---------------------------------------------:| | ||
| | | | ||
|
||
|
||
## 4 Overview | ||
|
||
A number of flags are required to turn on/off certain feature or control the behaviors of various features in SONiC. Currently, these flags are put into `DEVICE_METADATA` table. | ||
|
||
``` | ||
"DEVICE_METADATA": { | ||
"localhost": { | ||
"default_bgp_status": "down", | ||
"default_pfcwd_status": "enable", | ||
"synchronous_mode": "enable", | ||
"dhcp_server": "enable" | ||
} | ||
} | ||
``` | ||
As a result, the `DEVICE_METADATA` table is inflating rapidly as we are having more and more flags, although these flags seem not to be categorized into `DEVICE_METADATA` | ||
|
||
To have a better management of the flags, a new table `FLAGS` is introduced in this design. | ||
|
||
## 5 Design ## | ||
|
||
### 5.1 DB Schema | ||
|
||
A new table `FLAGS` is added into config_db. | ||
``` | ||
key = FLAGS | ||
|
||
;field = value | ||
FLAG_NAME = 1*255VCHAR ; FLAG_NAME must be unique, the value is a string, which can be 'enable'/'disable', 'down'/'up' or any string. | ||
``` | ||
Below is a sample of `FLAGS` table | ||
|
||
``` | ||
"FLAGS": { | ||
"default_bgp_status": "down", | ||
"default_pfcwd_status": "enable", | ||
"synchronous_mode": "enable", | ||
"dhcp_server": "enable" | ||
} | ||
``` | ||
|
||
### 5.2 How to update flags in `FLAGS` table | ||
|
||
#### 5.2.1 Set default value with `init_cfg.json` | ||
|
||
The default value of flags in `FLAGS` table can be set in `init_cfg.json` and loaded into db at system startup. These flags are usually set at image being build, and are unlikely to change at runtime. | ||
If the values in `config_db.json` is changed by user, it will not be rewritten back by `init_cfg.json` as `config_db.json` is loaded after `init_cfg.json` in [docker_image_ctl.j2](https://github.com/Azure/sonic-buildimage/blob/master/files/build_templates/docker_image_ctl.j2) | ||
|
||
``` | ||
if [ -r /etc/sonic/config_db$DEV.json ]; then | ||
if [ -r /etc/sonic/init_cfg.json ]; then | ||
$SONIC_CFGGEN -j /etc/sonic/init_cfg.json -j /etc/sonic/config_db$DEV.json --write-to-db | ||
else | ||
$SONIC_CFGGEN -j /etc/sonic/config_db$DEV.json --write-to-db | ||
fi | ||
fi | ||
``` | ||
For example, the value of `default_bgp_status` is down in `init_cfg.json` if `shutdown_bgp_on_start` is set to `y` when image is being built. If we modify the value of `default_bgp_status` in `config_db.json` to `up`, it will keep `up`. | ||
#### 5.2.2 Parse from `minigraph.xml` when loading minigraph | ||
|
||
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 commentThe 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 commentThe reason will be displayed to describe this comment to others. Learn more. The value in |
||
|
||
### 5.3 How to consume flags in `FLAGS` table | ||
|
||
#### 5.3.1 Consume at service startup or reload | ||
All of the flags in `FLAGS` table can be consumed at service startup or reload as we do now. We can use the flags to render templates or control the running path of code. | ||
|
||
#### 5.3.2 Consume on-the-fly without interrupting traffic | ||
The `FLAGS` table can be subscribed by components that are interested on the flags. Hence, the in-memory change of flags will be consumed by running service, and take effect without reloading if possible. | ||
|
||
## 6 Change required ## | ||
### 6.1 Template update | ||
1. Templates that generate default values in `DEVICE_METADATA|localhost` table are required to be updated. The generated flags will be put into `FLAGS` table now. | ||
2. Templates that depend on `DEVICE_METADATA|localhost` table are required to be updated. | ||
|
||
### 6.2 Yang model update | ||
A new Yang model is to be added to restrict the valid flags in `FLAGS` table. The existing entries for flags in current [sonic-device_metadata.yang](https://github.com/Azure/sonic-buildimage/blob/master/src/sonic-yang-models/yang-models/sonic-device_metadata.yang) are to be removed. | ||
|
||
### 6.3 Code change | ||
1. Update `db_migrator.py` to migrate flags from `DEVICE_METADATA|localhost` table to `FLAGS` table. Current flags include | ||
|
||
|Flag|Source| | ||
|--|--| | ||
|default_bgp_status| From init_cfg.json, the value is determined by option shutdown_bgp_on_start when image being built| | ||
|default_pfcwd_status|From init_cfg.json, the value is determined by option enable_pfcwd_on_start when image being built| | ||
|synchronous_mode|From init_cfg.json, the value is determined by option include_p4rt when image being built| | ||
|buffer_model|From init_cfg.json, the value is determined by option default_buffer_model when image being built| | ||
|dhcp_server| Parse from minigraph.xml| | ||
|
||
|
||
2. Add code to subscribe `FLAGS` table to get the update notification for components that is interested in the `FLAGS` change. Currently, all orchs and daemons don't support changing flag controlled behaviors without restarting service, so no code change is required for existing components. | ||
|
||
## 7 Test requirement | ||
TBA | ||
|
||
|
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 isenable
by default, but somehow we may need to change the value todisable
for some reason, then the table nameSYSTEM_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.