From 9507f45a0f6b60d7ad8f8304221fae7f7bfe880a Mon Sep 17 00:00:00 2001 From: Wei Zhang Date: Wed, 4 Sep 2019 19:56:23 +0800 Subject: [PATCH] CompatOCISpec: limit usage of CompatOCISpec Fixes: #2023 CompatOCISpec is used to gurantee backward compatbility for old runtime specs, after we convert CompatOCISpec to standard specs.Spec, we should use specs.Spec instead of CompatOCISpec, and CompatOCISpec should be useless from then. Spread usage of CompatOCISpec can make code structure confusing and making the runtime spec usage non-standard. Besides, this can be the very first step of removing CompatOCISpec from config's Annotations field. Signed-off-by: Wei Zhang --- cli/create.go | 2 +- cli/create_test.go | 12 +-- cli/delete_test.go | 32 ++++---- cli/exec.go | 6 +- cli/exec_test.go | 47 ++++++------ cli/main_test.go | 30 +------- cli/run_test.go | 5 +- cli/start_test.go | 13 ++-- containerd-shim-v2/container.go | 8 +- containerd-shim-v2/create.go | 4 +- containerd-shim-v2/create_test.go | 22 +++--- containerd-shim-v2/delete_test.go | 16 ++-- containerd-shim-v2/service.go | 4 +- containerd-shim-v2/utils.go | 6 +- containerd-shim-v2/utils_test.go | 42 +---------- pkg/katautils/create.go | 9 ++- pkg/katautils/create_test.go | 37 ++------- pkg/katautils/hook.go | 7 +- pkg/katautils/hook_test.go | 73 +++++++----------- virtcontainers/pkg/oci/utils.go | 108 +++++++++++++-------------- virtcontainers/pkg/oci/utils_test.go | 63 ++++++++-------- virtcontainers/types/sandbox.go | 19 +---- 22 files changed, 221 insertions(+), 344 deletions(-) diff --git a/cli/create.go b/cli/create.go index 77d356a10..0d3cf9952 100644 --- a/cli/create.go +++ b/cli/create.go @@ -118,7 +118,7 @@ func create(ctx context.Context, containerID, bundlePath, console, pidFilePath s return err } - containerType, err := ociSpec.ContainerType() + containerType, err := oci.ContainerType(ociSpec) if err != nil { return err } diff --git a/cli/create_test.go b/cli/create_test.go index 0428c3eaa..8f6c22bb3 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -322,7 +322,7 @@ func TestCreateInvalidContainerType(t *testing.T) { ociConfigFile := filepath.Join(bundlePath, "config.json") assert.True(katautils.FileExists(ociConfigFile)) - spec, err := readOCIConfigFile(ociConfigFile) + spec, err := oci.ParseConfigJSON(bundlePath) assert.NoError(err) // Force an invalid container type @@ -367,7 +367,7 @@ func TestCreateContainerInvalid(t *testing.T) { ociConfigFile := filepath.Join(bundlePath, "config.json") assert.True(katautils.FileExists(ociConfigFile)) - spec, err := readOCIConfigFile(ociConfigFile) + spec, err := oci.ParseConfigJSON(bundlePath) assert.NoError(err) @@ -432,7 +432,7 @@ func TestCreateProcessCgroupsPathSuccessful(t *testing.T) { ociConfigFile := filepath.Join(bundlePath, "config.json") assert.True(katautils.FileExists(ociConfigFile)) - spec, err := readOCIConfigFile(ociConfigFile) + spec, err := oci.ParseConfigJSON(bundlePath) assert.NoError(err) // Force sandbox-type container @@ -535,7 +535,7 @@ func TestCreateCreateCgroupsFilesFail(t *testing.T) { ociConfigFile := filepath.Join(bundlePath, "config.json") assert.True(katautils.FileExists(ociConfigFile)) - spec, err := readOCIConfigFile(ociConfigFile) + spec, err := oci.ParseConfigJSON(bundlePath) assert.NoError(err) // Force sandbox-type container @@ -622,7 +622,7 @@ func TestCreateCreateCreatePidFileFail(t *testing.T) { ociConfigFile := filepath.Join(bundlePath, "config.json") assert.True(katautils.FileExists(ociConfigFile)) - spec, err := readOCIConfigFile(ociConfigFile) + spec, err := oci.ParseConfigJSON(bundlePath) assert.NoError(err) // Force sandbox-type container @@ -697,7 +697,7 @@ func TestCreate(t *testing.T) { ociConfigFile := filepath.Join(bundlePath, "config.json") assert.True(katautils.FileExists(ociConfigFile)) - spec, err := readOCIConfigFile(ociConfigFile) + spec, err := oci.ParseConfigJSON(bundlePath) assert.NoError(err) // Force sandbox-type container diff --git a/cli/delete_test.go b/cli/delete_test.go index 77ded07f5..a04d1ceb8 100644 --- a/cli/delete_test.go +++ b/cli/delete_test.go @@ -128,22 +128,20 @@ func TestDeleteInvalidConfig(t *testing.T) { assert.False(vcmock.IsMockError(err)) } -func testConfigSetup(t *testing.T) (rootPath string, configPath string) { +func testConfigSetup(t *testing.T) (rootPath string, bundlePath string) { assert := assert.New(t) tmpdir, err := ioutil.TempDir("", "") assert.NoError(err) - bundlePath := filepath.Join(tmpdir, "bundle") + bundlePath = filepath.Join(tmpdir, "bundle") err = os.MkdirAll(bundlePath, testDirMode) assert.NoError(err) err = createOCIConfig(bundlePath) assert.NoError(err) - // config json path - configPath = filepath.Join(bundlePath, "config.json") - return tmpdir, configPath + return tmpdir, bundlePath } func TestDeleteSandbox(t *testing.T) { @@ -153,9 +151,9 @@ func TestDeleteSandbox(t *testing.T) { MockID: testSandboxID, } - rootPath, configPath := testConfigSetup(t) + rootPath, bundlePath := testConfigSetup(t) defer os.RemoveAll(rootPath) - configJSON, err := readOCIConfigJSON(configPath) + configJSON, err := readOCIConfigJSON(bundlePath) assert.NoError(err) path, err := createTempContainerIDMapping(sandbox.ID(), sandbox.ID()) @@ -231,9 +229,9 @@ func TestDeleteInvalidContainerType(t *testing.T) { MockID: testSandboxID, } - rootPath, configPath := testConfigSetup(t) + rootPath, bundlePath := testConfigSetup(t) defer os.RemoveAll(rootPath) - configJSON, err := readOCIConfigJSON(configPath) + configJSON, err := readOCIConfigJSON(bundlePath) assert.NoError(err) path, err := createTempContainerIDMapping(sandbox.ID(), sandbox.ID()) @@ -270,9 +268,9 @@ func TestDeleteSandboxRunning(t *testing.T) { MockID: testSandboxID, } - rootPath, configPath := testConfigSetup(t) + rootPath, bundlePath := testConfigSetup(t) defer os.RemoveAll(rootPath) - configJSON, err := readOCIConfigJSON(configPath) + configJSON, err := readOCIConfigJSON(bundlePath) assert.NoError(err) path, err := createTempContainerIDMapping(sandbox.ID(), sandbox.ID()) @@ -350,9 +348,9 @@ func TestDeleteRunningContainer(t *testing.T) { }, } - rootPath, configPath := testConfigSetup(t) + rootPath, bundlePath := testConfigSetup(t) defer os.RemoveAll(rootPath) - configJSON, err := readOCIConfigJSON(configPath) + configJSON, err := readOCIConfigJSON(bundlePath) assert.NoError(err) path, err := createTempContainerIDMapping(sandbox.MockContainers[0].ID(), sandbox.MockContainers[0].ID()) @@ -433,9 +431,9 @@ func TestDeleteContainer(t *testing.T) { }, } - rootPath, configPath := testConfigSetup(t) + rootPath, bundlePath := testConfigSetup(t) defer os.RemoveAll(rootPath) - configJSON, err := readOCIConfigJSON(configPath) + configJSON, err := readOCIConfigJSON(bundlePath) assert.NoError(err) path, err := createTempContainerIDMapping(sandbox.MockContainers[0].ID(), sandbox.MockContainers[0].ID()) @@ -533,9 +531,9 @@ func TestDeleteCLIFunctionSuccess(t *testing.T) { }, } - rootPath, configPath := testConfigSetup(t) + rootPath, bundlePath := testConfigSetup(t) defer os.RemoveAll(rootPath) - configJSON, err := readOCIConfigJSON(configPath) + configJSON, err := readOCIConfigJSON(bundlePath) assert.NoError(err) path, err := createTempContainerIDMapping(sandbox.ID(), sandbox.ID()) diff --git a/cli/exec.go b/cli/exec.go index 1c1574ce0..8ed80065e 100644 --- a/cli/exec.go +++ b/cli/exec.go @@ -23,7 +23,7 @@ import ( ) type execParams struct { - ociProcess oci.CompatOCIProcess + ociProcess specs.Process cID string pidFile string console string @@ -119,7 +119,7 @@ EXAMPLE: }, } -func generateExecParams(context *cli.Context, specProcess *oci.CompatOCIProcess) (execParams, error) { +func generateExecParams(context *cli.Context, specProcess *specs.Process) (execParams, error) { ctxArgs := context.Args() params := execParams{ @@ -133,7 +133,7 @@ func generateExecParams(context *cli.Context, specProcess *oci.CompatOCIProcess) } if context.String("process") != "" { - var ociProcess oci.CompatOCIProcess + var ociProcess specs.Process fileContent, err := ioutil.ReadFile(context.String("process")) if err != nil { diff --git a/cli/exec_test.go b/cli/exec_test.go index a916f66b0..bb2ea3554 100644 --- a/cli/exec_test.go +++ b/cli/exec_test.go @@ -14,13 +14,14 @@ import ( "path/filepath" "testing" - vc "github.com/kata-containers/runtime/virtcontainers" - vcAnnotations "github.com/kata-containers/runtime/virtcontainers/pkg/annotations" - "github.com/kata-containers/runtime/virtcontainers/pkg/oci" - "github.com/kata-containers/runtime/virtcontainers/pkg/vcmock" - "github.com/kata-containers/runtime/virtcontainers/types" + specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/stretchr/testify/assert" "github.com/urfave/cli" + + vc "github.com/kata-containers/runtime/virtcontainers" + vcAnnotations "github.com/kata-containers/runtime/virtcontainers/pkg/annotations" + "github.com/kata-containers/runtime/virtcontainers/pkg/vcmock" + "github.com/kata-containers/runtime/virtcontainers/types" ) func TestExecCLIFunction(t *testing.T) { @@ -90,9 +91,9 @@ func TestExecuteErrors(t *testing.T) { assert.False(vcmock.IsMockError(err)) // Container state undefined - rootPath, configPath := testConfigSetup(t) + rootPath, bundlePath := testConfigSetup(t) defer os.RemoveAll(rootPath) - configJSON, err := readOCIConfigJSON(configPath) + configJSON, err := readOCIConfigJSON(bundlePath) assert.NoError(err) annotations = map[string]string{ @@ -149,9 +150,9 @@ func TestExecuteErrorReadingProcessJson(t *testing.T) { flagSet.Parse([]string{testContainerID}) ctx := createCLIContext(flagSet) - rootPath, configPath := testConfigSetup(t) + rootPath, bundlePath := testConfigSetup(t) defer os.RemoveAll(rootPath) - configJSON, err := readOCIConfigJSON(configPath) + configJSON, err := readOCIConfigJSON(bundlePath) assert.NoError(err) annotations := map[string]string{ @@ -198,9 +199,9 @@ func TestExecuteErrorOpeningConsole(t *testing.T) { flagSet.Parse([]string{testContainerID}) ctx := createCLIContext(flagSet) - rootPath, configPath := testConfigSetup(t) + rootPath, bundlePath := testConfigSetup(t) defer os.RemoveAll(rootPath) - configJSON, err := readOCIConfigJSON(configPath) + configJSON, err := readOCIConfigJSON(bundlePath) assert.NoError(err) annotations := map[string]string{ @@ -265,9 +266,9 @@ func TestExecuteWithFlags(t *testing.T) { flagSet.Parse([]string{testContainerID, "/tmp/foo"}) ctx := createCLIContext(flagSet) - rootPath, configPath := testConfigSetup(t) + rootPath, bundlePath := testConfigSetup(t) defer os.RemoveAll(rootPath) - configJSON, err := readOCIConfigJSON(configPath) + configJSON, err := readOCIConfigJSON(bundlePath) assert.NoError(err) annotations := map[string]string{ @@ -355,9 +356,9 @@ func TestExecuteWithFlagsDetached(t *testing.T) { flagSet.Parse([]string{testContainerID, "/tmp/foo"}) ctx := createCLIContext(flagSet) - rootPath, configPath := testConfigSetup(t) + rootPath, bundlePath := testConfigSetup(t) defer os.RemoveAll(rootPath) - configJSON, err := readOCIConfigJSON(configPath) + configJSON, err := readOCIConfigJSON(bundlePath) assert.NoError(err) annotations := map[string]string{ @@ -434,9 +435,9 @@ func TestExecuteWithInvalidProcessJson(t *testing.T) { flagSet.Parse([]string{testContainerID}) ctx := createCLIContext(flagSet) - rootPath, configPath := testConfigSetup(t) + rootPath, bundlePath := testConfigSetup(t) defer os.RemoveAll(rootPath) - configJSON, err := readOCIConfigJSON(configPath) + configJSON, err := readOCIConfigJSON(bundlePath) assert.NoError(err) annotations := map[string]string{ @@ -486,9 +487,9 @@ func TestExecuteWithValidProcessJson(t *testing.T) { flagSet.Parse([]string{testContainerID, "/tmp/foo"}) ctx := createCLIContext(flagSet) - rootPath, configPath := testConfigSetup(t) + rootPath, bundlePath := testConfigSetup(t) defer os.RemoveAll(rootPath) - configJSON, err := readOCIConfigJSON(configPath) + configJSON, err := readOCIConfigJSON(bundlePath) assert.NoError(err) annotations := map[string]string{ @@ -587,9 +588,9 @@ func TestExecuteWithEmptyEnvironmentValue(t *testing.T) { flagSet.Parse([]string{testContainerID}) ctx := createCLIContext(flagSet) - rootPath, configPath := testConfigSetup(t) + rootPath, bundlePath := testConfigSetup(t) defer os.RemoveAll(rootPath) - configJSON, err := readOCIConfigJSON(configPath) + configJSON, err := readOCIConfigJSON(bundlePath) assert.NoError(err) annotations := map[string]string{ @@ -698,7 +699,7 @@ func TestGenerateExecParams(t *testing.T) { flagSet.String("apparmor", apparmor, "") ctx := createCLIContext(flagSet) - process := &oci.CompatOCIProcess{} + process := &specs.Process{} params, err := generateExecParams(ctx, process) assert.NoError(err) @@ -771,7 +772,7 @@ func TestGenerateExecParamsWithProcessJsonFile(t *testing.T) { defer os.Remove(processPath) - process := &oci.CompatOCIProcess{} + process := &specs.Process{} params, err := generateExecParams(ctx, process) assert.NoError(err) diff --git a/cli/main_test.go b/cli/main_test.go index 2515ce375..705722c24 100644 --- a/cli/main_test.go +++ b/cli/main_test.go @@ -150,10 +150,9 @@ func runUnitTests(m *testing.M) { os.Exit(ret) } -// Read fail that should contain a CompatOCISpec and +// Read fail that should contain a specs.Spec and // return its JSON representation on success -func readOCIConfigJSON(configFile string) (string, error) { - bundlePath := filepath.Dir(configFile) +func readOCIConfigJSON(bundlePath string) (string, error) { ociSpec, err := oci.ParseConfigJSON(bundlePath) if err != nil { return "", nil @@ -400,30 +399,7 @@ func makeOCIBundle(bundleDir string) error { return nil } -// readOCIConfig returns an OCI spec. -func readOCIConfigFile(configPath string) (oci.CompatOCISpec, error) { - if configPath == "" { - return oci.CompatOCISpec{}, errors.New("BUG: need config file path") - } - - data, err := ioutil.ReadFile(configPath) - if err != nil { - return oci.CompatOCISpec{}, err - } - - var ociSpec oci.CompatOCISpec - if err := json.Unmarshal(data, &ociSpec); err != nil { - return oci.CompatOCISpec{}, err - } - caps, err := oci.ContainerCapabilities(ociSpec) - if err != nil { - return oci.CompatOCISpec{}, err - } - ociSpec.Process.Capabilities = caps - return ociSpec, nil -} - -func writeOCIConfigFile(spec oci.CompatOCISpec, configPath string) error { +func writeOCIConfigFile(spec specs.Spec, configPath string) error { if configPath == "" { return errors.New("BUG: need config file path") } diff --git a/cli/run_test.go b/cli/run_test.go index 4f9640bc8..ef683fb2f 100644 --- a/cli/run_test.go +++ b/cli/run_test.go @@ -188,9 +188,6 @@ func testRunContainerSetup(t *testing.T) runContainerData { err = makeOCIBundle(bundlePath) assert.NoError(err) - // config json path - configPath := filepath.Join(bundlePath, specConfig) - // sandbox id and container id must be the same otherwise delete will not works sandbox := &vcmock.Sandbox{ MockID: testContainerID, @@ -208,7 +205,7 @@ func testRunContainerSetup(t *testing.T) runContainerData { runtimeConfig, err := newTestRuntimeConfig(tmpdir, consolePath, true) assert.NoError(err) - configJSON, err := readOCIConfigJSON(configPath) + configJSON, err := readOCIConfigJSON(bundlePath) assert.NoError(err) return runContainerData{ diff --git a/cli/start_test.go b/cli/start_test.go index 6c4c04380..28005a2fb 100644 --- a/cli/start_test.go +++ b/cli/start_test.go @@ -13,13 +13,14 @@ import ( "os" "testing" + "github.com/opencontainers/runtime-spec/specs-go" + "github.com/stretchr/testify/assert" + "github.com/urfave/cli" + "github.com/kata-containers/runtime/pkg/katautils" vc "github.com/kata-containers/runtime/virtcontainers" vcAnnotations "github.com/kata-containers/runtime/virtcontainers/pkg/annotations" - "github.com/kata-containers/runtime/virtcontainers/pkg/oci" "github.com/kata-containers/runtime/virtcontainers/pkg/vcmock" - "github.com/stretchr/testify/assert" - "github.com/urfave/cli" ) func TestStartInvalidArgs(t *testing.T) { @@ -61,7 +62,7 @@ func TestStartSandbox(t *testing.T) { assert.NoError(err) defer os.RemoveAll(path) - ociSpecJSON, err := json.Marshal(oci.CompatOCISpec{}) + ociSpecJSON, err := json.Marshal(specs.Spec{}) assert.NoError(err) testingImpl.StatusContainerFunc = func(ctx context.Context, sandboxID, containerID string) (vc.ContainerStatus, error) { @@ -139,7 +140,7 @@ func TestStartContainerSucessFailure(t *testing.T) { assert.NoError(err) defer os.RemoveAll(path) - ociSpecJSON, err := json.Marshal(oci.CompatOCISpec{}) + ociSpecJSON, err := json.Marshal(specs.Spec{}) assert.NoError(err) testingImpl.StatusContainerFunc = func(ctx context.Context, sandboxID, containerID string) (vc.ContainerStatus, error) { @@ -217,7 +218,7 @@ func TestStartCLIFunctionSuccess(t *testing.T) { assert.NoError(err) defer os.RemoveAll(path) - ociSpecJSON, err := json.Marshal(oci.CompatOCISpec{}) + ociSpecJSON, err := json.Marshal(specs.Spec{}) assert.NoError(err) testingImpl.StatusContainerFunc = func(ctx context.Context, sandboxID, containerID string) (vc.ContainerStatus, error) { diff --git a/containerd-shim-v2/container.go b/containerd-shim-v2/container.go index 9dffa2cfc..bcf89c524 100644 --- a/containerd-shim-v2/container.go +++ b/containerd-shim-v2/container.go @@ -11,15 +11,15 @@ import ( "github.com/containerd/containerd/api/types/task" "github.com/containerd/containerd/errdefs" taskAPI "github.com/containerd/containerd/runtime/v2/task" + "github.com/opencontainers/runtime-spec/specs-go" vc "github.com/kata-containers/runtime/virtcontainers" - "github.com/kata-containers/runtime/virtcontainers/pkg/oci" ) type container struct { s *service ttyio *ttyIO - spec *oci.CompatOCISpec + spec *specs.Spec exitTime time.Time execs map[string]*exec exitIOch chan struct{} @@ -35,14 +35,14 @@ type container struct { terminal bool } -func newContainer(s *service, r *taskAPI.CreateTaskRequest, containerType vc.ContainerType, spec *oci.CompatOCISpec) (*container, error) { +func newContainer(s *service, r *taskAPI.CreateTaskRequest, containerType vc.ContainerType, spec *specs.Spec) (*container, error) { if r == nil { return nil, errdefs.ToGRPCf(errdefs.ErrInvalidArgument, " CreateTaskRequest points to nil") } // in order to avoid deferencing a nil pointer in test if spec == nil { - spec = &oci.CompatOCISpec{} + spec = &specs.Spec{} } c := &container{ diff --git a/containerd-shim-v2/create.go b/containerd-shim-v2/create.go index 418c4108f..04b0cd50c 100644 --- a/containerd-shim-v2/create.go +++ b/containerd-shim-v2/create.go @@ -46,7 +46,7 @@ func create(ctx context.Context, s *service, r *taskAPI.CreateTaskRequest) (*con return nil, err } - containerType, err := ociSpec.ContainerType() + containerType, err := oci.ContainerType(*ociSpec) if err != nil { return nil, err } @@ -125,7 +125,7 @@ func create(ctx context.Context, s *service, r *taskAPI.CreateTaskRequest) (*con return container, nil } -func loadSpec(r *taskAPI.CreateTaskRequest) (*oci.CompatOCISpec, string, error) { +func loadSpec(r *taskAPI.CreateTaskRequest) (*specs.Spec, string, error) { // Checks the MUST and MUST NOT from OCI runtime specification bundlePath, err := validBundle(r.ID, r.Bundle) if err != nil { diff --git a/containerd-shim-v2/create_test.go b/containerd-shim-v2/create_test.go index eb5b67ad3..88b0b948f 100644 --- a/containerd-shim-v2/create_test.go +++ b/containerd-shim-v2/create_test.go @@ -15,14 +15,14 @@ import ( "github.com/containerd/containerd/namespaces" taskAPI "github.com/containerd/containerd/runtime/v2/task" - - vc "github.com/kata-containers/runtime/virtcontainers" - "github.com/kata-containers/runtime/virtcontainers/pkg/vcmock" + specs "github.com/opencontainers/runtime-spec/specs-go" + "github.com/stretchr/testify/assert" ktu "github.com/kata-containers/runtime/pkg/katatestutils" "github.com/kata-containers/runtime/pkg/katautils" - specs "github.com/opencontainers/runtime-spec/specs-go" - "github.com/stretchr/testify/assert" + vc "github.com/kata-containers/runtime/virtcontainers" + "github.com/kata-containers/runtime/virtcontainers/pkg/oci" + "github.com/kata-containers/runtime/virtcontainers/pkg/vcmock" ) func TestCreateSandboxSuccess(t *testing.T) { @@ -62,7 +62,7 @@ func TestCreateSandboxSuccess(t *testing.T) { ociConfigFile := filepath.Join(bundlePath, "config.json") assert.True(katautils.FileExists(ociConfigFile)) - spec, err := readOCIConfigFile(ociConfigFile) + spec, err := oci.ParseConfigJSON(bundlePath) assert.NoError(err) // Force sandbox-type container @@ -120,7 +120,7 @@ func TestCreateSandboxFail(t *testing.T) { ociConfigFile := filepath.Join(bundlePath, "config.json") assert.True(katautils.FileExists(ociConfigFile)) - spec, err := readOCIConfigFile(ociConfigFile) + spec, err := oci.ParseConfigJSON(bundlePath) assert.NoError(err) err = writeOCIConfigFile(spec, ociConfigFile) @@ -167,7 +167,7 @@ func TestCreateSandboxConfigFail(t *testing.T) { ociConfigFile := filepath.Join(bundlePath, "config.json") assert.True(katautils.FileExists(ociConfigFile)) - spec, err := readOCIConfigFile(ociConfigFile) + spec, err := oci.ParseConfigJSON(bundlePath) assert.NoError(err) quota := int64(0) @@ -231,7 +231,7 @@ func TestCreateContainerSuccess(t *testing.T) { ociConfigFile := filepath.Join(bundlePath, "config.json") assert.True(katautils.FileExists(ociConfigFile)) - spec, err := readOCIConfigFile(ociConfigFile) + spec, err := oci.ParseConfigJSON(bundlePath) assert.NoError(err) // set expected container type and sandboxID @@ -280,7 +280,7 @@ func TestCreateContainerFail(t *testing.T) { ociConfigFile := filepath.Join(bundlePath, "config.json") assert.True(katautils.FileExists(ociConfigFile)) - spec, err := readOCIConfigFile(ociConfigFile) + spec, err := oci.ParseConfigJSON(bundlePath) assert.NoError(err) spec.Annotations = make(map[string]string) @@ -340,7 +340,7 @@ func TestCreateContainerConfigFail(t *testing.T) { ociConfigFile := filepath.Join(bundlePath, "config.json") assert.True(katautils.FileExists(ociConfigFile)) - spec, err := readOCIConfigFile(ociConfigFile) + spec, err := oci.ParseConfigJSON(bundlePath) assert.NoError(err) // set the error containerType diff --git a/containerd-shim-v2/delete_test.go b/containerd-shim-v2/delete_test.go index 995e3d84e..3bc4dfb7d 100644 --- a/containerd-shim-v2/delete_test.go +++ b/containerd-shim-v2/delete_test.go @@ -13,8 +13,10 @@ import ( "testing" taskAPI "github.com/containerd/containerd/runtime/v2/task" - "github.com/kata-containers/runtime/virtcontainers/pkg/vcmock" "github.com/stretchr/testify/assert" + + "github.com/kata-containers/runtime/virtcontainers/pkg/oci" + "github.com/kata-containers/runtime/virtcontainers/pkg/vcmock" ) func TestDeleteContainerSuccessAndFail(t *testing.T) { @@ -24,9 +26,9 @@ func TestDeleteContainerSuccessAndFail(t *testing.T) { MockID: testSandboxID, } - rootPath, configPath := testConfigSetup(t) + rootPath, bundlePath := testConfigSetup(t) defer os.RemoveAll(rootPath) - _, err := readOCIConfigJSON(configPath) + _, err := oci.ParseConfigJSON(bundlePath) assert.NoError(err) s := &service{ @@ -42,20 +44,18 @@ func TestDeleteContainerSuccessAndFail(t *testing.T) { assert.NoError(err) } -func testConfigSetup(t *testing.T) (rootPath string, configPath string) { +func testConfigSetup(t *testing.T) (rootPath string, bundlePath string) { assert := assert.New(t) tmpdir, err := ioutil.TempDir("", "") assert.NoError(err) - bundlePath := filepath.Join(tmpdir, "bundle") + bundlePath = filepath.Join(tmpdir, "bundle") err = os.MkdirAll(bundlePath, testDirMode) assert.NoError(err) err = createOCIConfig(bundlePath) assert.NoError(err) - // config json path - configPath = filepath.Join(bundlePath, "config.json") - return tmpdir, configPath + return tmpdir, bundlePath } diff --git a/containerd-shim-v2/service.go b/containerd-shim-v2/service.go index 42f317816..a46fe8c2b 100644 --- a/containerd-shim-v2/service.go +++ b/containerd-shim-v2/service.go @@ -296,7 +296,7 @@ func (s *service) Cleanup(ctx context.Context) (_ *taskAPI.DeleteResponse, err e return nil, err } - containerType, err := ociSpec.ContainerType() + containerType, err := oci.ContainerType(ociSpec) if err != nil { return nil, err } @@ -308,7 +308,7 @@ func (s *service) Cleanup(ctx context.Context) (_ *taskAPI.DeleteResponse, err e return nil, err } case vc.PodContainer: - sandboxID, err := ociSpec.SandboxID() + sandboxID, err := oci.SandboxID(ociSpec) if err != nil { return nil, err } diff --git a/containerd-shim-v2/utils.go b/containerd-shim-v2/utils.go index 7580e2c0e..eee923595 100644 --- a/containerd-shim-v2/utils.go +++ b/containerd-shim-v2/utils.go @@ -92,13 +92,13 @@ func getAddress(ctx context.Context, bundlePath, id string) (string, error) { return "", err } - containerType, err := ociSpec.ContainerType() + containerType, err := oci.ContainerType(ociSpec) if err != nil { return "", err } if containerType == vc.PodContainer { - sandboxID, err := ociSpec.SandboxID() + sandboxID, err := oci.SandboxID(ociSpec) if err != nil { return "", err } @@ -124,7 +124,7 @@ func noNeedForOutput(detach bool, tty bool) bool { return true } -func removeNamespace(s *oci.CompatOCISpec, nsType specs.LinuxNamespaceType) { +func removeNamespace(s *specs.Spec, nsType specs.LinuxNamespaceType) { for i, n := range s.Linux.Namespaces { if n.Type == nsType { s.Linux.Namespaces = append(s.Linux.Namespaces[:i], s.Linux.Namespaces[i+1:]...) diff --git a/containerd-shim-v2/utils_test.go b/containerd-shim-v2/utils_test.go index 5a636b893..2470547f3 100644 --- a/containerd-shim-v2/utils_test.go +++ b/containerd-shim-v2/utils_test.go @@ -17,6 +17,8 @@ import ( "path/filepath" "strings" + "github.com/opencontainers/runtime-spec/specs-go" + ktu "github.com/kata-containers/runtime/pkg/katatestutils" "github.com/kata-containers/runtime/pkg/katautils" vc "github.com/kata-containers/runtime/virtcontainers" @@ -213,29 +215,6 @@ func newTestRuntimeConfig(dir, consolePath string, create bool) (oci.RuntimeConf }, nil } -// readOCIConfig returns an OCI spec. -func readOCIConfigFile(configPath string) (oci.CompatOCISpec, error) { - if configPath == "" { - return oci.CompatOCISpec{}, errors.New("BUG: need config file path") - } - - data, err := ioutil.ReadFile(configPath) - if err != nil { - return oci.CompatOCISpec{}, err - } - - var ociSpec oci.CompatOCISpec - if err := json.Unmarshal(data, &ociSpec); err != nil { - return oci.CompatOCISpec{}, err - } - caps, err := oci.ContainerCapabilities(ociSpec) - if err != nil { - return oci.CompatOCISpec{}, err - } - ociSpec.Process.Capabilities = caps - return ociSpec, nil -} - // realMakeOCIBundle will create an OCI bundle (including the "config.json" // config file) in the directory specified (which must already exist). // @@ -336,7 +315,7 @@ func createRootfs(dir string) error { return nil } -func writeOCIConfigFile(spec oci.CompatOCISpec, configPath string) error { +func writeOCIConfigFile(spec specs.Spec, configPath string) error { if configPath == "" { return errors.New("BUG: need config file path") } @@ -348,18 +327,3 @@ func writeOCIConfigFile(spec oci.CompatOCISpec, configPath string) error { return ioutil.WriteFile(configPath, bytes, testFileMode) } - -// Read fail that should contain a CompatOCISpec and -// return its JSON representation on success -func readOCIConfigJSON(configFile string) (string, error) { - bundlePath := filepath.Dir(configFile) - ociSpec, err := oci.ParseConfigJSON(bundlePath) - if err != nil { - return "", nil - } - ociSpecJSON, err := json.Marshal(ociSpec) - if err != nil { - return "", err - } - return string(ociSpecJSON), err -} diff --git a/pkg/katautils/create.go b/pkg/katautils/create.go index aa4a0747e..facf27324 100644 --- a/pkg/katautils/create.go +++ b/pkg/katautils/create.go @@ -13,6 +13,7 @@ import ( vc "github.com/kata-containers/runtime/virtcontainers" vf "github.com/kata-containers/runtime/virtcontainers/factory" "github.com/kata-containers/runtime/virtcontainers/pkg/oci" + specs "github.com/opencontainers/runtime-spec/specs-go" ) // GetKernelParamsFunc use a variable to allow tests to modify its value @@ -87,7 +88,7 @@ func HandleFactory(ctx context.Context, vci vc.VC, runtimeConfig *oci.RuntimeCon // For the given pod ephemeral volume is created only once // backed by tmpfs inside the VM. For successive containers // of the same pod the already existing volume is reused. -func SetEphemeralStorageType(ociSpec oci.CompatOCISpec) oci.CompatOCISpec { +func SetEphemeralStorageType(ociSpec specs.Spec) specs.Spec { for idx, mnt := range ociSpec.Mounts { if vc.IsEphemeralStorage(mnt.Source) { ociSpec.Mounts[idx].Type = vc.KataEphemeralDevType @@ -100,7 +101,7 @@ func SetEphemeralStorageType(ociSpec oci.CompatOCISpec) oci.CompatOCISpec { } // CreateSandbox create a sandbox container -func CreateSandbox(ctx context.Context, vci vc.VC, ociSpec oci.CompatOCISpec, runtimeConfig oci.RuntimeConfig, rootFs vc.RootFs, +func CreateSandbox(ctx context.Context, vci vc.VC, ociSpec specs.Spec, runtimeConfig oci.RuntimeConfig, rootFs vc.RootFs, containerID, bundlePath, console string, disableOutput, systemdCgroup, builtIn bool) (_ vc.VCSandbox, _ vc.Process, err error) { span, ctx := Trace(ctx, "createSandbox") defer span.Finish() @@ -175,7 +176,7 @@ func CreateSandbox(ctx context.Context, vci vc.VC, ociSpec oci.CompatOCISpec, ru } // CreateContainer create a container -func CreateContainer(ctx context.Context, vci vc.VC, sandbox vc.VCSandbox, ociSpec oci.CompatOCISpec, rootFs vc.RootFs, containerID, bundlePath, console string, disableOutput, builtIn bool) (vc.Process, error) { +func CreateContainer(ctx context.Context, vci vc.VC, sandbox vc.VCSandbox, ociSpec specs.Spec, rootFs vc.RootFs, containerID, bundlePath, console string, disableOutput, builtIn bool) (vc.Process, error) { var c vc.VCContainer span, ctx := Trace(ctx, "createContainer") @@ -199,7 +200,7 @@ func CreateContainer(ctx context.Context, vci vc.VC, sandbox vc.VCSandbox, ociSp contConfig.RootFs = rootFs } - sandboxID, err := ociSpec.SandboxID() + sandboxID, err := oci.SandboxID(ociSpec) if err != nil { return vc.Process{}, err } diff --git a/pkg/katautils/create_test.go b/pkg/katautils/create_test.go index 8fc928334..5170927d1 100644 --- a/pkg/katautils/create_test.go +++ b/pkg/katautils/create_test.go @@ -47,30 +47,7 @@ func init() { tc = ktu.NewTestConstraint(false) } -// readOCIConfig returns an OCI spec. -func readOCIConfigFile(configPath string) (oci.CompatOCISpec, error) { - if configPath == "" { - return oci.CompatOCISpec{}, errors.New("BUG: need config file path") - } - - data, err := ioutil.ReadFile(configPath) - if err != nil { - return oci.CompatOCISpec{}, err - } - - var ociSpec oci.CompatOCISpec - if err := json.Unmarshal(data, &ociSpec); err != nil { - return oci.CompatOCISpec{}, err - } - caps, err := oci.ContainerCapabilities(ociSpec) - if err != nil { - return oci.CompatOCISpec{}, err - } - ociSpec.Process.Capabilities = caps - return ociSpec, nil -} - -func writeOCIConfigFile(spec oci.CompatOCISpec, configPath string) error { +func writeOCIConfigFile(spec specs.Spec, configPath string) error { if configPath == "" { return errors.New("BUG: need config file path") } @@ -205,7 +182,7 @@ func TestSetEphemeralStorageType(t *testing.T) { assert.Nil(err) defer syscall.Unmount(ephePath, 0) - ociSpec := oci.CompatOCISpec{} + ociSpec := specs.Spec{} var ociMounts []specs.Mount mount := specs.Mount{ Source: ephePath, @@ -298,7 +275,7 @@ func TestCreateSandboxConfigFail(t *testing.T) { ociConfigFile := filepath.Join(bundlePath, "config.json") assert.True(FileExists(ociConfigFile)) - spec, err := readOCIConfigFile(ociConfigFile) + spec, err := oci.ParseConfigJSON(bundlePath) assert.NoError(err) quota := int64(0) @@ -346,7 +323,7 @@ func TestCreateSandboxFail(t *testing.T) { ociConfigFile := filepath.Join(bundlePath, "config.json") assert.True(FileExists(ociConfigFile)) - spec, err := readOCIConfigFile(ociConfigFile) + spec, err := oci.ParseConfigJSON(bundlePath) assert.NoError(err) rootFs := vc.RootFs{Mounted: true} @@ -376,7 +353,7 @@ func TestCreateContainerContainerConfigFail(t *testing.T) { ociConfigFile := filepath.Join(bundlePath, "config.json") assert.True(FileExists(ociConfigFile)) - spec, err := readOCIConfigFile(ociConfigFile) + spec, err := oci.ParseConfigJSON(bundlePath) assert.NoError(err) // Set invalid container type @@ -419,7 +396,7 @@ func TestCreateContainerFail(t *testing.T) { ociConfigFile := filepath.Join(bundlePath, "config.json") assert.True(FileExists(ociConfigFile)) - spec, err := readOCIConfigFile(ociConfigFile) + spec, err := oci.ParseConfigJSON(bundlePath) assert.NoError(err) // set expected container type and sandboxID @@ -469,7 +446,7 @@ func TestCreateContainer(t *testing.T) { ociConfigFile := filepath.Join(bundlePath, "config.json") assert.True(FileExists(ociConfigFile)) - spec, err := readOCIConfigFile(ociConfigFile) + spec, err := oci.ParseConfigJSON(bundlePath) assert.NoError(err) // set expected container type and sandboxID diff --git a/pkg/katautils/hook.go b/pkg/katautils/hook.go index c73ac2ab6..0f20a95ba 100644 --- a/pkg/katautils/hook.go +++ b/pkg/katautils/hook.go @@ -16,7 +16,6 @@ import ( "syscall" "time" - "github.com/kata-containers/runtime/virtcontainers/pkg/oci" "github.com/opencontainers/runtime-spec/specs-go" "github.com/opentracing/opentracing-go/log" "github.com/sirupsen/logrus" @@ -111,7 +110,7 @@ func runHooks(ctx context.Context, hooks []specs.Hook, cid, bundlePath, hookType } // PreStartHooks run the hooks before start container -func PreStartHooks(ctx context.Context, spec oci.CompatOCISpec, cid, bundlePath string) error { +func PreStartHooks(ctx context.Context, spec specs.Spec, cid, bundlePath string) error { // If no hook available, nothing needs to be done. if spec.Hooks == nil { return nil @@ -121,7 +120,7 @@ func PreStartHooks(ctx context.Context, spec oci.CompatOCISpec, cid, bundlePath } // PostStartHooks run the hooks just after start container -func PostStartHooks(ctx context.Context, spec oci.CompatOCISpec, cid, bundlePath string) error { +func PostStartHooks(ctx context.Context, spec specs.Spec, cid, bundlePath string) error { // If no hook available, nothing needs to be done. if spec.Hooks == nil { return nil @@ -131,7 +130,7 @@ func PostStartHooks(ctx context.Context, spec oci.CompatOCISpec, cid, bundlePath } // PostStopHooks run the hooks after stop container -func PostStopHooks(ctx context.Context, spec oci.CompatOCISpec, cid, bundlePath string) error { +func PostStopHooks(ctx context.Context, spec specs.Spec, cid, bundlePath string) error { // If no hook available, nothing needs to be done. if spec.Hooks == nil { return nil diff --git a/pkg/katautils/hook_test.go b/pkg/katautils/hook_test.go index 83dc65585..4cc0b6227 100644 --- a/pkg/katautils/hook_test.go +++ b/pkg/katautils/hook_test.go @@ -13,7 +13,6 @@ import ( ktu "github.com/kata-containers/runtime/pkg/katatestutils" . "github.com/kata-containers/runtime/virtcontainers/pkg/mock" - "github.com/kata-containers/runtime/virtcontainers/pkg/oci" "github.com/opencontainers/runtime-spec/specs-go" "github.com/stretchr/testify/assert" ) @@ -96,26 +95,22 @@ func TestPreStartHooks(t *testing.T) { ctx := context.Background() // Hooks field is nil - spec := oci.CompatOCISpec{} + spec := specs.Spec{} err := PreStartHooks(ctx, spec, "", "") assert.NoError(err) // Hooks list is empty - spec = oci.CompatOCISpec{ - Spec: specs.Spec{ - Hooks: &specs.Hooks{}, - }, + spec = specs.Spec{ + Hooks: &specs.Hooks{}, } err = PreStartHooks(ctx, spec, "", "") assert.NoError(err) // Run with timeout 0 hook := createHook(0) - spec = oci.CompatOCISpec{ - Spec: specs.Spec{ - Hooks: &specs.Hooks{ - Prestart: []specs.Hook{hook}, - }, + spec = specs.Spec{ + Hooks: &specs.Hooks{ + Prestart: []specs.Hook{hook}, }, } err = PreStartHooks(ctx, spec, testSandboxID, testBundlePath) @@ -123,11 +118,9 @@ func TestPreStartHooks(t *testing.T) { // Failure due to wrong hook hook = createWrongHook() - spec = oci.CompatOCISpec{ - Spec: specs.Spec{ - Hooks: &specs.Hooks{ - Prestart: []specs.Hook{hook}, - }, + spec = specs.Spec{ + Hooks: &specs.Hooks{ + Prestart: []specs.Hook{hook}, }, } err = PreStartHooks(ctx, spec, testSandboxID, testBundlePath) @@ -144,26 +137,22 @@ func TestPostStartHooks(t *testing.T) { ctx := context.Background() // Hooks field is nil - spec := oci.CompatOCISpec{} + spec := specs.Spec{} err := PostStartHooks(ctx, spec, "", "") assert.NoError(err) // Hooks list is empty - spec = oci.CompatOCISpec{ - Spec: specs.Spec{ - Hooks: &specs.Hooks{}, - }, + spec = specs.Spec{ + Hooks: &specs.Hooks{}, } err = PostStartHooks(ctx, spec, "", "") assert.NoError(err) // Run with timeout 0 hook := createHook(0) - spec = oci.CompatOCISpec{ - Spec: specs.Spec{ - Hooks: &specs.Hooks{ - Poststart: []specs.Hook{hook}, - }, + spec = specs.Spec{ + Hooks: &specs.Hooks{ + Poststart: []specs.Hook{hook}, }, } err = PostStartHooks(ctx, spec, testSandboxID, testBundlePath) @@ -171,11 +160,9 @@ func TestPostStartHooks(t *testing.T) { // Failure due to wrong hook hook = createWrongHook() - spec = oci.CompatOCISpec{ - Spec: specs.Spec{ - Hooks: &specs.Hooks{ - Poststart: []specs.Hook{hook}, - }, + spec = specs.Spec{ + Hooks: &specs.Hooks{ + Poststart: []specs.Hook{hook}, }, } err = PostStartHooks(ctx, spec, testSandboxID, testBundlePath) @@ -192,26 +179,22 @@ func TestPostStopHooks(t *testing.T) { ctx := context.Background() // Hooks field is nil - spec := oci.CompatOCISpec{} + spec := specs.Spec{} err := PostStopHooks(ctx, spec, "", "") assert.NoError(err) // Hooks list is empty - spec = oci.CompatOCISpec{ - Spec: specs.Spec{ - Hooks: &specs.Hooks{}, - }, + spec = specs.Spec{ + Hooks: &specs.Hooks{}, } err = PostStopHooks(ctx, spec, "", "") assert.NoError(err) // Run with timeout 0 hook := createHook(0) - spec = oci.CompatOCISpec{ - Spec: specs.Spec{ - Hooks: &specs.Hooks{ - Poststop: []specs.Hook{hook}, - }, + spec = specs.Spec{ + Hooks: &specs.Hooks{ + Poststop: []specs.Hook{hook}, }, } err = PostStopHooks(ctx, spec, testSandboxID, testBundlePath) @@ -219,11 +202,9 @@ func TestPostStopHooks(t *testing.T) { // Failure due to wrong hook hook = createWrongHook() - spec = oci.CompatOCISpec{ - Spec: specs.Spec{ - Hooks: &specs.Hooks{ - Poststop: []specs.Hook{hook}, - }, + spec = specs.Spec{ + Hooks: &specs.Hooks{ + Poststop: []specs.Hook{hook}, }, } err = PostStopHooks(ctx, spec, testSandboxID, testBundlePath) diff --git a/virtcontainers/pkg/oci/utils.go b/virtcontainers/pkg/oci/utils.go index e1e25c3b1..c4f26c8de 100644 --- a/virtcontainers/pkg/oci/utils.go +++ b/virtcontainers/pkg/oci/utils.go @@ -18,7 +18,7 @@ import ( criContainerdAnnotations "github.com/containerd/cri-containerd/pkg/annotations" crioAnnotations "github.com/cri-o/cri-o/pkg/annotations" - spec "github.com/opencontainers/runtime-spec/specs-go" + specs "github.com/opencontainers/runtime-spec/specs-go" "github.com/sirupsen/logrus" vc "github.com/kata-containers/runtime/virtcontainers" @@ -75,24 +75,24 @@ const ( const KernelModulesSeparator = ";" -// CompatOCIProcess is a structure inheriting from spec.Process defined +// compatOCIProcess is a structure inheriting from specs.Process defined // in runtime-spec/specs-go package. The goal is to be compatible with // both v1.0.0-rc4 and v1.0.0-rc5 since the latter introduced a change // about the type of the Capabilities field. // Refer to: https://github.com/opencontainers/runtime-spec/commit/37391fb -type CompatOCIProcess struct { - spec.Process +type compatOCIProcess struct { + specs.Process Capabilities interface{} `json:"capabilities,omitempty" platform:"linux"` //nolint:govet } -// CompatOCISpec is a structure inheriting from spec.Spec defined +// compatOCISpec is a structure inheriting from specs.Spec defined // in runtime-spec/specs-go package. It relies on the CompatOCIProcess // structure declared above, in order to be compatible with both // v1.0.0-rc4 and v1.0.0-rc5. // Refer to: https://github.com/opencontainers/runtime-spec/commit/37391fb -type CompatOCISpec struct { - spec.Spec - Process *CompatOCIProcess `json:"process,omitempty"` //nolint:govet +type compatOCISpec struct { + specs.Spec + Process *compatOCIProcess `json:"process,omitempty"` //nolint:govet } // FactoryConfig is a structure to set the VM factory configuration. @@ -162,7 +162,7 @@ func SetLogger(ctx context.Context, logger *logrus.Entry) { ociLog = logger.WithFields(fields) } -func cmdEnvs(spec CompatOCISpec, envs []types.EnvVar) []types.EnvVar { +func cmdEnvs(spec specs.Spec, envs []types.EnvVar) []types.EnvVar { for _, env := range spec.Process.Env { kv := strings.Split(env, "=") if len(kv) < 2 { @@ -179,7 +179,7 @@ func cmdEnvs(spec CompatOCISpec, envs []types.EnvVar) []types.EnvVar { return envs } -func newMount(m spec.Mount) vc.Mount { +func newMount(m specs.Mount) vc.Mount { return vc.Mount{ Source: m.Source, Destination: m.Destination, @@ -188,8 +188,8 @@ func newMount(m spec.Mount) vc.Mount { } } -func containerMounts(spec CompatOCISpec) []vc.Mount { - ociMounts := spec.Spec.Mounts +func containerMounts(spec specs.Spec) []vc.Mount { + ociMounts := spec.Mounts if ociMounts == nil { return []vc.Mount{} @@ -212,7 +212,7 @@ func contains(s []string, e string) bool { return false } -func newLinuxDeviceInfo(d spec.LinuxDevice) (*config.DeviceInfo, error) { +func newLinuxDeviceInfo(d specs.LinuxDevice) (*config.DeviceInfo, error) { allowedDeviceTypes := []string{"c", "b", "u", "p"} if !contains(allowedDeviceTypes, d.Type) { @@ -244,8 +244,8 @@ func newLinuxDeviceInfo(d spec.LinuxDevice) (*config.DeviceInfo, error) { return &deviceInfo, nil } -func containerDeviceInfos(spec CompatOCISpec) ([]config.DeviceInfo, error) { - ociLinuxDevices := spec.Spec.Linux.Devices +func containerDeviceInfos(spec specs.Spec) ([]config.DeviceInfo, error) { + ociLinuxDevices := spec.Linux.Devices if ociLinuxDevices == nil { return []config.DeviceInfo{}, nil @@ -264,9 +264,14 @@ func containerDeviceInfos(spec CompatOCISpec) ([]config.DeviceInfo, error) { return devices, nil } -func containerCapabilities(s CompatOCISpec) (types.LinuxCapabilities, error) { +// containerCapabilities return a LinuxCapabilities for virtcontainer +func containerCapabilities(s compatOCISpec) (specs.LinuxCapabilities, error) { + if s.Process == nil { + return specs.LinuxCapabilities{}, fmt.Errorf("containerCapabilities, Process is nil") + } + capabilities := s.Process.Capabilities - var c types.LinuxCapabilities + var c specs.LinuxCapabilities // In spec v1.0.0-rc4, capabilities was a list of strings. This was changed // to an object with v1.0.0-rc5. @@ -305,7 +310,7 @@ func containerCapabilities(s CompatOCISpec) (types.LinuxCapabilities, error) { list = append(list, str.(string)) } - c = types.LinuxCapabilities{ + c = specs.LinuxCapabilities{ Bounding: list, Effective: list, Inheritable: list, @@ -322,15 +327,7 @@ func containerCapabilities(s CompatOCISpec) (types.LinuxCapabilities, error) { return c, nil } -// ContainerCapabilities return a LinuxCapabilities for virtcontainer -func ContainerCapabilities(s CompatOCISpec) (types.LinuxCapabilities, error) { - if s.Process == nil { - return types.LinuxCapabilities{}, fmt.Errorf("ContainerCapabilities, Process is nil") - } - return containerCapabilities(s) -} - -func networkConfig(ocispec CompatOCISpec, config RuntimeConfig) (vc.NetworkConfig, error) { +func networkConfig(ocispec specs.Spec, config RuntimeConfig) (vc.NetworkConfig, error) { linux := ocispec.Linux if linux == nil { return vc.NetworkConfig{}, ErrNoLinux @@ -339,7 +336,7 @@ func networkConfig(ocispec CompatOCISpec, config RuntimeConfig) (vc.NetworkConfi var netConf vc.NetworkConfig for _, n := range linux.Namespaces { - if n.Type != spec.NetworkNamespace { + if n.Type != specs.NetworkNamespace { continue } @@ -366,26 +363,29 @@ func getConfigPath(bundlePath string) string { } // ParseConfigJSON unmarshals the config.json file. -func ParseConfigJSON(bundlePath string) (CompatOCISpec, error) { +func ParseConfigJSON(bundlePath string) (specs.Spec, error) { configPath := getConfigPath(bundlePath) ociLog.Debugf("converting %s", configPath) configByte, err := ioutil.ReadFile(configPath) if err != nil { - return CompatOCISpec{}, err + return specs.Spec{}, err } - var ocispec CompatOCISpec - if err := json.Unmarshal(configByte, &ocispec); err != nil { - return CompatOCISpec{}, err + var compSpec compatOCISpec + if err := json.Unmarshal(configByte, &compSpec); err != nil { + return specs.Spec{}, err } - caps, err := ContainerCapabilities(ocispec) + + caps, err := containerCapabilities(compSpec) if err != nil { - return CompatOCISpec{}, err + return specs.Spec{}, err } - ocispec.Process.Capabilities = caps - return ocispec, nil + compSpec.Spec.Process = &compSpec.Process.Process + compSpec.Spec.Process.Capabilities = &caps + + return compSpec.Spec, nil } // GetContainerType determines which type of container matches the annotations @@ -402,7 +402,7 @@ func GetContainerType(annotations map[string]string) (vc.ContainerType, error) { // ContainerType returns the type of container and if the container type was // found from CRI servers annotations. -func (spec *CompatOCISpec) ContainerType() (vc.ContainerType, error) { +func ContainerType(spec specs.Spec) (vc.ContainerType, error) { for _, key := range CRIContainerTypeKeyList { containerTypeVal, ok := spec.Annotations[key] if !ok { @@ -424,7 +424,7 @@ func (spec *CompatOCISpec) ContainerType() (vc.ContainerType, error) { // SandboxID determines the sandbox ID related to an OCI configuration. This function // is expected to be called only when the container type is "PodContainer". -func (spec *CompatOCISpec) SandboxID() (string, error) { +func SandboxID(spec specs.Spec) (string, error) { for _, key := range CRISandboxNameKeyList { sandboxID, ok := spec.Annotations[key] if ok { @@ -435,7 +435,7 @@ func (spec *CompatOCISpec) SandboxID() (string, error) { return "", fmt.Errorf("Could not find sandbox ID") } -func addAssetAnnotations(ocispec CompatOCISpec, config *vc.SandboxConfig) { +func addAssetAnnotations(ocispec specs.Spec, config *vc.SandboxConfig) { assetAnnotations := []string{ vcAnnotations.KernelPath, vcAnnotations.ImagePath, @@ -465,7 +465,7 @@ func addAssetAnnotations(ocispec CompatOCISpec, config *vc.SandboxConfig) { // SandboxConfig converts an OCI compatible runtime configuration file // to a virtcontainers sandbox configuration structure. -func SandboxConfig(ocispec CompatOCISpec, runtime RuntimeConfig, bundlePath, cid, console string, detach, systemdCgroup bool) (vc.SandboxConfig, error) { +func SandboxConfig(ocispec specs.Spec, runtime RuntimeConfig, bundlePath, cid, console string, detach, systemdCgroup bool) (vc.SandboxConfig, error) { containerConfig, err := ContainerConfig(ocispec, bundlePath, cid, console, detach) if err != nil { return vc.SandboxConfig{}, err @@ -528,7 +528,7 @@ func SandboxConfig(ocispec CompatOCISpec, runtime RuntimeConfig, bundlePath, cid // ContainerConfig converts an OCI compatible runtime configuration // file to a virtcontainers container configuration structure. -func ContainerConfig(ocispec CompatOCISpec, bundlePath, cid, console string, detach bool) (vc.ContainerConfig, error) { +func ContainerConfig(ocispec specs.Spec, bundlePath, cid, console string, detach bool) (vc.ContainerConfig, error) { ociSpecJSON, err := json.Marshal(ocispec) if err != nil { return vc.ContainerConfig{}, err @@ -564,17 +564,13 @@ func ContainerConfig(ocispec CompatOCISpec, bundlePath, cid, console string, det } if ocispec.Process != nil { - caps, ok := ocispec.Process.Capabilities.(types.LinuxCapabilities) - if !ok { - return vc.ContainerConfig{}, fmt.Errorf("Unexpected format for capabilities: %v", ocispec.Process.Capabilities) - } - cmd.Capabilities = caps + cmd.Capabilities = ocispec.Process.Capabilities } containerConfig := vc.ContainerConfig{ ID: cid, RootFs: rootfs, - ReadonlyRootfs: ocispec.Spec.Root.Readonly, + ReadonlyRootfs: ocispec.Root.Readonly, Cmd: cmd, Annotations: map[string]string{ vcAnnotations.ConfigJSONKey: string(ociSpecJSON), @@ -585,7 +581,7 @@ func ContainerConfig(ocispec CompatOCISpec, bundlePath, cid, console string, det Resources: *ocispec.Linux.Resources, } - cType, err := ocispec.ContainerType() + cType, err := ContainerType(ocispec) if err != nil { return vc.ContainerConfig{}, err } @@ -622,9 +618,9 @@ func getShmSize(c vc.ContainerConfig) (uint64, error) { } // StatusToOCIState translates a virtcontainers container status into an OCI state. -func StatusToOCIState(status vc.ContainerStatus) spec.State { - return spec.State{ - Version: spec.Version, +func StatusToOCIState(status vc.ContainerStatus) specs.State { + return specs.State{ + Version: specs.Version, ID: status.ID, Status: StateToOCIState(status.State.State), Pid: status.PID, @@ -684,15 +680,15 @@ func EnvVars(envs []string) ([]types.EnvVar, error) { // GetOCIConfig returns an OCI spec configuration from the annotation // stored into the container status. -func GetOCIConfig(status vc.ContainerStatus) (CompatOCISpec, error) { +func GetOCIConfig(status vc.ContainerStatus) (specs.Spec, error) { ociConfigStr, ok := status.Annotations[vcAnnotations.ConfigJSONKey] if !ok { - return CompatOCISpec{}, fmt.Errorf("Annotation[%s] not found", vcAnnotations.ConfigJSONKey) + return specs.Spec{}, fmt.Errorf("Annotation[%s] not found", vcAnnotations.ConfigJSONKey) } - var ociSpec CompatOCISpec + var ociSpec specs.Spec if err := json.Unmarshal([]byte(ociConfigStr), &ociSpec); err != nil { - return CompatOCISpec{}, err + return specs.Spec{}, err } return ociSpec, nil diff --git a/virtcontainers/pkg/oci/utils_test.go b/virtcontainers/pkg/oci/utils_test.go index d6edeb8e3..dafb2451c 100644 --- a/virtcontainers/pkg/oci/utils_test.go +++ b/virtcontainers/pkg/oci/utils_test.go @@ -135,7 +135,7 @@ func TestMinimalSandboxConfig(t *testing.T) { Interactive: true, Console: consolePath, NoNewPrivileges: true, - Capabilities: types.LinuxCapabilities{ + Capabilities: &specs.LinuxCapabilities{ Bounding: capList, Effective: capList, Inheritable: capList, @@ -168,17 +168,18 @@ func TestMinimalSandboxConfig(t *testing.T) { }, } - var minimalOCISpec CompatOCISpec + var minimalOCISpec compatOCISpec //Marshal and unmarshall json to compare sandboxConfig and expectedSandboxConfig err = json.Unmarshal([]byte(minimalConfig), &minimalOCISpec) assert.NoError(err) if minimalOCISpec.Process != nil { - caps, err := ContainerCapabilities(minimalOCISpec) + caps, err := containerCapabilities(minimalOCISpec) assert.NoError(err) - minimalOCISpec.Process.Capabilities = caps + minimalOCISpec.Spec.Process = &minimalOCISpec.Process.Process + minimalOCISpec.Spec.Process.Capabilities = &caps } - ociSpecJSON, err := json.Marshal(minimalOCISpec) + ociSpecJSON, err := json.Marshal(minimalOCISpec.Spec) assert.NoError(err) devInfo := config.DeviceInfo{ @@ -489,8 +490,8 @@ func TestGetContainerTypeFailure(t *testing.T) { assert.Equal(containerType, expected) } -func testContainerTypeSuccessful(t *testing.T, ociSpec CompatOCISpec, expected vc.ContainerType) { - containerType, err := ociSpec.ContainerType() +func testContainerTypeSuccessful(t *testing.T, ociSpec specs.Spec, expected vc.ContainerType) { + containerType, err := ContainerType(ociSpec) assert := assert.New(t) assert.NoError(err) @@ -498,7 +499,7 @@ func testContainerTypeSuccessful(t *testing.T, ociSpec CompatOCISpec, expected v } func TestContainerTypePodSandbox(t *testing.T) { - var ociSpec CompatOCISpec + var ociSpec specs.Spec ociSpec.Annotations = map[string]string{ annotations.ContainerType: annotations.ContainerTypeSandbox, @@ -508,7 +509,7 @@ func TestContainerTypePodSandbox(t *testing.T) { } func TestContainerTypePodContainer(t *testing.T) { - var ociSpec CompatOCISpec + var ociSpec specs.Spec ociSpec.Annotations = map[string]string{ annotations.ContainerType: annotations.ContainerTypeContainer, @@ -518,11 +519,11 @@ func TestContainerTypePodContainer(t *testing.T) { } func TestContainerTypePodSandboxEmptyAnnotation(t *testing.T) { - testContainerTypeSuccessful(t, CompatOCISpec{}, vc.PodSandbox) + testContainerTypeSuccessful(t, specs.Spec{}, vc.PodSandbox) } func TestContainerTypeFailure(t *testing.T) { - var ociSpec CompatOCISpec + var ociSpec specs.Spec expected := vc.UnknownContainerType unknownType := "unknown_type" assert := assert.New(t) @@ -531,13 +532,13 @@ func TestContainerTypeFailure(t *testing.T) { annotations.ContainerType: unknownType, } - containerType, err := ociSpec.ContainerType() + containerType, err := ContainerType(ociSpec) assert.Error(err) assert.Equal(containerType, expected) } func TestSandboxIDSuccessful(t *testing.T) { - var ociSpec CompatOCISpec + var ociSpec specs.Spec testSandboxID := "testSandboxID" assert := assert.New(t) @@ -545,16 +546,16 @@ func TestSandboxIDSuccessful(t *testing.T) { annotations.SandboxID: testSandboxID, } - sandboxID, err := ociSpec.SandboxID() + sandboxID, err := SandboxID(ociSpec) assert.NoError(err) assert.Equal(sandboxID, testSandboxID) } func TestSandboxIDFailure(t *testing.T) { - var ociSpec CompatOCISpec + var ociSpec specs.Spec assert := assert.New(t) - sandboxID, err := ociSpec.SandboxID() + sandboxID, err := SandboxID(ociSpec) assert.Error(err) assert.Empty(sandboxID) } @@ -590,7 +591,7 @@ func TestAddKernelParamInvalid(t *testing.T) { } func TestDeviceTypeFailure(t *testing.T) { - var ociSpec CompatOCISpec + var ociSpec specs.Spec invalidDeviceType := "f" ociSpec.Linux = &specs.Linux{} @@ -615,7 +616,7 @@ func TestContains(t *testing.T) { } func TestDevicePathEmpty(t *testing.T) { - var ociSpec CompatOCISpec + var ociSpec specs.Spec ociSpec.Linux = &specs.Linux{} ociSpec.Linux.Devices = []specs.LinuxDevice{ @@ -631,9 +632,9 @@ func TestDevicePathEmpty(t *testing.T) { } func TestContainerCapabilities(t *testing.T) { - var ociSpec CompatOCISpec + var ociSpec compatOCISpec - ociSpec.Process = &CompatOCIProcess{} + ociSpec.Process = &compatOCIProcess{} ociSpec.Process.Capabilities = map[string]interface{}{ "bounding": []interface{}{"CAP_KILL"}, "effective": []interface{}{"CAP_KILL", "CAP_LEASE"}, @@ -642,7 +643,7 @@ func TestContainerCapabilities(t *testing.T) { "ambient": []interface{}{""}, } - c, err := ContainerCapabilities(ociSpec) + c, err := containerCapabilities(ociSpec) assert.Nil(t, err) assert.Equal(t, c.Bounding, []string{"CAP_KILL"}) assert.Equal(t, c.Effective, []string{"CAP_KILL", "CAP_LEASE"}) @@ -652,7 +653,7 @@ func TestContainerCapabilities(t *testing.T) { ociSpec.Process.Capabilities = []interface{}{"CAP_LEASE", "CAP_SETUID"} - c, err = ContainerCapabilities(ociSpec) + c, err = containerCapabilities(ociSpec) assert.Nil(t, err) assert.Equal(t, c.Bounding, []string{"CAP_LEASE", "CAP_SETUID"}) assert.Equal(t, c.Effective, []string{"CAP_LEASE", "CAP_SETUID"}) @@ -662,7 +663,7 @@ func TestContainerCapabilities(t *testing.T) { ociSpec.Process.Capabilities = nil - c, err = ContainerCapabilities(ociSpec) + c, err = containerCapabilities(ociSpec) assert.Nil(t, err) assert.Equal(t, c.Bounding, []string(nil)) assert.Equal(t, c.Effective, []string(nil)) @@ -673,9 +674,9 @@ func TestContainerCapabilities(t *testing.T) { // use specs.Spec to decode the spec, the content of capabilities is [] string func TestCompatOCISpecWithArray(t *testing.T) { - compatOCISpec := CompatOCISpec{} + compatOCISpec := compatOCISpec{} err := json.Unmarshal([]byte(capabilitiesSpecArray), &compatOCISpec) - assert.Nil(t, err, "use CompatOCISpec to decode capabilitiesSpecArray failed") + assert.Nil(t, err, "use compatOCISpec to decode capabilitiesSpecArray failed") ociSpecJSON, err := json.Marshal(compatOCISpec) assert.Nil(t, err, "encode compatOCISpec failed") @@ -686,7 +687,7 @@ func TestCompatOCISpecWithArray(t *testing.T) { err = json.Unmarshal(ociSpecJSON, &ociSpec) assert.NotNil(t, err, "This test should fail") - caps, err := ContainerCapabilities(compatOCISpec) + caps, err := containerCapabilities(compatOCISpec) assert.Nil(t, err, "decode capabilities failed") compatOCISpec.Process.Capabilities = caps @@ -700,9 +701,9 @@ func TestCompatOCISpecWithArray(t *testing.T) { // use specs.Spec to decode the spec, the content of capabilities is struct func TestCompatOCISpecWithStruct(t *testing.T) { - compatOCISpec := CompatOCISpec{} + compatOCISpec := compatOCISpec{} err := json.Unmarshal([]byte(capabilitiesSpecStruct), &compatOCISpec) - assert.Nil(t, err, "use CompatOCISpec to decode capabilitiesSpecStruct failed") + assert.Nil(t, err, "use compatOCISpec to decode capabilitiesSpecStruct failed") ociSpecJSON, err := json.Marshal(compatOCISpec) assert.Nil(t, err, "encode compatOCISpec failed") @@ -810,10 +811,8 @@ func TestAddAssetAnnotations(t *testing.T) { AgentConfig: vc.KataAgentConfig{}, } - ocispec := CompatOCISpec{ - Spec: spec.Spec{ - Annotations: expectedAnnotations, - }, + ocispec := spec.Spec{ + Annotations: expectedAnnotations, } addAssetAnnotations(ocispec, &config) diff --git a/virtcontainers/types/sandbox.go b/virtcontainers/types/sandbox.go index 2259e86d5..655821c11 100644 --- a/virtcontainers/types/sandbox.go +++ b/virtcontainers/types/sandbox.go @@ -8,6 +8,8 @@ package types import ( "fmt" "strings" + + "github.com/opencontainers/runtime-spec/specs-go" ) // StateString is a string representing a sandbox state. @@ -225,21 +227,6 @@ type EnvVar struct { Value string } -// LinuxCapabilities specify the capabilities to keep when executing -// the process inside the container. -type LinuxCapabilities struct { - // Bounding is the set of capabilities checked by the kernel. - Bounding []string - // Effective is the set of capabilities checked by the kernel. - Effective []string - // Inheritable is the capabilities preserved across execve. - Inheritable []string - // Permitted is the limiting superset for effective capabilities. - Permitted []string - // Ambient is the ambient set of capabilities that are kept. - Ambient []string -} - // Cmd represents a command to execute in a running container. type Cmd struct { Args []string @@ -272,7 +259,7 @@ type Cmd struct { PrimaryGroup string WorkDir string Console string - Capabilities LinuxCapabilities + Capabilities *specs.LinuxCapabilities Interactive bool Detach bool