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

[VOQ][saidump] Install rdbtools into the docker base related containers. #16466

Merged
merged 10 commits into from
Nov 8, 2023
Merged

[VOQ][saidump] Install rdbtools into the docker base related containers. #16466

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
af63bf5
* [saidump]
JunhongMao Sep 1, 2023
c8f5996
* [saidump]
JunhongMao Sep 6, 2023
1a414d5
https://github.com/sonic-net/sonic-buildimage/pull/16466 fix based on…
JunhongMao Sep 19, 2023
f2215ec
Merge remote-tracking branch 'upstream/master'
JunhongMao Sep 27, 2023
65fc011
https://github.com/sonic-net/sonic-buildimage/pull/16466 fixing based…
JunhongMao Sep 27, 2023
aaac734
https://github.com/sonic-net/sonic-buildimage/pull/16466 fixing based…
JunhongMao Sep 30, 2023
add5b59
https://github.com/sonic-net/sonic-buildimage/pull/16466 fixing based…
JunhongMao Oct 2, 2023
af75a3c
Merge remote-tracking branch 'upstream/master'
JunhongMao Oct 3, 2023
a4fc033
Merge remote-tracking branch 'upstream/master'
JunhongMao Oct 12, 2023
8640e46
To process the review comment: https://github.com/sonic-net/sonic-bui…
JunhongMao Oct 27, 2023
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
4 changes: 4 additions & 0 deletions build_debian.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -729,6 +729,10 @@ SONIC_VERSION_CACHE=${SONIC_VERSION_CACHE} \
DBGOPT="${DBGOPT}" \
scripts/collect_host_image_version_files.sh $CONFIGURED_ARCH $IMAGE_DISTRO $TARGET_PATH $FILESYSTEM_ROOT

## Copy saidump.sh
sudo cp ./files/scripts/saidump.sh $FILESYSTEM_ROOT/usr/local/bin/saidump.sh
sudo chmod +x $FILESYSTEM_ROOT/usr/local/bin/saidump.sh

# Remove GCC
sudo LANG=C DEBIAN_FRONTEND=noninteractive chroot $FILESYSTEM_ROOT apt-get -y remove gcc

Expand Down
46 changes: 46 additions & 0 deletions files/scripts/saidump.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
#!/bin/bash
Copy link
Collaborator

Choose a reason for hiding this comment

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

please move this file to syncd/scripts

Copy link
Contributor Author

@JunhongMao JunhongMao Sep 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Hi Lguohan, there is an issue that's hard for putting saidump.sh in the syncd docker. Because the below command is not available inside the syncd docker due to the isolation between docker containers. I.e., the dump.rdb generated by the Redis save is invisible in the syncd container. So, one reasonable solution is to be leaving it in the host. The host has access to operate the dump.rdb.

  debug "saidump.sh: [2] Move dump.rdb to /var/run/redis$1/ in container database$1."
  docker exec database$1 sh -c "mv /var/lib/redis/dump.rdb /var/run/redis$1/"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@lguohan , please help to review it again. Thanks.

Copy link
Collaborator

Choose a reason for hiding this comment

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

thanks for the explanation. I feel we should still move this saidump.sh to syncd docker, to address the problem you listed, it is better to mount a shared folder between database docker and syncd docker, so that you can see the rdb file in syncd docker once dumped.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for your comments! I will fix it.

Copy link
Collaborator

Choose a reason for hiding this comment

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

i should have be more clear. when i say syncd docker, i really means sairedis repo, so that saidump.sh will be part of the sairedis package.

set -e

function debug()
{
/usr/bin/logger "$1"
}

save_saidump_by_rdb() {
DEV=$1 # ASIC index to operate on

if [ "$DEV" ]; then
debug "saidump.sh: [1] sonic-db-cli -n asic$DEV SAVE."
sonic-db-cli -n asic$DEV SAVE > /dev/null
else
debug "saidump.sh: [1] sonic-db-cli SAVE."
sonic-db-cli SAVE > /dev/null
fi

debug "saidump.sh: [2] Move dump.rdb to /var/run/redis$DEV/ in container database$DEV."
docker exec database$DEV sh -c "mv /var/lib/redis/dump.rdb /var/run/redis$DEV/"
debug "saidump.sh: [3] Run rdb command to convert the dump files into JSON files."
docker exec syncd$DEV sh -c "rdb --command json /var/run/redis$DEV/dump.rdb | tee /var/run/redis$DEV/dump.json > /dev/null"
debug "saidump.sh: [4] Run saidump -r to update the JSON files' format as same as the saidump before. Then we can get the saidump's result in standard output."
docker exec syncd$DEV sh -c "saidump -r /var/run/redis$DEV/dump.json -m 100"
debug "saidump.sh: [5] Clear temporary files."
sudo rm -f /var/run/redis$DEV/dump.rdb
sudo rm -f /var/run/redis$DEV/dump.json
}

NUM_ASICS=`python -c 'from sonic_py_common.multi_asic import get_num_asics; print(get_num_asics())'`

if (( $# == 0 && $NUM_ASICS == 1 )); then
Copy link
Contributor

Choose a reason for hiding this comment

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

there are sudo statements in save_saidump_by_rdb. When a user with only Read-only privileges executes show tech-support these will fail?

Copy link
Contributor Author

@JunhongMao JunhongMao Sep 20, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The command "show techsupport" calls "sudo generate_dump -v -t 5".
By checking the below related commands' mode bits, we can see they are all have "other user executable bit".
And I run the command "show techsupport" in a user with only Read-only privileges, then get correct saidump results.

$ ls `which generate_dump` -l
-rwxr-xr-x 1 root root 66028 Sep 19 14:32 /usr/local/bin/generate_dump
$ ls `which saidump.sh` -l
-rwxr-xr-x 1 root root 1752 Sep 21 15:28 /usr/local/bin/saidump.sh
$ ls `which rm` -l
-rwxr-xr-x 1 root root 72704 Sep 24  2020 /usr/bin/rm

So, finally, remove all sudo for concise.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@arlakshm , please review, thanks.

save_saidump_by_rdb
#validate if the argument is an integer
elif [[ "$1" =~ [0-9]+ ]] && (( $# == 1 && $1 >= 0 && $1 < $NUM_ASICS )) ; then
save_saidump_by_rdb $1
else
echo "The number of ASICS is $NUM_ASICS."
echo "Usage:"
echo "saidump.sh <ASIC Index> or NULL"
echo " <ASIC Index> should be 0 ~ NUM_ASICS."
echo " E.g. \"saidump.sh 1\" is OK when NUMASICS is 2."
echo " NULL"
echo " E.g. \"saidump.sh\" is OK when NUMASICS is 1."
fi
12 changes: 12 additions & 0 deletions platform/broadcom/docker-syncd-brcm-dnx/Dockerfile.j2
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,18 @@ RUN apt-get install -yf kmod
## BRCM uses ethtool to set host interface speed
RUN apt-get install -y ethtool

## Install gcc, libc6-dev and python3-dev for compiling python-lzf
RUN apt-get -y install build-essential libc6-dev python3-dev

## Install python-lzf
RUN pip3 install 'python-lzf==0.2.4'

## Install rdbtools
RUN pip3 install 'rdbtools==0.1.15'

## Uninstall gcc, libc6-dev and python3-dev for compiling python-lzf
RUN apt-get -y remove build-essential libc6-dev python3-dev

COPY ["files/dsserve", "files/bcmcmd", "start.sh", "start_led.sh", "bcmsh", "/usr/bin/"]
RUN chmod +x /usr/bin/dsserve /usr/bin/bcmcmd

Expand Down