diff --git a/integration/sandbox_clean_remove_test.go b/integration/sandbox_clean_remove_test.go new file mode 100644 index 000000000..16aa846a7 --- /dev/null +++ b/integration/sandbox_clean_remove_test.go @@ -0,0 +1,63 @@ +/* +Copyright 2017 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 integration + +import ( + "testing" + + "github.com/containerd/containerd" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "golang.org/x/net/context" + "k8s.io/kubernetes/pkg/kubelet/apis/cri/v1alpha1/runtime" +) + +func TestSandboxCleanRemove(t *testing.T) { + ctx := context.Background() + t.Logf("Create a sandbox") + sbConfig := PodSandboxConfig("sandbox", "clean-remove") + sb, err := runtimeService.RunPodSandbox(sbConfig) + require.NoError(t, err) + defer func() { + // Make sure the sandbox is cleaned up in any case. + runtimeService.StopPodSandbox(sb) + runtimeService.RemovePodSandbox(sb) + }() + + t.Logf("Should not be able to remove the sandbox when it's still running") + assert.Error(t, runtimeService.RemovePodSandbox(sb)) + + t.Logf("Delete sandbox task from containerd") + cntr, err := containerdClient.LoadContainer(ctx, sb) + require.NoError(t, err) + task, err := cntr.Task(ctx, nil) + require.NoError(t, err) + _, err = task.Delete(ctx, containerd.WithProcessKill) + require.NoError(t, err) + + t.Logf("Sandbox state should be NOTREADY") + status, err := runtimeService.PodSandboxStatus(sb) + require.NoError(t, err) + assert.Equal(t, runtime.PodSandboxState_SANDBOX_NOTREADY, status.GetState()) + + t.Logf("Should not be able to remove the sandbox when netns is not closed") + assert.Error(t, runtimeService.RemovePodSandbox(sb)) + + t.Logf("Should be able to remove the sandbox after properly stopped") + assert.NoError(t, runtimeService.StopPodSandbox(sb)) + assert.NoError(t, runtimeService.RemovePodSandbox(sb)) +} diff --git a/pkg/server/sandbox_remove.go b/pkg/server/sandbox_remove.go index fb21a723f..92e62f213 100644 --- a/pkg/server/sandbox_remove.go +++ b/pkg/server/sandbox_remove.go @@ -47,7 +47,6 @@ func (c *criContainerdService) RemovePodSandbox(ctx context.Context, r *runtime. id := sandbox.ID // Return error if sandbox container is not fully stopped. - // TODO(random-liu): [P0] Make sure network is torn down, may need to introduce a state. _, err = sandbox.Container.Task(ctx, nil) if err != nil && !errdefs.IsNotFound(err) { return nil, fmt.Errorf("failed to get sandbox container info for %q: %v", id, err) @@ -56,6 +55,11 @@ func (c *criContainerdService) RemovePodSandbox(ctx context.Context, r *runtime. return nil, fmt.Errorf("sandbox container %q is not fully stopped", id) } + // Return error if sandbox network namespace is not closed yet. + if sandbox.NetNS != nil && !sandbox.NetNS.Closed() { + return nil, fmt.Errorf("sandbox network namespace %q is not fully closed", sandbox.NetNS.GetPath()) + } + // Remove all containers inside the sandbox. // NOTE(random-liu): container could still be created after this point, Kubelet should // not rely on this behavior. diff --git a/pkg/store/sandbox/netns.go b/pkg/store/sandbox/netns.go index ec1aae08b..ee0d645ef 100644 --- a/pkg/store/sandbox/netns.go +++ b/pkg/store/sandbox/netns.go @@ -107,6 +107,7 @@ func (n *NetNS) Remove() error { if err := os.RemoveAll(path); err != nil { return fmt.Errorf("failed to remove netns: %v", err) } + n.restored = false } return nil } @@ -115,7 +116,7 @@ func (n *NetNS) Remove() error { func (n *NetNS) Closed() bool { n.Lock() defer n.Unlock() - return n.closed + return n.closed && !n.restored } // GetPath returns network namespace path for sandbox container