Skip to content

Commit 8fb244a

Browse files
fuweidfuweid
authored
Merge pull request #1525 from containerd/forcibly-remove-sandbox-container
Forcibly remove sandbox container
2 parents 5f50692 + 0c45d13 commit 8fb244a

5 files changed

Lines changed: 36 additions & 57 deletions

File tree

‎hack/utils.sh‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ ROOT="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"/..
1818

1919
# Not from vendor.conf.
2020
KUBERNETES_VERSION="v1.19.0-beta.2"
21-
CRITOOL_VERSION=${CRITOOL_VERSION:-75ef33dc2b4ecb08e0237d91de1b664909d262de}
21+
CRITOOL_VERSION=${CRITOOL_VERSION:-baca4a152dfe671fc17911a7af74bcb61680ee39}
2222
CRITOOL_PKG=github.com/kubernetes-sigs/cri-tools
2323
CRITOOL_REPO=github.com/kubernetes-sigs/cri-tools
2424

‎integration/sandbox_clean_remove_test.go‎

Lines changed: 0 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -21,61 +21,16 @@ import (
2121
"os"
2222
"path/filepath"
2323
"strings"
24-
"syscall"
2524
"testing"
2625
"time"
2726

28-
"github.com/containerd/containerd"
2927
runtimespec "github.com/opencontainers/runtime-spec/specs-go"
3028
"github.com/stretchr/testify/assert"
3129
"github.com/stretchr/testify/require"
32-
"golang.org/x/net/context"
3330
"golang.org/x/sys/unix"
3431
runtime "k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
3532
)
3633

37-
func TestSandboxCleanRemove(t *testing.T) {
38-
ctx := context.Background()
39-
t.Logf("Create a sandbox")
40-
sbConfig := PodSandboxConfig("sandbox", "clean-remove")
41-
sb, err := runtimeService.RunPodSandbox(sbConfig, *runtimeHandler)
42-
require.NoError(t, err)
43-
defer func() {
44-
// Make sure the sandbox is cleaned up in any case.
45-
runtimeService.StopPodSandbox(sb)
46-
runtimeService.RemovePodSandbox(sb)
47-
}()
48-
49-
t.Logf("Should not be able to remove the sandbox when it's still running")
50-
assert.Error(t, runtimeService.RemovePodSandbox(sb))
51-
52-
t.Logf("Delete sandbox task from containerd")
53-
cntr, err := containerdClient.LoadContainer(ctx, sb)
54-
require.NoError(t, err)
55-
task, err := cntr.Task(ctx, nil)
56-
require.NoError(t, err)
57-
// Kill the task with signal SIGKILL, once the task exited,
58-
// the TaskExit event will trigger the task.Delete().
59-
err = task.Kill(ctx, syscall.SIGKILL, containerd.WithKillAll)
60-
require.NoError(t, err)
61-
62-
t.Logf("Sandbox state should be NOTREADY")
63-
assert.NoError(t, Eventually(func() (bool, error) {
64-
status, err := runtimeService.PodSandboxStatus(sb)
65-
if err != nil {
66-
return false, err
67-
}
68-
return status.GetState() == runtime.PodSandboxState_SANDBOX_NOTREADY, nil
69-
}, time.Second, 30*time.Second), "sandbox state should become NOTREADY")
70-
71-
t.Logf("Should not be able to remove the sandbox when netns is not closed")
72-
assert.Error(t, runtimeService.RemovePodSandbox(sb))
73-
74-
t.Logf("Should be able to remove the sandbox after properly stopped")
75-
assert.NoError(t, runtimeService.StopPodSandbox(sb))
76-
assert.NoError(t, runtimeService.RemovePodSandbox(sb))
77-
}
78-
7934
func TestSandboxRemoveWithoutIPLeakage(t *testing.T) {
8035
const hostLocalCheckpointDir = "/var/lib/cni"
8136

‎pkg/server/container_remove.go‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/containerd/containerd/errdefs"
2222
"github.com/containerd/containerd/log"
2323
"github.com/pkg/errors"
24+
"github.com/sirupsen/logrus"
2425
"golang.org/x/net/context"
2526
runtime "k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
2627

@@ -29,7 +30,6 @@ import (
2930
)
3031

3132
// RemoveContainer removes the container.
32-
// TODO(random-liu): Forcibly stop container if it's running.
3333
func (c *criService) RemoveContainer(ctx context.Context, r *runtime.RemoveContainerRequest) (_ *runtime.RemoveContainerResponse, retErr error) {
3434
container, err := c.containerStore.Get(r.GetContainerId())
3535
if err != nil {
@@ -42,6 +42,17 @@ func (c *criService) RemoveContainer(ctx context.Context, r *runtime.RemoveConta
4242
}
4343
id := container.ID
4444

45+
// Forcibly stop the containers if they are in running or unknown state
46+
state := container.Status.Get().State()
47+
if state == runtime.ContainerState_CONTAINER_RUNNING ||
48+
state == runtime.ContainerState_CONTAINER_UNKNOWN {
49+
logrus.Infof("Forcibly stopping container %q", id)
50+
if err := c.stopContainer(ctx, container, 0); err != nil {
51+
return nil, errors.Wrapf(err, "failed to forcibly stop container %q", id)
52+
}
53+
54+
}
55+
4556
// Set removing state to prevent other start/remove operations against this container
4657
// while it's being removed.
4758
if err := setContainerRemoving(container); err != nil {

‎pkg/server/sandbox_remove.go‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/containerd/containerd/errdefs"
2222
"github.com/containerd/containerd/log"
2323
"github.com/pkg/errors"
24+
"github.com/sirupsen/logrus"
2425
"golang.org/x/net/context"
2526
runtime "k8s.io/cri-api/pkg/apis/runtime/v1alpha2"
2627

@@ -48,7 +49,10 @@ func (c *criService) RemovePodSandbox(ctx context.Context, r *runtime.RemovePodS
4849
// Return error if sandbox container is still running or unknown.
4950
state := sandbox.Status.Get().State
5051
if state == sandboxstore.StateReady || state == sandboxstore.StateUnknown {
51-
return nil, errors.Errorf("sandbox container %q is not fully stopped", id)
52+
logrus.Infof("Forcibly stopping sandbox %q", id)
53+
if err := c.stopPodSandbox(ctx, sandbox); err != nil {
54+
return nil, errors.Wrapf(err, "failed to forcibly stop sandbox %q", id)
55+
}
5256
}
5357

5458
// Return error if sandbox network namespace is not closed yet.

‎pkg/server/sandbox_stop.go‎

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,15 @@ func (c *criService) StopPodSandbox(ctx context.Context, r *runtime.StopPodSandb
3939
return nil, errors.Wrapf(err, "an error occurred when try to find sandbox %q",
4040
r.GetPodSandboxId())
4141
}
42+
43+
if err := c.stopPodSandbox(ctx, sandbox); err != nil {
44+
return nil, err
45+
}
46+
47+
return &runtime.StopPodSandboxResponse{}, nil
48+
}
49+
50+
func (c *criService) stopPodSandbox(ctx context.Context, sandbox sandboxstore.Sandbox) error {
4251
// Use the full sandbox id.
4352
id := sandbox.ID
4453

@@ -52,20 +61,20 @@ func (c *criService) StopPodSandbox(ctx context.Context, r *runtime.StopPodSandb
5261
}
5362
// Forcibly stop the container. Do not use `StopContainer`, because it introduces a race
5463
// if a container is removed after list.
55-
if err = c.stopContainer(ctx, container, 0); err != nil {
56-
return nil, errors.Wrapf(err, "failed to stop container %q", container.ID)
64+
if err := c.stopContainer(ctx, container, 0); err != nil {
65+
return errors.Wrapf(err, "failed to stop container %q", container.ID)
5766
}
5867
}
5968

6069
if err := c.cleanupSandboxFiles(id, sandbox.Config); err != nil {
61-
return nil, errors.Wrap(err, "failed to cleanup sandbox files")
70+
return errors.Wrap(err, "failed to cleanup sandbox files")
6271
}
6372

6473
// Only stop sandbox container when it's running or unknown.
6574
state := sandbox.Status.Get().State
6675
if state == sandboxstore.StateReady || state == sandboxstore.StateUnknown {
6776
if err := c.stopSandboxContainer(ctx, sandbox); err != nil {
68-
return nil, errors.Wrapf(err, "failed to stop sandbox container %q in %q state", id, state)
77+
return errors.Wrapf(err, "failed to stop sandbox container %q in %q state", id, state)
6978
}
7079
}
7180

@@ -74,21 +83,21 @@ func (c *criService) StopPodSandbox(ctx context.Context, r *runtime.StopPodSandb
7483
// Use empty netns path if netns is not available. This is defined in:
7584
// https://github.com/containernetworking/cni/blob/v0.7.0-alpha1/SPEC.md
7685
if closed, err := sandbox.NetNS.Closed(); err != nil {
77-
return nil, errors.Wrap(err, "failed to check network namespace closed")
86+
return errors.Wrap(err, "failed to check network namespace closed")
7887
} else if closed {
7988
sandbox.NetNSPath = ""
8089
}
8190
if err := c.teardownPodNetwork(ctx, sandbox); err != nil {
82-
return nil, errors.Wrapf(err, "failed to destroy network for sandbox %q", id)
91+
return errors.Wrapf(err, "failed to destroy network for sandbox %q", id)
8392
}
84-
if err = sandbox.NetNS.Remove(); err != nil {
85-
return nil, errors.Wrapf(err, "failed to remove network namespace for sandbox %q", id)
93+
if err := sandbox.NetNS.Remove(); err != nil {
94+
return errors.Wrapf(err, "failed to remove network namespace for sandbox %q", id)
8695
}
8796
}
8897

8998
log.G(ctx).Infof("TearDown network for sandbox %q successfully", id)
9099

91-
return &runtime.StopPodSandboxResponse{}, nil
100+
return nil
92101
}
93102

94103
// stopSandboxContainer kills the sandbox container.

0 commit comments

Comments
 (0)