From 4cdfe600e04e1e05b48a0dbc8d1f0e0f0213cc3f Mon Sep 17 00:00:00 2001 From: Sascha Grunert Date: Wed, 26 Apr 2023 10:56:03 +0200 Subject: [PATCH] Fix image pull error type `ErrRegistryUnavailable` The current error comparison `imagePullResult.err == ErrRegistryUnavailable` will never work with any remote runtime, because we produce gRPC errors which wrap a code and a description, like: ``` rpc error: code = Unknown desc = This is the error description ``` To be able to check custom error types from `pkg/kubelet/images/types.go`, we now strip the code if the status is unknown on image pull. Beside that, we use a string comparison to check against `ErrRegistryUnavailable.Error()`, because validating them via the `errors` package is not yet supported by grpc-go: https://github.com/grpc/grpc-go/issues/3616 Signed-off-by: Sascha Grunert --- pkg/kubelet/cri/remote/remote_image.go | 13 +++++++++++++ pkg/kubelet/images/image_manager.go | 5 ++++- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/pkg/kubelet/cri/remote/remote_image.go b/pkg/kubelet/cri/remote/remote_image.go index 1deff550fd8..f053345a06c 100644 --- a/pkg/kubelet/cri/remote/remote_image.go +++ b/pkg/kubelet/cri/remote/remote_image.go @@ -25,7 +25,9 @@ import ( "go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc" "go.opentelemetry.io/otel/trace" "google.golang.org/grpc" + "google.golang.org/grpc/codes" "google.golang.org/grpc/credentials/insecure" + "google.golang.org/grpc/status" utilfeature "k8s.io/apiserver/pkg/util/feature" tracing "k8s.io/component-base/tracing" "k8s.io/klog/v2" @@ -165,6 +167,17 @@ func (r *remoteImageService) pullImageV1(ctx context.Context, image *runtimeapi. }) if err != nil { klog.ErrorS(err, "PullImage from image service failed", "image", image.Image) + + // We can strip the code from unknown status errors since they add no value + // and will make them easier to read in the logs/events. + // + // It also ensures that checking custom error types from pkg/kubelet/images/types.go + // works in `imageManager.EnsureImageExists` (pkg/kubelet/images/image_manager.go). + statusErr, ok := status.FromError(err) + if ok && statusErr.Code() == codes.Unknown { + return "", errors.New(statusErr.Message()) + } + return "", err } diff --git a/pkg/kubelet/images/image_manager.go b/pkg/kubelet/images/image_manager.go index caf28cf7467..8ec8fb67a52 100644 --- a/pkg/kubelet/images/image_manager.go +++ b/pkg/kubelet/images/image_manager.go @@ -157,7 +157,10 @@ func (m *imageManager) EnsureImageExists(ctx context.Context, pod *v1.Pod, conta if imagePullResult.err != nil { m.logIt(ref, v1.EventTypeWarning, events.FailedToPullImage, logPrefix, fmt.Sprintf("Failed to pull image %q: %v", container.Image, imagePullResult.err), klog.Warning) m.backOff.Next(backOffKey, m.backOff.Clock.Now()) - if imagePullResult.err == ErrRegistryUnavailable { + + // 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 imagePullResult.err.Error() == ErrRegistryUnavailable.Error() { msg := fmt.Sprintf("image pull failed for %s because the registry is unavailable.", container.Image) return "", msg, imagePullResult.err }