Skip to content

Commit

Permalink
feat: improve "Fetching" message (wagoodman#482)
Browse files Browse the repository at this point in the history
The Fetching... message was confusing.

This replaces it with a clearer messages to avoid confusion.

Additional fix: show original error unless image is not found

Only try doing a pull if the image isn't found. Everything else should
just generate the error so the user can fix it.

Fixes wagoodman#360

Co-authored-by: Christian Höltje <[email protected]>
  • Loading branch information
joschi and docwhat committed Nov 6, 2024
1 parent fce748f commit 745cb1e
Show file tree
Hide file tree
Showing 6 changed files with 41 additions and 8 deletions.
5 changes: 5 additions & 0 deletions dive/image/docker/archive_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,11 @@ func NewResolverFromArchive() *archiveResolver {
return &archiveResolver{}
}

// Name returns the name of the resolver to display to the user.
func (r *archiveResolver) Name() string {
return "docker-archive"
}

func (r *archiveResolver) Fetch(path string) (*image.Image, error) {
reader, err := os.Open(path)
if err != nil {
Expand Down
18 changes: 14 additions & 4 deletions dive/image/docker/engine_resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,11 @@ func NewResolverFromEngine() *engineResolver {
return &engineResolver{}
}

// Name returns the name of the resolver to display to the user.
func (r *engineResolver) Name() string {
return "docker-engine"
}

func (r *engineResolver) Fetch(id string) (*image.Image, error) {
reader, err := r.fetchArchive(id)
if err != nil {
Expand Down Expand Up @@ -85,10 +90,15 @@ func (r *engineResolver) fetchArchive(id string) (io.ReadCloser, error) {
}
_, _, err = dockerClient.ImageInspectWithRaw(ctx, id)
if err != nil {
// don't use the API, the CLI has more informative output
fmt.Println("Handler not available locally. Trying to pull '" + id + "'...")
err = runDockerCmd("pull", id)
if err != nil {
// check if the error is due to the image not existing locally
if client.IsErrNotFound(err) {
fmt.Println("The image is not available locally. Trying to pull '" + id + "'...")
err = runDockerCmd("pull", id)
if err != nil {
return nil, err
}
} else {
// Some other error occurred, return it
return nil, err
}
}
Expand Down
5 changes: 5 additions & 0 deletions dive/image/podman/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ func NewResolverFromEngine() *resolver {
return &resolver{}
}

// Name returns the name of the resolver to display to the user.
func (r *resolver) Name() string {
return "podman"
}

func (r *resolver) Build(args []string) (*image.Image, error) {
id, err := buildImageFromCli(args)
if err != nil {
Expand Down
1 change: 1 addition & 0 deletions dive/image/resolver.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package image

type Resolver interface {
Name() string
Fetch(id string) (*Image, error)
Build(options []string) (*Image, error)
}
2 changes: 1 addition & 1 deletion runtime/run.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ func run(enableUi bool, options Options, imageResolver image.Resolver, events ev
}
} else {
events.message(utils.TitleFormat("Image Source: ") + options.Source.String() + "://" + options.Image)
events.message(utils.TitleFormat("Fetching image...") + " (this can take a while for large images)")
events.message(utils.TitleFormat("Extracting image from "+imageResolver.Name()+"...") + " (this can take a while for large images)")
img, err = imageResolver.Fetch(options.Image)
if err != nil {
events.exitWithErrorMessage("cannot fetch image", err)
Expand Down
18 changes: 15 additions & 3 deletions runtime/run_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ import (

type defaultResolver struct{}

func (r *defaultResolver) Name() string {
return "default-resolver"
}

func (r *defaultResolver) Fetch(id string) (*image.Image, error) {
archive, err := docker.TestLoadArchive("../.data/test-docker-image.tar")
if err != nil {
Expand All @@ -30,6 +34,10 @@ func (r *defaultResolver) Build(args []string) (*image.Image, error) {

type failedBuildResolver struct{}

func (r *failedBuildResolver) Name() string {
return "failed-build-resolver"
}

func (r *failedBuildResolver) Fetch(id string) (*image.Image, error) {
archive, err := docker.TestLoadArchive("../.data/test-docker-image.tar")
if err != nil {
Expand All @@ -44,6 +52,10 @@ func (r *failedBuildResolver) Build(args []string) (*image.Image, error) {

type failedFetchResolver struct{}

func (r *failedFetchResolver) Name() string {
return "failed-fetch-resolver"
}

func (r *failedFetchResolver) Fetch(id string) (*image.Image, error) {
return nil, fmt.Errorf("some fetch failure")
}
Expand Down Expand Up @@ -108,7 +120,7 @@ func TestRun(t *testing.T) {
},
events: []testEvent{
{stdout: "Image Source: docker://dive-example", stderr: "", errorOnExit: false, errMessage: ""},
{stdout: "Fetching image... (this can take a while for large images)", stderr: "", errorOnExit: false, errMessage: ""},
{stdout: "Extracting image from default-resolver... (this can take a while for large images)", stderr: "", errorOnExit: false, errMessage: ""},
{stdout: "Analyzing image...", stderr: "", errorOnExit: false, errMessage: ""},
{stdout: "Building cache...", stderr: "", errorOnExit: false, errMessage: ""},
},
Expand All @@ -126,7 +138,7 @@ func TestRun(t *testing.T) {
},
events: []testEvent{
{stdout: "Image Source: docker://dive-example", stderr: "", errorOnExit: false, errMessage: ""},
{stdout: "Fetching image... (this can take a while for large images)", stderr: "", errorOnExit: false, errMessage: ""},
{stdout: "Extracting image from default-resolver... (this can take a while for large images)", stderr: "", errorOnExit: false, errMessage: ""},
{stdout: "Analyzing image...", stderr: "", errorOnExit: false, errMessage: ""},
{stdout: "Building cache...", stderr: "", errorOnExit: false, errMessage: ""},
},
Expand Down Expand Up @@ -159,7 +171,7 @@ func TestRun(t *testing.T) {
},
events: []testEvent{
{stdout: "Image Source: docker://dive-example", stderr: "", errorOnExit: false, errMessage: ""},
{stdout: "Fetching image... (this can take a while for large images)", stderr: "", errorOnExit: false, errMessage: ""},
{stdout: "Extracting image from failed-fetch-resolver... (this can take a while for large images)", stderr: "", errorOnExit: false, errMessage: ""},
{stdout: "", stderr: "cannot fetch image", errorOnExit: true, errMessage: "some fetch failure"},
},
},
Expand Down

0 comments on commit 745cb1e

Please sign in to comment.