From 9f46e7a449a06934bfb4a9b4b9718c1f625b1693 Mon Sep 17 00:00:00 2001 From: Iceber Gu Date: Wed, 26 Mar 2025 15:00:08 +0800 Subject: [PATCH] integration/client: add tests for TaskOptions is not empty Co-authored-by: Wei Fu Signed-off-by: Iceber Gu --- integration/client/client_unix_test.go | 129 +++++++++++++++++++++ integration/client/container_linux_test.go | 13 +++ 2 files changed, 142 insertions(+) diff --git a/integration/client/client_unix_test.go b/integration/client/client_unix_test.go index 44c81ecce..0f8e0d6c7 100644 --- a/integration/client/client_unix_test.go +++ b/integration/client/client_unix_test.go @@ -19,12 +19,26 @@ package client import ( + "context" + "strings" + "sync" "testing" + "github.com/containerd/containerd/api/services/tasks/v1" + "github.com/containerd/containerd/api/types/runc/options" . "github.com/containerd/containerd/v2/client" "github.com/containerd/containerd/v2/integration/images" "github.com/containerd/containerd/v2/pkg/deprecation" + "github.com/containerd/containerd/v2/pkg/oci" + "github.com/containerd/containerd/v2/pkg/protobuf" + "github.com/containerd/containerd/v2/plugins" + "github.com/containerd/errdefs" + "github.com/containerd/errdefs/pkg/errgrpc" "github.com/containerd/platforms" + "github.com/containerd/typeurl/v2" + "github.com/google/go-cmp/cmp" + "github.com/stretchr/testify/require" + "google.golang.org/grpc" ) var ( @@ -63,3 +77,118 @@ func TestImagePullSchema1WithEmptyLayers(t *testing.T) { t.Fatal(err) } } + +func TestNewTaskWithRuntimeOption(t *testing.T) { + t.Parallel() + + fakeTasks := &fakeTaskService{ + TasksClient: tasks.NewTasksClient(nil), + createRequests: map[string]*tasks.CreateTaskRequest{}, + } + + cli, err := newClient(t, address, + WithServices(WithTaskClient(fakeTasks)), + ) + require.NoError(t, err) + defer cli.Close() + + var ( + image Image + ctx, cancel = testContext(t) + ) + defer cancel() + + image, err = cli.GetImage(ctx, testImage) + require.NoError(t, err) + + for _, tc := range []struct { + name string + runtimeOption *options.Options + taskOpts []NewTaskOpts + expectedOptions *options.Options + }{ + { + name: "should be empty options", + runtimeOption: &options.Options{ + BinaryName: "no-runc", + }, + expectedOptions: nil, + }, + { + name: "should overwrite IOUid/ShimCgroup", + runtimeOption: &options.Options{ + BinaryName: "no-runc", + ShimCgroup: "/abc", + IoUid: 1000, + SystemdCgroup: true, + }, + taskOpts: []NewTaskOpts{ + WithUIDOwner(2000), + WithGIDOwner(3000), + WithShimCgroup("/def"), + }, + expectedOptions: &options.Options{ + BinaryName: "no-runc", + ShimCgroup: "/def", + IoUid: 2000, + IoGid: 3000, + SystemdCgroup: true, + }, + }, + } { + t.Run(tc.name, func(t *testing.T) { + id := strings.Replace(t.Name(), "/", "_", -1) + + container, err := cli.NewContainer( + ctx, + id, + WithNewSnapshotView(id, image), + WithNewSpec(oci.WithImageConfig(image), withExitStatus(7)), + WithRuntime(plugins.RuntimeRuncV2, tc.runtimeOption), + ) + require.NoError(t, err) + defer container.Delete(ctx, WithSnapshotCleanup) + + _, err = container.NewTask(ctx, empty(), tc.taskOpts...) + require.NoError(t, err) + + fakeTasks.Lock() + req := fakeTasks.createRequests[id] + fakeTasks.Unlock() + + if tc.expectedOptions == nil { + require.Nil(t, req.Options) + return + } + + gotOptions := &options.Options{} + require.NoError(t, typeurl.UnmarshalTo(req.Options, gotOptions)) + require.True(t, cmp.Equal(tc.expectedOptions, gotOptions, protobuf.Compare)) + }) + } +} + +type fakeTaskService struct { + sync.Mutex + createRequests map[string]*tasks.CreateTaskRequest + tasks.TasksClient +} + +func (ts *fakeTaskService) Create(ctx context.Context, in *tasks.CreateTaskRequest, opts ...grpc.CallOption) (*tasks.CreateTaskResponse, error) { + ts.Lock() + defer ts.Unlock() + + ts.createRequests[in.ContainerID] = in + return &tasks.CreateTaskResponse{ + ContainerID: in.ContainerID, + Pid: 1, + }, nil +} + +func (ts *fakeTaskService) Get(ctx context.Context, in *tasks.GetRequest, opts ...grpc.CallOption) (*tasks.GetResponse, error) { + return nil, errgrpc.ToGRPC(errdefs.ErrNotFound) +} + +func (ts *fakeTaskService) Delete(ctx context.Context, in *tasks.DeleteTaskRequest, opts ...grpc.CallOption) (*tasks.DeleteResponse, error) { + return nil, errgrpc.ToGRPC(errdefs.ErrNotFound) +} diff --git a/integration/client/container_linux_test.go b/integration/client/container_linux_test.go index 6a6513446..d05eca3e6 100644 --- a/integration/client/container_linux_test.go +++ b/integration/client/container_linux_test.go @@ -1090,6 +1090,19 @@ func TestContainerRuntimeOptionsv2(t *testing.T) { if !strings.Contains(err.Error(), `"no-runc"`) { t.Errorf("task creation should have failed because of lack of executable. Instead failed with: %v", err.Error()) } + + // It doesn't matter what the NewTaskOpts function is. We are using an existing function in the client package, + // which will cause the TaskOptions in the new task request to be non-empty. + // https://github.com/containerd/containerd/issues/11568 + task, err = container.NewTask(ctx, empty(), WithNoNewKeyring) + if err == nil { + t.Errorf("task creation should have failed") + task.Delete(ctx) + return + } + if !strings.Contains(err.Error(), `"no-runc"`) { + t.Errorf("task creation should have failed because of lack of executable. Instead failed with: %v", err.Error()) + } } func TestContainerKillInitPidHost(t *testing.T) {