From 42a89d0dcd733cb21d32e145da3c2888e593ac4c Mon Sep 17 00:00:00 2001 From: Hui Zhu Date: Thu, 10 Jan 2019 09:33:54 +0800 Subject: [PATCH] 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 --- cli/create_test.go | 59 ------------------------------------ pkg/katautils/config.go | 40 ++++++++++++++++++++++++ pkg/katautils/config_test.go | 36 ++++++++++++++++++++++ pkg/katautils/create.go | 41 ------------------------- 4 files changed, 76 insertions(+), 100 deletions(-) diff --git a/cli/create_test.go b/cli/create_test.go index da0705c186..9e212af99e 100644 --- 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 index d3eca19bd3..3acaf75d34 100644 --- 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 index a54dbb220e..2f624972bd 100644 --- 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 index f12871bb17..0e07e8003c 100644 --- 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