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.

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

Jump to bottom

[gearbox] Add peer gbsyncd for swss if gearbox exists #10504

Merged
merged 3 commits into from
Apr 20, 2022
Merged

[gearbox] Add peer gbsyncd for swss if gearbox exists #10504

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b299b7c
Add peer gbsyncd if gearbox exists
jimmyzhai Apr 8, 2022
e7a6ff8
1/ Use ALL_PEERS_RUNNING 2/ Monitor container gbsyncd
jimmyzhai Apr 18, 2022
46191f9
Add gbsyncd to FEATURE table if needed
jimmyzhai Apr 19, 2022
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
6 changes: 5 additions & 1 deletion files/scripts/gbsyncd.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,11 @@
. /usr/local/bin/syncd_common.sh

function startplatform() {
:
# Add gbsyncd to FEATURE table, if not in. It did have same config as syncd.
if [ -z $($SONIC_DB_CLI CONFIG_DB HGET 'FEATURE|gbsyncd' state) ]; then
local CMD="local r=redis.call('DUMP', KEYS[1]); redis.call('RESTORE', KEYS[2], 0, r)"
$SONIC_DB_CLI CONFIG_DB EVAL "$CMD" 2 'FEATURE|syncd' 'FEATURE|gbsyncd'
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @jimmyzhai , @abdosi ,
shouldn't sonic-db-cli here query the global feature list but not the namespace.
This code adds the feature in namespace but the global feature list does not have the gbsyncd entry.
This does not fix the container_checker requirement which looks into the global feature table.

Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Docker syncd/gbsyncd is per-namespace. Should we have syncd/gbsyncd entry at per-namespace feature table?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure which code use the per-namespace entry for features but container_checker script only looks into global namespace for feture state and generates the per-namespace instance name based on the feature name from a global list. So I think gbsyncd also needs to be added to global list.
"show feature state" list the feature from global space and I do not see any namespace option for this CLI.
@abdosi , please comment. Thanks

Copy link
Contributor

Choose a reason for hiding this comment

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

Feature table entry is present both in global and namespace config db. Idea is the entry value will always be same and consistent across global and State DB.

When user enable/disable feature state using cli hostcfgd update value both global and namespace specific db's.
And when we do show commands we get the db value from global DB's and display them.

@jimmyzhai I agree with @anamehra can we update global Feature Table also when we are are doing for namespace so that table is consistent ?

Also with this change we will not need this #10834

cc @arlakshm

fi
}

function waitplatform() {
Expand Down
57 changes: 40 additions & 17 deletions files/scripts/swss.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,13 @@ start_peer_and_dependent_services() {
check_warm_boot

if [[ x"$WARM_BOOT" != x"true" ]]; then
if [[ ! -z $DEV ]]; then
/bin/systemctl start ${PEER}@$DEV
else
/bin/systemctl start ${PEER}
fi
for peer in ${PEER}; do
if [[ ! -z $DEV ]]; then
/bin/systemctl start ${peer}@$DEV
else
/bin/systemctl start ${peer}
fi
done
for dep in ${DEPENDENT}; do
/bin/systemctl start ${dep}
done
Expand All @@ -143,11 +145,13 @@ stop_peer_and_dependent_services() {
for dep in ${DEPENDENT}; do
/bin/systemctl stop ${dep}
done
if [[ ! -z $DEV ]]; then
/bin/systemctl stop ${PEER}@$DEV
else
/bin/systemctl stop ${PEER}
fi
for peer in ${PEER}; do
if [[ ! -z $DEV ]]; then
/bin/systemctl stop ${peer}@$DEV
else
/bin/systemctl stop ${peer}
fi
done
fi
}

Expand Down Expand Up @@ -207,11 +211,18 @@ wait() {
# NOTE: This assumes Docker containers share the same names as their
# corresponding services
for SECS in {1..60}; do
if [[ ! -z $DEV ]]; then
RUNNING=$(docker inspect -f '{{.State.Running}}' ${PEER}$DEV)
else
RUNNING=$(docker inspect -f '{{.State.Running}}' ${PEER})
fi
ALL_PEERS_RUNNING=true
for peer in ${PEER}; do
if [[ ! -z $DEV ]]; then
RUNNING=$(docker inspect -f '{{.State.Running}}' ${peer}$DEV)
else
RUNNING=$(docker inspect -f '{{.State.Running}}' ${peer})
fi
if [[ x"$RUNNING" != x"true" ]]; then
ALL_PEERS_RUNNING=false
break
fi
done
ALL_DEPS_RUNNING=true
for dep in ${MULTI_INST_DEPENDENT}; do
if [[ ! -z $DEV ]]; then
Expand All @@ -225,7 +236,7 @@ wait() {
fi
done

if [[ x"$RUNNING" == x"true" && x"$ALL_DEPS_RUNNING" == x"true" ]]; then
if [[ x"$ALL_PEERS_RUNNING" == x"true" && x"$ALL_DEPS_RUNNING" == x"true" ]]; then
break
else
sleep 1
Expand All @@ -243,7 +254,7 @@ wait() {
done

if [[ ! -z $DEV ]]; then
/usr/bin/docker-wait-any -s ${SERVICE}$DEV -d ${PEER}$DEV ${ALL_DEPS}
/usr/bin/docker-wait-any -s ${SERVICE}$DEV -d `printf "%s$DEV " ${PEER}` ${ALL_DEPS}
else
/usr/bin/docker-wait-any -s ${SERVICE} -d ${PEER} ${ALL_DEPS}
fi
Expand Down Expand Up @@ -283,6 +294,17 @@ stop() {
stop_peer_and_dependent_services
}

function check_peer_gbsyncd()
{
PLATFORM=`$SONIC_DB_CLI CONFIG_DB hget 'DEVICE_METADATA|localhost' platform`
HWSKU=`$SONIC_DB_CLI CONFIG_DB hget 'DEVICE_METADATA|localhost' hwsku`
GEARBOX_CONFIG=/usr/share/sonic/device/$PLATFORM/$HWSKU/$DEV/gearbox_config.json

if [ -f $GEARBOX_CONFIG ]; then
PEER="$PEER gbsyncd"
fi
}

if [ "$DEV" ]; then
NET_NS="$NAMESPACE_PREFIX$DEV" #name of the network namespace
SONIC_DB_CLI="sonic-db-cli -n $NET_NS"
Expand All @@ -291,6 +313,7 @@ else
SONIC_DB_CLI="sonic-db-cli"
fi

check_peer_gbsyncd
read_dependent_services

case "$1" in
Expand Down
8 changes: 8 additions & 0 deletions platform/components/docker-gbsyncd-credo/supervisord.conf.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,14 @@ autorestart=unexpected
startretries=0
exitcodes=0,3
events=PROCESS_STATE
buffer_size=1024

[eventlistener:supervisor-proc-exit-listener]
command=/usr/bin/supervisor-proc-exit-listener --container-name gbsyncd
events=PROCESS_STATE_EXITED,PROCESS_STATE_RUNNING
autostart=true
autorestart=unexpected
buffer_size=1024

[program:rsyslogd]
command=/usr/sbin/rsyslogd -n -iNONE
Expand Down