From aa405c8aac69d4ae4e7a21fb9a5dfb0a84f450b6 Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Thu, 11 May 2023 13:04:33 +0200 Subject: [PATCH] Allow runtimes to provide additional context on CRI pull errors Right now container runtimes have no way to provide additional context to the pull errors. We now loosen the constraints and check for additional messages after the actual CRI errors, which allows to enrich the verbosity of the warning events, for example: ``` Warning Failed 2s (x3 over 43s) kubelet Failed to pull image "localhost:5000/foo": RegistryUnavailable: pinging container registry localhost:5000: Get "http://localhost:5000/v2/": dial tcp [::1]:5000: connect: connection refused ``` Signed-off-by: Sascha Grunert --- pkg/kubelet/images/image_manager.go | 23 +++++++++++++++++++---- pkg/kubelet/images/image_manager_test.go | 23 ++++++++++++++++++++--- 2 files changed, 39 insertions(+), 7 deletions(-) diff --git a/pkg/kubelet/images/image_manager.go b/pkg/kubelet/images/image_manager.go index 8910f64097f..253713741ac 100644 --- a/pkg/kubelet/images/image_manager.go +++ b/pkg/kubelet/images/image_manager.go @@ -19,6 +19,7 @@ package images import ( "context" "fmt" + "strings" "time" dockerref "github.com/docker/distribution/reference" @@ -172,13 +173,27 @@ func (m *imageManager) EnsureImageExists(ctx context.Context, pod *v1.Pod, conta func evalCRIPullErr(container *v1.Container, err error) (errMsg string, errRes error) { // Error assertions via errors.Is is not supported by gRPC (remote runtime) errors right now. // See https://github.com/grpc/grpc-go/issues/3616 - if err.Error() == crierrors.ErrRegistryUnavailable.Error() { - errMsg = fmt.Sprintf("image pull failed for %s because the registry is unavailable.", container.Image) + if strings.HasPrefix(err.Error(), crierrors.ErrRegistryUnavailable.Error()) { + errMsg = fmt.Sprintf( + "image pull failed for %s because the registry is unavailable%s", + container.Image, + // Trim the error name from the message to convert errors like: + // "RegistryUnavailable: a more detailed explanation" to: + // "...because the registry is unavailable: a more detailed explanation" + strings.TrimPrefix(err.Error(), crierrors.ErrRegistryUnavailable.Error()), + ) return errMsg, crierrors.ErrRegistryUnavailable } - if err.Error() == crierrors.ErrSignatureValidationFailed.Error() { - errMsg = fmt.Sprintf("image pull failed for %s because the signature validation failed.", container.Image) + if strings.HasPrefix(err.Error(), crierrors.ErrSignatureValidationFailed.Error()) { + errMsg = fmt.Sprintf( + "image pull failed for %s because the signature validation failed%s", + container.Image, + // Trim the error name from the message to convert errors like: + // "SignatureValidationFailed: a more detailed explanation" to: + // "...because the signature validation failed: a more detailed explanation" + strings.TrimPrefix(err.Error(), crierrors.ErrSignatureValidationFailed.Error()), + ) return errMsg, crierrors.ErrSignatureValidationFailed } diff --git a/pkg/kubelet/images/image_manager_test.go b/pkg/kubelet/images/image_manager_test.go index 0e04c39e907..b6330657ebb 100644 --- a/pkg/kubelet/images/image_manager_test.go +++ b/pkg/kubelet/images/image_manager_test.go @@ -19,6 +19,7 @@ package images import ( "context" "errors" + "fmt" "sync" "testing" "time" @@ -435,7 +436,15 @@ func TestEvalCRIPullErr(t *testing.T) { input: crierrors.ErrRegistryUnavailable, assert: func(msg string, err error) { assert.ErrorIs(t, err, crierrors.ErrRegistryUnavailable) - assert.Contains(t, msg, "registry is unavailable") + assert.Equal(t, msg, "image pull failed for test because the registry is unavailable") + }, + }, + { + name: "registry is unavailable with additional error message", + input: fmt.Errorf("%v: foo", crierrors.ErrRegistryUnavailable), + assert: func(msg string, err error) { + assert.ErrorIs(t, err, crierrors.ErrRegistryUnavailable) + assert.Equal(t, msg, "image pull failed for test because the registry is unavailable: foo") }, }, { @@ -443,7 +452,15 @@ func TestEvalCRIPullErr(t *testing.T) { input: crierrors.ErrSignatureValidationFailed, assert: func(msg string, err error) { assert.ErrorIs(t, err, crierrors.ErrSignatureValidationFailed) - assert.Contains(t, msg, "signature validation failed") + assert.Equal(t, msg, "image pull failed for test because the signature validation failed") + }, + }, + { + name: "signature is invalid with additional error message (wrapped)", + input: fmt.Errorf("%w: bar", crierrors.ErrSignatureValidationFailed), + assert: func(msg string, err error) { + assert.ErrorIs(t, err, crierrors.ErrSignatureValidationFailed) + assert.Equal(t, msg, "image pull failed for test because the signature validation failed: bar") }, }, } { @@ -452,7 +469,7 @@ func TestEvalCRIPullErr(t *testing.T) { t.Run(tc.name, func(t *testing.T) { t.Parallel() - msg, err := evalCRIPullErr(&v1.Container{}, testInput) + msg, err := evalCRIPullErr(&v1.Container{Image: "test"}, testInput) testAssert(msg, err) }) }