-
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
[synchronous-mode] Document for configuration of synchronous mode #672
Conversation
1. Allow users to enable or disable synchronous mode via CLI. | ||
2. Follow the image-specified mode when upgrading an image with cold/warm/fast-reboot if the synchronous mode configuration is not explicitly specified by users in configDB. | ||
3. Use the user-specified mode when an explicit configuration of synchronous mode exists in configDB. | ||
4. Support enabling/disabling the synchronous mode when deploying a minigraph. |
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.
deploying a minigraph [](start = 56, length = 21)
parsing a minigraph and generating config_db.json #Closed
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.
Fixed.
|---------------------------|-----------------------------------| | ||
| enable | Enable synchronous mode. | | ||
| disable | Disable synchronous mode. | | ||
| Field does not exist | Orchagent and syncd use the mode specified by `orchagent.sh` and `syncd-init-common.sh`, respectively. | |
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.
Field does not exist [](start = 2, length = 20)
Let's be verbose. If the field does not exist, and user never use the CLI, will it appear in ConfigDB or not at all? #Closed
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.
Added a description for this scenario.
|---------------------------|-----------------------------------| | ||
| enable | Enable synchronous mode. | | ||
| disable | Disable synchronous mode. | | ||
| Field does not exist | Orchagent and syncd use the mode specified by `orchagent.sh` and `syncd-init-common.sh`, respectively. | |
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.
specified by
orchagent.sh
andsyncd-init-common.sh
[](start = 63, length = 54)
The design doc have some details not fixed:
- if Field does not exist, what is the behavior? Will new image change the behavior?
orchagent.sh
andsyncd-init-common.sh
will use the same mode, so better to define the behavior in one place. #Closed
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.
Added discussion in section 3.4 to clarify that new images can change the behavior if the field does not exist.
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.
As for 2, both orchagent.sh
and syncd-init-common.sh
follow the configuration in config_DB. The reason to make the field in config_DB empty as default and let the two files to choose their own mode with the field empty is to make sure the image can change the behavior accordingly with cold/warm/fast reboot if a user-specified mode does not exist. It is kind of like trading off this for better controllability in image upgrade. Since the mode defined by the two files are expected to change only once from async mode to sync mode, I think doing such a tradeoff should be ok.
### 3.1 Configuration Requirements | ||
1. Allow users to enable or disable synchronous mode via CLI. | ||
2. Follow the image-specified mode when upgrading an image with cold/warm/fast-reboot if the synchronous mode configuration is not explicitly specified by users in configDB. | ||
3. Use the user-specified mode when an explicit configuration of synchronous mode exists in configDB. |
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.
You need to provide design detail on how to achieve this. #Closed
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.
Added design details about where the configuration in configDB is read and applied.
1. Allow users to enable or disable synchronous mode via CLI. | ||
2. Follow the image-specified mode when upgrading an image with cold/warm/fast-reboot if the synchronous mode configuration is not explicitly specified by users in configDB. | ||
3. Use the user-specified mode when an explicit configuration of synchronous mode exists in configDB. | ||
4. Support enabling/disabling the synchronous mode when deploying a minigraph. |
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.
. [](start = 77, length = 1)
Do you need design for https://github.com/Azure/SONiC/wiki/L2-Switch-mode? #Closed
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.
I think this L2 switch mode does not need specific designs for it. I added Section 3.6 to discuss about the L2 switch mode.
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.
As comment
Document for the configuration design of synchronous mode for review.