Skip to content

Commit

Permalink
CompatOCISpec: limit usage of CompatOCISpec
Browse files Browse the repository at this point in the history 
Fixes: kata-containers#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 <weizhang555.zw@gmail.com>
  • Loading branch information
@WeiZhang555
WeiZhang555 committed Sep 5, 2019
1 parent 4176a7c commit 9507f45
Show file tree
Hide file tree
Showing 22 changed files with 219 additions and 342 deletions.
2 changes: 1 addition & 1 deletion cli/create.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
12 changes: 6 additions & 6 deletions cli/create_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
32 changes: 15 additions & 17 deletions cli/delete_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
Expand Down
6 changes: 3 additions & 3 deletions cli/exec.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import (
)

type execParams struct {
ociProcess oci.CompatOCIProcess
ociProcess specs.Process
cID string
pidFile string
console string
Expand Down Expand Up @@ -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{
Expand All @@ -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 {
Expand Down
43 changes: 22 additions & 21 deletions cli/exec_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,14 @@ import (
"path/filepath"
"testing"

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/oci"
"github.com/kata-containers/runtime/virtcontainers/pkg/vcmock"
"github.com/kata-containers/runtime/virtcontainers/types"
"github.com/stretchr/testify/assert"
"github.com/urfave/cli"
)

func TestExecCLIFunction(t *testing.T) {
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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{
Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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)

Expand Down
30 changes: 3 additions & 27 deletions cli/main_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
}
Expand Down
5 changes: 1 addition & 4 deletions cli/run_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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{
Expand Down
Loading

0 comments on commit 9507f45

Please sign in to comment.