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

Add message handling for when image build fails to pull and archive initial image #565

Closed
wants to merge 3 commits into from

Conversation

jsun-m
Copy link
Contributor

@jsun-m jsun-m commented Sep 24, 2024

No description provided.


_, err = b.containerRepo.GetContainerExitCode(containerId)
if err == nil {
outputChan <- common.OutputMsg{Done: true, Success: false, Msg: "Base image failed to load.\n"}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need to check if the exit code is > 0 because it looks like terminateContainer() calls SetContainerExitCode(). We also pass a reference of the exit code to terminateContainer(), which can be set from s.runcHandle.Run().

// a container address for the runc client to connect to.
// If it fails without a container address, we can still look up the exit code
// to terminate the build and notify the user.
for {
Copy link
Contributor

Choose a reason for hiding this comment

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

We might want to pass the calling function's context and check to see if its done using a select{} in the for loop. Doing this should guarantee that if someone disconnects or hits Ctl-C, this will exit regardless whats happening in the build workers.

@@ -199,6 +199,9 @@ func (b *Builder) Build(ctx context.Context, opts *BuildOpts, outputChan chan co
return err
}

// Get the hostname of the build container
go b.checkForInitialImagePullError(containerId, outputChan)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we can probably do this without an extra goroutine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ideally I check in the gateway to see if the image exists or not. But the packages that help do this require a shitton of other packages

Copy link
Contributor

@luke-lombardi luke-lombardi left a comment

Choose a reason for hiding this comment

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

@jsun-m Is this still required with the inspect and verify stuff now?

@jsun-m jsun-m closed this Oct 30, 2024
@jsun-m jsun-m deleted the jm/validate-container-image branch October 30, 2024 21:36
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