katautils: Move SetKernelParams from CreateSandbox to updateRuntimeCo…

…nfig Function SetKernelParams is just to update the runtimeConfig according to itself. It just around the configuration. So this patch moves it to updateRuntimeConfig. Fixes: #1106 Signed-off-by: Hui Zhu <[email protected]>
kata-containers · Jan 11, 2019 · 42a89d0 · 42a89d0
1 parent 8a07932
commit 42a89d0
Show file tree

Hide file tree

Showing 4 changed files with 76 additions and 100 deletions.
diff --git a/cli/create_test.go b/cli/create_test.go
@@ -720,62 +720,3 @@ func TestCreate(t *testing.T) {
 		os.RemoveAll(path)
 	}
 }
-
-func TestCreateInvalidKernelParams(t *testing.T) {
-	assert := assert.New(t)
-
-	path, err := ioutil.TempDir("", "containers-mapping")
-	assert.NoError(err)
-	defer os.RemoveAll(path)
-	ctrsMapTreePath = path
-	katautils.SetCtrsMapTreePath(ctrsMapTreePath)
-
-	tmpdir, err := ioutil.TempDir("", "")
-	assert.NoError(err)
-	defer os.RemoveAll(tmpdir)
-
-	runtimeConfig, err := newTestRuntimeConfig(tmpdir, testConsole, true)
-	assert.NoError(err)
-
-	bundlePath := filepath.Join(tmpdir, "bundle")
-
-	err = makeOCIBundle(bundlePath)
-	assert.NoError(err)
-
-	pidFilePath := filepath.Join(tmpdir, "pidfile.txt")
-
-	ociConfigFile := filepath.Join(bundlePath, "config.json")
-	assert.True(katautils.FileExists(ociConfigFile))
-
-	spec, err := readOCIConfigFile(ociConfigFile)
-	assert.NoError(err)
-
-	// Force createSandbox() to be called.
-	spec.Annotations = make(map[string]string)
-	spec.Annotations[testContainerTypeAnnotation] = testContainerTypeSandbox
-
-	// rewrite the file
-	err = writeOCIConfigFile(spec, ociConfigFile)
-	assert.NoError(err)
-
-	savedFunc := katautils.GetKernelParamsFunc
-	defer func() {
-		katautils.GetKernelParamsFunc = savedFunc
-	}()
-
-	katautils.GetKernelParamsFunc = func(needSystemd bool) []vc.Param {
-		return []vc.Param{
-			{
-				Key:   "",
-				Value: "",
-			},
-		}
-	}
-
-	for detach := range []bool{true, false} {
-		err := create(context.Background(), testContainerID, bundlePath, testConsole, pidFilePath, true, true, runtimeConfig)
-		assert.Errorf(err, "%+v", detach)
-		assert.False(vcmock.IsMockError(err))
-		os.RemoveAll(path)
-	}
-}
diff --git a/pkg/katautils/config.go b/pkg/katautils/config.go
@@ -643,6 +643,41 @@ func updateRuntimeConfigShim(configPath string, tomlConf tomlConfig, config *oci
 	return nil
 }
 
+// SetKernelParams adds the user-specified kernel parameters (from the
+// configuration file) to the defaults so that the former take priority.
+func SetKernelParams(runtimeConfig *oci.RuntimeConfig) error {
+	defaultKernelParams := GetKernelParamsFunc(needSystemd(runtimeConfig.HypervisorConfig))
+
+	if runtimeConfig.HypervisorConfig.Debug {
+		strParams := vc.SerializeParams(defaultKernelParams, "=")
+		formatted := strings.Join(strParams, " ")
+
+		kataUtilsLogger.WithField("default-kernel-parameters", formatted).Debug()
+	}
+
+	// retrieve the parameters specified in the config file
+	userKernelParams := runtimeConfig.HypervisorConfig.KernelParams
+
+	// reset
+	runtimeConfig.HypervisorConfig.KernelParams = []vc.Param{}
+
+	// first, add default values
+	for _, p := range defaultKernelParams {
+		if err := (runtimeConfig).AddKernelParam(p); err != nil {
+			return err
+		}
+	}
+
+	// now re-add the user-specified values so that they take priority.
+	for _, p := range userKernelParams {
+		if err := (runtimeConfig).AddKernelParam(p); err != nil {
+			return err
+		}
+	}
+
+	return nil
+}
+
 func updateRuntimeConfig(configPath string, tomlConf tomlConfig, config *oci.RuntimeConfig) error {
 	if err := updateRuntimeConfigHypervisor(configPath, tomlConf, config); err != nil {
 		return err
@@ -672,6 +707,11 @@ func updateRuntimeConfig(configPath string, tomlConf tomlConfig, config *oci.Run
 		Enable: tomlConf.Netmon.enable(),
 	}
 
+	err = SetKernelParams(config)
+	if err != nil {
+		return err
+	}
+
 	return nil
 }
 

diff --git a/pkg/katautils/config_test.go b/pkg/katautils/config_test.go
@@ -201,6 +201,11 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf
 		DisableNewNetNs: disableNewNetNs,
 	}
 
+	err = SetKernelParams(&runtimeConfig)
+	if err != nil {
+		return config, err
+	}
+
 	config = testRuntimeConfig{
 		RuntimeConfig:     runtimeConfig,
 		RuntimeConfigFile: configPath,
@@ -636,6 +641,10 @@ func TestMinimalRuntimeConfig(t *testing.T) {
 
 		NetmonConfig: expectedNetmonConfig,
 	}
+	err = SetKernelParams(&expectedConfig)
+	if err != nil {
+		t.Fatal(err)
+	}
 
 	if reflect.DeepEqual(config, expectedConfig) == false {
 		t.Fatalf("Got %+v\n expecting %+v", config, expectedConfig)
@@ -1378,6 +1387,33 @@ func TestUpdateRuntimeConfigurationFactoryConfig(t *testing.T) {
 	assert.Equal(expectedFactoryConfig, config.FactoryConfig)
 }
 
+func TestUpdateRuntimeConfigurationInvalidKernelParams(t *testing.T) {
+	assert := assert.New(t)
+
+	assert.NotEqual(defaultAgent, vc.HyperstartAgent)
+
+	config := oci.RuntimeConfig{}
+
+	tomlConf := tomlConfig{}
+
+	savedFunc := GetKernelParamsFunc
+	defer func() {
+		GetKernelParamsFunc = savedFunc
+	}()
+
+	GetKernelParamsFunc = func(needSystemd bool) []vc.Param {
+		return []vc.Param{
+			{
+				Key:   "",
+				Value: "",
+			},
+		}
+	}
+
+	err := updateRuntimeConfig("", tomlConf, &config)
+	assert.EqualError(err, "Empty kernel parameter")
+}
+
 func TestCheckHypervisorConfig(t *testing.T) {
 	assert := assert.New(t)
 

diff --git a/pkg/katautils/create.go b/pkg/katautils/create.go
@@ -9,7 +9,6 @@ package katautils
 import (
 	"context"
 	"fmt"
-	"strings"
 
 	vc "github.com/kata-containers/runtime/virtcontainers"
 	vf "github.com/kata-containers/runtime/virtcontainers/factory"
@@ -83,41 +82,6 @@ func HandleFactory(ctx context.Context, vci vc.VC, runtimeConfig *oci.RuntimeCon
 	vci.SetFactory(ctx, f)
 }
 
-// SetKernelParams adds the user-specified kernel parameters (from the
-// configuration file) to the defaults so that the former take priority.
-func SetKernelParams(runtimeConfig *oci.RuntimeConfig) error {
-	defaultKernelParams := GetKernelParamsFunc(needSystemd(runtimeConfig.HypervisorConfig))
-
-	if runtimeConfig.HypervisorConfig.Debug {
-		strParams := vc.SerializeParams(defaultKernelParams, "=")
-		formatted := strings.Join(strParams, " ")
-
-		kataUtilsLogger.WithField("default-kernel-parameters", formatted).Debug()
-	}
-
-	// retrieve the parameters specified in the config file
-	userKernelParams := runtimeConfig.HypervisorConfig.KernelParams
-
-	// reset
-	runtimeConfig.HypervisorConfig.KernelParams = []vc.Param{}
-
-	// first, add default values
-	for _, p := range defaultKernelParams {
-		if err := (runtimeConfig).AddKernelParam(p); err != nil {
-			return err
-		}
-	}
-
-	// now re-add the user-specified values so that they take priority.
-	for _, p := range userKernelParams {
-		if err := (runtimeConfig).AddKernelParam(p); err != nil {
-			return err
-		}
-	}
-
-	return nil
-}
-
 // SetEphemeralStorageType sets the mount type to 'ephemeral'
 // if the mount source path is provisioned by k8s for ephemeral storage.
 // For the given pod ephemeral volume is created only once
@@ -138,11 +102,6 @@ func CreateSandbox(ctx context.Context, vci vc.VC, ociSpec oci.CompatOCISpec, ru
 	span, ctx := Trace(ctx, "createSandbox")
 	defer span.Finish()
 
-	err := SetKernelParams(&runtimeConfig)
-	if err != nil {
-		return nil, vc.Process{}, err
-	}
-
 	sandboxConfig, err := oci.SandboxConfig(ociSpec, runtimeConfig, bundlePath, containerID, console, disableOutput, systemdCgroup)
 	if err != nil {
 		return nil, vc.Process{}, err