From 4b32819823c2b59934039d3a256afd1c131eedc0 Mon Sep 17 00:00:00 2001 From: Maksym Pavlenko Date: Wed, 16 Nov 2022 21:15:51 -0800 Subject: [PATCH] Remove duplicated helpers Signed-off-by: Maksym Pavlenko --- pkg/cri/sbserver/podsandbox/helpers.go | 62 ---------- pkg/cri/sbserver/podsandbox/helpers_test.go | 126 -------------------- pkg/cri/sbserver/podsandbox/sandbox_run.go | 8 +- 3 files changed, 2 insertions(+), 194 deletions(-) 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 c4de080b2..5cfe1d400 100644 --- a/pkg/cri/sbserver/podsandbox/sandbox_run.go +++ b/pkg/cri/sbserver/podsandbox/sandbox_run.go @@ -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 {