-
Notifications
You must be signed in to change notification settings - Fork 249
feat: Remove symbols from all binaries #3822
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
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR strips debug symbols from all Go binaries to reduce image sizes in a distroless environment.
- Added
-s -w
linker flags to Go build commands in Dockerfiles, CI scripts, and the Makefile to remove symbols. - Introduced
LD_BUILD_FLAGS
in the Makefile to propagate custom link flags in local builds.
Reviewed Changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
npm/windows.Dockerfile | Added -s -w to go build flags |
npm/linux.Dockerfile | Added -s -w to go build flags |
cns/Dockerfile.tmpl | Added -s -w to go build flags |
cns/Dockerfile | Added -s -w to go build flags |
cni/Dockerfile.tmpl | Added -s -w to multiple go build commands |
cni/Dockerfile | Added -s -w to multiple go build commands |
bpf-prog/ipv6-hp-bpf/linux.Dockerfile | Added -s -w to go build flags |
azure-ipam/Dockerfile | Added -s -w to go build and dropgz commands |
azure-ip-masq-merger/Dockerfile | Added -s -w to go build flags |
Makefile | Introduced LD_BUILD_FLAGS and applied it to targets |
.pipelines/build/scripts/npm.sh | Added -s -w to go build flags |
.pipelines/build/scripts/ipv6-hp-bpf.sh | Added -s -w to go build flags |
.pipelines/build/scripts/dropgz.sh | Added -s -w to go build flags |
.pipelines/build/scripts/cns.sh | Added -s -w to go build flags |
.pipelines/build/scripts/cni.sh | Added -s -w to go build flags |
.pipelines/build/scripts/azure-ipam.sh | Added -s -w to go build flags |
.pipelines/build/scripts/azure-ip-masq-merger.sh | Added -s -w to go build flags |
Comments suppressed due to low confidence (1)
Makefile:23
- [nitpick] Please add a comment above this line to explain the purpose and usage of
LD_BUILD_FLAGS
, so that developers understand how to use it in local builds.
LD_BUILD_FLAGS ?= ""
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These changes look fine to me. This might not affect current NPM releases as currently NPM releases from v1.6 and not master. Just to be sure can you queue NPM scale and conformance tests with:
/azp run NPM Scale Test
/azp run NPM Conformance Tests
/azp run NPM Scale Test, NPM Conformance Tests |
Azure Pipelines successfully started running 2 pipeline(s). |
Reason for Change:
In a distroless environment we lose the need for debug symbols to be present on binaries. Debug symbol removal from binaries generally saves 20%-35% of HDD space.
For comparison linux/amd64 bins
CNS 91MB v 63MB
CNI 82MB v 56MB
IPAM 49MB v 34MB
NPM 66MB v 45MB
ipv6-hp-bpf < TODO > Cant seem to build on wsl2
azure-ip-masq-merger 20MB v 13MB
Issue Fixed:
Requirements:
Notes:
Want to move to leveraging our make commands for binary generation. This is a placeholder until that effort can be picked up.