Skip to content
This repository was archived by the owner on Mar 9, 2022. It is now read-only.

Commit b2568d2

Browse files
Random-LiuRandom-Liu
committed
Do not SIGKILL container if container stop is cancelled.
Signed-off-by: Lantao Liu <[email protected]>
1 parent 3c81b30 commit b2568d2

4 files changed

Lines changed: 116 additions & 4 deletions

File tree

‎integration/container_stop_test.go‎

Lines changed: 91 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
/*
2+
Copyright 2019 The containerd Authors.
3+
4+
Licensed under the Apache License, Version 2.0 (the "License");
5+
you may not use this file except in compliance with the License.
6+
You may obtain a copy of the License at
7+
8+
http://www.apache.org/licenses/LICENSE-2.0
9+
10+
Unless required by applicable law or agreed to in writing, software
11+
distributed under the License is distributed on an "AS IS" BASIS,
12+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
See the License for the specific language governing permissions and
14+
limitations under the License.
15+
*/
16+
17+
package integration
18+
19+
import (
20+
"context"
21+
"testing"
22+
"time"
23+
24+
"github.com/stretchr/testify/assert"
25+
"github.com/stretchr/testify/require"
26+
runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2"
27+
)
28+
29+
func TestContainerStopCancellation(t *testing.T) {
30+
t.Log("Create a pod sandbox")
31+
sbConfig := PodSandboxConfig("sandbox", "cancel-container-stop")
32+
sb, err := runtimeService.RunPodSandbox(sbConfig)
33+
require.NoError(t, err)
34+
defer func() {
35+
assert.NoError(t, runtimeService.StopPodSandbox(sb))
36+
assert.NoError(t, runtimeService.RemovePodSandbox(sb))
37+
}()
38+
39+
const (
40+
testImage = "busybox"
41+
containerName = "test-container"
42+
)
43+
t.Logf("Pull test image %q", testImage)
44+
img, err := imageService.PullImage(&runtime.ImageSpec{Image: testImage}, nil)
45+
require.NoError(t, err)
46+
defer func() {
47+
assert.NoError(t, imageService.RemoveImage(&runtime.ImageSpec{Image: img}))
48+
}()
49+
50+
t.Log("Create a container which traps sigterm")
51+
cnConfig := ContainerConfig(
52+
containerName,
53+
testImage,
54+
WithCommand("sh", "-c", `trap "echo ignore sigterm" TERM; sleep 1000`),
55+
)
56+
cn, err := runtimeService.CreateContainer(sb, cnConfig, sbConfig)
57+
require.NoError(t, err)
58+
59+
t.Log("Start the container")
60+
require.NoError(t, runtimeService.StartContainer(cn))
61+
62+
t.Log("Stop the container with 3s timeout, but 1s context timeout")
63+
// Note that with container pid namespace, the sleep process
64+
// is pid 1, and SIGTERM sent by `StopContainer` will be ignored.
65+
rawClient, err := RawRuntimeClient()
66+
require.NoError(t, err)
67+
ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
68+
defer cancel()
69+
_, err = rawClient.StopContainer(ctx, &runtime.StopContainerRequest{
70+
ContainerId: cn,
71+
Timeout: 3,
72+
})
73+
assert.Error(t, err)
74+
75+
t.Log("The container should still be running even after 5 seconds")
76+
assert.NoError(t, Consistently(func() (bool, error) {
77+
s, err := runtimeService.ContainerStatus(cn)
78+
if err != nil {
79+
return false, err
80+
}
81+
return s.GetState() == runtime.ContainerState_CONTAINER_RUNNING, nil
82+
}, 100*time.Millisecond, 5*time.Second))
83+
84+
t.Log("Stop the container with 1s timeout, without shorter context timeout")
85+
assert.NoError(t, runtimeService.StopContainer(cn, 1))
86+
87+
t.Log("The container state should be exited")
88+
s, err := runtimeService.ContainerStatus(cn)
89+
require.NoError(t, err)
90+
assert.Equal(t, s.GetState(), runtime.ContainerState_CONTAINER_EXITED)
91+
}

‎integration/test_utils.go‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -261,6 +261,26 @@ func Eventually(f CheckFunc, period, timeout time.Duration) error {
261261
}
262262
}
263263

264+
// Consistently makes sure that f consistently returns true without
265+
// error before timeout exceeds. If f returns error, Consistently
266+
// will return the same error immediately.
267+
func Consistently(f CheckFunc, period, timeout time.Duration) error {
268+
start := time.Now()
269+
for {
270+
ok, err := f()
271+
if !ok {
272+
return errors.New("get false")
273+
}
274+
if err != nil {
275+
return err
276+
}
277+
if time.Since(start) >= timeout {
278+
return nil
279+
}
280+
time.Sleep(period)
281+
}
282+
}
283+
264284
// Randomize adds uuid after a string.
265285
func Randomize(str string) string {
266286
return str + "-" + util.GenerateID()

‎pkg/server/container_stop.go‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -111,8 +111,9 @@ func (c *criService) stopContainer(ctx context.Context, container containerstore
111111
return errors.Wrapf(err, "failed to stop container %q", id)
112112
}
113113

114-
if err = c.waitContainerStop(ctx, container, timeout); err == nil {
115-
return nil
114+
if err = c.waitContainerStop(ctx, container, timeout); err == nil || errors.Cause(err) == ctx.Err() {
115+
// Do not SIGKILL container if the context is cancelled.
116+
return err
116117
}
117118
logrus.WithError(err).Errorf("An error occurs during waiting for container %q to be stopped", id)
118119
}
@@ -156,7 +157,7 @@ func (c *criService) waitContainerStop(ctx context.Context, container containers
156157
defer timeoutTimer.Stop()
157158
select {
158159
case <-ctx.Done():
159-
return errors.Errorf("wait container %q is cancelled", container.ID)
160+
return errors.Wrapf(ctx.Err(), "wait container %q is cancelled", container.ID)
160161
case <-timeoutTimer.C:
161162
return errors.Errorf("wait container %q stop timeout", container.ID)
162163
case <-container.Stopped():

‎pkg/server/sandbox_stop.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,7 @@ func (c *criService) waitSandboxStop(ctx context.Context, sandbox sandboxstore.S
129129
defer timeoutTimer.Stop()
130130
select {
131131
case <-ctx.Done():
132-
return errors.Errorf("wait sandbox container %q is cancelled", sandbox.ID)
132+
return errors.Wrapf(ctx.Err(), "wait sandbox container %q is cancelled", sandbox.ID)
133133
case <-timeoutTimer.C:
134134
return errors.Errorf("wait sandbox container %q stop timeout", sandbox.ID)
135135
case <-sandbox.Stopped():

0 commit comments

Comments
 (0)