Add integration test.

Signed-off-by: Lantao Liu <lantaol@google.com>

integration/restart_test.go

@@ -19,7 +19,6 @@ package integration
 import (
 	"sort"
 	"testing"
-	"time"
 
 	"github.com/containerd/containerd"
 	"github.com/stretchr/testify/assert"
@@ -140,27 +139,8 @@ func TestContainerdRestart(t *testing.T) {
 	imagesBeforeRestart, err := imageService.ListImages(nil)
 	assert.NoError(t, err)
 
-	t.Logf("Kill containerd")
+	t.Logf("Restart containerd")
-	require.NoError(t, KillProcess("containerd"))
+	RestartContainerd(t)
-	defer func() {
-		assert.NoError(t, Eventually(func() (bool, error) {
-			return ConnectDaemons() == nil, nil
-		}, time.Second, 30*time.Second), "make sure containerd is running before test finish")
-	}()
-
-	t.Logf("Wait until containerd is killed")
-	require.NoError(t, Eventually(func() (bool, error) {
-		pid, err := PidOf("containerd")
-		if err != nil {
-			return false, err
-		}
-		return pid == 0, nil
-	}, time.Second, 30*time.Second), "wait for containerd to be killed")
-
-	t.Logf("Wait until containerd is restarted")
-	require.NoError(t, Eventually(func() (bool, error) {
-		return ConnectDaemons() == nil, nil
-	}, time.Second, 30*time.Second), "wait for containerd to be restarted")
 
 	t.Logf("Check sandbox and container state after restart")
 	loadedSandboxes, err := runtimeService.ListPodSandbox(&runtime.PodSandboxFilter{})

integration/sandbox_clean_remove_test.go

@@ -17,14 +17,23 @@ limitations under the License.
 package integration
 
 import (
+	"encoding/json"
+	"io/ioutil"
+	"os"
+	"path/filepath"
+	"strings"
 	"testing"
 	"time"
 
 	"github.com/containerd/containerd"
+	runtimespec "github.com/opencontainers/runtime-spec/specs-go"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/net/context"
+	"golang.org/x/sys/unix"
 	runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2"
+
+	"github.com/containerd/cri/pkg/server"
 )
 
 func TestSandboxCleanRemove(t *testing.T) {
@@ -66,3 +75,100 @@ func TestSandboxCleanRemove(t *testing.T) {
 	assert.NoError(t, runtimeService.StopPodSandbox(sb))
 	assert.NoError(t, runtimeService.RemovePodSandbox(sb))
 }
+
+func TestSandboxRemoveWithoutIPLeakage(t *testing.T) {
+	ctx := context.Background()
+	const hostLocalCheckpointDir = "/var/lib/cni"
+
+	t.Logf("Make sure host-local ipam is in use")
+	config, err := CRIConfig()
+	require.NoError(t, err)
+	fs, err := ioutil.ReadDir(config.NetworkPluginConfDir)
+	require.NoError(t, err)
+	require.NotEmpty(t, fs)
+	f := filepath.Join(config.NetworkPluginConfDir, fs[0].Name())
+	cniConfig, err := ioutil.ReadFile(f)
+	require.NoError(t, err)
+	if !strings.Contains(string(cniConfig), "host-local") {
+		t.Skip("host-local ipam is not in use")
+	}
+
+	t.Logf("Create a sandbox")
+	sbConfig := PodSandboxConfig("sandbox", "remove-without-ip-leakage")
+	sb, err := runtimeService.RunPodSandbox(sbConfig, *runtimeHandler)
+	require.NoError(t, err)
+	defer func() {
+		// Make sure the sandbox is cleaned up in any case.
+		runtimeService.StopPodSandbox(sb)
+		runtimeService.RemovePodSandbox(sb)
+	}()
+
+	t.Logf("Get pod information")
+	client, err := RawRuntimeClient()
+	require.NoError(t, err)
+	resp, err := client.PodSandboxStatus(ctx, &runtime.PodSandboxStatusRequest{
+		PodSandboxId: sb,
+		Verbose:      true,
+	})
+	require.NoError(t, err)
+	status := resp.GetStatus()
+	info := resp.GetInfo()
+	ip := status.GetNetwork().GetIp()
+	require.NotEmpty(t, ip)
+	var sbInfo server.SandboxInfo
+	require.NoError(t, json.Unmarshal([]byte(info["info"]), &sbInfo))
+	require.NotNil(t, sbInfo.RuntimeSpec.Linux)
+	var netNS string
+	for _, n := range sbInfo.RuntimeSpec.Linux.Namespaces {
+		if n.Type == runtimespec.NetworkNamespace {
+			netNS = n.Path
+		}
+	}
+	require.NotEmpty(t, netNS, "network namespace should be set")
+
+	t.Logf("Should be able to find the pod ip in host-local checkpoint")
+	checkIP := func(ip string) bool {
+		found := false
+		filepath.Walk(hostLocalCheckpointDir, func(_ string, info os.FileInfo, _ error) error {
+			if info != nil && info.Name() == ip {
+				found = true
+			}
+			return nil
+		})
+		return found
+	}
+	require.True(t, checkIP(ip))
+
+	t.Logf("Kill sandbox container")
+	require.NoError(t, KillPid(int(sbInfo.Pid)))
+
+	t.Logf("Unmount network namespace")
+	// The umount will take effect after containerd is stopped.
+	require.NoError(t, unix.Unmount(netNS, unix.MNT_DETACH))
+
+	t.Logf("Restart containerd")
+	RestartContainerd(t)
+
+	t.Logf("Sandbox state should be NOTREADY")
+	assert.NoError(t, Eventually(func() (bool, error) {
+		status, err := runtimeService.PodSandboxStatus(sb)
+		if err != nil {
+			return false, err
+		}
+		return status.GetState() == runtime.PodSandboxState_SANDBOX_NOTREADY, nil
+	}, time.Second, 30*time.Second), "sandbox state should become NOTREADY")
+
+	t.Logf("Network namespace should have been removed")
+	_, err = os.Stat(netNS)
+	assert.True(t, os.IsNotExist(err))
+
+	t.Logf("Should still be able to find the pod ip in host-local checkpoint")
+	assert.True(t, checkIP(ip))
+
+	t.Logf("Should be able to remove the sandbox after properly stopped")
+	assert.NoError(t, runtimeService.StopPodSandbox(sb))
+	assert.NoError(t, runtimeService.RemovePodSandbox(sb))
+
+	t.Logf("Should not be able to find the pod ip in host-local checkpoint")
+	assert.False(t, checkIP(ip))
+}

integration/test_utils.go

@@ -24,11 +24,14 @@ import (
 	"os/exec"
 	"strconv"
 	"strings"
+	"testing"
 	"time"
 
 	"github.com/containerd/containerd"
 	"github.com/pkg/errors"
 	"github.com/sirupsen/logrus"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
 	"google.golang.org/grpc"
 	"k8s.io/kubernetes/pkg/kubelet/apis/cri"
 	runtime "k8s.io/kubernetes/pkg/kubelet/apis/cri/runtime/v1alpha2"
@@ -295,6 +298,15 @@ func KillProcess(name string) error {
 	return nil
 }
 
+// KillPid kills the process by pid. kill is used.
+func KillPid(pid int) error {
+	output, err := exec.Command("kill", strconv.Itoa(pid)).CombinedOutput()
+	if err != nil {
+		return errors.Errorf("failed to kill %d - error: %v, output: %q", pid, err, output)
+	}
+	return nil
+}
+
 // PidOf returns pid of a process by name.
 func PidOf(name string) (int, error) {
 	b, err := exec.Command("pidof", name).CombinedOutput()
@@ -308,8 +320,8 @@ func PidOf(name string) (int, error) {
 	return strconv.Atoi(output)
 }
 
-// CRIConfig gets current cri config from containerd.
+// RawRuntimeClient returns a raw grpc runtime service client.
-func CRIConfig() (*criconfig.Config, error) {
+func RawRuntimeClient() (runtime.RuntimeServiceClient, error) {
 	addr, dialer, err := kubeletutil.GetAddressAndDialer(*criEndpoint)
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to get dialer")
@@ -320,8 +332,16 @@ func CRIConfig() (*criconfig.Config, error) {
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to connect cri endpoint")
 	}
-	client := runtime.NewRuntimeServiceClient(conn)
+	return runtime.NewRuntimeServiceClient(conn), nil
-	resp, err := client.Status(ctx, &runtime.StatusRequest{Verbose: true})
+}
+
+// CRIConfig gets current cri config from containerd.
+func CRIConfig() (*criconfig.Config, error) {
+	client, err := RawRuntimeClient()
+	if err != nil {
+		return nil, errors.Wrap(err, "failed to get raw runtime client")
+	}
+	resp, err := client.Status(context.Background(), &runtime.StatusRequest{Verbose: true})
 	if err != nil {
 		return nil, errors.Wrap(err, "failed to get status")
 	}
@@ -331,3 +351,21 @@ func CRIConfig() (*criconfig.Config, error) {
 	}
 	return config, nil
 }
+
+func RestartContainerd(t *testing.T) {
+	require.NoError(t, KillProcess("containerd"))
+
+	// Use assert so that the 3rd wait always runs, this makes sure
+	// containerd is running before this function returns.
+	assert.NoError(t, Eventually(func() (bool, error) {
+		pid, err := PidOf("containerd")
+		if err != nil {
+			return false, err
+		}
+		return pid == 0, nil
+	}, time.Second, 30*time.Second), "wait for containerd to be killed")
+
+	require.NoError(t, Eventually(func() (bool, error) {
+		return ConnectDaemons() == nil, nil
+	}, time.Second, 30*time.Second), "wait for containerd to be restarted")
+}

pkg/server/container_status.go

@@ -99,7 +99,8 @@ func toCRIContainerStatus(container containerstore.Container, spec *runtime.Imag
 	}
 }
 
-type containerInfo struct {
+// ContainerInfo is extra information for a container.
+type ContainerInfo struct {
 	// TODO(random-liu): Add sandboxID in CRI container status.
 	SandboxID      string                   `json:"sandboxID"`
 	Pid            uint32                   `json:"pid"`
@@ -122,7 +123,7 @@ func toCRIContainerInfo(ctx context.Context, container containerstore.Container,
 	status := container.Status.Get()
 
 	// TODO(random-liu): Change CRI status info to use array instead of map.
-	ci := &containerInfo{
+	ci := &ContainerInfo{
 		SandboxID: container.SandboxID,
 		Pid:       status.Pid,
 		Removing:  status.Removing,

pkg/server/sandbox_status.go

@@ -99,8 +99,9 @@ func toCRISandboxStatus(meta sandboxstore.Metadata, status sandboxstore.Status,
 	}
 }
 
+// SandboxInfo is extra information for sandbox.
 // TODO (mikebrow): discuss predefining constants structures for some or all of these field names in CRI
-type sandboxInfo struct {
+type SandboxInfo struct {
 	Pid            uint32                    `json:"pid"`
 	Status         string                    `json:"processStatus"`
 	NetNSClosed    bool                      `json:"netNamespaceClosed"`
@@ -132,7 +133,7 @@ func toCRISandboxInfo(ctx context.Context, sandbox sandboxstore.Sandbox) (map[st
 		processStatus = taskStatus.Status
 	}
 
-	si := &sandboxInfo{
+	si := &SandboxInfo{
 		Pid:            sandbox.Status.Get().Pid,
 		RuntimeHandler: sandbox.RuntimeHandler,
 		Status:         string(processStatus),