[synchronous-mode] Add template file for synchronous mode #5644
Conversation
| @@ -14,7 +14,7 @@ ORCHAGENT_ARGS="-d /var/log/swss " | |||
| ORCHAGENT_ARGS+="-b 8192 " | |||
|
|
|||
| # Set synchronous mode if it is enabled in CONFIG_DB | |||
| SYNC_MODE=$(sonic-cfggen -d -v DEVICE_METADATA.localhost.synchronous_mode) | |||
| SYNC_MODE=$(sonic-cfggen -d -t /etc/sonic/synchronous_mode.j2 | jq -r '.synchronous_mode') | |||
There was a problem hiding this comment.
sonic-cfggen [](start = 12, length = 12)
sonic-cfggen is slow in the warm/fast reboot critical timeline. So we optimize them aggressively.
It is possible to limit only one call in each script to generate everything into a big variable, and extract each config from the variable. #Closed
There was a problem hiding this comment.
Reduced sonic-cfggen usage.
|
retest vsimage please |
rules/scripts.mk
Outdated
| @@ -23,11 +23,15 @@ $(SYSCTL_NET_CONFIG)_PATH = files/image_config/sysctl | |||
| UPDATE_CHASSISDB_CONFIG_SCRIPT = update_chassisdb_config | |||
| $(UPDATE_CHASSISDB_CONFIG_SCRIPT)_PATH = files/scripts | |||
|
|
|||
| SWSS_SYNCD_VARS_TEMPLATE = swss_syncd_vars.j2 | |||
There was a problem hiding this comment.
SWSS_SYNCD_VARS_TEMPLATE [](start = 0, length = 24)
Wondering why original code don't need it. #Closed
There was a problem hiding this comment.
It is needed here to make the file available to be copied to dvs and p4 docker. The template file was not available in the platforms.
| fi | ||
|
|
||
| # Retrieve SWSS vars from sonic-cfggen | ||
| SWSS_VARS=$(sonic-cfggen -d -y /etc/sonic/sonic_version.yml -t $SWSS_VARS_FILE) |
There was a problem hiding this comment.
$SWSS_VARS_FILE [](start = 63, length = 15)
If SWSS vars template file not found, the command substitution will fail, so you can catch the error and exit. It is too verbose to check every input files. #Closed
There was a problem hiding this comment.
Removed the verbose check for the template file.
| @@ -1,7 +1,7 @@ | |||
| #!/usr/bin/env bash | |||
|
|
|||
| EXIT_SWSS_VARS_FILE_NOT_FOUND=1 | |||
| SWSS_VARS_FILE=/usr/share/sonic/templates/swss_vars.j2 | |||
| SWSS_VARS_FILE=/etc/sonic/swss_syncd_vars.j2 | |||
There was a problem hiding this comment.
/etc/sonic/swss_syncd_vars.j2 [](start = 15, length = 29)
Does it make more sense to keep it in original directory? User are not expected to modify this file.
BTW, keep the original filename still make sense to me. #Closed
There was a problem hiding this comment.
The original directory /usr/share/sonic/templates is only available in swss docker. The reason to move it to /etc/sonic is to make the same template file accessible to both swss and syncd. Also, as this template is going to be accessed by syncd as well, I renamed to reflect this.
|
retest mellanox please |
|
@qiluo-msft, what do you need @tahmed-dev to review? can you mark it? |
|
@tahmed-dev Could you help review |
@qiluo-msft sure. |
|
The change LGTM, however I just find the case for the |
|
@shi-su wonder if symlink-ing the file in the source code base would be a viable solution. |
| @@ -23,11 +23,15 @@ $(SYSCTL_NET_CONFIG)_PATH = files/image_config/sysctl | |||
| UPDATE_CHASSISDB_CONFIG_SCRIPT = update_chassisdb_config | |||
| $(UPDATE_CHASSISDB_CONFIG_SCRIPT)_PATH = files/scripts | |||
|
|
|||
| SWSS_VARS_TEMPLATE = swss_vars.j2 | |||
| $(SWSS_VARS_TEMPLATE)_PATH = files/build_templates | |||
|
|
|||
| SONIC_COPY_FILES += $(CONFIGDB_LOAD_SCRIPT) \ | |||
There was a problem hiding this comment.
SONIC_COPY_FILES [](start = 0, length = 16)
Why we don't need to change this before?
There was a problem hiding this comment.
The template file was previously put in the docker-orchagent directory, so it could be seen by orchagent docker. Since we want to put this file into more than one docker, I moved it into a shared place and added this change.
|
retest vs please |
|
retest broadcom please |
|
retest broadcom please |
…5644) The orchagent and syncd need to have the same default synchronous mode configuration. This PR adds a template file to translate the default value in CONFIG_DB (empty field) to an explicit mode so that the orchagent and syncd could have the same default mode.
- Why I did it
The orchagent and syncd need to have the same default synchronous mode configuration. This PR adds a template file to translate the default value in CONFIG_DB (empty field) to an explicit mode so that the orchagent and syncd could have the same default mode.
- How I did it
Add a template file to /etc/sonic/ directory so that both syncd and orchagent could access the same template file to determine the default mode.
- How to verify it
Verified with kvm and dvs locally
- Which release branch to backport (provide reason below if selected)
- Description for the changelog
- A picture of a cute animal (not mandatory but encouraged)