Updated Makefile infrastructure to build debug images. #2753

renukamanavalan · 2019-04-05T23:35:30Z

As a sample, platform/broadcom/docker-orchagent-brcm.mk is updated to add a docker-orchagent-brcm-dbg.gz target.

Now "BLDENV=stretch make target/docker-orchagent-brcm-dbg.gz" will build the debug image.

NOTE: If you don't specify NOSTRETcH=1, it implicitly calls "make stretch", which builds all stretch targets and that would include debug dockers too.

This debug image can be used in any linux box to inspect core file. If your module's external dependency can be suitably mocked, you my even manually run it inside.

"docker run -it --entrypoint=/bin/bash e47a8fb8ed38"

You may map the core file path to this docker run.

- What I did

- How I did it

- How to verify it
BLDENV=stretch make target/docker-orchagent-brcm-dbg.gz

- Description for the changelog

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

As a sample, platform/broadcom/docker-orchagent-brcm.mk is updated to add a docker-orchagent-brcm-dbg.gz target. Now "BLDENV=stretch make target/docker-orchagent-brcm-dbg.gz" will build the debug image. NOTE: If you don't specify NOSTRETcH=1, it implicitly calls "make stretch", which builds all stretch targets and that would include debug dockers too. This debug image can be used in any linux box to inspect core file. If your module's external dependency can be suitably mocked, you my even manually run it inside. "docker run -it --entrypoint=/bin/bash e47a8fb8ed38" You may map the core file path to this docker run.

… to help readability.

renukamanavalan · 2019-04-06T03:13:47Z

Let me look at the failures

…UG_TOOLS=y. When this change ('building debug docker image transparently') is extended to all dockers, this flag would become redundant. Yet, there can be some test based use cases that rely on this flag. Until after all the dockers gets their debug images by default and we switch all use cases of this flag to use the newly built debug images, we need to maintain the existing behavior.

renukamanavalan · 2019-04-08T02:57:11Z

Tested by comparing Dockerfiles used for base-strerch, config-engine-stretch and orchagent-brcm, by creating them in the following 4 scenarios

With no update to any code
1.a) With INSTALL_DEBUG_TOOLS not set
1.b) With INSTALL_DEBUG_TOOLS set
With all the updates to code applied
2.a) With INSTALL_DEBUG_TOOLS not set
2.b) With INSTALL_DEBUG_TOOLS set

Has built sonic-broadcom.bin with all updates, Need to see, how to vet it, for a double confirmation.

lguohan · 2019-04-10T21:07:45Z

slave.mk

+ # Export variables for j2. Use path for unique variable names, e.g. docker_orchagent_debs
+ $(eval export $(subst -,_,$(notdir $($*.gz_PATH)))_dbg_debs=$(shell printf "$(subst $(SPACE),\n,$(call expand,$($*.gz_DBG_DEPENDS),RDEPENDS))\n" | awk '!a[$$0]++'))
+ $(eval export $(subst -,_,$(notdir $($*.gz_PATH)))_image_dbgs=$(shell printf "$(subst $(SPACE),\n,$(call expand,$($*.gz_DBG_IMAGE_PACKAGES)))\n" | awk '!a[$$0]++'))
+ ./build_dbg_j2.sh $* $(subst -,_,$(notdir $($*.gz_PATH)))_dbg_debs $(subst -,_,$(notdir $($*.gz_PATH)))_image_dbgs > $($*.gz_PATH)/Dockerfile-dbg.j2


rename to build_debug_docker_j2.sh for better clarity.

lguohan · 2019-04-10T21:08:30Z

build_dbg_j2.sh

+RUN apt-get clean -y; apt-get autoclean -y; apt-get autoremove -y
+RUN rm -rf /debs
+
+CMD ["/usr/bin/supervisord"]


this should be the same as the base docker, it should be removed.

Actually, it loses it significance as base docker has ENTRYPOINT.

BTW, I would like to make it CMD in the base docker image, which allows an easy override. But it can wait, until a bigger change is in demand.

lguohan · 2019-04-10T21:22:28Z

slave.mk

+ --build-arg guid=$(GUID) \
+ --build-arg docker_container_name=$($*.gz_CONTAINER_NAME) \
+ --build-arg frr_user_uid=$(FRR_USER_UID) \
+ --build-arg frr_user_gid=$(FRR_USER_GID) \


build-arg for user, uid, guid, frr_user_uid, frr_user_gid are not needed.

2) Debug template builder: renamed build_dbg_j2.sh to build_debug_docker_j2.sh 3) Dropped insignifcant statement CMD from debug Docker file, as base docker has Entrypoint.

lguohan · 2019-04-11T01:36:23Z

slave.mk

 --build-arg docker_container_name=$($*.gz_CONTAINER_NAME) \
- --build-arg frr_user_uid=$(FRR_USER_UID) \
- --build-arg frr_user_gid=$(FRR_USER_GID) \


we need to keep those for the base docker images, we do not need them for the debug docker images.

lguohan · 2019-04-11T01:45:15Z

slave.mk

-ifeq ($(SONIC_ROUTING_STACK),frr)
-$(info "FRR_USER_UID" : "$(FRR_USER_UID)")
-$(info "FRR_USER_GID" : "$(FRR_USER_GID)")
-endif


lguohan · 2019-04-11T01:45:47Z

rules/docker-base-stretch.mk

 GDB = gdb
 GDBSERVER = gdbserver
 VIM = vim
 OPENSSH = openssh-client
 SSHPASS = sshpass
 STRACE = strace
-$(DOCKER_BASE_STRETCH)_DBG_PACKAGES += $(GDB) $(GDBSERVER) $(VIM) $(OPENSSH) $(SSHPASS) $(STRACE)
+$(DOCKER_BASE_STRETCH)_DBG_IMAGE_PACKAGES += $(GDB) $(GDBSERVER) $(VIM) $(OPENSSH) $(SSHPASS) $(STRACE)
+ifeq ($(INSTALL_DEBUG_TOOLS),y)


since we are install those debug packages in the DBG docker images, we likely do not need this option now.

Eventually we would not need. Below is my comments (copy/paste from commit comments):

"
Tweaked the changes to retain the existing behavior w.r.t INSTALL_DEBUG_TOOLS=y.

When this change ('building debug docker image transparently') is extended to all dockers, this flag would become redundant. Yet, there can be some test based use cases that rely on this flag.

Until after all the dockers gets their debug images by default and we switch all use cases of this flag to use the newly built debug images, we need to maintain the existing behavior.
"

"User, uid, guid, frr-uid & frr-guid" are required for all docker images, with exception of debug images.

…(SONIC_STRETCH_DOCKERS_FOR_INSTALLERS) and build debug-dockers only for those to be built and debug target is available.

…c-net#2753) * Updated Makefile infrastructure to build debug images. As a sample, platform/broadcom/docker-orchagent-brcm.mk is updated to add a docker-orchagent-brcm-dbg.gz target. Now "BLDENV=stretch make target/docker-orchagent-brcm-dbg.gz" will build the debug image. This debug image can be used in any linux box to inspect core file. If your module's external dependency can be suitably mocked, you my even manually run it inside. "docker run -it --entrypoint=/bin/bash e47a8fb8ed38" You may map the core file path to this docker run. * Dropped the regular binary using DBG_PACKAGES and a small name change to help readability. * Tweaked the changes to retain the existing behavior w.r.t INSTALL_DEBUG_TOOLS=y. When this change ('building debug docker image transparently') is extended to all dockers, this flag would become redundant. Yet, there can be some test based use cases that rely on this flag. Until after all the dockers gets their debug images by default and we switch all use cases of this flag to use the newly built debug images, we need to maintain the existing behavior.

Update sonic-utilities submodule pointer to include the following: * 88ffb167 [config]config reload should generate sysinfo if missing ([sonic-net#2778](sonic-net/sonic-utilities#2778)) * 7443b9e5 [sonic-package-manager] support extension with multiple YANG modules ([sonic-net#2752](sonic-net/sonic-utilities#2752)) * 522c3a9e [sonic-package-manager] add support for multiple CLI plugin files ([sonic-net#2753](sonic-net/sonic-utilities#2753)) * b38fcfd1 [show][muxcable] fix RC ([sonic-net#2812](sonic-net/sonic-utilities#2812)) * 7e24463f [chassis]: remote cli commands infra for sonic chassis ([sonic-net#2701](sonic-net/sonic-utilities#2701)) * bee593e4 [DPB]Fixing typo in config breakout output ([sonic-net#2802](sonic-net/sonic-utilities#2802)) * ada603c5 [config]Support multi-asic Golden Config override ([sonic-net#2738](sonic-net/sonic-utilities#2738)) * 88a7daa8 [show][barefoot] replace shell=True ([sonic-net#2699](sonic-net/sonic-utilities#2699)) * 5e99edb5 [sonic_package_manager] replace shell=True ([sonic-net#2726](sonic-net/sonic-utilities#2726)) * b547bb45 [acl-loader] Only add default deny rule when table is L3 or L3V6 ([sonic-net#2796](sonic-net/sonic-utilities#2796)) Signed-off-by: dprital <drorp@nvidia.com>

Why I did it 69abbc3c - (HEAD, origin/master, origin/HEAD) Revert "[GCU] Complete RDMA Platform Validation Checks [device][platform] Update Inventec new platform d6356 #2791" DellEMC S6100 Watchdog Support #2854 (8 minutes ago) 4fead896 - [sonic-package-manager] fix CLI plugin compatibility issue [sonic-utilities] advance submodule head to latest #2842 (27 hours ago) db61efca - [vlan][dhcp_relay] Clear dhcpv6 relay counter while deleting vlan ([201811] [services] Restart SwSS service upon unexpected critical process exit #2852) (33 hours ago) d5544b4a - [config] Generate sysinfo as needed when override config ([minigraph]: Add mirror type v6 condition #2836) (6 days ago) f258e2a3 - [GCU] Complete RDMA Platform Validation Checks ([device][platform] Update Inventec new platform d6356 #2791) (6 days ago) b4f4e63e - Revert "Revert frr route check ([mlnx] fix url inconsistency in fw.mk #2761)" (Support TACACS Accounting #2762) (7 days ago) 3d89589f - Update pcieutil error message on loading common pcie module (Enable Debugs in BCM Kernel-bde and Knet Modules #2786) (11 days ago) e6aacd37 - Update TRANSCEIVER_INFO table after CDB FW upgrade (Remove unused packages in docker images and host (#2807) #2837) (2 weeks ago) 33d665c4 - replace shell=True, replace xml, and replace exit() ([mellanox-simx] add ability to build simx-compatiable image #2664) (2 weeks ago) 9e510a83 - [chassis][voq[Add "config fabric port ..." commands and tests. (Watchdog enable/disable in DellEMC S6100 #2730) (2 weeks ago) aeb0dbc1 - Fix the invalid variable issue when set-fips in uboot (fix bug in file sonic-cfggen #2834) (3 weeks ago) 1e73632d - [test]: add UT coverage for GCU (Feed device info to orchagent process #2818) (3 weeks ago) 3a9995b6 - [config]Support multi-asic Golden Config override with fix ([mellanox] Update Mellanox MFT packedge #2825) (3 weeks ago) 3fb32588 - Revert "[chassis]: remote cli commands infra for sonic chassis ([mellanox] add makefiles to build Mellanox SDK from sources #2701)" ([dhcp_relay] Base DHCP Relay Docker container on Debian Stretch #2832) (3 weeks ago) 2ffe6e37 - [show][mlnx] replace shell=True, replace xml (Add support of HwSKU Mellanox-SN2700-C28D8 #2700) (3 weeks ago) a5091bba - [sonic_sku_create] remove shell=True, replace exit() with sys.exit() (removed exec from script which that prevents the further lines to be … #2816) (3 weeks ago) 71ef4f16 - [build] Fix base OS compilation issue caused by incompatibility with requests >= 2.29.0. ([201811][sairedis][utilities] advance sub module heads #2830) (3 weeks ago) 1097373b - [show] Added alias interface mode support for 'show interfaces counters ...' command ([kernel]: update sonic kernel to 4.9.0-8-2 #2468) (4 weeks ago) <Julian Chang - TW> 589375fc - correctly parsing complete ipv6 vnet info ([201811][mellanox] Update Mellanox FW version to 13.1910.0928 #2827) (4 weeks ago) 634ac77c - LAG keepalive script to reduce lacp session wait during warm-reboot (Set proper hostname on containers startup #2806) (4 weeks ago) 331c9de0 - [config]: Dynamically start and stop ndppd ([Arista] Add QoS needed files for Arista 7170 #2814) (4 weeks ago) d1f307d0 - [GCU]Fix rdma check failure ([device/celestica]: Add fwutil #2824) (4 weeks ago) ce81a340 - Revert "[config]Support multi-asic Golden Config override (Before issue “sonic-clear counters”, “show interface counters” result not complete #2738)" ([BGP docker]: start bgp_eoiu_mark service to populate bgp eoiu marker… #2823) (4 weeks ago) 61e0e810 - Added platform plugin support in load_minigraph ([db migrator] migrate the DB to latest schema when needed #2808) (4 weeks ago) d4355a96 - Change default CDB run mode to non-hitless (Revert "Watchdog enable/disable in DellEMC S6100 " #2817) (4 weeks ago) 88ffb167 - [config]config reload should generate sysinfo if missing ([Mellanox] Update SAI #2778) (4 weeks ago) 7443b9e5 - [sonic-package-manager] support extension with multiple YANG modules (dhcp_relay service stopped with "systemctl stop swss" but not restarted with "systemctl restart swss" #2752) (4 weeks ago) 522c3a9e - [sonic-package-manager] add support for multiple CLI plugin files (Updated Makefile infrastructure to build debug images. #2753) (4 weeks ago) b38fcfd1 - [show][muxcable] fix show mux hwmode muxdirection RC (syncd-rpc.mk: Fix stretch dockers build failure #2812) (5 weeks ago) 7e24463f - [chassis]: remote cli commands infra for sonic chassis ([mellanox] add makefiles to build Mellanox SDK from sources #2701) (6 weeks ago) bee593e4 - [DPB]Fixing typo in config breakout output ([submodule update]: Quagga bgpd crash fix #2802) (6 weeks ago) ada603c5 - [config]Support multi-asic Golden Config override (Before issue “sonic-clear counters”, “show interface counters” result not complete #2738) (6 weeks ago) 88a7daa8 - [show][barefoot] replace shell=True ([teamd] retry creating team_port after interface info changed #2699) (6 weeks ago) 5e99edb5 - [sonic_package_manager] replace shell=True (Upgrade Mellanox HW-MGMT: fix high CPU utilization issue #2726) (6 weeks ago) b547bb45 - [acl-loader] Only add default deny rule when table is L3 or L3V6 ([201811] [radvd] Build radvd from source; Patch so as not to treat out-of-range MTU as an error #2796) (6 weeks ago)

Why I did it 69abbc3c - (HEAD, origin/master, origin/HEAD) Revert "[GCU] Complete RDMA Platform Validation Checks [device][platform] Update Inventec new platform d6356 sonic-net#2791" DellEMC S6100 Watchdog Support sonic-net#2854 (8 minutes ago) 4fead896 - [sonic-package-manager] fix CLI plugin compatibility issue [sonic-utilities] advance submodule head to latest sonic-net#2842 (27 hours ago) db61efca - [vlan][dhcp_relay] Clear dhcpv6 relay counter while deleting vlan ([201811] [services] Restart SwSS service upon unexpected critical process exit sonic-net#2852) (33 hours ago) d5544b4a - [config] Generate sysinfo as needed when override config ([minigraph]: Add mirror type v6 condition sonic-net#2836) (6 days ago) f258e2a3 - [GCU] Complete RDMA Platform Validation Checks ([device][platform] Update Inventec new platform d6356 sonic-net#2791) (6 days ago) b4f4e63e - Revert "Revert frr route check ([mlnx] fix url inconsistency in fw.mk sonic-net#2761)" (Support TACACS Accounting sonic-net#2762) (7 days ago) 3d89589f - Update pcieutil error message on loading common pcie module (Enable Debugs in BCM Kernel-bde and Knet Modules sonic-net#2786) (11 days ago) e6aacd37 - Update TRANSCEIVER_INFO table after CDB FW upgrade (Remove unused packages in docker images and host (sonic-net#2807) sonic-net#2837) (2 weeks ago) 33d665c4 - replace shell=True, replace xml, and replace exit() ([mellanox-simx] add ability to build simx-compatiable image sonic-net#2664) (2 weeks ago) 9e510a83 - [chassis][voq[Add "config fabric port ..." commands and tests. (Watchdog enable/disable in DellEMC S6100 sonic-net#2730) (2 weeks ago) aeb0dbc1 - Fix the invalid variable issue when set-fips in uboot (fix bug in file sonic-cfggen sonic-net#2834) (3 weeks ago) 1e73632d - [test]: add UT coverage for GCU (Feed device info to orchagent process sonic-net#2818) (3 weeks ago) 3a9995b6 - [config]Support multi-asic Golden Config override with fix ([mellanox] Update Mellanox MFT packedge sonic-net#2825) (3 weeks ago) 3fb32588 - Revert "[chassis]: remote cli commands infra for sonic chassis ([mellanox] add makefiles to build Mellanox SDK from sources sonic-net#2701)" ([dhcp_relay] Base DHCP Relay Docker container on Debian Stretch sonic-net#2832) (3 weeks ago) 2ffe6e37 - [show][mlnx] replace shell=True, replace xml (Add support of HwSKU Mellanox-SN2700-C28D8 sonic-net#2700) (3 weeks ago) a5091bba - [sonic_sku_create] remove shell=True, replace exit() with sys.exit() (removed exec from script which that prevents the further lines to be … sonic-net#2816) (3 weeks ago) 71ef4f16 - [build] Fix base OS compilation issue caused by incompatibility with requests >= 2.29.0. ([201811][sairedis][utilities] advance sub module heads sonic-net#2830) (3 weeks ago) 1097373b - [show] Added alias interface mode support for 'show interfaces counters ...' command ([kernel]: update sonic kernel to 4.9.0-8-2 sonic-net#2468) (4 weeks ago) <Julian Chang - TW> 589375fc - correctly parsing complete ipv6 vnet info ([201811][mellanox] Update Mellanox FW version to 13.1910.0928 sonic-net#2827) (4 weeks ago) 634ac77c - LAG keepalive script to reduce lacp session wait during warm-reboot (Set proper hostname on containers startup sonic-net#2806) (4 weeks ago) 331c9de0 - [config]: Dynamically start and stop ndppd ([Arista] Add QoS needed files for Arista 7170 sonic-net#2814) (4 weeks ago) d1f307d0 - [GCU]Fix rdma check failure ([device/celestica]: Add fwutil sonic-net#2824) (4 weeks ago) ce81a340 - Revert "[config]Support multi-asic Golden Config override (Before issue “sonic-clear counters”, “show interface counters” result not complete sonic-net#2738)" ([BGP docker]: start bgp_eoiu_mark service to populate bgp eoiu marker… sonic-net#2823) (4 weeks ago) 61e0e810 - Added platform plugin support in load_minigraph ([db migrator] migrate the DB to latest schema when needed sonic-net#2808) (4 weeks ago) d4355a96 - Change default CDB run mode to non-hitless (Revert "Watchdog enable/disable in DellEMC S6100 " sonic-net#2817) (4 weeks ago) 88ffb167 - [config]config reload should generate sysinfo if missing ([Mellanox] Update SAI sonic-net#2778) (4 weeks ago) 7443b9e5 - [sonic-package-manager] support extension with multiple YANG modules (dhcp_relay service stopped with "systemctl stop swss" but not restarted with "systemctl restart swss" sonic-net#2752) (4 weeks ago) 522c3a9e - [sonic-package-manager] add support for multiple CLI plugin files (Updated Makefile infrastructure to build debug images. sonic-net#2753) (4 weeks ago) b38fcfd1 - [show][muxcable] fix show mux hwmode muxdirection RC (syncd-rpc.mk: Fix stretch dockers build failure sonic-net#2812) (5 weeks ago) 7e24463f - [chassis]: remote cli commands infra for sonic chassis ([mellanox] add makefiles to build Mellanox SDK from sources sonic-net#2701) (6 weeks ago) bee593e4 - [DPB]Fixing typo in config breakout output ([submodule update]: Quagga bgpd crash fix sonic-net#2802) (6 weeks ago) ada603c5 - [config]Support multi-asic Golden Config override (Before issue “sonic-clear counters”, “show interface counters” result not complete sonic-net#2738) (6 weeks ago) 88a7daa8 - [show][barefoot] replace shell=True ([teamd] retry creating team_port after interface info changed sonic-net#2699) (6 weeks ago) 5e99edb5 - [sonic_package_manager] replace shell=True (Upgrade Mellanox HW-MGMT: fix high CPU utilization issue sonic-net#2726) (6 weeks ago) b547bb45 - [acl-loader] Only add default deny rule when table is L3 or L3V6 ([201811] [radvd] Build radvd from source; Patch so as not to treat out-of-range MTU as an error sonic-net#2796) (6 weeks ago)

renukamanavalan requested a review from lguohan April 5, 2019 23:35

Dropped the regular binary using DBG_PACKAGES and a small name change…

042d51e

… to help readability.

lguohan reviewed Apr 10, 2019

View reviewed changes

1) slave.mk - Dropped unused Docker build args

4443c70

2) Debug template builder: renamed build_dbg_j2.sh to build_debug_docker_j2.sh 3) Dropped insignifcant statement CMD from debug Docker file, as base docker has Entrypoint.

lguohan reviewed Apr 11, 2019

View reviewed changes

renukamanavalan added 3 commits April 11, 2019 17:05

Reverted some changes, per review comments.

7b84e64

"User, uid, guid, frr-uid & frr-guid" are required for all docker images, with exception of debug images.

Merge remote-tracking branch 'upstream/master'

c717e50

Get in sync with the new update that filters out dockers to be built …

60a001c

…(SONIC_STRETCH_DOCKERS_FOR_INSTALLERS) and build debug-dockers only for those to be built and debug target is available.

lguohan approved these changes Apr 14, 2019

View reviewed changes

lguohan merged commit c846fb9 into sonic-net:master Apr 14, 2019

renukamanavalan mentioned this pull request Apr 28, 2019

Build sonic-broadcom.bin using debug dockers for all stretch based dockers #2833

Merged

StormLiangMS mentioned this pull request May 30, 2023

[submodule][master] sonic-utilities #15264

Merged

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updated Makefile infrastructure to build debug images. #2753

Updated Makefile infrastructure to build debug images. #2753

renukamanavalan commented Apr 5, 2019

renukamanavalan commented Apr 6, 2019

renukamanavalan commented Apr 8, 2019

lguohan Apr 10, 2019

renukamanavalan Apr 10, 2019

lguohan Apr 10, 2019

renukamanavalan Apr 10, 2019

renukamanavalan Apr 10, 2019

lguohan Apr 10, 2019

renukamanavalan Apr 10, 2019

lguohan Apr 11, 2019 •

edited

Loading

lguohan Apr 11, 2019

lguohan Apr 11, 2019

renukamanavalan Apr 11, 2019

Updated Makefile infrastructure to build debug images. #2753

Updated Makefile infrastructure to build debug images. #2753

Conversation

renukamanavalan commented Apr 5, 2019

renukamanavalan commented Apr 6, 2019

renukamanavalan commented Apr 8, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lguohan Apr 11, 2019 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lguohan Apr 11, 2019 •

edited

Loading