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

Built stripped binaries when release #6686

Conversation

afshin-deriv
Copy link
Contributor

Signed-off-by: Afshin Paydar afshin.paydar@binary.com

Description

  • Built binaries as small as possible to reduce docker images size, by stripping symbols from binaries only when release.
  • type of fix: kind/enhancement

Related issues/PRs

Todos

  • Tests
  • Documentation
  • Release note

Release Note

TBD

Reminder for the reviewer

Make sure that this PR has the correct labels and milestone set.

Every PR needs one docs-* label.

  • docs-pr-required: This change requires a change to the documentation that has not been completed yet.
  • docs-completed: This change has all necessary documentation completed.
  • docs-not-required: This change has no user-facing impact and requires no docs.

Every PR needs one release-note-* label.

  • release-note-required: This PR has user-facing changes. Most PRs should have this label.
  • release-note-not-required: This PR has no user-facing changes.

Other optional labels:

  • cherry-pick-candidate: This PR should be cherry-picked to an earlier release. For bug fixes only.
  • needs-operator-pr: This PR is related to install and requires a corresponding change to the operator.

@afshin-deriv afshin-deriv requested a review from a team as a code owner September 11, 2022 08:37
@CLAassistant
Copy link

CLAassistant commented Sep 11, 2022

CLA assistant check
All committers have signed the CLA.

@marvin-tigera marvin-tigera added this to the Calico v3.25.0 milestone Sep 11, 2022
@marvin-tigera marvin-tigera added docs-pr-required Change is not yet documented release-note-required Change has user-facing impact (no matter how small) labels Sep 11, 2022
Copy link
Member

@caseydavenport caseydavenport left a comment

Choose a reason for hiding this comment

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

Thanks for working on this!

I've added some thoughts, let me know what you think.

.semaphore/semaphore-scheduled-builds.yml Outdated Show resolved Hide resolved
.semaphore/semaphore-scheduled-builds.yml Outdated Show resolved Hide resolved
pod2daemon/Makefile Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
pod2daemon/Makefile Outdated Show resolved Hide resolved
pod2daemon/Makefile Outdated Show resolved Hide resolved
pod2daemon/Makefile Outdated Show resolved Hide resolved
Copy link
Member

@caseydavenport caseydavenport left a comment

Choose a reason for hiding this comment

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

Added a few more comments - also please double check that each of the components builds after your changes. I believe some of these go build commands are not valid and will fail in their current form.

@caseydavenport caseydavenport added the docs-not-required Docs not required for this change label Sep 20, 2022
@marvin-tigera marvin-tigera removed the docs-pr-required Change is not yet documented label Sep 20, 2022
@caseydavenport
Copy link
Member

@afshinpaydar-binary is this something you're still planning to work on?

@mgleung mgleung modified the milestones: Calico v3.25.0, Calico v3.26.0 Jan 5, 2023
@afshin-deriv
Copy link
Contributor Author

afshin-deriv commented Jan 31, 2023

@caseydavenport Could you please take a look at these changes again?

@afshin-deriv afshin-deriv force-pushed the afshinpaydar/built_stripped_binaries branch from 423d0c9 to 01aa337 Compare February 1, 2023 09:41
Copy link
Member

@caseydavenport caseydavenport left a comment

Choose a reason for hiding this comment

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

Sorry for the long delay here. It looks like this will need a rebase though.

Just one minor comment from me otherwise.

kube-controllers/Makefile Outdated Show resolved Hide resolved
@caseydavenport
Copy link
Member

/sem-approve

@caseydavenport
Copy link
Member

@afshin-deriv Also, I'm afraid we can't accept this PR unless you sign the CLA to make the bot happy.

apiserver/Makefile Outdated Show resolved Hide resolved
@afshin-deriv afshin-deriv force-pushed the afshinpaydar/built_stripped_binaries branch from 3505758 to 9402b9e Compare March 5, 2023 03:39
@caseydavenport
Copy link
Member

/sem-approve

@caseydavenport
Copy link
Member

Starting with UID : 1001
CGO enabled, switching GOROOT to /usr/local/go-cgo.
sh: 1: Syntax error: Unterminated quoted string
Makefile:81: recipe for target 'bin/calico-typha-amd64' failed

^ Builds seem to be failing with this error

apiserver/Makefile Outdated Show resolved Hide resolved
typha/Makefile Outdated Show resolved Hide resolved
@afshin-deriv afshin-deriv force-pushed the afshinpaydar/built_stripped_binaries branch from a8a9704 to e9e1813 Compare April 5, 2023 13:31
node/Makefile Outdated Show resolved Hide resolved
Signed-off-by: Afshin Paydar <afshin.paydar@binary.com>
@afshin-deriv afshin-deriv force-pushed the afshinpaydar/built_stripped_binaries branch from e9e1813 to 2673da1 Compare April 20, 2023 13:56
@caseydavenport
Copy link
Member

Is this still in progress?

@radTuti radTuti modified the milestones: Calico v3.28.0, Calico v3.28.1 May 3, 2024
@marvin-tigera marvin-tigera removed release-note-required Change has user-facing impact (no matter how small) docs-not-required Docs not required for this change labels Jun 18, 2024
@hjiawei hjiawei mentioned this pull request Jun 18, 2024
3 tasks
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.

8 participants