Merge pull request #124739 from saschagrunert/timeout-error

Remove CRI `TimeoutError` type

pkg/kubelet/cri/remote/remote_runtime.go

@@ -37,7 +37,6 @@ import (
 	runtimeapi "k8s.io/cri-api/pkg/apis/runtime/v1"
 	"k8s.io/klog/v2"
 	"k8s.io/kubernetes/pkg/kubelet/util"
-	"k8s.io/kubernetes/pkg/probe/exec"
 
 	utilexec "k8s.io/utils/exec"
 )
@@ -64,8 +63,13 @@ const (
 // versions.
 type CRIVersion string
 
-// ErrContainerStatusNil indicates that the returned container status is nil.
+var (
-var ErrContainerStatusNil = errors.New("container status is nil")
+	// ErrContainerStatusNil indicates that the returned container status is nil.
+	ErrContainerStatusNil = errors.New("container status is nil")
+
+	// ErrCommandTimedOut indicates that the exec sync command timed.
+	ErrCommandTimedOut = errors.New("command timed out")
+)
 
 const (
 	// CRIVersionV1 references the v1 CRI API.
@@ -494,9 +498,9 @@ func (r *remoteRuntimeService) execSyncV1(ctx context.Context, containerID strin
 	if err != nil {
 		klog.ErrorS(err, "ExecSync cmd from runtime service failed", "containerID", containerID, "cmd", cmd)
 
-		// interpret DeadlineExceeded gRPC errors as timedout probes
+		// interpret DeadlineExceeded gRPC errors as timedout errors
 		if status.Code(err) == codes.DeadlineExceeded {
-			err = exec.NewTimeoutError(fmt.Errorf("command %q timed out", strings.Join(cmd, " ")), timeout)
+			err = fmt.Errorf("%w: %q timed out after %s", ErrCommandTimedOut, strings.Join(cmd, " "), timeout)
 		}
 
 		return nil, nil, err

pkg/probe/exec/errors.go

@@ -1,47 +0,0 @@
-/*
-Copyright 2020 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package exec
-
-import (
-	"time"
-)
-
-// NewTimeoutError returns a new TimeoutError.
-func NewTimeoutError(err error, timeout time.Duration) *TimeoutError {
-	return &TimeoutError{
-		err:     err,
-		timeout: timeout,
-	}
-}
-
-// TimeoutError is an error returned on exec probe timeouts. It should be returned by CRI implementations
-// in order for the exec prober to interpret exec timeouts as failed probes.
-// TODO: this error type can likely be removed when we support CRI errors.
-type TimeoutError struct {
-	err     error
-	timeout time.Duration
-}
-
-// Error returns the error string.
-func (t *TimeoutError) Error() string {
-	return t.err.Error()
-}
-
-// Timeout returns the timeout duration of the exec probe.
-func (t *TimeoutError) Timeout() time.Duration {
-	return t.timeout
-}

pkg/probe/exec/errors_test.go

@@ -1,51 +0,0 @@
-/*
-Copyright 2023 The Kubernetes Authors.
-
-Licensed under the Apache License, Version 2.0 (the "License");
-you may not use this file except in compliance with the License.
-You may obtain a copy of the License at
-
-    http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing, software
-distributed under the License is distributed on an "AS IS" BASIS,
-WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-See the License for the specific language governing permissions and
-limitations under the License.
-*/
-
-package exec
-
-import (
-	"errors"
-	"testing"
-	"time"
-)
-
-func TestErrors(t *testing.T) {
-	tests := []struct {
-		err     error
-		timeout time.Duration
-		message string
-	}{
-		{
-			err:     errors.New("some error message"),
-			timeout: time.Hour * 8,
-			message: "some error message",
-		},
-	}
-
-	for i, test := range tests {
-		testErr := NewTimeoutError(test.err, test.timeout)
-
-		if testErr == nil {
-			t.Errorf("[%d] expected error a TimeoutError, got nil", i)
-		}
-		if msg := testErr.Error(); msg != test.message {
-			t.Errorf("[%d] expected error message %q, got %q", i, test.message, msg)
-		}
-		if timeout := testErr.Timeout(); timeout != test.timeout {
-			t.Errorf("[%d] expected timeout %q, got %q", i, test.timeout, timeout)
-		}
-	}
-}

pkg/probe/exec/exec.go

@@ -18,9 +18,11 @@ package exec
 
 import (
 	"bytes"
+	"errors"
 
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	"k8s.io/kubernetes/pkg/features"
+	"k8s.io/kubernetes/pkg/kubelet/cri/remote"
 	"k8s.io/kubernetes/pkg/kubelet/util/ioutils"
 	"k8s.io/kubernetes/pkg/probe"
 
@@ -69,14 +71,12 @@ func (pr execProber) Probe(e exec.Cmd) (probe.Result, string, error) {
 			return probe.Failure, string(data), nil
 		}
 
-		timeoutErr, ok := err.(*TimeoutError)
+		if errors.Is(err, remote.ErrCommandTimedOut) {
-		if ok {
 			if utilfeature.DefaultFeatureGate.Enabled(features.ExecProbeTimeout) {
-				// When exec probe timeout, data is empty, so we should return timeoutErr.Error() as the stdout.
+				return probe.Failure, err.Error(), nil
-				return probe.Failure, timeoutErr.Error(), nil
 			}
 
-			klog.Warningf("Exec probe timed out after %s but ExecProbeTimeout feature gate was disabled", timeoutErr.Timeout())
+			klog.Warningf("Exec probe timed out but ExecProbeTimeout feature gate was disabled")
 		}
 
 		return probe.Unknown, "", err

pkg/probe/exec/exec_test.go

@@ -21,11 +21,11 @@ import (
 	"io"
 	"strings"
 	"testing"
-	"time"
 
 	utilfeature "k8s.io/apiserver/pkg/util/feature"
 	featuregatetesting "k8s.io/component-base/featuregate/testing"
 	"k8s.io/kubernetes/pkg/features"
+	"k8s.io/kubernetes/pkg/kubelet/cri/remote"
 	"k8s.io/kubernetes/pkg/probe"
 )
 
@@ -128,9 +128,9 @@ func TestExec(t *testing.T) {
 		// Unhealthy
 		{probe.Failure, false, true, "Fail", "", &fakeExitError{true, 1}},
 		// Timeout
-		{probe.Failure, false, true, "", "command testcmd timed out", NewTimeoutError(fmt.Errorf("command testcmd timed out"), time.Second)},
+		{probe.Failure, false, true, "", remote.ErrCommandTimedOut.Error() + ": command testcmd timed out", fmt.Errorf("%w: command testcmd timed out", remote.ErrCommandTimedOut)},
 		// ExecProbeTimeout
-		{probe.Unknown, true, false, "", "", NewTimeoutError(fmt.Errorf("command testcmd timed out"), time.Second)},
+		{probe.Unknown, true, false, "", "", fmt.Errorf("%w: command testcmd timed out", remote.ErrCommandTimedOut)},
 	}
 
 	for i, test := range tests {