Merged
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #206 +/- ##
==========================================
+ Coverage 75.08% 75.16% +0.07%
==========================================
Files 58 58
Lines 3263 3265 +2
Branches 161 161
==========================================
+ Hits 2450 2454 +4
+ Misses 708 706 -2
Partials 105 105 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
86e2f2f to
0eaa2ac
Compare
Contributor
Author
|
@ogorman89 another FYI for you, these are the build changes I've been working on |
0eaa2ac to
3e128cc
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Related Links
Description and Motivation
DISPATCH_MODE=recurse, and then set it tolocalinside the container build and for theskctlbuild.bookworm, so there shouldn't be any library mismatches or whatever. In the future if we change the version in the docker images we'll need to also update the build version.fs_groupsecurity context for the pods to ensure that they can read the created volumes: https://kubernetes.io/docs/tasks/configure-pod-container/security-context/#set-the-security-context-for-a-podobjcopyon MacOS is not guaranteed to existsymbol-file path/to/symbols.dbginsiderust-gdband it will load them.objcopyissues, and I don't really want to make that a dependency/prereq for any of this.skctl,nextest, and friends use the defaultdev/releaseprofiles, and the docker builds use a custom profile. This lets us have a slightly smaller.builddirectory without quite so many random profile subdirs in there, but it still keeps build artifacts separate for different build modes.s/echo/printf/in the MakefileHow
build-in-dockerscript to handle all of the objcopy nonsense, because it was getting really ugly to stuff that into the Makefile (also renamed thehackdirectory toscripts, which feels more professional)Test Steps
make build imageworksBUILD_MODE=release make build imageworksmake runshows all containers running locally on my laptop;skctl run --disable-metrics --driver-image `cat .build/sk-driver-image`reports a working simulation and the driver pod starts upOther Notes
The "image build" process just copies whatever binary last existed in
.build/into the docker image; in essence we're doing a multi-stage build but using our makefiles to manage the multiple stages. This is probably not the ideal way to do things, but I like keepingbuildandimageas separate steps in the Make process. In any case, the implication of this is that if you, for whatever reason, runmake build, and then later runBUILD_MODE=release make image, it will create "release" docker images using the "debug" binaries. I thought about trying to fix this but I've already done too much bikeshedding here, so decided to punt on this for later.Marking as a "fix" so this shows up in the changelog/release notes. Ostensibly it was a bug to not have a "release/slimmed down" docker image.
I certify that this PR does not contain any code that has been generated with GitHub Copilot or any other AI-based code generation tool, in accordance with this project's policies.