Use stable working directory paths #407
Merged
Conversation
This file contains 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
|
Thanks for having a look @rquitales 🙇 |
squaremo
force-pushed
the
stable-build-directory
branch
from
6845c61 to
581425f
Compare
|
I built a dev image for this: |
squaremo
force-pushed
the
stable-build-directory
branch
from
581425f to
4f498a4
Compare
squaremo
force-pushed
the
stable-build-directory
branch
2 times, most recently
from
f3ee53f to
d167ee1
Compare
rquitales
reviewed
Feb 7, 2023
rquitales
approved these changes
Feb 7, 2023
Previously, the operator asked the OS to create a fresh tmp directory for each stack run. This is a reasonable enough design, since it means builds are always from a fresh clone, can't interfere with one another, and can be safely deleted afterwards. But it also meant that the go build cache would treat every source file as distinct source code, and grow its build cache accordingly -- to the tune of tens or hundreds of megabytes a build. This commit changes the working directory to a stable path (still under /tmp), named for the stack object. Since each stack object is uniquely named and is processed serially, there should be no collisions. (There's also no security benefit to having random directory names, since a program or user will ill intent can always just poke around in the filesystem to find what they want). This also centralises the creation of the directory, which removes a bit of repetition. Signed-off-by: Michael Bridgen <[email protected]>
A simple test to check that the controller doesn't cause the go build cache to grow, unduly. Essentially: 1. Run the stack 2. Don't change anything 3. Run the stack again 4. Check that the Go build cache didn't change between 1. and 3. If a stack is built in a fresh tmp directory each time (as it was previously), this test will fail, because at least the stack's main.go will look like a new file to the cache. Signed-off-by: Michael Bridgen <[email protected]>
Update the setup-go actions to V3, and use its cache feature. This will help builds in general; I'm also hoping it will help with the test involving test/testdata/go-build, which can time out because it spends a minute or so downloading the (minimal) dependencies for the Go program therein. Signed-off-by: Michael Bridgen <[email protected]>
Most of the time we want to fail quite fast if a stack fails to run, but some stacks -- the one involving Go -- just take a while to run. So: let a timeout be supplied. Signed-off-by: Michael Bridgen <[email protected]>
Signed-off-by: Michael Bridgen <[email protected]>
When there's tests running concurrently, it's possible that something will change the environment -- notably, $GOCACHE -- so, this commit adds a check that the cache dir is the same before and after the test has run. This is not bullet-proof, but is simple and has a decent chance of catching errors of logic in the tests. Signed-off-by: Michael Bridgen <[email protected]> Co-authored-by: Ramon Quitales <[email protected]>
squaremo
force-pushed
the
stable-build-directory
branch
from
6a0aaff to
146fb53
Compare
This was referenced Feb 13, 2023
Closed
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.
Previously, the operator asked the OS to create a fresh tmp directory for each stack run. This is a reasonable enough design, since it means builds are always from a fresh clone, can't interfere with one another, and can be safely deleted afterwards. But it also meant that the go build cache would treat every source file as distinct source code, and grow its build cache accordingly -- to the tune of tens or hundreds of megabytes a build.
This commit changes the working directory to a stable path (still under /tmp), named for the stack object. Since each stack object is uniquely named and is processed serially, there should be no collisions. (There's also no security benefit to having random directory names, since a program or user will ill intent can always just poke around in the filesystem to find what they want).
This also centralises the creation of the directory, which removes a bit of repetition.
Fixes #381.
Further notes:
This will have a doubly beneficial effect on Go projects, since the go build cache will work as intended -- so, no unbounded disk use, but also much faster builds. I checked this by making a project which vendors its libraries and using it in a stack:
pulumi new kubernetes-go, thengo mod vendor; git add .; git commit -m "Initial revision"watch du -sh /tmp/.cache/go-buildWhen using stable paths, that cache grows to 320MB the first time it processes the stack, then stays there. Without this change, the cache directory grew by (just about) 320MB each time the stack was processed.