Merge pull request #117935 from saschagrunert/cri-errors-additional

Allow runtimes to provide additional context on CRI pull errors

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() {
+	if strings.HasPrefix(err.Error(), crierrors.ErrRegistryUnavailable.Error()) {
-		errMsg = fmt.Sprintf("image pull failed for %s because the registry is unavailable.", container.Image)
+		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() {
+	if strings.HasPrefix(err.Error(), crierrors.ErrSignatureValidationFailed.Error()) {
-		errMsg = fmt.Sprintf("image pull failed for %s because the signature validation failed.", container.Image)
+		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
 	}
 

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)
 		})
 	}