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

switch to SDK container for builds #525

Merged
merged 16 commits into from
Nov 20, 2019
Merged

switch to SDK container for builds #525

merged 16 commits into from
Nov 20, 2019

Conversation

bcressey
Copy link
Contributor

Issue #, if available:
#426

Description of changes:
The major change is moving the build of the "host" toolchain - C, C++, Go, and Rust compilers - into a separate container. The existing build system is still used to produce "target" binaries and libraries.

The split isn't as clean as we might wish since we're juggling some other concerns. Both Rust and Go want a C library and headers for their build, so we need to build copies of glibc and musl for that, which in turn require kernel headers.

Including the kernel headers in the SDK has the advantage of standardizing a particular kernel version for all package builds. It removes the kernel as a bottleneck in the build process, and opens the door to building alternative kernel packages.

We build Rust in the SDK since we want to ship the standard library built for the target in our images to reduce the size of Rust binaries, and therefore want tighter control over where and how the library is built.

We build Go for alignment with the other toolchains, even though it's quick to build.

The musl-based toolchain is new and is intended to support building our migration binaries, which need to be statically linked to be compatible across a wide range of image releases.

Testing done:
Built images, created AMI, launched instances. Cluster passed conformance tests.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@bcressey bcressey requested review from tjkirch and iliana November 15, 2019 18:17
Copy link
Contributor

@tjkirch tjkirch left a comment

Choose a reason for hiding this comment

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

Very impressive. I know how much work this took, and it'll be a huge improvement for us.

Makefile.toml Outdated Show resolved Hide resolved
Makefile.toml Outdated Show resolved Hide resolved
extras/sdk-container/Dockerfile Outdated Show resolved Hide resolved
extras/sdk-container/Dockerfile Outdated Show resolved Hide resolved
extras/sdk-container/Dockerfile Outdated Show resolved Hide resolved
extras/sdk-container/Dockerfile Show resolved Hide resolved
Copy link
Contributor

@tjkirch tjkirch left a comment

Choose a reason for hiding this comment

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

My approval stands, just nit/rebase comments remain above.

@jahkeup
Copy link
Member

jahkeup commented Nov 19, 2019

Looks like the build failed because there's a new tool needed:

required program 'unlz4' not found 

Can you add a command to install this package to the install steps of the buildspec in tools/infra/buildspec/thar-pr-build.yml? Note: we're using CodeBuild's ubuntu image so the package is liblz4-tool.

@jahkeup
Copy link
Member

jahkeup commented Nov 19, 2019

@bcressey scratch the comment, looks like you're switching to a more widely available gzip 👍

Signed-off-by: Ben Cressey <[email protected]>
Signed-off-by: Ben Cressey <[email protected]>
Signed-off-by: Ben Cressey <[email protected]>
Signed-off-by: Ben Cressey <[email protected]>
Signed-off-by: Ben Cressey <[email protected]>
Signed-off-by: Ben Cressey <[email protected]>
Signed-off-by: Ben Cressey <[email protected]>
Signed-off-by: Ben Cressey <[email protected]>
@bcressey bcressey merged commit 38d4939 into develop Nov 20, 2019
@bcressey bcressey deleted the sdk branch November 20, 2019 13:22
@iliana iliana added this to the v0.2.0 milestone Nov 20, 2019
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