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.

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

Makefile: Extend to build debug docker images for all stretch dockers #2789

Merged
merged 17 commits into from
Apr 20, 2019

Conversation

renukamanavalan
Copy link
Contributor

@renukamanavalan renukamanavalan commented Apr 15, 2019

- What I did
Overall goal: Build debug images for every stretch docker.

An earlier PR (#2789) made the first cut, by transforming broadcom/orchagent to build target/docker-orhagent-dbg.gz.

Changes in this PR:

  1. Made docker-orchagent build to be platform independent.
    1.1) Created rules/docker_orchagent.mk
    1.2) Removed platform//docker-orchagent-*.mk
    1.3) Removed the corresponding entry from platform/
    /rules.mk

  2. Extended the debug docker image build to stretch based syncd dockers.
    2.1) For now, only mellanox & barefoot are stretch based.
    2.2) All the common variable definitions are put in one place platform/template/docker-syncd-base.mk
    2.3) platform/[mellanox, bfn]/docker-syncd-[mlnx, bfn].mk are updated as detailed below.
    2.3.1) Set platform code and include template base file
    2.3.2) Add the dependencies & debug dependencies and any update over what base template offers.

  3. Extended all stretch based non-platform dockers to build debug dockers too.
    3.1) Affected are:
    docker-database.mk,
    docker-platform-monitor.mk,
    docker-router-advertiser.mk,
    docker-teamd.mk,
    docker-telemetry.mk

Next: Build debug flavor of final images with regular dockers replaced with debug dockers where available.

- How I did it

- How to verify it
make stretch, will build target/docker=*-dbg.gz versions for all stretch based dockers.

- 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.
…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.
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.
"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.
Where needed a platform entry can override the template.
This avoids duplication, hence easier to maintain.
Just take the platform code and do the rest in template.
@renukamanavalan renukamanavalan self-assigned this Apr 16, 2019
@renukamanavalan renukamanavalan requested a review from lguohan April 16, 2019 22:37
@lguohan
Copy link
Collaborator

lguohan commented Apr 16, 2019

can you add better description of the pr and also a clear title?

@lguohan
Copy link
Collaborator

lguohan commented Apr 17, 2019

swss docker is the same for all platforms, instead of using template, let's move it out of platform folder.

…e under rules/docker-orchagent.mk

2) Extened debug image to all stretch dockers
@renukamanavalan renukamanavalan changed the title Makefile: Simplify through template Makefile: Extend to build debug docker images for all stretch dockers Apr 18, 2019
rules/docker-teamd.mk Outdated Show resolved Hide resolved
rules/docker-telemetry.mk Outdated Show resolved Hide resolved
rules/docker-database.mk Outdated Show resolved Hide resolved
Copy link
Collaborator

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

as comments

1) Dropped LIBSAIREDIS_DBG from database, teamd, router-advertiser, telemetry, and platform-monitor docker*.mk files from _DBG_DEPENDS list
2) W.r.t docker make for syncd, moved DEPENDS from template to specific makefile and let the template has stuff that is applicable to all.
@@ -36,7 +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
DBG_IMAGE_MARK = dbg
Copy link
Contributor

@jleveque jleveque Apr 18, 2019

Choose a reason for hiding this comment

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

Suggest change name to DBG_IMAGE_SUFFIX, unless it may be used elsewhere as something other than a suffix.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Wish I could. But I have to use this as prefix for final image, as images have different extensions (unlike dockers, which all ends with .gz) as .bin, .swi, .raw, .... If I make it suffix, it would be something like sonic-broadcom-dbg.bin, sonic-broadcom-dbg.swi. With all Makefile variables having 'sonic-broadcom.bin' as prefix, stripping off dbg from the middle leads to complex rules. Even if you would make 'sonic-broadcom' as prefix, stripping off dbg- is not easy one within Make and it would make Makefile more unreadable.

Next PR will be the one for final image.

Copy link
Contributor

@jleveque jleveque Apr 19, 2019

Choose a reason for hiding this comment

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

Based on your description, it's not really ever being used as a prefix; it's always used as a suffix to the filename (not counting what trails after the '.', because those are technically file extensions). Unless I'm missing something, as long as it's always used like, <filename>-dbg.<extension>, it could still be renamed DBG_IMAGE_SUFFIX and be correct and more clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I guess, I did not make it clear. Let us just wait till the next PR, which is going to use it as "prefix" for reasons, which can be better explained, when you see the PR.

If we still decide to change, we will do it in the next PR.

BTW, the next PR will follow as soon as this gets approved.

@renukamanavalan renukamanavalan requested a review from lguohan April 19, 2019 03:44
include $(PLATFORM_PATH)/../template/docker-syncd-base.mk

$(DOCKER_SYNCD_BASE)_DEPENDS += $(SYNCD)
$(DOCKER_SYNCD_BASE)_PYTHON_DEBS += $(MLNX_SFPD)
Copy link
Collaborator

Choose a reason for hiding this comment

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

this is mellanox specific.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops! copy/paste error.
Let me fix it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

THANK YOU!

Copy link
Collaborator

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

misplaced the package in syncd bfn.

also please update the commit message to describe what has been done.

Copy link
Collaborator

@lguohan lguohan left a comment

Choose a reason for hiding this comment

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

:shipit:

@lguohan lguohan merged commit ba0ca01 into sonic-net:master Apr 20, 2019
MichelMoriniaux pushed a commit to criteo-forks/sonic-buildimage that referenced this pull request May 28, 2019
…h dockers (sonic-net#2789)

Overall goal: Build debug images for every stretch docker.

An earlier PR (sonic-net#2789) made the first cut, by transforming broadcom/orchagent to build target/docker-orhagent-dbg.gz.

Changes in this PR:

Made docker-orchagent build to be platform independent.
1.1) Created rules/docker_orchagent.mk
1.2) Removed platform//docker-orchagent-*.mk
1.3) Removed the corresponding entry from platform//rules.mk

Extended the debug docker image build to stretch based syncd dockers.
2.1) For now, only mellanox & barefoot are stretch based.
2.2) All the common variable definitions are put in one place platform/template/docker-syncd-base.mk
2.3) platform/[mellanox, bfn]/docker-syncd-[mlnx, bfn].mk are updated as detailed below.
2.3.1) Set platform code and include template base file
2.3.2) Add the dependencies & debug dependencies and any update over what base template offers.

Extended all stretch based non-platform dockers to build debug dockers too.
3.1) Affected are:
docker-database.mk,
docker-platform-monitor.mk,
docker-router-advertiser.mk,
docker-teamd.mk,
docker-telemetry.mk

Next: Build debug flavor of final images with regular dockers replaced with debug dockers where available.
StormLiangMS pushed a commit to StormLiangMS/sonic-buildimage that referenced this pull request Jul 18, 2022
yxieca pushed a commit that referenced this pull request Jun 12, 2023
…lly (#15387)

src/sonic-swss

* 2aec547 - (HEAD -> 202205, origin/202205) [pfcwd] Enhance DLR_INIT based recovery and DLR_PACKET_ACTION for broadcom platforms (#2807) (4 days ago) [Neetha John]
* fa4acd3 - Fix to substract the macsec sectag size from port MTU during InitializePort (#2789) (5 days ago) [judyjoseph]
mihirpat1 pushed a commit to mihirpat1/sonic-buildimage that referenced this pull request Jun 14, 2023
dprital added a commit to dprital/sonic-buildimage that referenced this pull request Jun 20, 2023
Update sonic-utilities submodule pointer to include the following:
* 0b629ba1 Revert [chassis][voq] Clear fabric counters queue/port (2789) ([sonic-net#2882](sonic-net/sonic-utilities#2882))
* 3ba8241a [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. ([sonic-net#2839](sonic-net/sonic-utilities#2839))
* fceef2ed [chassis][voq] Clear fabric counters queue/port ([sonic-net#2789](sonic-net/sonic-utilities#2789))
* 659ba24b [syslog] Adjust runningconfiguration syslog command ([sonic-net#2843](sonic-net/sonic-utilities#2843))
* 46fba26f [db_migrator] add required protocol field in ROUTE_TABLE ([sonic-net#2766](sonic-net/sonic-utilities#2766))
* f186376e Fix issue: show interfaces transceiver eeprom -d should display same entry for CMIS cable ([sonic-net#2864](sonic-net/sonic-utilities#2864))
* de491798 fix precedence in portstat CLI ([sonic-net#2874](sonic-net/sonic-utilities#2874))

Signed-off-by: dprital <drorp@nvidia.com>
dprital added a commit to dprital/sonic-buildimage that referenced this pull request Jun 21, 2023
Update sonic-utilities submodule pointer to include the following:
* 0b629ba1 Revert [chassis][voq] Clear fabric counters queue/port (2789) ([sonic-net#2882](sonic-net/sonic-utilities#2882))
* 3ba8241a [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. ([sonic-net#2839](sonic-net/sonic-utilities#2839))
* fceef2ed [chassis][voq] Clear fabric counters queue/port ([sonic-net#2789](sonic-net/sonic-utilities#2789))
* 659ba24b [syslog] Adjust runningconfiguration syslog command ([sonic-net#2843](sonic-net/sonic-utilities#2843))
* 46fba26f [db_migrator] add required protocol field in ROUTE_TABLE ([sonic-net#2766](sonic-net/sonic-utilities#2766))
* f186376e Fix issue: show interfaces transceiver eeprom -d should display same entry for CMIS cable ([sonic-net#2864](sonic-net/sonic-utilities#2864))
* de491798 fix precedence in portstat CLI ([sonic-net#2874](sonic-net/sonic-utilities#2874))

Signed-off-by: dprital <drorp@nvidia.com>
dgsudharsan added a commit to dgsudharsan/sonic-buildimage that referenced this pull request Jul 11, 2023
Update sonic-utilities submodule pointer to include the following:
* ff380e04 [hash]: Implement GH frontend ([sonic-net#2580](sonic-net/sonic-utilities#2580))
* 61bad064 [db_migrator] Set correct CURRENT_VERSION, extend UT ([sonic-net#2895](sonic-net/sonic-utilities#2895))
* 6b8ee47c [CLI][Show][BGP] Show BGP Change for no neighbor scenario ([sonic-net#2885](sonic-net/sonic-utilities#2885))
* 73d8d633 [doc] Update Command-Reference.md, change show bgp peer command to show bfd peer ([sonic-net#2750](sonic-net/sonic-utilities#2750))
* 7bc08c28 [db_migrator] Remove hardcoded config and migrate config from minigraph ([sonic-net#2887](sonic-net/sonic-utilities#2887))
* b1aa9426 [generate_dump]: Enhance show techsupport for Marvell platform ([sonic-net#2676](sonic-net/sonic-utilities#2676))
* 316b14c0 Add support for secure upgrade ([sonic-net#2698](sonic-net/sonic-utilities#2698))
* dc2945bc [dns] Implement config and show commands for static DNS. ([sonic-net#2737](sonic-net/sonic-utilities#2737))
* 8414a709 [chassis][multi asic] change acl_loader to use tcp socket for db communication ([sonic-net#2525](sonic-net/sonic-utilities#2525))
* 0b629ba1 Revert [chassis][voq] Clear fabric counters queue/port (2789) ([sonic-net#2882](sonic-net/sonic-utilities#2882))
* 3ba8241a [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. ([sonic-net#2839](sonic-net/sonic-utilities#2839))
* fceef2ed [chassis][voq] Clear fabric counters queue/port ([sonic-net#2789](sonic-net/sonic-utilities#2789))

Signed-off-by: dgsudharsan <sudharsand@nvidia.com>
liat-grozovik pushed a commit that referenced this pull request Jul 11, 2023
Update sonic-utilities submodule pointer to include the following:
* ff380e04 [hash]: Implement GH frontend ([#2580](sonic-net/sonic-utilities#2580))
* 61bad064 [db_migrator] Set correct CURRENT_VERSION, extend UT ([#2895](sonic-net/sonic-utilities#2895))
* 6b8ee47c [CLI][Show][BGP] Show BGP Change for no neighbor scenario ([#2885](sonic-net/sonic-utilities#2885))
* 73d8d633 [doc] Update Command-Reference.md, change show bgp peer command to show bfd peer ([#2750](sonic-net/sonic-utilities#2750))
* 7bc08c28 [db_migrator] Remove hardcoded config and migrate config from minigraph ([#2887](sonic-net/sonic-utilities#2887))
* b1aa9426 [generate_dump]: Enhance show techsupport for Marvell platform ([#2676](sonic-net/sonic-utilities#2676))
* 316b14c0 Add support for secure upgrade ([#2698](sonic-net/sonic-utilities#2698))
* dc2945bc [dns] Implement config and show commands for static DNS. ([#2737](sonic-net/sonic-utilities#2737))
* 8414a709 [chassis][multi asic] change acl_loader to use tcp socket for db communication ([#2525](sonic-net/sonic-utilities#2525))
* 0b629ba1 Revert [chassis][voq] Clear fabric counters queue/port (2789) ([#2882](sonic-net/sonic-utilities#2882))
* 3ba8241a [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. ([#2839](sonic-net/sonic-utilities#2839))
* fceef2ed [chassis][voq] Clear fabric counters queue/port ([#2789](sonic-net/sonic-utilities#2789))

Signed-off-by: dgsudharsan <sudharsand@nvidia.com>
mssonicbld added a commit that referenced this pull request Jul 11, 2023
…atically (#15456)

#### Why I did it
src/sonic-utilities
```
* ff380e04 - (HEAD -> master, origin/master, origin/HEAD) [hash]: Implement GH frontend (#2580) (13 hours ago) [Nazarii Hnydyn]
* 61bad064 - [db_migrator] Set correct CURRENT_VERSION, extend UT (#2895) (4 days ago) [Vadym Hlushko]
* 6b8ee47c - [CLI][Show][BGP] Show BGP Change for no neighbor scenario (#2885) (6 days ago) [Dev Ojha]
* 73d8d633 - [doc] Update Command-Reference.md, change "show bgp peer" command to "show bfd peer" (#2750) (11 days ago) [PinghaoQu]
* 7bc08c28 - [db_migrator] Remove hardcoded config and migrate config from minigraph (#2887) (11 days ago) [Vaibhav Hemant Dixit]
* b1aa9426 - [generate_dump]: Enhance show techsupport for Marvell platform (#2676) (11 days ago) [pavannaregundi]
* 316b14c0 - Add support for secure upgrade (#2698) (2 weeks ago) [ycoheNvidia]
* dc2945bc - [dns] Implement config and show commands for static DNS. (#2737) (2 weeks ago) [Oleksandr Ivantsiv]
* 8414a709 - [chassis][multi asic] change acl_loader to use tcp socket for db communication (#2525) (2 weeks ago) [Arvindsrinivasan Lakshmi Narasimhan]
* 0b629ba1 - Revert "[chassis][voq] Clear fabric counters queue/port (#2789)" (#2882) (3 weeks ago) [RoRonoa]
* 3ba8241a - [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. (#2839) (4 weeks ago) [Vadym Hlushko]
* fceef2ed - [chassis][voq] Clear fabric counters queue/port (#2789) (4 weeks ago) [jfeng-arista]
```
#### How I did it
#### How to verify it
#### Description for the changelog
sonic-otn pushed a commit to sonic-otn/sonic-buildimage that referenced this pull request Sep 20, 2023
Update sonic-utilities submodule pointer to include the following:
* ff380e04 [hash]: Implement GH frontend ([sonic-net#2580](sonic-net/sonic-utilities#2580))
* 61bad064 [db_migrator] Set correct CURRENT_VERSION, extend UT ([sonic-net#2895](sonic-net/sonic-utilities#2895))
* 6b8ee47c [CLI][Show][BGP] Show BGP Change for no neighbor scenario ([sonic-net#2885](sonic-net/sonic-utilities#2885))
* 73d8d633 [doc] Update Command-Reference.md, change show bgp peer command to show bfd peer ([sonic-net#2750](sonic-net/sonic-utilities#2750))
* 7bc08c28 [db_migrator] Remove hardcoded config and migrate config from minigraph ([sonic-net#2887](sonic-net/sonic-utilities#2887))
* b1aa9426 [generate_dump]: Enhance show techsupport for Marvell platform ([sonic-net#2676](sonic-net/sonic-utilities#2676))
* 316b14c0 Add support for secure upgrade ([sonic-net#2698](sonic-net/sonic-utilities#2698))
* dc2945bc [dns] Implement config and show commands for static DNS. ([sonic-net#2737](sonic-net/sonic-utilities#2737))
* 8414a709 [chassis][multi asic] change acl_loader to use tcp socket for db communication ([sonic-net#2525](sonic-net/sonic-utilities#2525))
* 0b629ba1 Revert [chassis][voq] Clear fabric counters queue/port (2789) ([sonic-net#2882](sonic-net/sonic-utilities#2882))
* 3ba8241a [db_migtrator] Add migration of FLEX_COUNTER_DELAY_STATUS during 1911->master upgrade + fast-reboot. Add UT. ([sonic-net#2839](sonic-net/sonic-utilities#2839))
* fceef2ed [chassis][voq] Clear fabric counters queue/port ([sonic-net#2789](sonic-net/sonic-utilities#2789))

Signed-off-by: dgsudharsan <sudharsand@nvidia.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants