virtcontainers: Remove ConfigJSONKey annotation

Currently we already saved "BundlePath" in sandbox and config's config, with `BundlePath`, we can easily get config of json format from "BundlePath"/config.json, So ConfigJSONKey is a duplicate and unnecessary field and we can remove it for saving more space from persist data. Fixes: #215 Signed-off-by: Zhang Wei <[email protected]>
kata-containers · Apr 16, 2018 · 29d2a54 · 29d2a54
1 parent ad5669f
commit 29d2a54
Show file tree

Hide file tree

Showing 8 changed files with 38 additions and 94 deletions.
diff --git a/cli/create.go b/cli/create.go
@@ -233,7 +233,7 @@ func createSandbox(ociSpec oci.CompatOCISpec, runtimeConfig oci.RuntimeConfig,
 		return vc.Process{}, err
 	}
 
-	sandboxConfig, err := oci.SandboxConfig(ociSpec, runtimeConfig, bundlePath, containerID, console, disableOutput)
+	sandboxConfig, err := oci.SandboxConfig(runtimeConfig, bundlePath, containerID, console, disableOutput)
 	if err != nil {
 		return vc.Process{}, err
 	}

diff --git a/cli/delete_test.go b/cli/delete_test.go
@@ -170,8 +170,6 @@ func TestDeleteSandbox(t *testing.T) {
 	}
 
 	configPath := testConfigSetup(t)
-	configJSON, err := readOCIConfigJSON(configPath)
-	assert.NoError(err)
 
 	testingImpl.ListSandboxFunc = func() ([]vc.SandboxStatus, error) {
 		return []vc.SandboxStatus{
@@ -182,7 +180,6 @@ func TestDeleteSandbox(t *testing.T) {
 						ID: sandbox.ID(),
 						Annotations: map[string]string{
 							vcAnnotations.ContainerTypeKey: string(vc.PodSandbox),
-							vcAnnotations.ConfigJSONKey:    configJSON,
 						},
 						State: vc.State{
 							State: "ready",
@@ -233,9 +230,6 @@ func TestDeleteInvalidContainerType(t *testing.T) {
 	}
 
 	configPath := testConfigSetup(t)
-	configJSON, err := readOCIConfigJSON(configPath)
-	assert.NoError(err)
-
 	testingImpl.ListSandboxFunc = func() ([]vc.SandboxStatus, error) {
 		return []vc.SandboxStatus{
 			{
@@ -245,7 +239,6 @@ func TestDeleteInvalidContainerType(t *testing.T) {
 						ID: sandbox.ID(),
 						Annotations: map[string]string{
 							vcAnnotations.ContainerTypeKey: "InvalidType",
-							vcAnnotations.ConfigJSONKey:    configJSON,
 						},
 						State: vc.State{
 							State: "created",
@@ -274,8 +267,6 @@ func TestDeleteSandboxRunning(t *testing.T) {
 	}
 
 	configPath := testConfigSetup(t)
-	configJSON, err := readOCIConfigJSON(configPath)
-	assert.NoError(err)
 
 	testingImpl.ListSandboxFunc = func() ([]vc.SandboxStatus, error) {
 		return []vc.SandboxStatus{
@@ -286,7 +277,6 @@ func TestDeleteSandboxRunning(t *testing.T) {
 						ID: sandbox.ID(),
 						Annotations: map[string]string{
 							vcAnnotations.ContainerTypeKey: string(vc.PodSandbox),
-							vcAnnotations.ConfigJSONKey:    configJSON,
 						},
 						State: vc.State{
 							State: "running",
@@ -346,8 +336,6 @@ func TestDeleteRunningContainer(t *testing.T) {
 	}
 
 	configPath := testConfigSetup(t)
-	configJSON, err := readOCIConfigJSON(configPath)
-	assert.NoError(err)
 
 	testingImpl.ListSandboxFunc = func() ([]vc.SandboxStatus, error) {
 		return []vc.SandboxStatus{
@@ -358,7 +346,6 @@ func TestDeleteRunningContainer(t *testing.T) {
 						ID: sandbox.MockContainers[0].ID(),
 						Annotations: map[string]string{
 							vcAnnotations.ContainerTypeKey: string(vc.PodContainer),
-							vcAnnotations.ConfigJSONKey:    configJSON,
 						},
 						State: vc.State{
 							State: "running",
@@ -419,8 +406,6 @@ func TestDeleteContainer(t *testing.T) {
 	}
 
 	configPath := testConfigSetup(t)
-	configJSON, err := readOCIConfigJSON(configPath)
-	assert.NoError(err)
 
 	testingImpl.ListSandboxFunc = func() ([]vc.SandboxStatus, error) {
 		return []vc.SandboxStatus{
@@ -431,7 +416,6 @@ func TestDeleteContainer(t *testing.T) {
 						ID: sandbox.MockContainers[0].ID(),
 						Annotations: map[string]string{
 							vcAnnotations.ContainerTypeKey: string(vc.PodContainer),
-							vcAnnotations.ConfigJSONKey:    configJSON,
 						},
 						State: vc.State{
 							State: "ready",
@@ -511,8 +495,6 @@ func TestDeleteCLIFunctionSuccess(t *testing.T) {
 	}
 
 	configPath := testConfigSetup(t)
-	configJSON, err := readOCIConfigJSON(configPath)
-	assert.NoError(err)
 
 	testingImpl.ListSandboxFunc = func() ([]vc.SandboxStatus, error) {
 		return []vc.SandboxStatus{
@@ -523,7 +505,6 @@ func TestDeleteCLIFunctionSuccess(t *testing.T) {
 						ID: sandbox.ID(),
 						Annotations: map[string]string{
 							vcAnnotations.ContainerTypeKey: string(vc.PodSandbox),
-							vcAnnotations.ConfigJSONKey:    configJSON,
 						},
 						State: vc.State{
 							State: "ready",

diff --git a/cli/exec_test.go b/cli/exec_test.go
@@ -91,12 +91,9 @@ func TestExecuteErrors(t *testing.T) {
 
 	// Container state undefined
 	configPath := testConfigSetup(t)
-	configJSON, err := readOCIConfigJSON(configPath)
-	assert.NoError(err)
 
 	annotations = map[string]string{
 		vcAnnotations.ContainerTypeKey: string(vc.PodSandbox),
-		vcAnnotations.ConfigJSONKey:    configJSON,
 	}
 
 	containerState := vc.State{}
@@ -149,12 +146,9 @@ func TestExecuteErrorReadingProcessJson(t *testing.T) {
 	ctx := cli.NewContext(cli.NewApp(), flagSet, nil)
 
 	configPath := testConfigSetup(t)
-	configJSON, err := readOCIConfigJSON(configPath)
-	assert.NoError(err)
 
 	annotations := map[string]string{
 		vcAnnotations.ContainerTypeKey: string(vc.PodSandbox),
-		vcAnnotations.ConfigJSONKey:    configJSON,
 	}
 
 	state := vc.State{
@@ -193,12 +187,9 @@ func TestExecuteErrorOpeningConsole(t *testing.T) {
 	ctx := cli.NewContext(cli.NewApp(), flagSet, nil)
 
 	configPath := testConfigSetup(t)
-	configJSON, err := readOCIConfigJSON(configPath)
-	assert.NoError(err)
 
 	annotations := map[string]string{
 		vcAnnotations.ContainerTypeKey: string(vc.PodSandbox),
-		vcAnnotations.ConfigJSONKey:    configJSON,
 	}
 
 	state := vc.State{
@@ -255,12 +246,9 @@ func TestExecuteWithFlags(t *testing.T) {
 	ctx := cli.NewContext(cli.NewApp(), flagSet, nil)
 
 	configPath := testConfigSetup(t)
-	configJSON, err := readOCIConfigJSON(configPath)
-	assert.NoError(err)
 
 	annotations := map[string]string{
 		vcAnnotations.ContainerTypeKey: string(vc.PodSandbox),
-		vcAnnotations.ConfigJSONKey:    configJSON,
 	}
 
 	state := vc.State{
@@ -339,12 +327,9 @@ func TestExecuteWithFlagsDetached(t *testing.T) {
 	ctx := cli.NewContext(cli.NewApp(), flagSet, nil)
 
 	configPath := testConfigSetup(t)
-	configJSON, err := readOCIConfigJSON(configPath)
-	assert.NoError(err)
 
 	annotations := map[string]string{
 		vcAnnotations.ContainerTypeKey: string(vc.PodSandbox),
-		vcAnnotations.ConfigJSONKey:    configJSON,
 	}
 
 	state := vc.State{
@@ -413,12 +398,9 @@ func TestExecuteWithInvalidProcessJson(t *testing.T) {
 	ctx := cli.NewContext(cli.NewApp(), flagSet, nil)
 
 	configPath := testConfigSetup(t)
-	configJSON, err := readOCIConfigJSON(configPath)
-	assert.NoError(err)
 
 	annotations := map[string]string{
 		vcAnnotations.ContainerTypeKey: string(vc.PodSandbox),
-		vcAnnotations.ConfigJSONKey:    configJSON,
 	}
 
 	state := vc.State{
@@ -460,12 +442,9 @@ func TestExecuteWithValidProcessJson(t *testing.T) {
 	ctx := cli.NewContext(cli.NewApp(), flagSet, nil)
 
 	configPath := testConfigSetup(t)
-	configJSON, err := readOCIConfigJSON(configPath)
-	assert.NoError(err)
 
 	annotations := map[string]string{
 		vcAnnotations.ContainerTypeKey: string(vc.PodContainer),
-		vcAnnotations.ConfigJSONKey:    configJSON,
 	}
 
 	state := vc.State{
@@ -552,12 +531,9 @@ func TestExecuteWithInvalidEnvironment(t *testing.T) {
 	ctx := cli.NewContext(cli.NewApp(), flagSet, nil)
 
 	configPath := testConfigSetup(t)
-	configJSON, err := readOCIConfigJSON(configPath)
-	assert.NoError(err)
 
 	annotations := map[string]string{
 		vcAnnotations.ContainerTypeKey: string(vc.PodContainer),
-		vcAnnotations.ConfigJSONKey:    configJSON,
 	}
 
 	state := vc.State{

diff --git a/cli/run_test.go b/cli/run_test.go
@@ -160,7 +160,6 @@ type runContainerData struct {
 	pidFilePath   string
 	consolePath   string
 	bundlePath    string
-	configJSON    string
 	sandbox       *vcmock.Sandbox
 	runtimeConfig oci.RuntimeConfig
 	process       *os.Process
@@ -212,14 +211,10 @@ func testRunContainerSetup(t *testing.T) runContainerData {
 	runtimeConfig, err := newTestRuntimeConfig(tmpdir, consolePath, true)
 	assert.NoError(err)
 
-	configJSON, err := readOCIConfigJSON(configPath)
-	assert.NoError(err)
-
 	return runContainerData{
 		pidFilePath:   pidFilePath,
 		consolePath:   consolePath,
 		bundlePath:    bundlePath,
-		configJSON:    configJSON,
 		sandbox:       sandbox,
 		runtimeConfig: runtimeConfig,
 		process:       cmd.Process,
@@ -261,7 +256,6 @@ func TestRunContainerSuccessful(t *testing.T) {
 						ID: d.sandbox.ID(),
 						Annotations: map[string]string{
 							vcAnnotations.ContainerTypeKey: string(vc.PodContainer),
-							vcAnnotations.ConfigJSONKey:    d.configJSON,
 						},
 					},
 				},
@@ -337,7 +331,6 @@ func TestRunContainerDetachSuccessful(t *testing.T) {
 						ID: d.sandbox.ID(),
 						Annotations: map[string]string{
 							vcAnnotations.ContainerTypeKey: string(vc.PodContainer),
-							vcAnnotations.ConfigJSONKey:    d.configJSON,
 						},
 					},
 				},
@@ -410,7 +403,6 @@ func TestRunContainerDeleteFail(t *testing.T) {
 						ID: d.sandbox.ID(),
 						Annotations: map[string]string{
 							vcAnnotations.ContainerTypeKey: string(vc.PodContainer),
-							vcAnnotations.ConfigJSONKey:    d.configJSON,
 						},
 					},
 				},
@@ -486,7 +478,6 @@ func TestRunContainerWaitFail(t *testing.T) {
 						ID: d.sandbox.ID(),
 						Annotations: map[string]string{
 							vcAnnotations.ContainerTypeKey: string(vc.PodContainer),
-							vcAnnotations.ConfigJSONKey:    d.configJSON,
 						},
 					},
 				},
@@ -570,7 +561,6 @@ func TestRunContainerStartFail(t *testing.T) {
 						ID: d.sandbox.ID(),
 						Annotations: map[string]string{
 							vcAnnotations.ContainerTypeKey: string(vc.PodContainer),
-							vcAnnotations.ConfigJSONKey:    d.configJSON,
 						},
 					},
 				},

diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go
@@ -21,6 +21,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"io/ioutil"
 	"os"
 	"path/filepath"
 	"strconv"
@@ -42,7 +43,7 @@ var (
 	defaultKataDeviceID         = "channel0"
 	defaultKataID               = "charch0"
 	errorMissingProxy           = errors.New("Missing proxy pointer")
-	errorMissingOCISpec         = errors.New("Missing OCI specification")
+	errorMissingOCIBundlePath   = errors.New("Missing OCI bundle path")
 	kataHostSharedDir           = "/run/kata-containers/shared/sandboxes/"
 	kataGuestSharedDir          = "/run/kata-containers/shared/containers/"
 	mountGuest9pTag             = "kataShared"
@@ -658,9 +659,9 @@ func (k *kataAgent) rollbackFailingContainerCreation(c *Container) {
 }
 
 func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process, err error) {
-	ociSpecJSON, ok := c.config.Annotations[vcAnnotations.ConfigJSONKey]
+	bundlePath, ok := c.config.Annotations[vcAnnotations.BundlePathKey]
 	if !ok {
-		return nil, errorMissingOCISpec
+		return nil, errorMissingOCIBundlePath
 	}
 
 	var ctrStorages []*grpc.Storage
@@ -738,8 +739,8 @@ func (k *kataAgent) createContainer(sandbox *Sandbox, c *Container) (p *Process,
 		}
 	}
 
-	ociSpec := &specs.Spec{}
-	if err = json.Unmarshal([]byte(ociSpecJSON), ociSpec); err != nil {
+	ociSpec, err := ParseConfigJSON(bundlePath)
+	if err != nil {
 		return nil, err
 	}
 
@@ -962,3 +963,22 @@ func (k *kataAgent) sendReq(request interface{}) (interface{}, error) {
 		return nil, fmt.Errorf("Unknown gRPC type %T", req)
 	}
 }
+
+// ParseConfigJSON unmarshals the config.json file.
+// TODO: this is duplicate with virtcontainers/pkg/oci/utilsgo:ParseConfigJSON
+// but we can't use that function because there will be cycle dependency problem
+// better find a way to eliminate duplicate codes @weizhang555
+func ParseConfigJSON(bundlePath string) (*specs.Spec, error) {
+	configPath := filepath.Join(bundlePath, "config.json")
+	configByte, err := ioutil.ReadFile(configPath)
+	if err != nil {
+		return nil, err
+	}
+
+	var ocispec specs.Spec
+	if err := json.Unmarshal(configByte, &ocispec); err != nil {
+		return nil, err
+	}
+
+	return &ocispec, nil
+}
diff --git a/virtcontainers/pkg/annotations/annotations.go b/virtcontainers/pkg/annotations/annotations.go
@@ -17,7 +17,7 @@
 package annotations
 
 const (
-	vcAnnotationsPrefix = "com.github.containers.virtcontainers."
+	vcAnnotationsPrefix = "com.github.katacontainers.runtime.virtcontainers."
 
 	// KernelPath is a sandbox annotation for passing a per container path pointing at the kernel needed to boot the container VM.
 	KernelPath = vcAnnotationsPrefix + "KernelPath"
@@ -52,9 +52,6 @@ const (
 	// AssetHashType is the hash type used for assets verification
 	AssetHashType = vcAnnotationsPrefix + "AssetHashType"
 
-	// ConfigJSONKey is the annotation key to fetch the OCI configuration.
-	ConfigJSONKey = vcAnnotationsPrefix + "pkg.oci.config"
-
 	// BundlePathKey is the annotation key to fetch the OCI configuration file path.
 	BundlePathKey = vcAnnotationsPrefix + "pkg.oci.bundle_path"