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 panic condition on docker pull failure #1968

Merged
merged 3 commits into from
Jul 27, 2023
Merged

Conversation

wagoodman
Copy link
Contributor

@wagoodman wagoodman commented Jul 27, 2023

When pulling a docker image and there is an issue, there is a panic when attempting to close the source:

❯ go run ./cmd/syft python:slim -o json
  ⠼ Pulling image
 panic: value method github.com/anchore/syft/syft/source.StereoscopeImageSource.Close called using nil *StereoscopeImageSource pointer
 
 goroutine 9 [running]:
 github.com/anchore/syft/syft/source.(*StereoscopeImageSource).Close(0xc00011c3e0?)
         <autogenerated>:1 +0x86
 github.com/anchore/syft/cmd/syft/cli/packages.execWorker.func1()
         /Users/wagoodman/code/syft/cmd/syft/cli/packages/packages.go:109 +0x92a
created by github.com/anchore/syft/cmd/syft/cli/packages.execWorker
        /Users/wagoodman/code/syft/cmd/syft/cli/packages/packages.go:57 +0xf7
 exit status 2

When fixing the panic, it was noticed that the UI was hung (but no more panicing!). There seems to be an underlying issue to fix in last-event handling in the syft event loop and error conditions in stereoscope, both of which have been deferred. In the meantime there was an additional change to not wait for the UI to teardown in kill situations, resulting in the following output:

❯ go run ./cmd/syft python:slims -o json
 ⠦ Pulling image                   
2023/07/27 11:18:59 error during command execution: 1 error occurred:
        * failed to construct source from user input "python:slims": unable to load image: unable to use DockerDaemon source: pull failed: Error response from daemon: manifest for python:slims not found: manifest unknown: manifest unknown

exit status 1

@wagoodman wagoodman self-assigned this Jul 27, 2023
@wagoodman wagoodman added the bug Something isn't working label Jul 27, 2023
@wagoodman wagoodman requested a review from a team July 27, 2023 15:20
@wagoodman wagoodman marked this pull request as ready for review July 27, 2023 15:20
Signed-off-by: Alex Goodman <[email protected]>
@github-actions
Copy link

github-actions bot commented Jul 27, 2023

Benchmark Test Results

Benchmark results from the latest changes vs base branch
goos: linux%0Agoarch: amd64%0Apkg: github.com/anchore/syft/test/integration%0Acpu: Intel(R) Xeon(R) Platinum 8272CL CPU @ 2.60GHz%0A                                                              │ ./.tmp/benchmark-4ee70f7.txt │%0A                                                              │            sec/op            │%0AImagePackageCatalogers/alpmdb-cataloger-2                                       12.31m ±  2%25%0AImagePackageCatalogers/apkdb-cataloger-2                                        740.7µ ±  5%25%0AImagePackageCatalogers/binary-cataloger-2                                       212.3µ ±  2%25%0AImagePackageCatalogers/dpkgdb-cataloger-2                                       632.2µ ±  5%25%0AImagePackageCatalogers/dotnet-portable-executable-cataloger-2                   22.25µ ±  1%25%0AImagePackageCatalogers/go-module-binary-cataloger-2                             97.05µ ± 16%25%0AImagePackageCatalogers/java-cataloger-2                                         13.97m ±  2%25%0AImagePackageCatalogers/graalvm-native-image-cataloger-2                         98.46µ ±  2%25%0AImagePackageCatalogers/javascript-package-cataloger-2                           403.2µ ±  1%25%0AImagePackageCatalogers/nix-store-cataloger-2                                    290.3µ ±  1%25%0AImagePackageCatalogers/php-composer-installed-cataloger-2                       833.6µ ±  0%25%0AImagePackageCatalogers/portage-cataloger-2                                      503.9µ ±  1%25%0AImagePackageCatalogers/python-package-cataloger-2                               3.506m ±  3%25%0AImagePackageCatalogers/r-package-cataloger-2                                    218.4µ ±  1%25%0AImagePackageCatalogers/rpm-db-cataloger-2                                       577.4µ ±  1%25%0AImagePackageCatalogers/ruby-gemspec-cataloger-2                                 952.0µ ±  1%25%0AImagePackageCatalogers/sbom-cataloger-2                                         121.9µ ±  2%25%0Ageomean                                                                         505.4µ%0A%0A                                                              │ ./.tmp/benchmark-4ee70f7.txt │%0A                                                              │             B/op             │%0AImagePackageCatalogers/alpmdb-cataloger-2                                       5.119Mi ± 0%25%0AImagePackageCatalogers/apkdb-cataloger-2                                        204.8Ki ± 0%25%0AImagePackageCatalogers/binary-cataloger-2                                       30.19Ki ± 0%25%0AImagePackageCatalogers/dpkgdb-cataloger-2                                       168.8Ki ± 0%25%0AImagePackageCatalogers/dotnet-portable-executable-cataloger-2                   3.696Ki ± 0%25%0AImagePackageCatalogers/go-module-binary-cataloger-2                             9.906Ki ± 0%25%0AImagePackageCatalogers/java-cataloger-2                                         2.823Mi ± 0%25%0AImagePackageCatalogers/graalvm-native-image-cataloger-2                         8.594Ki ± 0%25%0AImagePackageCatalogers/javascript-package-cataloger-2                           94.19Ki ± 0%25%0AImagePackageCatalogers/nix-store-cataloger-2                                    49.14Ki ± 0%25%0AImagePackageCatalogers/php-composer-installed-cataloger-2                       186.6Ki ± 0%25%0AImagePackageCatalogers/portage-cataloger-2                                      119.9Ki ± 0%25%0AImagePackageCatalogers/python-package-cataloger-2                               1.004Mi ± 0%25%0AImagePackageCatalogers/r-package-cataloger-2                                    53.29Ki ± 0%25%0AImagePackageCatalogers/rpm-db-cataloger-2                                       180.9Ki ± 0%25%0AImagePackageCatalogers/ruby-gemspec-cataloger-2                                 144.0Ki ± 0%25%0AImagePackageCatalogers/sbom-cataloger-2                                         14.20Ki ± 0%25%0Ageomean                                                                         100.3Ki%0A%0A                                                              │ ./.tmp/benchmark-4ee70f7.txt │%0A                                                              │          allocs/op           │%0AImagePackageCatalogers/alpmdb-cataloger-2                                        87.74k ± 0%25%0AImagePackageCatalogers/apkdb-cataloger-2                                         4.182k ± 0%25%0AImagePackageCatalogers/binary-cataloger-2                                         830.0 ± 0%25%0AImagePackageCatalogers/dpkgdb-cataloger-2                                        3.002k ± 0%25%0AImagePackageCatalogers/dotnet-portable-executable-cataloger-2                     132.0 ± 0%25%0AImagePackageCatalogers/go-module-binary-cataloger-2                               281.0 ± 0%25%0AImagePackageCatalogers/java-cataloger-2                                          39.88k ± 0%25%0AImagePackageCatalogers/graalvm-native-image-cataloger-2                           228.0 ± 0%25%0AImagePackageCatalogers/javascript-package-cataloger-2                            1.342k ± 0%25%0AImagePackageCatalogers/nix-store-cataloger-2                                      895.0 ± 0%25%0AImagePackageCatalogers/php-composer-installed-cataloger-2                        4.079k ± 0%25%0AImagePackageCatalogers/portage-cataloger-2                                       2.268k ± 0%25%0AImagePackageCatalogers/python-package-cataloger-2                                16.44k ± 0%25%0AImagePackageCatalogers/r-package-cataloger-2                                      929.0 ± 0%25%0AImagePackageCatalogers/rpm-db-cataloger-2                                        3.989k ± 0%25%0AImagePackageCatalogers/ruby-gemspec-cataloger-2                                  2.447k ± 0%25%0AImagePackageCatalogers/sbom-cataloger-2                                           394.0 ± 0%25%0Ageomean                                                                          2.051k

@wagoodman wagoodman merged commit bbd2d42 into main Jul 27, 2023
9 checks passed
@wagoodman wagoodman deleted the fix-docker-pull-hang branch July 27, 2023 15:32
GijsCalis pushed a commit to GijsCalis/syft that referenced this pull request Feb 19, 2024
* [wip] add image pull error handlers

Signed-off-by: Alex Goodman <[email protected]>

* fix panic and ui hang on docker pull failure

Signed-off-by: Alex Goodman <[email protected]>

* linter fix

Signed-off-by: Alex Goodman <[email protected]>

---------

Signed-off-by: Alex Goodman <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants