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

Default remove builder #552

Merged
merged 1 commit into from
Jul 17, 2023

Conversation

joshrwolf
Copy link
Contributor

Change the default for runners that create a persisting build environment (everything except bwrap) to always destroy the build environment when the build succeeds/fails.

This means runners like docker/kubernetes don't leave dangling resources around on failures unless the user opts in (--debug-runner).

I originally coupled this to the existing --debug flag, but then changed my mind and used --debug-runner. I'm not married to the semantics so please suggest otherwise!

@joshrwolf joshrwolf force-pushed the default-remove-builder branch 3 times, most recently from 949e136 to 1f0f09e Compare July 17, 2023 13:08
@joshrwolf joshrwolf marked this pull request as ready for review July 17, 2023 13:10
@joshrwolf joshrwolf requested a review from a team as a code owner July 17, 2023 13:10
@joshrwolf joshrwolf requested review from rawlingsj and removed request for a team July 17, 2023 13:10
imjasonh
imjasonh previously approved these changes Jul 17, 2023
Signed-off-by: Josh Wolf <josh@wolfs.io>
@@ -1645,6 +1654,13 @@ func (b *Build) BuildPackage(ctx context.Context) error {
if err := b.Runner.StartPod(ctx, cfg); err != nil {
return fmt.Errorf("unable to start pod: %w", err)
}
if !b.DebugRunner {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if !b.DebugRunner {
if !b.DebugRunner && !b.IsBuildLess() {

Copy link
Contributor Author

@joshrwolf joshrwolf Jul 17, 2023

Choose a reason for hiding this comment

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

this conditional is already inside of the outer:

if !b.IsBuildLess() {

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, got it.

@joshrwolf joshrwolf merged commit 2dde56c into chainguard-dev:main Jul 17, 2023
@joshrwolf joshrwolf deleted the default-remove-builder branch July 17, 2023 19:46
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.

3 participants