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 <james.o.hunt@intel.com>

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

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

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)
 

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 {

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 {

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)
 

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

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

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