Skip to content

Commit

Permalink
Enhance error messages around image to tag resolving. (#5920)
Browse files Browse the repository at this point in the history
* Enhance error messages around image to tag resolving.

We currently surface the plain error returned from the tag resolution without further explanation. This adds wrapping around those errors to explain in which step it fails and adds wrapping around the outer layer to make sure the user gets enough context to be able to tell what exactly failed.

Fixes #5410

* Make the linter happy.
  • Loading branch information
markusthoemmes authored and knative-prow-robot committed Nov 1, 2019
1 parent 7c304a6 commit e8fe5ef
Show file tree
Hide file tree
Showing 3 changed files with 12 additions and 10 deletions.
10 changes: 5 additions & 5 deletions pkg/reconciler/revision/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ func (r *digestResolver) Resolve(
registriesToSkip sets.String) (string, error) {
kc, err := k8schain.New(r.client, opt)
if err != nil {
return "", err
return "", fmt.Errorf("failed to initialize authentication: %w", err)
}

if _, err := name.NewDigest(image, name.WeakValidation); err == nil {
Expand All @@ -102,7 +102,7 @@ func (r *digestResolver) Resolve(

tag, err := name.NewTag(image, name.WeakValidation)
if err != nil {
return "", err
return "", fmt.Errorf("failed to parse image name %q into a tag: %w", image, err)
}

if registriesToSkip.Has(tag.Registry.RegistryStr()) {
Expand All @@ -114,7 +114,7 @@ func (r *digestResolver) Resolve(
}
desc, err := remote.Get(tag, remote.WithTransport(r.transport), remote.WithAuthFromKeychain(kc), remote.WithPlatform(platform))
if err != nil {
return "", err
return "", fmt.Errorf("failed to fetch image information: %w", err)
}

// TODO(#3997): Use remote.Get to resolve manifest lists to digests as well
Expand All @@ -123,11 +123,11 @@ func (r *digestResolver) Resolve(
case types.OCIImageIndex, types.DockerManifestList:
img, err := desc.Image()
if err != nil {
return "", err
return "", fmt.Errorf("failed to get image reference: %w", err)
}
dgst, err := img.Digest()
if err != nil {
return "", err
return "", fmt.Errorf("failed to get image digest: %w", err)
}
return fmt.Sprintf("%s@%s", tag.Repository.String(), dgst), nil
default:
Expand Down
2 changes: 2 additions & 0 deletions pkg/reconciler/revision/revision.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ package revision

import (
"context"
"fmt"
"reflect"
"strings"

Expand Down Expand Up @@ -142,6 +143,7 @@ func (c *Reconciler) reconcileDigest(ctx context.Context, rev *v1alpha1.Revision
digest, err := c.resolver.Resolve(rev.Spec.GetContainer().Image,
opt, cfgs.Deployment.RegistriesSkippingTagResolving)
if err != nil {
err = fmt.Errorf("failed to resolve image to digest: %w", err)
rev.Status.MarkContainerHealthyFalse(v1alpha1.ContainerMissing,
v1alpha1.RevisionContainerMissingMessage(
rev.Spec.GetContainer().Image, err.Error()))
Expand Down
10 changes: 5 additions & 5 deletions pkg/reconciler/revision/revision_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -261,20 +261,20 @@ func (r *fixedResolver) Resolve(_ string, _ k8schain.Options, _ sets.String) (st
}

type errorResolver struct {
error string
err error
}

func (r *errorResolver) Resolve(_ string, _ k8schain.Options, _ sets.String) (string, error) {
return "", errors.New(r.error)
return "", r.err
}

func TestResolutionFailed(t *testing.T) {
ctx, cancel, _, controller, _ := newTestController(t)
defer cancel()

// Unconditionally return this error during resolution.
errorMessage := "I am the expected error message, hear me ROAR!"
controller.Reconciler.(*Reconciler).resolver = &errorResolver{errorMessage}
innerError := errors.New("i am the expected error message, hear me ROAR!")
controller.Reconciler.(*Reconciler).resolver = &errorResolver{innerError}

rev := testRevision()
config := testConfiguration()
Expand All @@ -295,7 +295,7 @@ func TestResolutionFailed(t *testing.T) {
Status: corev1.ConditionFalse,
Reason: "ContainerMissing",
Message: v1alpha1.RevisionContainerMissingMessage(
rev.Spec.GetContainer().Image, errorMessage),
rev.Spec.GetContainer().Image, "failed to resolve image to digest: "+innerError.Error()),
LastTransitionTime: got.LastTransitionTime,
Severity: apis.ConditionSeverityError,
}
Expand Down

0 comments on commit e8fe5ef

Please sign in to comment.