Skip to content

[synchronous-mode] Add template file for synchronous mode #5644

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.

Sign up for GitHub

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

Merged
merged 6 commits into from
Oct 23, 2020
Merged

[synchronous-mode] Add template file for synchronous mode #5644

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6248c49
add synchronous mode template file
Oct 15, 2020
54bbc32
update orchagent.sh to use synchronous_mode.sh
Oct 15, 2020
54240c6
fix typo
Oct 16, 2020
74c6afc
Combine template file and reduce sonic-cfggen usage
Oct 16, 2020
030247f
update orchagent.sh
Oct 16, 2020
b7eaa05
move swss_var.j2 to docker-config-engine
Oct 21, 2020
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
2 changes: 1 addition & 1 deletion dockers/docker-orchagent/orchagent.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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
Copy link
Collaborator

@qiluo-msft qiluo-msft Oct 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

/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

Copy link
Contributor Author

@shi-su shi-su Oct 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


if [ ! -f "$SWSS_VARS_FILE" ]; then
echo "SWSS vars template file not found"
Expand Down
3 changes: 3 additions & 0 deletions files/build_templates/sonic_debian_extension.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -625,3 +625,6 @@ sudo cp $BUILD_SCRIPTS_DIR/mask_disabled_services.py $FILESYSTEM_ROOT/tmp/
sudo chmod a+x $FILESYSTEM_ROOT/tmp/mask_disabled_services.py
sudo LANG=C chroot $FILESYSTEM_ROOT /tmp/mask_disabled_services.py
sudo rm -rf $FILESYSTEM_ROOT/tmp/mask_disabled_services.py

# Copy synchronous mode configuration template
sudo cp $BUILD_TEMPLATES/swss_syncd_vars.j2 $FILESYSTEM_ROOT_ETC_SONIC/
3 changes: 1 addition & 2 deletions dockers/docker-orchagent/swss_vars.j2 → files/build_templates/swss_syncd_vars.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,5 @@
"asic_type": "{{ asic_type }}",
"asic_id": "{{ DEVICE_METADATA.localhost.asic_id }}",
"mac": "{{ DEVICE_METADATA.localhost.mac }}",
"synchronous_mode": "{{ DEVICE_METADATA.localhost.synchronous_mode }}"
"synchronous_mode": {% if DEVICE_METADATA.localhost.synchronous_mode == "enable" %}"enable"{% else %}"disable"{% endif %}
}

3 changes: 2 additions & 1 deletion platform/p4/docker-sonic-p4.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,8 @@ $(DOCKER_SONIC_P4)_DEPENDS += $(QUAGGA)

$(DOCKER_SONIC_P4)_FILES += $(CONFIGDB_LOAD_SCRIPT) \
$(ARP_UPDATE_SCRIPT) \
$(ARP_UPDATE_VARS_TEMPLATE)
$(ARP_UPDATE_VARS_TEMPLATE) \
$(SWSS_SYNCD_VARS_TEMPLATE)

$(DOCKER_SONIC_P4)_LOAD_DOCKERS += $(DOCKER_CONFIG_ENGINE)
SONIC_DOCKER_IMAGES += $(DOCKER_SONIC_P4)
1 change: 1 addition & 0 deletions platform/p4/docker-sonic-p4/Dockerfile.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ COPY ["supervisord.conf", "/etc/supervisor/conf.d/"]
COPY ["files/configdb-load.sh", "/usr/bin/"]
COPY ["files/arp_update", "/usr/bin"]
COPY ["files/arp_update_vars.j2", "/usr/share/sonic/templates/"]
COPY ["files/swss_syncd_vars.j2", "/etc/sonic/"]
RUN echo "docker-sonic-p4" > /etc/hostname
RUN touch /etc/quagga/zebra.conf

Expand Down
15 changes: 13 additions & 2 deletions platform/p4/docker-sonic-p4/orchagent.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,17 @@
#!/usr/bin/env bash

MAC_ADDRESS=$(sonic-cfggen -d -v 'DEVICE_METADATA.localhost.mac')
EXIT_SWSS_VARS_FILE_NOT_FOUND=1
SWSS_VARS_FILE=/etc/sonic/swss_syncd_vars.j2

if [ ! -f "$SWSS_VARS_FILE" ]; then
echo "SWSS vars template file not found"
exit $EXIT_SWSS_VARS_FILE_NOT_FOUND
fi

# Retrieve SWSS vars from sonic-cfggen
SWSS_VARS=$(sonic-cfggen -d -y /etc/sonic/sonic_version.yml -t $SWSS_VARS_FILE)

MAC_ADDRESS=$(echo $SWSS_VARS | jq -r '.mac')
if [ "$MAC_ADDRESS" == "None" ] || [ -z "$MAC_ADDRESS" ]; then
MAC_ADDRESS=$(ip link show eth0 | grep ether | awk '{print $2}')
logger "Mac address not found in Device Metadata, Falling back to eth0"
Expand All @@ -14,7 +25,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=$(echo $SWSS_VARS | jq -r '.synchronous_mode')
if [ "$SYNC_MODE" == "enable" ]; then
ORCHAGENT_ARGS+="-s "
fi
Expand Down
3 changes: 2 additions & 1 deletion platform/vs/docker-sonic-vs.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,8 @@ $(DOCKER_SONIC_VS)_FILES += $(CONFIGDB_LOAD_SCRIPT) \
$(BUFFERS_CONFIG_TEMPLATE) \
$(QOS_CONFIG_TEMPLATE) \
$(SONIC_VERSION) \
$(UPDATE_CHASSISDB_CONFIG_SCRIPT)
$(UPDATE_CHASSISDB_CONFIG_SCRIPT) \
$(SWSS_SYNCD_VARS_TEMPLATE)

$(DOCKER_SONIC_VS)_LOAD_DOCKERS += $(DOCKER_CONFIG_ENGINE_BUSTER)
SONIC_DOCKER_IMAGES += $(DOCKER_SONIC_VS)
1 change: 1 addition & 0 deletions platform/vs/docker-sonic-vs/Dockerfile.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,7 @@ COPY ["chassis_db.py", "/usr/bin/"]

COPY ["platform.json", "/usr/share/sonic/device/x86_64-kvm_x86_64-r0/"]
COPY ["hwsku.json", "/usr/share/sonic/device/x86_64-kvm_x86_64-r0/Force10-S6000/"]
COPY ["files/swss_syncd_vars.j2", "/etc/sonic/"]

# Workaround the tcpdump issue
RUN mv /usr/sbin/tcpdump /usr/bin/tcpdump
Expand Down
16 changes: 13 additions & 3 deletions platform/vs/docker-sonic-vs/orchagent.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,18 @@ else
export platform=$fake_platform
fi

CFG_VARS=$(sonic-cfggen -d --var-json 'DEVICE_METADATA')
MAC_ADDRESS=$(echo $CFG_VARS | jq -r '.localhost.mac')
EXIT_SWSS_VARS_FILE_NOT_FOUND=1
SWSS_VARS_FILE=/etc/sonic/swss_syncd_vars.j2

if [ ! -f "$SWSS_VARS_FILE" ]; then
echo "SWSS vars template file not found"
exit $EXIT_SWSS_VARS_FILE_NOT_FOUND
fi

# Retrieve SWSS vars from sonic-cfggen
SWSS_VARS=$(sonic-cfggen -d -y /etc/sonic/sonic_version.yml -t $SWSS_VARS_FILE)
Copy link
Collaborator

@qiluo-msft qiluo-msft Oct 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

$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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed the verbose check for the template file.


MAC_ADDRESS=$(echo $SWSS_VARS | jq -r '.mac')
if [ "$MAC_ADDRESS" == "None" ] || [ -z "$MAC_ADDRESS" ]; then
MAC_ADDRESS=$(ip link show eth0 | grep ether | awk '{print $2}')
logger "Mac address not found in Device Metadata, Falling back to eth0"
Expand All @@ -21,7 +31,7 @@ ORCHAGENT_ARGS="-d /var/log/swss "
ORCHAGENT_ARGS+="-b 8192 "

# Set synchronous mode if it is enabled in CONFIG_DB
SYNC_MODE=$(echo $CFG_VARS | jq -r '.localhost.synchronous_mode')
SYNC_MODE=$(echo $SWSS_VARS | jq -r '.synchronous_mode')
if [ "$SYNC_MODE" == "enable" ]; then
ORCHAGENT_ARGS+="-s "
fi
Expand Down
6 changes: 5 additions & 1 deletion rules/scripts.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Collaborator

@qiluo-msft qiluo-msft Oct 16, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SWSS_SYNCD_VARS_TEMPLATE [](start = 0, length = 24)

Wondering why original code don't need it. #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

$(SWSS_SYNCD_VARS_TEMPLATE)_PATH = files/build_templates

SONIC_COPY_FILES += $(CONFIGDB_LOAD_SCRIPT) \
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SONIC_COPY_FILES [](start = 0, length = 16)

Why we don't need to change this before?

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

$(ARP_UPDATE_SCRIPT) \
$(ARP_UPDATE_VARS_TEMPLATE) \
$(BUFFERS_CONFIG_TEMPLATE) \
$(QOS_CONFIG_TEMPLATE) \
$(SUPERVISOR_PROC_EXIT_LISTENER_SCRIPT) \
$(SYSCTL_NET_CONFIG) \
$(UPDATE_CHASSISDB_CONFIG_SCRIPT)
$(UPDATE_CHASSISDB_CONFIG_SCRIPT) \
$(SWSS_SYNCD_VARS_TEMPLATE)