Mock out command execution (#2115)

Signed-off-by: Nikki Attea <nikki@sensu.io>

.appveyor.yml

@@ -38,7 +38,6 @@ install:
   - go version
   - go env
   - mkdir %GOPATH%\bin
-  - ps: .\build.ps1 build_tools
 
 platform:
   - x64

.circleci/config.yml

@@ -43,10 +43,8 @@ jobs:
       - run: sudo $CIRCLE_WORKING_DIRECTORY/build/ci/circle-ci-setup.sh
       - run: $SENSU_RELEASE_REPO/install-awscli.sh
 
-      # *Now* get to initial linting/building...
+      # *Now* get to initial linting
       - run: GOFMT=/usr/local/go/bin/gofmt ./build/ci/gofmt-linting-check.sh
-      - run: ./build.sh build_tools
-      - run: make
 
       # Post packages to S3
       - run: $SENSU_RELEASE_REPO/collect-circle-log.sh $CIRCLE_WORKING_DIRECTORY/out log-build.txt

.travis.yml

@@ -25,7 +25,6 @@ install:
 # Travis clones with depth=50 by default. If the most recent tag is more than
 # 50 commits back, git describe will fail. Fetch tags and full history.
 - "git fetch --unshallow --tags"
-- "./build.sh build_tools"
 script: "./build.sh $TEST_SUITE"
 jobs:
   include:

CHANGELOG.md

@@ -44,6 +44,7 @@ the older versions are now deprecated.
 - Web ui entity recent events are sorted by last ok
 - Deprecated --custom-attributes in the sensu-agent command, changed to
 --extended-attributes.
+- Interfaced command execution and mocked it for testing.
 
 ### Fixed
 - Fixed a bug in `sensuctl configure` where an output format called `none` could

agent/agent.go

@@ -18,6 +18,7 @@ import (
 
 	"github.com/atlassian/gostatsd/pkg/statsd"
 	"github.com/sensu/sensu-go/agent/assetmanager"
+	"github.com/sensu/sensu-go/command"
 	"github.com/sensu/sensu-go/handler"
 	"github.com/sensu/sensu-go/system"
 	"github.com/sensu/sensu-go/transport"
@@ -192,6 +193,7 @@ type Agent struct {
 	conn            transport.Transport
 	context         context.Context
 	entity          *types.Entity
+	executor        command.Executor
 	handler         *handler.MessageHandler
 	header          http.Header
 	inProgress      map[string]*types.CheckConfig
@@ -217,6 +219,7 @@ func NewAgent(config *Config) *Agent {
 		handler:         handler.NewMessageHandler(),
 		inProgress:      make(map[string]*types.CheckConfig),
 		inProgressMu:    &sync.Mutex{},
+		executor:        command.NewExecutor(),
 		stopping:        make(chan struct{}),
 		stopped:         make(chan struct{}),
 		sendq:           make(chan *transport.Message, 10),

agent/check_handler.go

@@ -74,7 +74,7 @@ func (a *Agent) executeCheck(request *types.CheckRequest) {
 
 	// Inject the dependenices into PATH, LD_LIBRARY_PATH & CPATH so that they are
 	// availabe when when the command is executed.
-	ex := &command.Execution{
+	ex := command.ExecutionRequest{
 		Env:          append(assets.Env(), check.EnvVars...),
 		Command:      checkConfig.Command,
 		Timeout:      int(checkConfig.Timeout),
@@ -99,20 +99,21 @@ func (a *Agent) executeCheck(request *types.CheckRequest) {
 		return
 	}
 
-	if _, err := command.ExecuteCommand(context.Background(), ex); err != nil {
+	checkExec, err := a.executor.Execute(context.Background(), ex)
+	if err != nil {
 		event.Check.Output = err.Error()
 	} else {
-		event.Check.Output = ex.Output
+		event.Check.Output = checkExec.Output
 	}
 
-	event.Check.Duration = ex.Duration
-	event.Check.Status = uint32(ex.Status)
+	event.Check.Duration = checkExec.Duration
+	event.Check.Status = uint32(checkExec.Status)
 
 	event.Entity = a.getAgentEntity()
 	event.Timestamp = time.Now().Unix()
 
 	if len(checkHooks) != 0 {
-		event.Check.Hooks = a.ExecuteHooks(request, ex.Status)
+		event.Check.Hooks = a.ExecuteHooks(request, checkExec.Status)
 	}
 
 	// Instantiate metrics in the event if the check is attempting to extract metrics

agent/check_handler_internal_test.go

@@ -2,29 +2,23 @@ package agent
 
 import (
 	"encoding/json"
-	"fmt"
-	"io/ioutil"
-	"os"
-	"path/filepath"
 	"testing"
 	"time"
 
-	"github.com/sensu/sensu-go/testing/testutil"
+	"github.com/sensu/sensu-go/command"
+	"github.com/sensu/sensu-go/testing/mockexecutor"
+
 	"github.com/sensu/sensu-go/transport"
 	"github.com/sensu/sensu-go/types"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 	"github.com/stretchr/testify/require"
 )
 
-var binDir = filepath.Join("..", "bin")
-var toolsDir = filepath.Join(binDir, "tools")
-
 func TestHandleCheck(t *testing.T) {
 	assert := assert.New(t)
 
 	checkConfig := types.FixtureCheckConfig("check")
-	truePath := testutil.CommandPath(filepath.Join(toolsDir, "true"))
-	checkConfig.Command = truePath
 
 	request := &types.CheckRequest{Config: checkConfig, Issued: time.Now().Unix()}
 	payload, err := json.Marshal(request)
@@ -34,6 +28,10 @@ func TestHandleCheck(t *testing.T) {
 
 	config := FixtureConfig()
 	agent := NewAgent(config)
+	ex := &mockexecutor.MockExecutor{}
+	agent.executor = ex
+	execution := command.FixtureExecutionResponse(0, "")
+	ex.On("Execute", mock.Anything, mock.Anything).Return(execution, nil)
 	ch := make(chan *transport.Message, 5)
 	agent.sendq = ch
 
@@ -61,13 +59,12 @@ func TestExecuteCheck(t *testing.T) {
 	agent := NewAgent(config)
 	ch := make(chan *transport.Message, 1)
 	agent.sendq = ch
-
-	truePath := testutil.CommandPath(filepath.Join(toolsDir, "true"))
-	checkConfig.Command = truePath
-	checkConfig.Timeout = 10
+	ex := &mockexecutor.MockExecutor{}
+	agent.executor = ex
+	execution := command.FixtureExecutionResponse(0, "")
+	ex.On("Execute", mock.Anything, mock.Anything).Return(execution, nil)
 
 	agent.executeCheck(request)
-
 	msg := <-ch
 
 	event := &types.Event{}
@@ -76,11 +73,8 @@ func TestExecuteCheck(t *testing.T) {
 	assert.Equal(uint32(0), event.Check.Status)
 	assert.False(event.HasMetrics())
 
-	falsePath := testutil.CommandPath(filepath.Join(toolsDir, "false"))
-	checkConfig.Command = falsePath
-
+	execution.Status = 1
 	agent.executeCheck(request)
-
 	msg = <-ch
 
 	event = &types.Event{}
@@ -89,45 +83,44 @@ func TestExecuteCheck(t *testing.T) {
 	assert.Equal(uint32(1), event.Check.Status)
 	assert.NotZero(event.Check.Issued)
 
-	sleepPath := testutil.CommandPath(filepath.Join(toolsDir, "sleep"), "5")
-	checkConfig.Command = sleepPath
-	checkConfig.Timeout = 1
-
+	execution.Status = 127
+	execution.Output = "command not found"
 	agent.executeCheck(request)
+	msg = <-ch
 
+	event = &types.Event{}
+	assert.NoError(json.Unmarshal(msg.Payload, event))
+	assert.NotZero(event.Timestamp)
+	assert.Equal(uint32(127), event.Check.Status)
+	assert.Equal("command not found", event.Check.Output)
+	assert.NotZero(event.Check.Issued)
+
+	execution.Status = 2
+	execution.Output = ""
+	agent.executeCheck(request)
 	msg = <-ch
 
 	event = &types.Event{}
 	assert.NoError(json.Unmarshal(msg.Payload, event))
 	assert.NotZero(event.Timestamp)
 	assert.Equal(uint32(2), event.Check.Status)
 
-	checkConfig.Command = truePath
 	checkConfig.OutputMetricHandlers = nil
 	checkConfig.OutputMetricFormat = ""
-
+	execution.Status = 0
+	execution.Output = "metric.foo 1 123456789\nmetric.bar 2 987654321"
+	ex.On("Execute", mock.Anything, mock.Anything).Return(execution, nil)
 	agent.executeCheck(request)
-
 	msg = <-ch
 
 	event = &types.Event{}
 	assert.NoError(json.Unmarshal(msg.Payload, event))
 	assert.NotZero(event.Timestamp)
 	assert.False(event.HasMetrics())
 
-	metrics := "metric.foo 1 123456789\nmetric.bar 2 987654321"
-	f, err := ioutil.TempFile("", "metric")
-	assert.NoError(err)
-	_, err = fmt.Fprintln(f, metrics)
-	require.NoError(t, err)
-	f.Close()
-	defer os.Remove(f.Name())
 	checkConfig.OutputMetricFormat = types.GraphiteOutputMetricFormat
-	catPath := testutil.CommandPath(filepath.Join(toolsDir, "cat"), f.Name())
-	checkConfig.Command = catPath
-
+	ex.On("Execute", mock.Anything, mock.Anything).Return(execution, nil)
 	agent.executeCheck(request)
-
 	msg = <-ch
 
 	event = &types.Event{}

agent/hook.go

@@ -53,7 +53,7 @@ func (a *Agent) executeHook(hookConfig *types.HookConfig, check string) *types.H
 	}
 
 	// Instantiate the execution command
-	ex := &command.Execution{
+	ex := command.ExecutionRequest{
 		Command:      hookConfig.Command,
 		Timeout:      int(hookConfig.Timeout),
 		InProgress:   a.inProgress,
@@ -71,14 +71,15 @@ func (a *Agent) executeHook(hookConfig *types.HookConfig, check string) *types.H
 		ex.Input = string(input)
 	}
 
-	if _, err := command.ExecuteCommand(context.Background(), ex); err != nil {
+	hookExec, err := a.executor.Execute(context.Background(), ex)
+	if err != nil {
 		hook.Output = err.Error()
 	} else {
-		hook.Output = ex.Output
+		hook.Output = hookExec.Output
 	}
 
-	hook.Duration = ex.Duration
-	hook.Status = int32(ex.Status)
+	hook.Duration = hookExec.Duration
+	hook.Status = int32(hookExec.Status)
 
 	return hook
 }

agent/hook_test.go

@@ -1,13 +1,15 @@
 package agent
 
 import (
-	"path/filepath"
 	"testing"
 
-	"github.com/sensu/sensu-go/testing/testutil"
+	"github.com/sensu/sensu-go/command"
+	"github.com/sensu/sensu-go/testing/mockexecutor"
+
 	"github.com/sensu/sensu-go/transport"
 	"github.com/sensu/sensu-go/types"
 	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
 )
 
 func TestExecuteHook(t *testing.T) {
@@ -20,23 +22,23 @@ func TestExecuteHook(t *testing.T) {
 	agent := NewAgent(config)
 	ch := make(chan *transport.Message, 1)
 	agent.sendq = ch
-
-	truePath := testutil.CommandPath(filepath.Join(toolsDir, "true"))
-	hookConfig.Command = truePath
+	ex := &mockexecutor.MockExecutor{}
+	agent.executor = ex
+	execution := command.FixtureExecutionResponse(0, "")
+	ex.On("Execute", mock.Anything, mock.Anything).Return(execution, nil)
 
 	hook := agent.executeHook(hookConfig, "check")
 
 	assert.NotZero(hook.Executed)
-	assert.Equal(hook.Status, int32(0))
-	assert.Equal(hook.Output, "")
-
-	hookConfig.Command = "printf hello"
+	assert.Equal(int32(0), hook.Status)
+	assert.Equal("", hook.Output)
 
+	execution.Output = "hello"
 	hook = agent.executeHook(hookConfig, "check")
 
 	assert.NotZero(hook.Executed)
-	assert.Equal(hook.Status, int32(0))
-	assert.Equal(hook.Output, "hello")
+	assert.Equal(int32(0), hook.Status)
+	assert.Equal("hello", hook.Output)
 }
 
 func TestPrepareHook(t *testing.T) {

backend/pipelined/handle.go

@@ -177,8 +177,8 @@ func (p *Pipelined) expandHandlers(ctx context.Context, handlers []string, level
 
 // pipeHandler fork/executes a child process for a Sensu pipe handler
 // command and writes the mutated eventData to it via STDIN.
-func (p *Pipelined) pipeHandler(handler *types.Handler, eventData []byte) (*command.Execution, error) {
-	handlerExec := &command.Execution{}
+func (p *Pipelined) pipeHandler(handler *types.Handler, eventData []byte) (*command.ExecutionResponse, error) {
+	handlerExec := command.ExecutionRequest{}
 	handlerExec.Command = handler.Command
 	handlerExec.Timeout = int(handler.Timeout)
 	handlerExec.Env = handler.EnvVars
@@ -191,7 +191,7 @@ func (p *Pipelined) pipeHandler(handler *types.Handler, eventData []byte) (*comm
 		"handler":      handler.Name,
 	}
 
-	result, err := command.ExecuteCommand(context.Background(), handlerExec)
+	result, err := p.executor.Execute(context.Background(), handlerExec)
 
 	if err != nil {
 		logger.WithFields(fields).WithError(err).Error("failed to execute event pipe handler")

backend/pipelined/handle_test.go

@@ -12,6 +12,7 @@ import (
 	"testing"
 
 	storre "github.com/sensu/sensu-go/backend/store"
+	"github.com/sensu/sensu-go/command"
 	"github.com/sensu/sensu-go/rpc"
 	"github.com/sensu/sensu-go/testing/mockstore"
 	"github.com/sensu/sensu-go/types"
@@ -155,6 +156,7 @@ func TestPipelinedExpandHandlers(t *testing.T) {
 
 func TestPipelinedPipeHandler(t *testing.T) {
 	p := &Pipelined{}
+	p.executor = &command.ExecutionRequest{}
 
 	handler := types.FakeHandlerCommand("cat")
 	handler.Type = "pipe"

backend/pipelined/mutate.go

@@ -111,7 +111,7 @@ func (p *Pipelined) onlyCheckOutputMutator(event *types.Event) []byte {
 // STDIN, and captures the command output (STDOUT/ERR) to be used as
 // the mutated event data for a Sensu event handler.
 func (p *Pipelined) pipeMutator(mutator *types.Mutator, event *types.Event) ([]byte, error) {
-	mutatorExec := &command.Execution{}
+	mutatorExec := command.ExecutionRequest{}
 	mutatorExec.Command = mutator.Command
 	mutatorExec.Timeout = int(mutator.Timeout)
 	mutatorExec.Env = mutator.EnvVars
@@ -123,7 +123,7 @@ func (p *Pipelined) pipeMutator(mutator *types.Mutator, event *types.Event) ([]b
 
 	mutatorExec.Input = string(eventData[:])
 
-	result, err := command.ExecuteCommand(context.Background(), mutatorExec)
+	result, err := p.executor.Execute(context.Background(), mutatorExec)
 
 	if err != nil {
 		return nil, err

backend/pipelined/pipelined.go

@@ -7,6 +7,7 @@ import (
 
 	"github.com/sensu/sensu-go/backend/messaging"
 	"github.com/sensu/sensu-go/backend/store"
+	"github.com/sensu/sensu-go/command"
 	"github.com/sensu/sensu-go/rpc"
 	"github.com/sensu/sensu-go/types"
 )
@@ -35,6 +36,7 @@ type Pipelined struct {
 	store             store.Store
 	bus               messaging.MessageBus
 	extensionExecutor ExtensionExecutorGetterFunc
+	executor          command.Executor
 }
 
 // Config configures a Pipelined.
@@ -58,6 +60,7 @@ func New(c Config, options ...Option) (*Pipelined, error) {
 		wg:                &sync.WaitGroup{},
 		errChan:           make(chan error, 1),
 		eventChan:         make(chan interface{}, 100),
+		executor:          command.NewExecutor(),
 	}
 	for _, o := range options {
 		if err := o(p); err != nil {