From b309dc54806e27c24631ad7ac69e36bd3db02b72 Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Mon, 15 Apr 2019 10:52:42 +0100 Subject: [PATCH] agent: Provide explicit config options for the agent Previously, the agent behaviour was controlled entirely using the `kernel_params=` config option. This mechanism suffers from a subtle problem - the runtime is not aware of how the agent will behave. From now on, all significant agent options will be controlled from the agent section in the configuration file. This allows the runtime to be more aware of -- and in control of -- such agent settings. It would also allow the underlying kernel CLI options to be modified in the future if required. This PR adds the only useful agent option as an explicit option by adding an `enable_debug=true` option to the Kata agent section in `configuration.toml`. This allows controlling agent debug to be handled in the same manner as the other debug options. This change is somewhat foundational: it permits the agent to be handled consistently with other config file sections which is useful, but arguably not essential (the old way worked). However, the new way of handling agent options will be essential when introducing agent tracing control as the runtime must be aware of the agent trace mode to allow the runtime to modify its behaviour accordingly. Signed-off-by: James O. D. Hunt --- cli/config/configuration-fc.toml.in | 5 +++-- cli/config/configuration-qemu.toml.in | 5 +++-- cli/kata-env.go | 27 +++++++++++++++++++------ cli/kata-env_test.go | 29 +++++++++++++++++++++++++-- pkg/katautils/config.go | 26 +++++++++++++++++++++++- pkg/katautils/config_test.go | 13 ++++++++++++ virtcontainers/kata_agent.go | 13 ++++++++++++ virtcontainers/kata_agent_test.go | 23 +++++++++++++++++++++ virtcontainers/vm_test.go | 2 +- 9 files changed, 129 insertions(+), 14 deletions(-) diff --git a/cli/config/configuration-fc.toml.in b/cli/config/configuration-fc.toml.in index 2c2a14e790..7cc052b728 100644 --- a/cli/config/configuration-fc.toml.in +++ b/cli/config/configuration-fc.toml.in @@ -222,8 +222,9 @@ path = "@SHIMPATH@" #enable_tracing = true [agent.@PROJECT_TYPE@] -# There is no field for this section. The goal is only to be able to -# specify which type of agent the user wants to use. +# If enabled, make the agent display debug-level messages. +# (default: disabled) +#enable_debug = true [netmon] # If enabled, the network monitoring process gets started when the diff --git a/cli/config/configuration-qemu.toml.in b/cli/config/configuration-qemu.toml.in index a70ca97531..0c875fd4f2 100644 --- a/cli/config/configuration-qemu.toml.in +++ b/cli/config/configuration-qemu.toml.in @@ -274,8 +274,9 @@ path = "@SHIMPATH@" #enable_tracing = true [agent.@PROJECT_TYPE@] -# There is no field for this section. The goal is only to be able to -# specify which type of agent the user wants to use. +# If enabled, make the agent display debug-level messages. +# (default: disabled) +#enable_debug = true [netmon] # If enabled, the network monitoring process gets started when the diff --git a/cli/kata-env.go b/cli/kata-env.go index 8608bf97d1..0d6883bb75 100644 --- a/cli/kata-env.go +++ b/cli/kata-env.go @@ -1,4 +1,4 @@ -// Copyright (c) 2017-2018 Intel Corporation +// Copyright (c) 2017-2019 Intel Corporation // // SPDX-License-Identifier: Apache-2.0 // @@ -27,7 +27,7 @@ import ( // // XXX: Increment for every change to the output format // (meaning any change to the EnvInfo type). -const formatVersion = "1.0.21" +const formatVersion = "1.0.22" // MetaInfo stores information on the format of the output itself type MetaInfo struct { @@ -112,7 +112,8 @@ type ShimInfo struct { // AgentInfo stores agent details type AgentInfo struct { - Type string + Type string + Debug bool } // DistroInfo stores host operating system distribution details. @@ -308,12 +309,23 @@ func getShimInfo(config oci.RuntimeConfig) (ShimInfo, error) { return shim, nil } -func getAgentInfo(config oci.RuntimeConfig) AgentInfo { +func getAgentInfo(config oci.RuntimeConfig) (AgentInfo, error) { agent := AgentInfo{ Type: string(config.AgentType), } - return agent + switch config.AgentType { + case vc.KataContainersAgent: + agentConfig, ok := config.AgentConfig.(vc.KataAgentConfig) + if !ok { + return AgentInfo{}, errors.New("cannot determine Kata agent config") + } + agent.Debug = agentConfig.Debug + default: + // Nothing useful to report for the other agent types + } + + return agent, nil } func getHypervisorInfo(config oci.RuntimeConfig) HypervisorInfo { @@ -361,7 +373,10 @@ func getEnvInfo(configFile string, config oci.RuntimeConfig) (env EnvInfo, err e return EnvInfo{}, err } - agent := getAgentInfo(config) + agent, err := getAgentInfo(config) + if err != nil { + return EnvInfo{}, err + } hypervisor := getHypervisorInfo(config) diff --git a/cli/kata-env_test.go b/cli/kata-env_test.go index 3bb1b589c7..23c0d65de1 100644 --- a/cli/kata-env_test.go +++ b/cli/kata-env_test.go @@ -42,6 +42,7 @@ var ( runtimeTrace = false shimDebug = false netmonDebug = false + agentDebug = false ) // makeVersionBinary creates a shell script with the specified file @@ -153,6 +154,7 @@ func makeRuntimeConfig(prefixDir string) (configFile string, config oci.RuntimeC ProxyDebug: proxyDebug, ShimDebug: shimDebug, NetmonDebug: netmonDebug, + AgentDebug: agentDebug, } runtimeConfig := katatestutils.MakeRuntimeConfigFileData(configFileOptions) @@ -206,8 +208,15 @@ func getExpectedShimDetails(config oci.RuntimeConfig) (ShimInfo, error) { } func getExpectedAgentDetails(config oci.RuntimeConfig) (AgentInfo, error) { + + agentConfig, ok := config.AgentConfig.(vc.KataAgentConfig) + if !ok { + return AgentInfo{}, fmt.Errorf("expected KataAgentConfig, got %T", config.AgentConfig) + } + return AgentInfo{ - Type: string(config.AgentType), + Type: string(config.AgentType), + Debug: agentConfig.Debug, }, nil } @@ -486,6 +495,7 @@ func TestEnvGetEnvInfo(t *testing.T) { runtimeDebug = toggle runtimeTrace = toggle shimDebug = toggle + agentDebug = toggle configFile, config, err := makeRuntimeConfig(tmpdir) assert.NoError(t, err) @@ -799,8 +809,23 @@ func TestEnvGetAgentInfo(t *testing.T) { expectedAgent, err := getExpectedAgentDetails(config) assert.NoError(t, err) - agent := getAgentInfo(config) + agent, err := getAgentInfo(config) + assert.NoError(t, err) + assert.Equal(t, expectedAgent, agent) + + agentConfig, ok := config.AgentConfig.(vc.KataAgentConfig) + assert.True(t, ok) + + agentConfig.Debug = true + config.AgentConfig = agentConfig + agent, err = getAgentInfo(config) + assert.NoError(t, err) + assert.True(t, agent.Debug) + + config.AgentConfig = "I am the wrong type" + _, err = getAgentInfo(config) + assert.Error(t, err) } func testEnvShowTOMLSettings(t *testing.T, tmpdir string, tmpfile *os.File) error { diff --git a/pkg/katautils/config.go b/pkg/katautils/config.go index 24554d5b65..354c83c258 100644 --- a/pkg/katautils/config.go +++ b/pkg/katautils/config.go @@ -136,6 +136,7 @@ type shim struct { } type agent struct { + Debug bool `toml:"enable_debug"` } type netmon struct { @@ -388,6 +389,10 @@ func (s shim) trace() bool { return s.Tracing } +func (a agent) debug() bool { + return a.Debug +} + func (n netmon) enable() bool { return n.Enable } @@ -630,21 +635,29 @@ func updateRuntimeConfigProxy(configPath string, tomlConf tomlConfig, config *oc func updateRuntimeConfigAgent(configPath string, tomlConf tomlConfig, config *oci.RuntimeConfig, builtIn bool) error { if builtIn { + var agentConfig vc.KataAgentConfig + + // If the agent config section isn't a Kata one, just default + // to everything being disabled. + agentConfig, _ = config.AgentConfig.(vc.KataAgentConfig) + config.AgentType = vc.KataContainersAgent config.AgentConfig = vc.KataAgentConfig{ LongLiveConn: true, UseVSock: config.HypervisorConfig.UseVSock, + Debug: agentConfig.Debug, } return nil } - for k := range tomlConf.Agent { + for k, agent := range tomlConf.Agent { switch k { case kataAgentTableType: config.AgentType = vc.KataContainersAgent config.AgentConfig = vc.KataAgentConfig{ UseVSock: config.HypervisorConfig.UseVSock, + Debug: agent.debug(), } default: return fmt.Errorf("%s agent type is not supported", k) @@ -705,6 +718,17 @@ func SetKernelParams(runtimeConfig *oci.RuntimeConfig) error { } } + // next, check for agent specific kernel params + if agentConfig, ok := runtimeConfig.AgentConfig.(vc.KataAgentConfig); ok { + params := vc.KataAgentKernelParams(agentConfig) + + for _, p := range params { + 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 { diff --git a/pkg/katautils/config_test.go b/pkg/katautils/config_test.go index 0bb7065f28..163d9c9d39 100644 --- a/pkg/katautils/config_test.go +++ b/pkg/katautils/config_test.go @@ -33,6 +33,7 @@ var ( runtimeTrace = false shimDebug = false netmonDebug = false + agentDebug = false ) type testRuntimeConfig struct { @@ -109,6 +110,7 @@ func createAllRuntimeConfigFiles(dir, hypervisor string) (config testRuntimeConf ProxyDebug: proxyDebug, ShimDebug: shimDebug, NetmonDebug: netmonDebug, + AgentDebug: agentDebug, } runtimeConfigFileData := katatestutils.MakeRuntimeConfigFileData(configFileOptions) @@ -1197,6 +1199,17 @@ func TestShimDefaults(t *testing.T) { assert.True(s.trace()) } +func TestAgentDefaults(t *testing.T) { + assert := assert.New(t) + + a := agent{} + + assert.Equal(a.debug(), a.Debug) + + a.Debug = true + assert.Equal(a.debug(), a.Debug) +} + func TestGetDefaultConfigFilePaths(t *testing.T) { assert := assert.New(t) diff --git a/virtcontainers/kata_agent.go b/virtcontainers/kata_agent.go index 80aa151cc1..87cd4442f0 100644 --- a/virtcontainers/kata_agent.go +++ b/virtcontainers/kata_agent.go @@ -88,6 +88,7 @@ var ( type KataAgentConfig struct { LongLiveConn bool UseVSock bool + Debug bool } type kataVSOCK struct { @@ -175,6 +176,18 @@ func (k *kataAgent) generateVMSocket(id string, c KataAgentConfig) error { return nil } +// KataAgentKernelParams returns a list of Kata Agent specific kernel +// parameters. +func KataAgentKernelParams(config KataAgentConfig) []Param { + var params []Param + + if config.Debug { + params = append(params, Param{Key: "agent.log", Value: "debug"}) + } + + return params +} + func (k *kataAgent) init(ctx context.Context, sandbox *Sandbox, config interface{}) (err error) { // save k.ctx = sandbox.ctx diff --git a/virtcontainers/kata_agent_test.go b/virtcontainers/kata_agent_test.go index dfab29ed3f..b1e27f1d9e 100644 --- a/virtcontainers/kata_agent_test.go +++ b/virtcontainers/kata_agent_test.go @@ -934,3 +934,26 @@ func TestKataCleanupSandbox(t *testing.T) { t.Fatalf("%s still exists\n", dir) } } + +func TestKataAgentKernelParams(t *testing.T) { + assert := assert.New(t) + + config := KataAgentConfig{} + + params := KataAgentKernelParams(config) + assert.Empty(params) + + config.Debug = true + + params = KataAgentKernelParams(config) + assert.NotEmpty(params) + + assert.Len(params, 1) + + expected := Param{ + Key: "agent.log", + Value: "debug", + } + + assert.Equal(params[0], expected) +} diff --git a/virtcontainers/vm_test.go b/virtcontainers/vm_test.go index 90025d2381..7b3583b4bb 100644 --- a/virtcontainers/vm_test.go +++ b/virtcontainers/vm_test.go @@ -118,7 +118,7 @@ func TestVMConfigGrpc(t *testing.T) { HypervisorType: QemuHypervisor, HypervisorConfig: newQemuConfig(), AgentType: KataContainersAgent, - AgentConfig: KataAgentConfig{false, true}, + AgentConfig: KataAgentConfig{false, true, false}, ProxyType: NoopProxyType, }