katautils: Move SetKernelParams from CreateSandbox to updateRuntimeConfig

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 <teawater@hyper.sh>

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)
-	}
-}

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
 }
 

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)
 

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