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

fix:[#1525] handle container creation failure #1526

Merged
merged 1 commit into from
Oct 12, 2024

Conversation

jardon
Copy link
Contributor

@jardon jardon commented Aug 22, 2024

distrobox fumbles the error handling for container creation. the exit command is nested inside of the creation success conditional statement. this PR adds an else statement and handles the subprocess error by passing it up

changes:

  • print error message and exit with subprocess return code

here's the output now that the error is handled:

jardon@lagann:~/Projects/distrobox$ ./distrobox create --additional-flags jammy      --image docker.io/library/ubuntu:jammy --name apx-test --no-entry --yes --pull 
Trying to pull docker.io/library/ubuntu:jammy...
Getting image source signatures
Copying blob 857cc8cb19c0 skipped: already exists  
Copying config 53a843653c done   | 
Writing manifest to image destination
53a843653cbcd9e10be207e951d907dc2481d9c222de57d24cfcac32e5165188
Creating 'apx-test' using image docker.io/library/ubuntu:jammy	Resolving "jammy" using unqualified-search registries (/etc/containers/registries.conf)
Trying to pull docker.io/library/jammy:latest...
Error: initializing source docker://jammy:latest: reading manifest latest in docker.io/library/jammy: requested access to the resource is denied
 [ ERR ] failed to create container.
jardon@lagann:~/Projects/distrobox$ echo $?
125

Closes #1525

print error message and exit with subprocess return code
@jardon
Copy link
Contributor Author

jardon commented Sep 2, 2024

@89luca89 you mind looking at this? yes, it stemmed from an error with apx but it shows that a non-zero exit with podman resulted in a 0 exit for distrobox

Copy link

@softmoth softmoth left a comment

Choose a reason for hiding this comment

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

This looks good to me. NOTE: removing the exit $? within the if branch is OK, because it would be the last statement executed in the script and is therefore redundant. However, it might be best to leave it in just as an explicit indicator that "Hey, we rely on this exit status here!", and to have a nice symmetry between the if and else clauses.

@89luca89 89luca89 merged commit 2fcdd6e into 89luca89:main Oct 12, 2024
@89luca89
Copy link
Owner

Thanks @jardon and @softmoth for looking into it!

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.

[Error] Unhandled podman exit code
3 participants