Skip to content

Update telemetry entry sh to support k8s start and keep compatible with systemd start flow #24061

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
qiluo-msft merged 3 commits into from
Sep 26, 2025
Merged

Update telemetry entry sh to support k8s start and keep compatible with systemd start flow #24061

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2709f1e
Update telemetry entry sh to support k8s start and keep compatible with
FengPan-Frank Sep 20, 2025
8aa0b04
Update telemetry entry sh to support k8s start and keep compatible with
FengPan-Frank Sep 20, 2025
79c31ca
Merge branch 'telemetrysh' of https://github.com/FengPan-Frank/sonic-…
FengPan-Frank Sep 25, 2025
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
40 changes: 38 additions & 2 deletions dockers/docker-sonic-telemetry/docker-telemetry-entry.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,44 @@
#!/usr/bin/env bash

# Control with env var launch_by, only used if container is launched via k8s
if [[ "${launch_by:-}" == "k8s" ]]; then
Copy link
Copy Markdown

@make1980 make1980 Sep 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

how do you pass in the environment variable? this is started by supervisord but I assume the environment variable set by k8s will also be available? #Closed

Copy link
Copy Markdown
Contributor Author

@FengPan-Frank FengPan-Frank Sep 21, 2025

Choose a reason for hiding this comment

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

This env will not be set by default so that existing mgmt test will keep working as originally, when k8s rollout telemetry image this env will be set via daemonset config

echo "Running Part 1 because launch_by=k8s"

set -euo pipefail

# --- Create /usr/share/sonic/hwsku symlink (single-ASIC) ---
ENV_FILE="/etc/sonic/sonic-environment"
[[ -r "$ENV_FILE" ]] || { echo "ERROR: $ENV_FILE missing or unreadable."; exit 1; }
# shellcheck source=/dev/null
. "$ENV_FILE"
[[ -n "${PLATFORM:-}" && -n "${HWSKU:-}" ]] || { echo "ERROR: PLATFORM/HWSKU not set in $ENV_FILE."; exit 1; }

resolve() { for p in "$@"; do [[ -e "$p" ]] && { printf '%s\n' "$p"; return 0; }; done; return 1; }
SRC_PLATFORM="$(resolve "/usr/share/sonic/device/${PLATFORM}" "/usr/share/sonic/device/device/${PLATFORM}")" \
Copy link
Copy Markdown

@make1980 make1980 Sep 24, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

/usr/share/sonic/device/device/${PLATFORM}

what's this? shouldn't we remove it? #Closed

Copy link
Copy Markdown
Contributor Author

@FengPan-Frank FengPan-Frank Sep 25, 2025

Choose a reason for hiding this comment

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

seems indeed redundant check, removed.

|| { echo "ERROR: platform dir not found for PLATFORM=$PLATFORM."; exit 1; }
SRC_HWSKU="$(resolve "/usr/share/sonic/device/${PLATFORM}/${HWSKU}" "/usr/share/sonic/device/device/${PLATFORM}/${HWSKU}")" \
|| { echo "ERROR: hwsku dir not found for HWSKU=$HWSKU."; exit 1; }

mkdir -p /usr/share/sonic

DST="/usr/share/sonic/platform"
rm -rf -- "$DST"
ln -s -- "$SRC_PLATFORM" "$DST"

DST="/usr/share/sonic/hwsku"
rm -rf -- "$DST"
ln -s -- "$SRC_HWSKU" "$DST"
Copy link
Copy Markdown
Collaborator

@qiluo-msft qiluo-msft Sep 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

why you need to create symlinks? Is it relating to k8s features? #Closed

Copy link
Copy Markdown
Contributor Author

@FengPan-Frank FengPan-Frank Sep 21, 2025

Choose a reason for hiding this comment

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

not for k8s feature, this was used in currently telemetry.sh

sonic-buildimage/files/build_templates/docker_image_ctl.j2

Line 664 in 1571dd0

docker create {{docker_image_run_opt}} \
, we only try to launch telemetry with the same parameter/mount/env but from k8s instead of systemd path.

Copy link
Copy Markdown

@make1980 make1980 Sep 21, 2025

Choose a reason for hiding this comment

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

k8s can only have the volume name as static instead of generating volume based on label of the node - so we create the symlink at run time based on the node's sku/platform.

Copy link
Copy Markdown
Collaborator

@qiluo-msft qiluo-msft Sep 24, 2025

Choose a reason for hiding this comment

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

The 2 DST folders above are RO mounted folder. I do not understand why you need to rm them and create symlink.

Copy link
Copy Markdown
Contributor Author

@FengPan-Frank FengPan-Frank Sep 24, 2025

Choose a reason for hiding this comment

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

added this logic to first remove the symlink/file/directory at DST in case they're not expected and then run "ln -s" just once.

Copy link
Copy Markdown

@make1980 make1980 Sep 24, 2025

Choose a reason for hiding this comment

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

@qiluo-msft - the DST folders are not mounted. the mounted folder is /usr/share/sonic/device/


[[ "$(readlink -f /usr/share/sonic/platform)" == "$(readlink -f "$SRC_PLATFORM")" ]] || { echo "ERROR: platform symlink verification failed."; exit 1; }
[[ "$(readlink -f /usr/share/sonic/hwsku)" == "$(readlink -f "$SRC_HWSKU")" ]] || { echo "ERROR: hwsku symlink verification failed."; exit 1; }
else
echo "Skipping Part 1 (launch_by=${launch_by:-unset})"
fi

# ---check FEATURE table for switch ---
while true; do
STATE=$(redis-cli -n 4 HGET "FEATURE|telemetry" state)
if [ "$STATE" == "enabled" ]; then
STATE=$(redis-cli -n 4 HGET "FEATURE|telemetry" state || true)
if [[ "${STATE}" == "enabled" ]]; then
echo "Telemetry is enabled. Starting service..."
break
else
Expand Down
Loading