Skip to content

Commit b41d6f4

Browse files
qiutongsqiutongs
committed
Update container with sandbox metadata after NetNS is created
Signed-off-by: Qiutong Song <[email protected]>
1 parent 31f9d13 commit b41d6f4

6 files changed

Lines changed: 232 additions & 28 deletions

File tree

‎integration/main_test.go‎

Lines changed: 86 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,20 +17,24 @@
1717
package integration
1818

1919
import (
20+
"bytes"
2021
"context"
2122
"encoding/json"
2223
"errors"
2324
"flag"
2425
"fmt"
26+
"io"
2527
"os"
2628
"path/filepath"
2729
goruntime "runtime"
2830
"strconv"
2931
"strings"
32+
"syscall"
3033
"testing"
3134
"time"
3235

3336
"github.com/containerd/containerd"
37+
"github.com/containerd/containerd/containers"
3438
cri "github.com/containerd/containerd/integration/cri-api/pkg/apis"
3539
_ "github.com/containerd/containerd/integration/images" // Keep this around to parse `imageListFile` command line var
3640
"github.com/containerd/containerd/integration/remote"
@@ -403,12 +407,12 @@ func Randomize(str string) string {
403407
}
404408

405409
// KillProcess kills the process by name. pkill is used.
406-
func KillProcess(name string) error {
410+
func KillProcess(name string, signal syscall.Signal) error {
407411
var command []string
408412
if goruntime.GOOS == "windows" {
409413
command = []string{"taskkill", "/IM", name, "/F"}
410414
} else {
411-
command = []string{"pkill", "-x", fmt.Sprintf("^%s$", name)}
415+
command = []string{"pkill", "-" + strconv.Itoa(int(signal)), "-x", fmt.Sprintf("^%s$", name)}
412416
}
413417

414418
output, err := exec.Command(command[0], command[1:]...).CombinedOutput()
@@ -444,6 +448,80 @@ func PidOf(name string) (int, error) {
444448
return strconv.Atoi(output)
445449
}
446450

451+
// PidsOf returns pid(s) of a process by name
452+
func PidsOf(name string) ([]int, error) {
453+
if len(name) == 0 {
454+
return []int{}, fmt.Errorf("name is required")
455+
}
456+
457+
procDirFD, err := os.Open("/proc")
458+
if err != nil {
459+
return nil, fmt.Errorf("failed to open /proc: %w", err)
460+
}
461+
defer procDirFD.Close()
462+
463+
res := []int{}
464+
for {
465+
fileInfos, err := procDirFD.Readdir(100)
466+
if err != nil {
467+
if err == io.EOF {
468+
break
469+
}
470+
return nil, fmt.Errorf("failed to readdir: %w", err)
471+
}
472+
473+
for _, fileInfo := range fileInfos {
474+
if !fileInfo.IsDir() {
475+
continue
476+
}
477+
478+
pid, err := strconv.Atoi(fileInfo.Name())
479+
if err != nil {
480+
continue
481+
}
482+
483+
exePath, err := os.Readlink(filepath.Join("/proc", fileInfo.Name(), "exe"))
484+
if err != nil {
485+
continue
486+
}
487+
488+
if strings.HasSuffix(exePath, name) {
489+
res = append(res, pid)
490+
}
491+
}
492+
}
493+
return res, nil
494+
}
495+
496+
// PidEnvs returns the environ of pid in key-value pairs.
497+
func PidEnvs(pid int) (map[string]string, error) {
498+
envPath := filepath.Join("/proc", strconv.Itoa(pid), "environ")
499+
500+
b, err := os.ReadFile(envPath)
501+
if err != nil {
502+
return nil, fmt.Errorf("failed to read %s: %w", envPath, err)
503+
}
504+
505+
values := bytes.Split(b, []byte{0})
506+
if len(values) == 0 {
507+
return nil, nil
508+
}
509+
510+
res := make(map[string]string)
511+
for _, value := range values {
512+
value := strings.TrimSpace(string(value))
513+
if len(value) == 0 {
514+
continue
515+
}
516+
517+
parts := strings.SplitN(value, "=", 2)
518+
if len(parts) == 2 {
519+
res[parts[0]] = parts[1]
520+
}
521+
}
522+
return res, nil
523+
}
524+
447525
// RawRuntimeClient returns a raw grpc runtime service client.
448526
func RawRuntimeClient() (runtime.RuntimeServiceClient, error) {
449527
addr, dialer, err := dialer.GetAddressAndDialer(*criEndpoint)
@@ -500,8 +578,8 @@ func SandboxInfo(id string) (*runtime.PodSandboxStatus, *server.SandboxInfo, err
500578
return status, &info, nil
501579
}
502580

503-
func RestartContainerd(t *testing.T) {
504-
require.NoError(t, KillProcess(*containerdBin))
581+
func RestartContainerd(t *testing.T, signal syscall.Signal) {
582+
require.NoError(t, KillProcess(*containerdBin, signal))
505583

506584
// Use assert so that the 3rd wait always runs, this makes sure
507585
// containerd is running before this function returns.
@@ -534,3 +612,7 @@ func EnsureImageExists(t *testing.T, imageName string) string {
534612

535613
return imgID
536614
}
615+
616+
func GetContainer(id string) (containers.Container, error) {
617+
return containerdClient.ContainerService().Get(context.Background(), id)
618+
}

‎integration/restart_test.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -165,7 +165,7 @@ func TestContainerdRestart(t *testing.T) {
165165
assert.NoError(t, err)
166166

167167
t.Logf("Restart containerd")
168-
RestartContainerd(t)
168+
RestartContainerd(t, syscall.SIGTERM)
169169

170170
t.Logf("Check sandbox and container state after restart")
171171
loadedSandboxes, err := runtimeService.ListPodSandbox(&runtime.PodSandboxFilter{})

‎integration/sandbox_run_rollback_test.go‎

Lines changed: 136 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,16 +26,24 @@ import (
2626
"path/filepath"
2727
"runtime"
2828
"strings"
29+
"sync"
30+
"syscall"
2931
"testing"
32+
"time"
3033

34+
runtimespec "github.com/opencontainers/runtime-spec/specs-go"
35+
"github.com/stretchr/testify/assert"
3136
"github.com/stretchr/testify/require"
3237
criapiv1 "k8s.io/cri-api/pkg/apis/runtime/v1"
3338

39+
"github.com/containerd/containerd/pkg/cri/store/sandbox"
3440
"github.com/containerd/containerd/pkg/failpoint"
41+
"github.com/containerd/typeurl"
3542
)
3643

3744
const (
3845
failpointRuntimeHandler = "runc-fp"
46+
failpointCNIBinary = "cni-bridge-fp"
3947

4048
failpointShimPrefixKey = "io.containerd.runtime.v2.shim.failpoint."
4149

@@ -138,7 +146,7 @@ func TestRunPodSandboxWithShimDeleteFailure(t *testing.T) {
138146

139147
if restart {
140148
t.Log("Restart containerd")
141-
RestartContainerd(t)
149+
RestartContainerd(t, syscall.SIGTERM)
142150

143151
t.Log("ListPodSandbox with the specific label")
144152
l, err = runtimeService.ListPodSandbox(&criapiv1.PodSandboxFilter{Id: sb.Id})
@@ -211,7 +219,7 @@ func TestRunPodSandboxWithShimStartAndTeardownCNIFailure(t *testing.T) {
211219

212220
if restart {
213221
t.Log("Restart containerd")
214-
RestartContainerd(t)
222+
RestartContainerd(t, syscall.SIGTERM)
215223

216224
t.Log("ListPodSandbox with the specific label")
217225
l, err = runtimeService.ListPodSandbox(&criapiv1.PodSandboxFilter{Id: sb.Id})
@@ -229,6 +237,132 @@ func TestRunPodSandboxWithShimStartAndTeardownCNIFailure(t *testing.T) {
229237
t.Run("JustCleanup", testCase(false))
230238
}
231239

240+
// TestRunPodSandboxWithShimStartAndTeardownCNISlow should keep the sandbox
241+
// record if failed to rollback CNI API.
242+
func TestRunPodSandboxAndTeardownCNISlow(t *testing.T) {
243+
if runtime.GOOS != "linux" {
244+
t.Skip()
245+
}
246+
if os.Getenv("ENABLE_CRI_SANDBOXES") != "" {
247+
t.Skip()
248+
}
249+
250+
t.Log("Init PodSandboxConfig with specific key")
251+
sbName := t.Name()
252+
labels := map[string]string{
253+
sbName: "true",
254+
}
255+
sbConfig := PodSandboxConfig(sbName, "failpoint", WithPodLabels(labels))
256+
257+
t.Log("Inject CNI failpoint")
258+
conf := &failpointConf{
259+
// Delay 1 day
260+
Add: "1*delay(86400000)",
261+
}
262+
injectCNIFailpoint(t, sbConfig, conf)
263+
264+
var wg sync.WaitGroup
265+
wg.Add(1)
266+
267+
go func() {
268+
defer wg.Done()
269+
t.Log("Create a sandbox")
270+
_, err := runtimeService.RunPodSandbox(sbConfig, failpointRuntimeHandler)
271+
require.Error(t, err)
272+
require.ErrorContains(t, err, "error reading from server: EOF")
273+
}()
274+
275+
assert.NoError(t, ensureCNIAddRunning(t, sbName), "check that failpoint CNI.Add is running")
276+
277+
// Use SIGKILL to prevent containerd server gracefulshutdown which may cause indeterministic invocation of defer functions
278+
t.Log("Restart containerd")
279+
RestartContainerd(t, syscall.SIGKILL)
280+
281+
wg.Wait()
282+
283+
t.Log("ListPodSandbox with the specific label")
284+
l, err := runtimeService.ListPodSandbox(&criapiv1.PodSandboxFilter{LabelSelector: labels})
285+
require.NoError(t, err)
286+
require.Len(t, l, 1)
287+
288+
sb := l[0]
289+
290+
defer func() {
291+
t.Log("Cleanup leaky sandbox")
292+
err := runtimeService.StopPodSandbox(sb.Id)
293+
assert.NoError(t, err)
294+
err = runtimeService.RemovePodSandbox(sb.Id)
295+
require.NoError(t, err)
296+
}()
297+
298+
assert.Equal(t, sb.State, criapiv1.PodSandboxState_SANDBOX_NOTREADY)
299+
assert.Equal(t, sb.Metadata.Name, sbConfig.Metadata.Name)
300+
assert.Equal(t, sb.Metadata.Namespace, sbConfig.Metadata.Namespace)
301+
assert.Equal(t, sb.Metadata.Uid, sbConfig.Metadata.Uid)
302+
assert.Equal(t, sb.Metadata.Attempt, sbConfig.Metadata.Attempt)
303+
304+
t.Log("Get sandbox info")
305+
_, info, err := SandboxInfo(sb.Id)
306+
require.NoError(t, err)
307+
require.False(t, info.NetNSClosed)
308+
309+
var netNS string
310+
for _, n := range info.RuntimeSpec.Linux.Namespaces {
311+
if n.Type == runtimespec.NetworkNamespace {
312+
netNS = n.Path
313+
}
314+
}
315+
assert.NotEmpty(t, netNS, "network namespace should be set")
316+
317+
t.Log("Get sandbox container")
318+
c, err := GetContainer(sb.Id)
319+
require.NoError(t, err)
320+
any, ok := c.Extensions["io.cri-containerd.sandbox.metadata"]
321+
require.True(t, ok, "sandbox metadata should exist in extension")
322+
i, err := typeurl.UnmarshalAny(any)
323+
require.NoError(t, err)
324+
require.IsType(t, &sandbox.Metadata{}, i)
325+
metadata, ok := i.(*sandbox.Metadata)
326+
require.True(t, ok)
327+
assert.NotEmpty(t, metadata.NetNSPath)
328+
assert.Equal(t, netNS, metadata.NetNSPath, "network namespace path should be the same in runtime spec and sandbox metadata")
329+
}
330+
331+
func ensureCNIAddRunning(t *testing.T, sbName string) error {
332+
return Eventually(func() (bool, error) {
333+
pids, err := PidsOf(failpointCNIBinary)
334+
if err != nil || len(pids) == 0 {
335+
return false, err
336+
}
337+
338+
for _, pid := range pids {
339+
envs, err := PidEnvs(pid)
340+
if err != nil {
341+
t.Logf("failed to read environ of pid %v: %v: skip it", pid, err)
342+
continue
343+
}
344+
345+
args, ok := envs["CNI_ARGS"]
346+
if !ok {
347+
t.Logf("expected CNI_ARGS env but got nothing, skip pid=%v", pid)
348+
continue
349+
}
350+
351+
for _, arg := range strings.Split(args, ";") {
352+
kv := strings.SplitN(arg, "=", 2)
353+
if len(kv) != 2 {
354+
continue
355+
}
356+
357+
if kv[0] == "K8S_POD_NAME" && kv[1] == sbName {
358+
return true, nil
359+
}
360+
}
361+
}
362+
return false, nil
363+
}, time.Second, 30*time.Second)
364+
}
365+
232366
// failpointConf is used to describe cmdAdd/cmdDel/cmdCheck command's failpoint.
233367
type failpointConf struct {
234368
Add string `json:"cmdAdd"`

‎pkg/cri/server/container_update_resources_other.go‎

Lines changed: 0 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -23,10 +23,6 @@ import (
2323
"context"
2424
"fmt"
2525

26-
"github.com/containerd/containerd"
27-
"github.com/containerd/containerd/containers"
28-
"github.com/containerd/typeurl"
29-
runtimespec "github.com/opencontainers/runtime-spec/specs-go"
3026
runtime "k8s.io/cri-api/pkg/apis/runtime/v1"
3127

3228
containerstore "github.com/containerd/containerd/pkg/cri/store/container"
@@ -48,19 +44,3 @@ func (c *criService) UpdateContainerResources(ctx context.Context, r *runtime.Up
4844
}
4945
return &runtime.UpdateContainerResourcesResponse{}, nil
5046
}
51-
52-
// TODO: Copied from container_update_resources.go because that file is not built for darwin.
53-
// updateContainerSpec updates container spec.
54-
func updateContainerSpec(ctx context.Context, cntr containerd.Container, spec *runtimespec.Spec) error {
55-
any, err := typeurl.MarshalAny(spec)
56-
if err != nil {
57-
return fmt.Errorf("failed to marshal spec %+v: %w", spec, err)
58-
}
59-
if err := cntr.Update(ctx, func(ctx context.Context, client *containerd.Client, c *containers.Container) error {
60-
c.Spec = any
61-
return nil
62-
}); err != nil {
63-
return fmt.Errorf("failed to update container spec: %w", err)
64-
}
65-
return nil
66-
}

‎pkg/cri/server/sandbox_run.go‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -290,7 +290,12 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox
290290

291291
// Update network namespace in the container's spec
292292
c.updateNetNamespacePath(spec, sandbox.NetNSPath)
293-
if err := updateContainerSpec(ctx, container, spec); err != nil {
293+
294+
if err := container.Update(ctx,
295+
// Update spec of the container
296+
containerd.UpdateContainerOpts(containerd.WithSpec(spec)),
297+
// Update sandbox metadata to include NetNS info
298+
containerd.UpdateContainerOpts(containerd.WithContainerExtension(sandboxMetadataExtension, &sandbox.Metadata))); err != nil {
294299
return nil, fmt.Errorf("failed to update the network namespace for the sandbox container %q: %w", id, err)
295300
}
296301

‎script/test/utils.sh‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,9 @@ run_crictl() {
275275
# keepalive runs a command and keeps it alive.
276276
# keepalive process is eventually killed in test_teardown.
277277
keepalive() {
278+
# The command may return non-zero and we want to continue this script.
279+
# e.g. containerd receives SIGKILL
280+
set +e
278281
local command=$1
279282
echo "${command}"
280283
local wait_period=$2

0 commit comments

Comments
 (0)