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

Updated Makefile infrastructure to build debug images. #2753

Merged
merged 7 commits into from
Apr 14, 2019
Merged

Updated Makefile infrastructure to build debug images. #2753

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
941eb7d
Updated Makefile infrastructure to build debug images.
renukamanavalan Apr 5, 2019
042d51e
Dropped the regular binary using DBG_PACKAGES and a small name change…
renukamanavalan Apr 6, 2019
24f38f2
Tweaked the changes to retain the existing behavior w.r.t INSTALL_DEB…
renukamanavalan Apr 8, 2019
4443c70
1) slave.mk - Dropped unused Docker build args
renukamanavalan Apr 11, 2019
7b84e64
Reverted some changes, per review comments.
renukamanavalan Apr 11, 2019
c717e50
Merge remote-tracking branch 'upstream/master'
renukamanavalan Apr 12, 2019
60a001c
Get in sync with the new update that filters out dockers to be built …
renukamanavalan Apr 12, 2019
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
45 changes: 45 additions & 0 deletions build_dbg_j2.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,45 @@
#! /bin/bash

echo "
FROM $1

ARG docker_container_name

## Make apt-get non-interactive
ENV DEBIAN_FRONTEND=noninteractive

{% if $2 is defined %}
{% if $2|length %}

COPY \
{% for deb in $2.split(' ') -%}
debs/{{ deb }}{{' '}}
{%- endfor -%}
debs/

RUN dpkg -i \
{% for deb in $2.split(' ') -%}
debs/{{ deb }}{{' '}}
{%- endfor %}

{% endif %}
{% endif %}

{% if $3 is defined %}
{% if $3|length %}

RUN apt-get install -f -y \
{% for dbg in $3.split(' ') -%}
{{ dbg }}{{' '}}
{%- endfor %}

{% endif %}
{% endif %}


## Clean up
RUN apt-get clean -y; apt-get autoclean -y; apt-get autoremove -y
RUN rm -rf /debs

CMD ["/usr/bin/supervisord"]
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

"
18 changes: 15 additions & 3 deletions platform/broadcom/docker-orchagent-brcm.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,18 +1,30 @@
# docker image for orchagent

DOCKER_ORCHAGENT_BRCM = docker-orchagent-brcm.gz
DOCKER_ORCHAGENT_BRCM_STEM = docker-orchagent-brcm
DOCKER_ORCHAGENT_BRCM = $(DOCKER_ORCHAGENT_BRCM_STEM).gz
DOCKER_ORCHAGENT_BRCM_DBG = $(DOCKER_ORCHAGENT_BRCM_STEM)$(DBG_IMAGE_MARK).gz

$(DOCKER_ORCHAGENT_BRCM)_PATH = $(DOCKERS_PATH)/docker-orchagent
$(DOCKER_ORCHAGENT_BRCM)_DEPENDS += $(SWSS) $(REDIS_TOOLS)
ifeq ($(INSTALL_DEBUG_TOOLS), y)
$(DOCKER_ORCHAGENT_BRCM)_DEPENDS += $(SWSS_DBG) \
$(DOCKER_ORCHAGENT_BRCM)_DBG_DEPENDS = $(SWSS_DBG) \
$(LIBSWSSCOMMON_DBG) \
$(LIBSAIREDIS_DBG)
ifeq ($(INSTALL_DEBUG_TOOLS), y)
$(DOCKER_ORCHAGENT_BRCM)_DEPENDS += $($(DOCKER_ORCHAGENT_BRCM)_DBG_DEPENDS)
endif
$(DOCKER_ORCHAGENT_BRCM)_DBG_DEPENDS += $($(DOCKER_CONFIG_ENGINE_STRETCH)_DBG_DEPENDS)

$(DOCKER_ORCHAGENT_BRCM)_LOAD_DOCKERS += $(DOCKER_CONFIG_ENGINE_STRETCH)
$(DOCKER_ORCHAGENT_BRCM)_DBG_IMAGE_PACKAGES = $($(DOCKER_CONFIG_ENGINE_STRETCH)_DBG_IMAGE_PACKAGES)

SONIC_DOCKER_IMAGES += $(DOCKER_ORCHAGENT_BRCM)
SONIC_STRETCH_DOCKERS += $(DOCKER_ORCHAGENT_BRCM)
SONIC_INSTALL_DOCKER_IMAGES += $(DOCKER_ORCHAGENT_BRCM)

SONIC_DOCKER_DBG_IMAGES += $(DOCKER_ORCHAGENT_BRCM_DBG)
SONIC_STRETCH_DBG_DOCKERS += $(DOCKER_ORCHAGENT_BRCM_DBG)
SONIC_INSTALL_DOCKER_DBG_IMAGES += $(DOCKER_ORCHAGENT_BRCM_DBG)

$(DOCKER_ORCHAGENT_BRCM)_CONTAINER_NAME = swss
$(DOCKER_ORCHAGENT_BRCM)_RUN_OPT += --net=host --privileged -t
$(DOCKER_ORCHAGENT_BRCM)_RUN_OPT += -v /etc/network/interfaces:/etc/network/interfaces:ro
Expand Down
5 changes: 3 additions & 2 deletions rules/docker-base-stretch.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,14 +5,15 @@ $(DOCKER_BASE_STRETCH)_PATH = $(DOCKERS_PATH)/docker-base-stretch
$(DOCKER_BASE_STRETCH)_DEPENDS += $(SUPERVISOR)
$(DOCKER_BASE_STRETCH)_DEPENDS += $(SOCAT)

ifeq ($(INSTALL_DEBUG_TOOLS),y)
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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

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.
"

$(DOCKER_BASE_STRETCH)_DBG_PACKAGES += $($(DOCKER_BASE_STRETCH)_DBG_IMAGE_PACKAGES)
endif

SONIC_STRETCH_DOCKERS += $(DOCKER_BASE_STRETCH)
4 changes: 4 additions & 0 deletions rules/docker-config-engine-stretch.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,8 @@ $(DOCKER_CONFIG_ENGINE_STRETCH)_PATH = $(DOCKERS_PATH)/docker-config-engine-stre
$(DOCKER_CONFIG_ENGINE_STRETCH)_PYTHON_WHEELS += $(SWSSSDK_PY2)
$(DOCKER_CONFIG_ENGINE_STRETCH)_PYTHON_WHEELS += $(SONIC_CONFIG_ENGINE)
$(DOCKER_CONFIG_ENGINE_STRETCH)_LOAD_DOCKERS += $(DOCKER_BASE_STRETCH)

$(DOCKER_CONFIG_ENGINE_STRETCH)_DBG_DEPENDS = $($(DOCKER_BASE_STRETCH)_DBG_DEPENDS)
$(DOCKER_CONFIG_ENGINE_STRETCH)_DBG_IMAGE_PACKAGES = $($(DOCKER_BASE_STRETCH)_DBG_IMAGE_PACKAGES)

SONIC_STRETCH_DOCKERS += $(DOCKER_CONFIG_ENGINE_STRETCH)
40 changes: 39 additions & 1 deletion slave.mk
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ PYTHON_WHEELS_PATH = $(TARGET_PATH)/python-wheels
PROJECT_ROOT = $(shell pwd)
STRETCH_DEBS_PATH = $(TARGET_PATH)/debs/stretch
STRETCH_FILES_PATH = $(TARGET_PATH)/files/stretch
DBG_IMAGE_MARK = -dbg

CONFIGURED_PLATFORM := $(shell [ -f .platform ] && cat .platform || echo generic)
PLATFORM_PATH = platform/$(CONFIGURED_PLATFORM)
Expand Down Expand Up @@ -474,8 +475,10 @@ SONIC_TARGET_LIST += $(addprefix $(TARGET_PATH)/, $(SONIC_SIMPLE_DOCKER_IMAGES))
# jessie docker images only in jessie slave docker
ifeq ($(BLDENV),stretch)
DOCKER_IMAGES := $(SONIC_STRETCH_DOCKERS)
DOCKER_DBG_IMAGES := $(SONIC_STRETCH_DBG_DOCKERS)
else
DOCKER_IMAGES := $(filter-out $(SONIC_STRETCH_DOCKERS), $(SONIC_DOCKER_IMAGES))
DOCKER_DBG_IMAGES := $(filter-out $(SONIC_STRETCH_DBG_DOCKERS), $(SONIC_DOCKER_DBG_IMAGES))
endif

# Targets for building docker images
Expand Down Expand Up @@ -522,9 +525,42 @@ $(addprefix $(TARGET_PATH)/, $(DOCKER_IMAGES)) : $(TARGET_PATH)/%.gz : .platform

SONIC_TARGET_LIST += $(addprefix $(TARGET_PATH)/, $(DOCKER_IMAGES))

# Targets for building docker images
$(addprefix $(TARGET_PATH)/, $(DOCKER_DBG_IMAGES)) : $(TARGET_PATH)/%$(DBG_IMAGE_MARK).gz : .platform docker-start \
$$(addprefix $(DEBS_PATH)/,$$($$*.gz_DBG_DEPENDS)) \
$$(addsuffix -load,$$(addprefix $(TARGET_PATH)/,$$*.gz))
$(HEADER)
mkdir -p $($*.gz_PATH)/debs $(LOG)
sudo mount --bind $(DEBS_PATH) $($*.gz_PATH)/debs $(LOG)
# 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
Copy link
Collaborator

Choose a reason for hiding this comment

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

rename to build_debug_docker_j2.sh for better clarity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok

j2 $($*.gz_PATH)/Dockerfile-dbg.j2 > $($*.gz_PATH)/Dockerfile-dbg
docker info $(LOG)
docker build --squash --no-cache \
--build-arg http_proxy=$(HTTP_PROXY) \
--build-arg https_proxy=$(HTTPS_PROXY) \
--build-arg user=$(USER) \
--build-arg uid=$(UID) \
--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) \
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

--label Tag=$(SONIC_GET_VERSION) \
--file $($*.gz_PATH)/Dockerfile-dbg \
-t $*-dbg $($*.gz_PATH) $(LOG)
docker save $*-dbg | gzip -c > $@
# Clean up
if [ -f $($*.gz_PATH).patch/series ]; then pushd $($*.gz_PATH) && quilt pop -a -f; popd; fi
$(FOOTER)

SONIC_TARGET_LIST += $(addprefix $(TARGET_PATH)/, $(DOCKER_DBG_IMAGES))

DOCKER_LOAD_TARGETS = $(addsuffix -load,$(addprefix $(TARGET_PATH)/, \
$(SONIC_SIMPLE_DOCKER_IMAGES) \
$(DOCKER_IMAGES)))

$(DOCKER_LOAD_TARGETS) : $(TARGET_PATH)/%.gz-load : .platform docker-start $$(TARGET_PATH)/$$*.gz
$(HEADER)
docker load -i $(TARGET_PATH)/$*.gz $(LOG)
Expand Down Expand Up @@ -658,6 +694,7 @@ $(SONIC_CLEAN_FILES) : $(FILES_PATH)/%-clean : .platform

SONIC_CLEAN_TARGETS += $(addsuffix -clean,$(addprefix $(TARGET_PATH)/, \
$(SONIC_DOCKER_IMAGES) \
$(SONIC_DOCKER_DBG_IMAGES) \
$(SONIC_SIMPLE_DOCKER_IMAGES) \
$(SONIC_INSTALLERS)))
$(SONIC_CLEAN_TARGETS) : $(TARGET_PATH)/%-clean : .platform
Expand All @@ -681,7 +718,8 @@ all : .platform $$(addprefix $(TARGET_PATH)/,$$(SONIC_ALL))

stretch : $$(addprefix $(DEBS_PATH)/,$$(SONIC_STRETCH_DEBS)) \
$$(addprefix $(FILES_PATH)/,$$(SONIC_STRETCH_FILES)) \
$$(addprefix $(TARGET_PATH)/,$$(SONIC_STRETCH_DOCKERS))
$$(addprefix $(TARGET_PATH)/,$$(SONIC_STRETCH_DOCKERS)) \
$$(addprefix $(TARGET_PATH)/,$$(SONIC_STRETCH_DBG_DOCKERS))


###############################################################################
Expand Down