diff --git a/pkg/cri/sbserver/container_create.go b/pkg/cri/sbserver/container_create.go index 27e468c38..f1c70a38b 100644 --- a/pkg/cri/sbserver/container_create.go +++ b/pkg/cri/sbserver/container_create.go @@ -57,6 +57,8 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta return nil, fmt.Errorf("failed to find sandbox id %q: %w", r.GetPodSandboxId(), err) } sandboxID := sandbox.ID + + // TODO: Add PID endpoint to Controller interface. s, err := sandbox.Container.Task(ctx, nil) if err != nil { return nil, fmt.Errorf("failed to get sandbox container task: %w", err) @@ -104,11 +106,6 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta } start := time.Now() - // Run container using the same runtime with sandbox. - sandboxInfo, err := sandbox.Container.Info(ctx) - if err != nil { - return nil, fmt.Errorf("failed to get sandbox %q info: %w", sandboxID, err) - } // Create container root directory. containerRootDir := c.getContainerRootDir(id) @@ -236,16 +233,18 @@ func (c *criService) CreateContainer(ctx context.Context, r *runtime.CreateConta containerLabels := buildLabels(config.Labels, image.ImageSpec.Config.Labels, containerKindContainer) - runtimeOptions, err := getRuntimeOptions(sandboxInfo) + sandboxInfo, err := c.client.SandboxStore().Get(ctx, sandboxID) if err != nil { - return nil, fmt.Errorf("failed to get runtime options: %w", err) + return nil, fmt.Errorf("unable to get sandbox %q metdata: %w", sandboxID, err) } opts = append(opts, containerd.WithSpec(spec, specOpts...), - containerd.WithRuntime(sandboxInfo.Runtime.Name, runtimeOptions), + containerd.WithRuntime(sandboxInfo.Runtime.Name, sandboxInfo.Runtime.Options), containerd.WithContainerLabels(containerLabels), - containerd.WithContainerExtension(containerMetadataExtension, &meta)) + containerd.WithContainerExtension(containerMetadataExtension, &meta), + ) + var cntr containerd.Container if cntr, err = c.client.NewContainer(ctx, id, opts...); err != nil { return nil, fmt.Errorf("failed to create containerd container: %w", err) diff --git a/pkg/cri/sbserver/podsandbox/helpers.go b/pkg/cri/sbserver/podsandbox/helpers.go index d3f90888c..307add4dd 100644 --- a/pkg/cri/sbserver/podsandbox/helpers.go +++ b/pkg/cri/sbserver/podsandbox/helpers.go @@ -33,18 +33,11 @@ import ( criconfig "github.com/containerd/containerd/pkg/cri/config" imagestore "github.com/containerd/containerd/pkg/cri/store/image" ctrdutil "github.com/containerd/containerd/pkg/cri/util" - runtimeoptions "github.com/containerd/containerd/pkg/runtimeoptions/v1" - "github.com/containerd/containerd/plugin" "github.com/containerd/containerd/reference/docker" - "github.com/containerd/containerd/runtime/linux/runctypes" - runcoptions "github.com/containerd/containerd/runtime/v2/runc/options" - "github.com/containerd/typeurl" runtimespec "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" - runhcsoptions "github.com/Microsoft/hcsshim/cmd/containerd-shim-runhcs-v1/options" imagedigest "github.com/opencontainers/go-digest" - "github.com/pelletier/go-toml" ) const ( @@ -61,8 +54,6 @@ const ( containerKindSandbox = "sandbox" // sandboxMetadataExtension is an extension name that identify metadata of sandbox in CreateContainerRequest sandboxMetadataExtension = criContainerdPrefix + ".sandbox.metadata" - // runtimeRunhcsV1 is the runtime type for runhcs. - runtimeRunhcsV1 = "io.containerd.runhcs.v1" // MetadataKey is the key used for storing metadata in the sandbox extensions MetadataKey = "metadata" ) @@ -170,59 +161,6 @@ func parseImageReferences(refs []string) ([]string, []string) { return tags, digests } -// generateRuntimeOptions generates runtime options from cri plugin config. -func generateRuntimeOptions(r criconfig.Runtime, c criconfig.Config) (interface{}, error) { - if r.Options == nil { - if r.Type != plugin.RuntimeLinuxV1 { - return nil, nil - } - // This is a legacy config, generate runctypes.RuncOptions. - return &runctypes.RuncOptions{ - Runtime: r.Engine, - RuntimeRoot: r.Root, - SystemdCgroup: c.SystemdCgroup, - }, nil - } - optionsTree, err := toml.TreeFromMap(r.Options) - if err != nil { - return nil, err - } - options := getRuntimeOptionsType(r.Type) - if err := optionsTree.Unmarshal(options); err != nil { - return nil, err - } - return options, nil -} - -// getRuntimeOptionsType gets empty runtime options by the runtime type name. -func getRuntimeOptionsType(t string) interface{} { - switch t { - case plugin.RuntimeRuncV1: - fallthrough - case plugin.RuntimeRuncV2: - return &runcoptions.Options{} - case plugin.RuntimeLinuxV1: - return &runctypes.RuncOptions{} - case runtimeRunhcsV1: - return &runhcsoptions.Options{} - default: - return &runtimeoptions.Options{} - } -} - -// getRuntimeOptions get runtime options from container metadata. -func getRuntimeOptions(c containers.Container) (interface{}, error) { - from := c.Runtime.Options - if from == nil || from.GetValue() == nil { - return nil, nil - } - opts, err := typeurl.UnmarshalAny(from) - if err != nil { - return nil, err - } - return opts, nil -} - // getPassthroughAnnotations filters requested pod annotations by comparing // against permitted annotations for the given runtime. func getPassthroughAnnotations(podAnnotations map[string]string, diff --git a/pkg/cri/sbserver/podsandbox/helpers_test.go b/pkg/cri/sbserver/podsandbox/helpers_test.go index 1edb2cada..0e18d0c42 100644 --- a/pkg/cri/sbserver/podsandbox/helpers_test.go +++ b/pkg/cri/sbserver/podsandbox/helpers_test.go @@ -22,21 +22,11 @@ import ( "strings" "testing" - "github.com/containerd/containerd/containers" "github.com/containerd/containerd/oci" - criconfig "github.com/containerd/containerd/pkg/cri/config" - "github.com/containerd/containerd/plugin" - "github.com/containerd/containerd/protobuf/types" "github.com/containerd/containerd/reference/docker" - "github.com/containerd/containerd/runtime/linux/runctypes" - runcoptions "github.com/containerd/containerd/runtime/v2/runc/options" - "github.com/containerd/typeurl" - imagedigest "github.com/opencontainers/go-digest" runtimespec "github.com/opencontainers/runtime-spec/specs-go" - "github.com/pelletier/go-toml" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" ) // TestGetUserFromImage tests the logic of getting image uid or user name of image user. @@ -158,112 +148,6 @@ func TestParseImageReferences(t *testing.T) { assert.Equal(t, expectedDigests, digests) } -func TestGenerateRuntimeOptions(t *testing.T) { - nilOpts := ` -systemd_cgroup = true -[containerd] - no_pivot = true - default_runtime_name = "default" -[containerd.runtimes.legacy] - runtime_type = "` + plugin.RuntimeLinuxV1 + `" -[containerd.runtimes.runc] - runtime_type = "` + plugin.RuntimeRuncV1 + `" -[containerd.runtimes.runcv2] - runtime_type = "` + plugin.RuntimeRuncV2 + `" -` - nonNilOpts := ` -systemd_cgroup = true -[containerd] - no_pivot = true - default_runtime_name = "default" -[containerd.runtimes.legacy] - runtime_type = "` + plugin.RuntimeLinuxV1 + `" -[containerd.runtimes.legacy.options] - Runtime = "legacy" - RuntimeRoot = "/legacy" -[containerd.runtimes.runc] - runtime_type = "` + plugin.RuntimeRuncV1 + `" -[containerd.runtimes.runc.options] - BinaryName = "runc" - Root = "/runc" - NoNewKeyring = true -[containerd.runtimes.runcv2] - runtime_type = "` + plugin.RuntimeRuncV2 + `" -[containerd.runtimes.runcv2.options] - BinaryName = "runc" - Root = "/runcv2" - NoNewKeyring = true -` - var nilOptsConfig, nonNilOptsConfig criconfig.Config - tree, err := toml.Load(nilOpts) - require.NoError(t, err) - err = tree.Unmarshal(&nilOptsConfig) - require.NoError(t, err) - require.Len(t, nilOptsConfig.Runtimes, 3) - - tree, err = toml.Load(nonNilOpts) - require.NoError(t, err) - err = tree.Unmarshal(&nonNilOptsConfig) - require.NoError(t, err) - require.Len(t, nonNilOptsConfig.Runtimes, 3) - - for desc, test := range map[string]struct { - r criconfig.Runtime - c criconfig.Config - expectedOptions interface{} - }{ - "when options is nil, should return nil option for io.containerd.runc.v1": { - r: nilOptsConfig.Runtimes["runc"], - c: nilOptsConfig, - expectedOptions: nil, - }, - "when options is nil, should return nil option for io.containerd.runc.v2": { - r: nilOptsConfig.Runtimes["runcv2"], - c: nilOptsConfig, - expectedOptions: nil, - }, - "when options is nil, should use legacy fields for legacy runtime": { - r: nilOptsConfig.Runtimes["legacy"], - c: nilOptsConfig, - expectedOptions: &runctypes.RuncOptions{ - SystemdCgroup: true, - }, - }, - "when options is not nil, should be able to decode for io.containerd.runc.v1": { - r: nonNilOptsConfig.Runtimes["runc"], - c: nonNilOptsConfig, - expectedOptions: &runcoptions.Options{ - BinaryName: "runc", - Root: "/runc", - NoNewKeyring: true, - }, - }, - "when options is not nil, should be able to decode for io.containerd.runc.v2": { - r: nonNilOptsConfig.Runtimes["runcv2"], - c: nonNilOptsConfig, - expectedOptions: &runcoptions.Options{ - BinaryName: "runc", - Root: "/runcv2", - NoNewKeyring: true, - }, - }, - "when options is not nil, should be able to decode for legacy runtime": { - r: nonNilOptsConfig.Runtimes["legacy"], - c: nonNilOptsConfig, - expectedOptions: &runctypes.RuncOptions{ - Runtime: "legacy", - RuntimeRoot: "/legacy", - }, - }, - } { - t.Run(desc, func(t *testing.T) { - opts, err := generateRuntimeOptions(test.r, test.c) - assert.NoError(t, err) - assert.Equal(t, test.expectedOptions, opts) - }) - } -} - func TestEnvDeduplication(t *testing.T) { for desc, test := range map[string]struct { existing []string @@ -471,13 +355,3 @@ func TestEnsureRemoveAllWithFile(t *testing.T) { t.Fatal(err) } } - -func TestGetRuntimeOptions(t *testing.T) { - _, err := getRuntimeOptions(containers.Container{}) - require.NoError(t, err) - - var pbany *types.Any // This is nil. - var typeurlAny typeurl.Any = pbany // This is typed nil. - _, err = getRuntimeOptions(containers.Container{Runtime: containers.RuntimeInfo{Options: typeurlAny}}) - require.NoError(t, err) -} diff --git a/pkg/cri/sbserver/podsandbox/sandbox_run.go b/pkg/cri/sbserver/podsandbox/sandbox_run.go index d5aef016f..5cfe1d400 100644 --- a/pkg/cri/sbserver/podsandbox/sandbox_run.go +++ b/pkg/cri/sbserver/podsandbox/sandbox_run.go @@ -86,7 +86,7 @@ func (c *Controller) Start(ctx context.Context, id string) (resp *api.Controller return nil, fmt.Errorf("failed to get image from containerd %q: %w", image.ID, err) } - ociRuntime, err := c.getSandboxRuntime(config, sandboxInfo.Runtime.Name) + ociRuntime, err := c.getSandboxRuntime(config, metadata.RuntimeHandler) if err != nil { return nil, fmt.Errorf("failed to get sandbox runtime: %w", err) } @@ -131,11 +131,6 @@ func (c *Controller) Start(ctx context.Context, id string) (resp *api.Controller sandboxLabels := buildLabels(config.Labels, image.ImageSpec.Config.Labels, containerKindSandbox) - runtimeOpts, err := generateRuntimeOptions(ociRuntime, c.config) - if err != nil { - return nil, fmt.Errorf("failed to generate runtime options: %w", err) - } - snapshotterOpt := snapshots.WithLabels(snapshots.FilterInheritedLabels(config.Annotations)) opts := []containerd.NewContainerOpts{ containerd.WithSnapshotter(c.runtimeSnapshotter(ctx, ociRuntime)), @@ -143,7 +138,8 @@ func (c *Controller) Start(ctx context.Context, id string) (resp *api.Controller containerd.WithSpec(spec, specOpts...), containerd.WithContainerLabels(sandboxLabels), containerd.WithContainerExtension(sandboxMetadataExtension, &metadata), - containerd.WithRuntime(ociRuntime.Type, runtimeOpts)} + containerd.WithRuntime(ociRuntime.Type, sandboxInfo.Runtime.Options), + } container, err := c.client.NewContainer(ctx, id, opts...) if err != nil { @@ -263,6 +259,7 @@ func (c *Controller) Start(ctx context.Context, id string) (resp *api.Controller SandboxID: id, Pid: task.Pid(), CreatedAt: protobuf.ToTimestamp(info.CreatedAt), + Labels: labels, } return resp, nil diff --git a/pkg/cri/sbserver/sandbox_run.go b/pkg/cri/sbserver/sandbox_run.go index 4fda0b739..90642f2cb 100644 --- a/pkg/cri/sbserver/sandbox_run.go +++ b/pkg/cri/sbserver/sandbox_run.go @@ -86,10 +86,29 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox } }() - sandboxInfo := sb.Sandbox{ - ID: id, - // TODO: runtime handler can be an empty string, should use default one and enable back validation of this field in metadata store. - Runtime: sb.RuntimeOpts{Name: r.GetRuntimeHandler()}, + var ( + err error + sandboxInfo = sb.Sandbox{ID: id} + ) + + ociRuntime, err := c.getSandboxRuntime(config, r.GetRuntimeHandler()) + if err != nil { + return nil, fmt.Errorf("unable to get OCI runtime for sandbox %q: %w", id, err) + } + + sandboxInfo.Runtime.Name = ociRuntime.Type + + // Retrieve runtime options + runtimeOpts, err := generateRuntimeOptions(ociRuntime, c.config) + if err != nil { + return nil, fmt.Errorf("failed to generate sandbox runtime options: %w", err) + } + + if runtimeOpts != nil { + sandboxInfo.Runtime.Options, err = typeurl.MarshalAny(runtimeOpts) + if err != nil { + return nil, fmt.Errorf("failed to marshal runtime options: %w", err) + } } // Save sandbox name @@ -127,11 +146,7 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox } }() - var ( - podNetwork = true - err error - ) - + podNetwork := true if goruntime.GOOS != "windows" && config.GetLinux().GetSecurityContext().GetNamespaceOptions().GetNetwork() == runtime.NamespaceMode_NODE { // Pod network is not needed on linux with host network. @@ -143,6 +158,11 @@ func (c *criService) RunPodSandbox(ctx context.Context, r *runtime.RunPodSandbox podNetwork = false } + // No CNI on darwin yet + if goruntime.GOOS == "darwin" { + podNetwork = false + } + if podNetwork { netStart := time.Now() // If it is not in host network namespace then create a namespace and set the sandbox diff --git a/pkg/cri/sbserver/sandbox_status.go b/pkg/cri/sbserver/sandbox_status.go index 97e8aae4e..2426dd0a0 100644 --- a/pkg/cri/sbserver/sandbox_status.go +++ b/pkg/cri/sbserver/sandbox_status.go @@ -45,12 +45,13 @@ func (c *criService) PodSandboxStatus(ctx context.Context, r *runtime.PodSandbox status := toCRISandboxStatus(sandbox.Metadata, sandbox.Status.Get(), ip, additionalIPs) if status.GetCreatedAt() == 0 { // CRI doesn't allow CreatedAt == 0. - info, err := sandbox.Container.Info(ctx) + sandboxInfo, err := c.client.SandboxStore().Get(ctx, r.GetPodSandboxId()) if err != nil { - return nil, fmt.Errorf("failed to get CreatedAt for sandbox container in %q state: %w", status.State, err) + return nil, fmt.Errorf("failed to get sandbox %q from metadata store: %w", r.GetPodSandboxId(), err) } - status.CreatedAt = info.CreatedAt.UnixNano() + status.CreatedAt = sandboxInfo.CreatedAt.UnixNano() } + if !r.GetVerbose() { return &runtime.PodSandboxStatusResponse{Status: status}, nil } @@ -76,10 +77,15 @@ func (c *criService) getIPs(sandbox sandboxstore.Sandbox) (string, []string, err // responsible for reporting the IP. return "", nil, nil } + if goruntime.GOOS == "windows" && config.GetWindows().GetSecurityContext().GetHostProcess() { return "", nil, nil } + if goruntime.GOOS == "darwin" { + return "", nil, nil + } + if closed, err := sandbox.NetNS.Closed(); err != nil { return "", nil, fmt.Errorf("check network namespace closed: %w", err) } else if closed { diff --git a/pkg/cri/sbserver/service.go b/pkg/cri/sbserver/service.go index 289b6a2b1..78880d822 100644 --- a/pkg/cri/sbserver/service.go +++ b/pkg/cri/sbserver/service.go @@ -27,14 +27,12 @@ import ( "time" "github.com/containerd/containerd" - sandboxapi "github.com/containerd/containerd/api/services/sandbox/v1" "github.com/containerd/containerd/oci" "github.com/containerd/containerd/pkg/cri/sbserver/podsandbox" "github.com/containerd/containerd/pkg/cri/streaming" "github.com/containerd/containerd/pkg/kmutex" "github.com/containerd/containerd/plugin" "github.com/containerd/containerd/sandbox" - "github.com/containerd/containerd/sandbox/proxy" runtime_alpha "github.com/containerd/containerd/third_party/k8s.io/cri-api/pkg/apis/runtime/v1alpha2" "github.com/containerd/go-cni" "github.com/sirupsen/logrus" @@ -191,7 +189,7 @@ func NewCRIService(config criconfig.Config, client *containerd.Client) (CRIServi // Load all sandbox controllers(pod sandbox controller and remote shim controller) c.sandboxControllers[criconfig.ModePodSandbox] = podsandbox.New(config, client, c.sandboxStore, c.os, c, c.baseOCISpecs) - c.sandboxControllers[criconfig.ModeShim] = proxy.NewSandboxController(sandboxapi.NewControllerClient(client.Conn())) + c.sandboxControllers[criconfig.ModeShim] = client.SandboxController() return c, nil } diff --git a/services/sandbox/controller_local.go b/services/sandbox/controller_local.go index 4215a59dc..ec62317f9 100644 --- a/services/sandbox/controller_local.go +++ b/services/sandbox/controller_local.go @@ -112,6 +112,7 @@ func (c *controllerLocal) Create(ctx context.Context, in *api.ControllerCreateRe Rootfs: in.Rootfs, Options: in.Options, }); err != nil { + // TODO: Delete sandbox shim here. return nil, fmt.Errorf("failed to start sandbox %s: %w", in.SandboxID, err) }