respect ExecProbeTimeout

This commit is contained in:
Jack Francis 2021-03-12 13:47:56 -08:00
parent fcee7a0105
commit 5a43067915
3 changed files with 75 additions and 16 deletions

View File

@ -26,8 +26,10 @@ import (
dockertypes "github.com/docker/docker/api/types"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/tools/remotecommand"
"k8s.io/klog/v2"
"k8s.io/kubernetes/pkg/features"
kubecontainer "k8s.io/kubernetes/pkg/kubelet/container"
"k8s.io/kubernetes/pkg/kubelet/dockershim/libdocker"
)
@ -106,7 +108,7 @@ func (*NativeExecHandler) ExecInContainer(ctx context.Context, client libdocker.
ExecStarted: execStarted,
}
if timeout > 0 {
if timeout > 0 && utilfeature.DefaultFeatureGate.Enabled(features.ExecProbeTimeout) {
var cancel context.CancelFunc
ctx, cancel = context.WithTimeout(ctx, timeout)
defer cancel()

View File

@ -29,7 +29,11 @@ import (
"github.com/golang/mock/gomock"
"github.com/stretchr/testify/assert"
utilfeature "k8s.io/apiserver/pkg/util/feature"
"k8s.io/client-go/tools/remotecommand"
featuregatetesting "k8s.io/component-base/featuregate/testing"
"k8s.io/kubernetes/pkg/features"
"k8s.io/kubernetes/pkg/kubelet/dockershim/libdocker"
mockclient "k8s.io/kubernetes/pkg/kubelet/dockershim/libdocker/testing"
)
@ -43,6 +47,8 @@ func TestExecInContainer(t *testing.T) {
returnStartExec error
returnInspectExec1 *dockertypes.ContainerExecInspect
returnInspectExec2 error
execProbeTimeout bool
startExecDelay time.Duration
expectError error
}{{
description: "ExecInContainer succeeds",
@ -57,6 +63,7 @@ func TestExecInContainer(t *testing.T) {
ExitCode: 0,
Pid: 100},
returnInspectExec2: nil,
execProbeTimeout: true,
expectError: nil,
}, {
description: "CreateExec returns an error",
@ -66,6 +73,7 @@ func TestExecInContainer(t *testing.T) {
returnStartExec: nil,
returnInspectExec1: nil,
returnInspectExec2: nil,
execProbeTimeout: true,
expectError: fmt.Errorf("failed to exec in container - Exec setup failed - error in CreateExec()"),
}, {
description: "StartExec returns an error",
@ -75,6 +83,7 @@ func TestExecInContainer(t *testing.T) {
returnStartExec: fmt.Errorf("error in StartExec()"),
returnInspectExec1: nil,
returnInspectExec2: nil,
execProbeTimeout: true,
expectError: fmt.Errorf("error in StartExec()"),
}, {
description: "InspectExec returns an error",
@ -84,6 +93,7 @@ func TestExecInContainer(t *testing.T) {
returnStartExec: nil,
returnInspectExec1: nil,
returnInspectExec2: fmt.Errorf("error in InspectExec()"),
execProbeTimeout: true,
expectError: fmt.Errorf("error in InspectExec()"),
}, {
description: "ExecInContainer returns context DeadlineExceeded",
@ -98,7 +108,30 @@ func TestExecInContainer(t *testing.T) {
ExitCode: 0,
Pid: 100},
returnInspectExec2: nil,
execProbeTimeout: true,
expectError: context.DeadlineExceeded,
}, {
description: "[ExecProbeTimeout=true] StartExec that takes longer than the probe timeout returns context.DeadlineExceeded",
timeout: 1 * time.Second,
returnCreateExec1: &dockertypes.IDResponse{ID: "12345678"},
returnCreateExec2: nil,
startExecDelay: 5 * time.Second,
returnStartExec: fmt.Errorf("error in StartExec()"),
returnInspectExec1: nil,
returnInspectExec2: nil,
execProbeTimeout: true,
expectError: context.DeadlineExceeded,
}, {
description: "[ExecProbeTimeout=false] StartExec that takes longer than the probe timeout returns a error",
timeout: 1 * time.Second,
returnCreateExec1: &dockertypes.IDResponse{ID: "12345678"},
returnCreateExec2: nil,
startExecDelay: 5 * time.Second,
returnStartExec: fmt.Errorf("error in StartExec()"),
returnInspectExec1: nil,
returnInspectExec2: nil,
execProbeTimeout: false,
expectError: fmt.Errorf("error in StartExec()"),
}}
eh := &NativeExecHandler{}
@ -110,23 +143,27 @@ func TestExecInContainer(t *testing.T) {
var resize <-chan remotecommand.TerminalSize
for _, tc := range testcases {
t.Logf("TestCase: %q", tc.description)
tc := tc
t.Run(tc.description, func(t *testing.T) {
t.Parallel()
defer featuregatetesting.SetFeatureGateDuringTest(t, utilfeature.DefaultFeatureGate, features.ExecProbeTimeout, tc.execProbeTimeout)()
mockClient := mockclient.NewMockInterface(ctrl)
mockClient.EXPECT().CreateExec(gomock.Any(), gomock.Any()).Return(
tc.returnCreateExec1,
tc.returnCreateExec2)
mockClient.EXPECT().StartExec(gomock.Any(), gomock.Any(), gomock.Any()).Return(tc.returnStartExec)
mockClient.EXPECT().InspectExec(gomock.Any()).Return(
tc.returnInspectExec1,
tc.returnInspectExec2)
mockClient := mockclient.NewMockInterface(ctrl)
mockClient.EXPECT().CreateExec(gomock.Any(), gomock.Any()).Return(
tc.returnCreateExec1,
tc.returnCreateExec2)
mockClient.EXPECT().StartExec(gomock.Any(), gomock.Any(), gomock.Any()).Do(func(_ string, _ dockertypes.ExecStartCheck, _ libdocker.StreamOptions) { time.Sleep(tc.startExecDelay) }).Return(tc.returnStartExec)
mockClient.EXPECT().InspectExec(gomock.Any()).Return(
tc.returnInspectExec1,
tc.returnInspectExec2)
// use parent context of 2 minutes since that's the default remote
// runtime connection timeout used by dockershim
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
defer cancel()
err := eh.ExecInContainer(ctx, mockClient, container, cmd, stdin, stdout, stderr, false, resize, tc.timeout)
assert.Equal(t, tc.expectError, err)
// use parent context of 2 minutes since that's the default remote
// runtime connection timeout used by dockershim
ctx, cancel := context.WithTimeout(context.Background(), 2*time.Minute)
defer cancel()
err := eh.ExecInContainer(ctx, mockClient, container, cmd, stdin, stdout, stderr, false, resize, tc.timeout)
assert.Equal(t, tc.expectError, err)
})
}
}

View File

@ -246,6 +246,26 @@ var _ = SIGDescribe("Probing container", func() {
runReadinessFailTest(f, pod, time.Minute)
})
/*
Release: v1.21
Testname: Pod liveness probe, container exec timeout, restart
Description: A Pod is created with liveness probe with a Exec action on the Pod. If the liveness probe call does not return within the timeout specified, liveness probe MUST restart the Pod. When ExecProbeTimeout feature gate is disabled and cluster is using dockershim, the timeout is ignored BUT a failing liveness probe MUST restart the Pod.
*/
ginkgo.It("should be restarted with a failing exec liveness probe that took longer than the timeout", func() {
// The ExecProbeTimeout feature gate exists to allow backwards-compatibility with pre-1.20 cluster behaviors using dockershim, where livenessProbe timeouts were ignored
// If ExecProbeTimeout feature gate is disabled on a dockershim cluster, timeout enforcement for exec livenessProbes is disabled, but a failing liveness probe MUST still trigger a restart
// Note ExecProbeTimeout=false is not recommended for non-dockershim clusters (e.g., containerd), and this test will fail if run against such a configuration
cmd := []string{"/bin/sh", "-c", "sleep 600"}
livenessProbe := &v1.Probe{
Handler: execHandler([]string{"/bin/sh", "-c", "sleep 10 & exit 1"}),
InitialDelaySeconds: 15,
TimeoutSeconds: 1,
FailureThreshold: 1,
}
pod := busyBoxPodSpec(nil, livenessProbe, cmd)
RunLivenessTest(f, pod, 1, defaultObservationTimeout)
})
/*
Release: v1.14
Testname: Pod http liveness probe, redirected to a local address