From e803a7f870ea530afaaf0e3c7917d26dcb2ecc4f Mon Sep 17 00:00:00 2001 From: "James O. D. Hunt" Date: Mon, 15 Apr 2019 09:44:57 +0100 Subject: [PATCH] agent: Return an error, not just an interface Make `newAgentConfig()` return an explicit error rather than handling the error scenario by simply returning the `error` object in the `interface{}` return type. The old behaviour was confusing and inconsistent with the other functions creating a new config type (shim, proxy, etc). Signed-off-by: James O. D. Hunt --- virtcontainers/agent.go | 10 +++++----- virtcontainers/agent_test.go | 7 ++++++- virtcontainers/sandbox.go | 6 +++++- 3 files changed, 16 insertions(+), 7 deletions(-) diff --git a/virtcontainers/agent.go b/virtcontainers/agent.go index 56721bf9a7..fb24cfa4b5 100644 --- a/virtcontainers/agent.go +++ b/virtcontainers/agent.go @@ -91,19 +91,19 @@ func newAgent(agentType AgentType) agent { } // newAgentConfig returns an agent config from a generic SandboxConfig interface. -func newAgentConfig(agentType AgentType, agentConfig interface{}) interface{} { +func newAgentConfig(agentType AgentType, agentConfig interface{}) (interface{}, error) { switch agentType { case NoopAgentType: - return nil + return nil, nil case KataContainersAgent: var kataAgentConfig KataAgentConfig err := mapstructure.Decode(agentConfig, &kataAgentConfig) if err != nil { - return err + return nil, err } - return kataAgentConfig + return kataAgentConfig, nil default: - return nil + return nil, nil } } diff --git a/virtcontainers/agent_test.go b/virtcontainers/agent_test.go index 4706bdaf4d..21840df61e 100644 --- a/virtcontainers/agent_test.go +++ b/virtcontainers/agent_test.go @@ -86,7 +86,12 @@ func TestNewAgentFromUnknownAgentType(t *testing.T) { } func testNewAgentConfig(t *testing.T, config SandboxConfig, expected interface{}) { - agentConfig := newAgentConfig(config.AgentType, config.AgentConfig) + agentConfig, err := newAgentConfig(config.AgentType, config.AgentConfig) + if err != nil { + t.Fatal(err) + + } + if reflect.DeepEqual(agentConfig, expected) == false { t.Fatal() } diff --git a/virtcontainers/sandbox.go b/virtcontainers/sandbox.go index 8d3046d46f..5ca4395422 100644 --- a/virtcontainers/sandbox.go +++ b/virtcontainers/sandbox.go @@ -583,7 +583,11 @@ func newSandbox(ctx context.Context, sandboxConfig SandboxConfig, factory Factor return nil, err } - agentConfig := newAgentConfig(sandboxConfig.AgentType, sandboxConfig.AgentConfig) + agentConfig, err := newAgentConfig(sandboxConfig.AgentType, sandboxConfig.AgentConfig) + if err != nil { + return nil, err + } + if err = s.agent.init(ctx, s, agentConfig); err != nil { return nil, err }