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

[HLD] Add SYSTEM_DEFAULTS table in config_db to control the behavior of SONiC #982

Merged
merged 5 commits into from
May 23, 2022
Merged
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
108 changes: 108 additions & 0 deletions doc/sonic-flags/control-sonic-behaviors-with-sonic-flags.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,108 @@
# 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": {
Copy link
Contributor

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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.

"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 vaule 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.

#### 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`.
Copy link
Contributor

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?

Copy link
Contributor Author

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


### 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.
2. Update the code of component to subscribe `FLAGS` table to watch the update.
Copy link
Contributor

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.

Copy link
Contributor Author

@bingwang-ms bingwang-ms Apr 18, 2022

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?

Copy link
Contributor

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

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. Thanks


## 7 Test requirement
TBA