Skip to content

Commit

Permalink
return Updater error codes
Browse files Browse the repository at this point in the history
  • Loading branch information
jakecoffman committed Dec 22, 2023
1 parent c7aa6e7 commit 207b71c
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 9 deletions.
2 changes: 1 addition & 1 deletion cmd/dependabot/internal/cmd/update.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ func NewUpdateCommand() *cobra.Command {
if errors.Is(err, context.DeadlineExceeded) {
log.Fatalf("update timed out after %s", flags.timeout)
}
log.Fatalf("failed to run updater: %v", err)
log.Fatalf("updater failure: %v", err)
}

return nil
Expand Down
2 changes: 1 addition & 1 deletion internal/infra/proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,7 @@ func (p *Proxy) Close() (err error) {
}()

// Check the error code if the container has already exited, so we can pass it along to the caller. If the proxy
//crashes we want the CLI to error out. Unlike the Updater it should never stop on its own.
//crashes we want the CLI to error out.
containerInfo, inspectErr := p.cli.ContainerInspect(context.Background(), p.containerID)
if inspectErr != nil {
return fmt.Errorf("failed to inspect proxy container: %w", inspectErr)
Expand Down
6 changes: 5 additions & 1 deletion internal/infra/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,11 @@ func runContainers(ctx context.Context, params RunParams) (err error) {
if err != nil {
return err
}
defer updater.Close()
defer func() {
if updaterErr := updater.Close(); updaterErr != nil {
err = updaterErr
}
}()

// put the clone dir in the updater container to be used by during the update
if params.LocalDir != "" {
Expand Down
34 changes: 29 additions & 5 deletions internal/infra/updater.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,19 +251,29 @@ func (u *Updater) RunCmd(ctx context.Context, cmd, user string, env ...string) e
_, _ = io.Copy(os.Stderr, prefixer.New(r, "updater | "))
}()

// blocks until update is complete or ctl-c
ch := make(chan struct{})
go func() {
_, _ = stdcopy.StdCopy(w, w, execResp.Reader)
ch <- struct{}{}
}()

// blocks until update is complete or ctl-c
select {
case <-ctx.Done():
return ctx.Err()
case <-ch:
}

// check the exit code of the command
execInspect, err := u.cli.ContainerExecInspect(ctx, execCreate.ID)
if err != nil {
return fmt.Errorf("failed to inspect exec: %w", err)
}

if execInspect.ExitCode != 0 {
return fmt.Errorf("updater exited with code: %v", execInspect.ExitCode)
}

return nil
}

Expand All @@ -282,10 +292,24 @@ func (u *Updater) Wait(ctx context.Context, condition container.WaitCondition) e
}

// Close kills and deletes the container and deletes updater mount paths related to the run.
func (u *Updater) Close() error {
return u.cli.ContainerRemove(context.Background(), u.containerID, types.ContainerRemoveOptions{
Force: true,
})
func (u *Updater) Close() (err error) {
defer func() {
removeErr := u.cli.ContainerRemove(context.Background(), u.containerID, types.ContainerRemoveOptions{Force: true})
if removeErr != nil {
err = fmt.Errorf("failed to remove proxy container: %w", removeErr)
}
}()

// Handle non-zero exit codes.
containerInfo, inspectErr := u.cli.ContainerInspect(context.Background(), u.containerID)
if inspectErr != nil {
return fmt.Errorf("failed to inspect proxy container: %w", inspectErr)
}
if containerInfo.State.ExitCode != 0 {
return fmt.Errorf("updater container exited with non-zero exit code: %d", containerInfo.State.ExitCode)
}

return
}

// JobFile is the payload passed to file updater containers.
Expand Down
24 changes: 24 additions & 0 deletions testdata/scripts/crash.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
exec docker build -t crashy-updater .

! dependabot update go_modules dependabot/cli --updater-image crashy-updater
stderr 'updater exited with code: 2'

exec docker rmi -f crashy-updater

-- Dockerfile --
FROM ubuntu:22.04

RUN useradd dependabot

COPY --chown=dependabot --chmod=755 update-ca-certificates /usr/bin/update-ca-certificates
COPY --chown=dependabot --chmod=755 run bin/run

-- update-ca-certificates --
#!/usr/bin/env bash

echo "Updating CA certificates..."

-- run --
#!/usr/bin/env bash

exit 2
2 changes: 1 addition & 1 deletion testdata/scripts/local.txt
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
exec docker build -qt local-updater .

# The ls command in run will fail since this isn't a real updater
dependabot update go_modules dependabot-fixtures/go-modules-lib --updater-image local-updater
! dependabot update go_modules dependabot-fixtures/go-modules-lib --updater-image local-updater
stderr 'No such file or directory'

# The local flag should create the repo directory and put my-repo in it
Expand Down

0 comments on commit 207b71c

Please sign in to comment.