[202511][ci][broadcom] Build syncd-rpc docker for legacy-th image by lipxu · Pull Request #26215 · sonic-net/sonic-buildimage

lipxu · 2026-03-17T01:00:42Z

Why I did it

The legacy-th (Tomahawk) platform requires a dedicated syncd-rpc docker to enable RPC-based SAI testing. Previously only docker-syncd-brcm-rpc was built for the standard broadcom image; there was no equivalent for the legacy-th image variant. This adds docker-syncd-brcm-legacy-th-rpc to support RPC-based SAI testing on Tomahawk legacy platforms.

Work item tracking

Microsoft ADO (number only):

How I did it

Added platform/broadcom/docker-syncd-brcm-legacy-th-rpc.mk — Makefile for the new docker image
Added platform/broadcom/docker-syncd-brcm-legacy-th-rpc/Dockerfile.j2 — Dockerfile template
Added platform/broadcom/docker-syncd-brcm-legacy-th-rpc/base_image_files/bcm_common — BCM common script
Added platform/broadcom/docker-syncd-brcm-legacy-th-rpc/base_image_files/bcmcmd — bcmcmd script
Added platform/broadcom/docker-syncd-brcm-legacy-th-rpc/base_image_files/bcmsh — bcmsh script
Added platform/broadcom/docker-syncd-brcm-legacy-th-rpc/ptf_nn_agent.conf — PTF NN agent config
Updated platform/broadcom/rules.mk to include the new docker in the build

How to verify it

Build the broadcom legacy-th image with the new code
Confirm docker-syncd-brcm-legacy-th-rpc image is produced alongside docker-syncd-brcm-rpc
Boot a Tomahawk legacy-th device with the new image
Verify the syncd-rpc container starts successfully

Which release branch to backport (provide reason below if selected)

Tested branch (Please provide the tested image version)

Description for the changelog

Add docker-syncd-brcm-legacy-th-rpc docker to support RPC-based SAI testing on Tomahawk legacy platforms.

Link to config_db schema for YANG module changes

N/A

A picture of a cute animal (not mandatory but encouraged)

🐧

mssonicbld · 2026-03-17T01:00:51Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2026-03-17T01:01:06Z

Azure Pipelines successfully started running 1 pipeline(s).

Add docker-syncd-brcm-legacy-th-rpc docker and build it in the pipeline alongside the existing docker-syncd-brcm-rpc build step. Cherry-picked from internal commit f2ff21395671ea0b75354604e84b3d007be57ab4 Note: .pipelines/template-stages.yml excluded (ADO-internal only) Signed-off-by: Liping Xu <xuliping@microsoft.com> Signed-off-by: xuliping@microsoft.com <xuliping@microsoft.com>

mssonicbld · 2026-03-17T05:05:25Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2026-03-17T05:05:40Z

Azure Pipelines successfully started running 1 pipeline(s).

platform/broadcom/docker-syncd-brcm-legacy-th-rpc.mk

XuChen-MSFT

Review Summary

Compared against docker-syncd-brcm-rpc (standard RPC) and docker-syncd-brcm-legacy-th (non-RPC base). Shell scripts (bcm_common, bcmcmd, bcmsh) are byte-identical to the legacy-th base — verified by SHA match. Dockerfile.j2 correctly adapts the brcm-rpc template with proper Jinja2 variable renaming (docker_syncd_brcm_legacy_th_rpc_debs). Overall structure looks good.

Issues

1. Missing .dep file (echoing @saiarcot895)
docker-syncd-brcm-rpc.dep exists for the standard RPC docker. A corresponding docker-syncd-brcm-legacy-th-rpc.dep should be added following the same pattern:

DPATH       := $($(DOCKER_SYNCD_BRCM_LEGACY_TH_RPC)_PATH)
DEP_FILES   := $(SONIC_COMMON_FILES_LIST) $(PLATFORM_PATH)/docker-syncd-brcm-legacy-th-rpc.mk $(PLATFORM_PATH)/docker-syncd-brcm-legacy-th-rpc.dep
DEP_FILES   += $(SONIC_COMMON_BASE_FILES_LIST)
DEP_FILES   += $(shell git ls-files $(DPATH))

$(DOCKER_SYNCD_BRCM_LEGACY_TH_RPC)_CACHE_MODE  := GIT_CONTENT_SHA
$(DOCKER_SYNCD_BRCM_LEGACY_TH_RPC)_DEP_FLAGS   := $(SONIC_COMMON_FLAGS_LIST)
$(DOCKER_SYNCD_BRCM_LEGACY_TH_RPC)_DEP_FILES   := $(DEP_FILES)

2. $(SSWSYNCD) dependency placement inconsistency (docker-syncd-brcm-legacy-th-rpc.mk line 6)
In the standard docker-syncd-brcm-rpc.mk, $(SSWSYNCD) is inside the ifeq ($(INSTALL_DEBUG_TOOLS), y) block (debug-only). In this PR it's an unconditional top-level dependency. The non-RPC legacy-th base also has it unconditional, so this may be intentional — but please confirm whether the RPC container truly needs sswsyncd in non-debug builds.

3. Consider adding _AFTER build ordering (docker-syncd-brcm-legacy-th-rpc.mk)
The non-RPC legacy-th base has $(DOCKER_SYNCD_LEGACY_TH_BASE)_AFTER = $(DOCKER_SYNCD_DNX_BASE) to prevent build race conditions. The RPC variant may need a similar directive (e.g., _AFTER = $(DOCKER_SYNCD_LEGACY_TH_BASE)) to ensure the base image is built first.

- Move \ inside ifeq (INSTALL_DEBUG_TOOLS) block to align with docker-syncd-brcm-rpc.mk (only needed for debug builds) - Add _AFTER = \ to enforce build ordering and prevent race conditions with the base image - Add docker-syncd-brcm-legacy-th-rpc.dep for cache invalidation support, following the same pattern as docker-syncd-brcm-rpc.dep Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: xuliping@microsoft.com <xuliping@microsoft.com>

mssonicbld · 2026-03-20T03:38:06Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2026-03-20T03:38:19Z

Azure Pipelines successfully started running 1 pipeline(s).

Replace bcmcmd/bcmsh with the simpler single-ASIC versions used by docker-syncd-brcm-rpc, and remove bcm_common. The RPC container is used for SAI testing on single-ASIC setups, so the multi-ASIC capable scripts from the non-RPC legacy-th base are unnecessary. This aligns all RPC containers to use consistent, simpler host-side BCM scripts. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: xuliping@microsoft.com <xuliping@microsoft.com>

mssonicbld · 2026-03-20T04:14:12Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2026-03-20T04:14:25Z

Azure Pipelines successfully started running 1 pipeline(s).

…ncd-brcm-legacy-th-rpc.mk _LOAD_DOCKERS already implies build ordering dependency on the base image, consistent with docker-syncd-brcm-rpc.mk which also uses _LOAD_DOCKERS without any _AFTER directive. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: xuliping@microsoft.com <xuliping@microsoft.com>

mssonicbld · 2026-03-20T05:05:43Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2026-03-20T05:05:57Z

Azure Pipelines successfully started running 1 pipeline(s).

lipxu · 2026-03-20T06:00:29Z

@XuChen-MSFT Thank you for the thorough review! Here's how each item was addressed:

1. Missing .dep file — Fixed in commit 0b2ffc238. Added platform/broadcom/docker-syncd-brcm-legacy-th-rpc.dep following the same pattern as docker-syncd-brcm-rpc.dep.

2. $(SSWSYNCD) dependency placement — Fixed in commit 0b2ffc238. Moved $(SSWSYNCD) inside the ifeq ($(INSTALL_DEBUG_TOOLS), y) block to be consistent with docker-syncd-brcm-rpc.mk. The RPC container is used for SAI testing and does not require sswsyncd in non-debug builds.

3. _AFTER build ordering — After further investigation, we found that _LOAD_DOCKERS already implies the build ordering dependency (the base image must be built first). The standard docker-syncd-brcm-rpc.mk also uses _LOAD_DOCKERS without any _AFTER directive, confirming it is not needed. Therefore _AFTER was not added (removed in commit c9fa32809).

…rcm-legacy-th.mk PR sonic-net#25071 migrated warmboot mounts from the static -v /host/warmboot:/var/warmboot in .mk files to a dynamic -v /host/warmbootDEV:/var/warmboot generated at runtime via docker_image_ctl.j2 to support multi-ASIC devices. However, it missed updating docker-syncd-brcm-legacy-th.mk. This commit applies the same fix to align with the changes introduced in PR sonic-net#25071. Note: docker-syncd-brcm-legacy-th-rpc.mk will be updated in a follow-up commit once PR sonic-net#26215 is merged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: xuliping@microsoft.com <xuliping@microsoft.com>

XuChen-MSFT

All 3 review items have been addressed:

Missing .dep file — ✅ docker-syncd-brcm-legacy-th-rpc.dep added following docker-syncd-brcm-rpc.dep pattern
** placement** — ✅ Moved inside ifeq (, y) block, consistent with docker-syncd-brcm-rpc.mk
_AFTER build ordering — ✅ Not needed — _LOAD_DOCKERS already implies build ordering, consistent with standard RPC docker

Code is correct and consistent with codebase patterns. Approving.

XuChen-MSFT

All 3 review items have been addressed:

Missing .dep file — Added docker-syncd-brcm-legacy-th-rpc.dep following docker-syncd-brcm-rpc.dep pattern
SSWSYNCD placement — Moved inside ifeq (INSTALL_DEBUG_TOOLS) block, consistent with docker-syncd-brcm-rpc.mk
_AFTER build ordering — Not needed — _LOAD_DOCKERS already implies build ordering, consistent with standard RPC docker

Code is correct and consistent with codebase patterns. Approving.

XuChen-MSFT

All review comments have been addressed. Approving.

XuChen-MSFT

All review comments have been addressed. Approving.

saiarcot895 · 2026-03-20T21:46:35Z

platform/broadcom/docker-syncd-brcm-legacy-th-rpc/Dockerfile.j2

+
+FROM $BASE
+
+RUN --mount=type=bind,from=base,target=/changes-to-image rsync -axAX --no-D --exclude=/sys --exclude=/proc --exclude=/dev --exclude=resolv.conf /changes-to-image/ /


Use the rsync_from_builder_stage here.

Thank you for the suggestion! Fixed in commit cd3c343e4.

XuChen-MSFT

All review comments have been addressed. Approving.

XuChen-MSFT

All review comments have been addressed. Approving.

mssonicbld · 2026-03-21T09:31:23Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2026-03-21T09:31:36Z

Azure Pipelines successfully started running 1 pipeline(s).

Use the rsync_from_builder_stage() Jinja2 macro instead of the inline rsync command to apply builder stage changes. The macro also includes the --omit-dir-times flag which was missing from the raw command. Signed-off-by: xuliping@microsoft.com <xuliping@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>

mssonicbld · 2026-03-21T09:37:13Z

/azp run Azure.sonic-buildimage

azure-pipelines · 2026-03-21T09:37:25Z

Azure Pipelines successfully started running 1 pipeline(s).

…rcm-legacy-th.mk PR sonic-net#25071 migrated warmboot mounts from the static -v /host/warmboot:/var/warmboot in .mk files to a dynamic -v /host/warmbootDEV:/var/warmboot generated at runtime via docker_image_ctl.j2 to support multi-ASIC devices. However, it missed updating docker-syncd-brcm-legacy-th.mk. This commit applies the same fix to align with the changes introduced in PR sonic-net#25071. Note: docker-syncd-brcm-legacy-th-rpc.mk will be updated in a follow-up commit once PR sonic-net#26215 is merged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Signed-off-by: xuliping@microsoft.com <xuliping@microsoft.com>

lipxu requested a review from lguohan as a code owner March 17, 2026 01:00

lipxu force-pushed the dev/xuliping/20260317_202511_rpcdocker branch from bbc33db to b011dbb Compare March 17, 2026 05:05

saiarcot895 reviewed Mar 17, 2026

View reviewed changes

platform/broadcom/docker-syncd-brcm-legacy-th-rpc.mk Show resolved Hide resolved

saiarcot895 reviewed Mar 17, 2026

View reviewed changes

platform/broadcom/docker-syncd-brcm-legacy-th-rpc.mk Show resolved Hide resolved

XuChen-MSFT reviewed Mar 19, 2026

View reviewed changes

lipxu mentioned this pull request Mar 20, 2026

[platform/broadcom]: Remove static warmboot mount from docker-syncd-brcm-legacy-th.mk #26292

Open

6 tasks

XuChen-MSFT approved these changes Mar 20, 2026

View reviewed changes

saiarcot895 reviewed Mar 20, 2026

View reviewed changes

saiarcot895 requested changes Mar 20, 2026

View reviewed changes

XuChen-MSFT approved these changes Mar 20, 2026

View reviewed changes

XuChen-MSFT approved these changes Mar 21, 2026

View reviewed changes

lipxu force-pushed the dev/xuliping/20260317_202511_rpcdocker branch from c0e0208 to cd3c343 Compare March 21, 2026 09:37

saiarcot895 approved these changes Mar 23, 2026

View reviewed changes

vmittal-msft approved these changes Mar 23, 2026

View reviewed changes

vmittal-msft merged commit c473f31 into sonic-net:202511 Mar 23, 2026
13 checks passed

lipxu mentioned this pull request Mar 23, 2026

[ci][broadcom] Build syncd-rpc docker for legacy-th image #26340

Merged

7 tasks

lipxu changed the title ~~[ci][broadcom] Build syncd-rpc docker for legacy-th image~~ [202511][ci][broadcom] Build syncd-rpc docker for legacy-th image Mar 26, 2026


		FROM $BASE

		RUN --mount=type=bind,from=base,target=/changes-to-image rsync -axAX --no-D --exclude=/sys --exclude=/proc --exclude=/dev --exclude=resolv.conf /changes-to-image/ /

Conversation

lipxu commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why I did it

Work item tracking

How I did it

How to verify it

Which release branch to backport (provide reason below if selected)

Tested branch (Please provide the tested image version)

Description for the changelog

Link to config_db schema for YANG module changes

A picture of a cute animal (not mandatory but encouraged)

Uh oh!

mssonicbld commented Mar 17, 2026

Uh oh!

azure-pipelines bot commented Mar 17, 2026

Uh oh!

mssonicbld commented Mar 17, 2026

Uh oh!

azure-pipelines bot commented Mar 17, 2026

Uh oh!

Uh oh!

Uh oh!

XuChen-MSFT left a comment

Choose a reason for hiding this comment

Review Summary

Issues

Uh oh!

mssonicbld commented Mar 20, 2026

Uh oh!

azure-pipelines bot commented Mar 20, 2026

Uh oh!

mssonicbld commented Mar 20, 2026

Uh oh!

azure-pipelines bot commented Mar 20, 2026

Uh oh!

mssonicbld commented Mar 20, 2026

Uh oh!

azure-pipelines bot commented Mar 20, 2026

Uh oh!

lipxu commented Mar 20, 2026

Uh oh!

XuChen-MSFT left a comment

Choose a reason for hiding this comment

Uh oh!

XuChen-MSFT left a comment

Choose a reason for hiding this comment

Uh oh!

XuChen-MSFT left a comment

Choose a reason for hiding this comment

Uh oh!

XuChen-MSFT left a comment

Choose a reason for hiding this comment

Uh oh!

saiarcot895 Mar 20, 2026

Choose a reason for hiding this comment

Uh oh!

lipxu Mar 21, 2026

Choose a reason for hiding this comment

Uh oh!

XuChen-MSFT left a comment

Choose a reason for hiding this comment

Uh oh!

XuChen-MSFT left a comment

Choose a reason for hiding this comment

Uh oh!

mssonicbld commented Mar 21, 2026

Uh oh!

azure-pipelines bot commented Mar 21, 2026

Uh oh!

mssonicbld commented Mar 21, 2026

Uh oh!

azure-pipelines bot commented Mar 21, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

lipxu commented Mar 17, 2026 •

edited

Loading