adjust interfaces of cmdutil

mackerelio · Jan 6, 2018 · d159eb2 · d159eb2
1 parent cf267fb
commit d159eb2
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 68 deletions.
diff --git a/cmdutil/cmdutil.go b/cmdutil/cmdutil.go
@@ -26,6 +26,8 @@ var cmdBase = []string{"sh", "-c"}
 // CommandOption carries a timeout duration.
 type CommandOption struct {
 	TimeoutDuration time.Duration
+	User            string
+	Env             []string
 }
 
 func init() {
@@ -35,23 +37,23 @@ func init() {
 }
 
 // RunCommand runs command (in two string) and returns stdout, stderr strings and its exit code.
-func RunCommand(command, user string, env []string, opt CommandOption) (stdout, stderr string, exitCode int, err error) {
+func RunCommand(command string, opt CommandOption) (stdout, stderr string, exitCode int, err error) {
 	cmdArgs := append(cmdBase, command)
-	return RunCommandArgs(cmdArgs, user, env, opt)
+	return RunCommandArgs(cmdArgs, opt)
 }
 
 // RunCommandArgs run the command
-func RunCommandArgs(cmdArgs []string, user string, env []string, opt CommandOption) (stdout, stderr string, exitCode int, err error) {
+func RunCommandArgs(cmdArgs []string, opt CommandOption) (stdout, stderr string, exitCode int, err error) {
 	args := append([]string{}, cmdArgs...)
-	if user != "" {
+	if opt.User != "" {
 		if runtime.GOOS == "windows" {
-			logger.Warningf("RunCommand ignore option: user = %q", user)
+			logger.Warningf("RunCommand ignore option: user = %q", opt.User)
 		} else {
-			args = append([]string{"sudo", "-Eu", user}, args...)
+			args = append([]string{"sudo", "-Eu", opt.User}, args...)
 		}
 	}
 	cmd := exec.Command(args[0], args[1:]...)
-	cmd.Env = append(os.Environ(), env...)
+	cmd.Env = append(os.Environ(), opt.Env...)
 	tio := &timeout.Timeout{
 		Cmd:       cmd,
 		Duration:  defaultTimeoutDuration,

diff --git a/cmdutil/cmdutil_test.go b/cmdutil/cmdutil_test.go
@@ -16,7 +16,7 @@ var testCmdOpt = CommandOption{
 }
 
 func TestRunCommand(t *testing.T) {
-	stdout, stderr, exitCode, err := RunCommand("echo 1", "", nil, testCmdOpt)
+	stdout, stderr, exitCode, err := RunCommand("echo 1", testCmdOpt)
 	if runtime.GOOS == "windows" {
 		stdout = strings.Replace(stdout, "\r\n", "\n", -1)
 		stderr = strings.Replace(stderr, "\r\n", "\n", -1)
@@ -59,7 +59,7 @@ func TestRunCommandWithTimeout(t *testing.T) {
 	if runtime.GOOS == "windows" {
 		command, tmpdir = makeSleep(t)
 	}
-	stdout, stderr, _, err := RunCommand(command, "", nil, testCmdOpt)
+	stdout, stderr, _, err := RunCommand(command, testCmdOpt)
 	if stdout != "" {
 		t.Errorf("stdout shoud be empty")
 	}
@@ -80,7 +80,10 @@ func TestRunCommandWithEnv(t *testing.T) {
 		command = `echo %TEST_RUN_COMMAND_ENV%`
 	}
 
-	stdout, stderr, exitCode, err := RunCommand(command, "", []string{"TEST_RUN_COMMAND_ENV=mackerel-agent"}, testCmdOpt)
+	stdout, stderr, exitCode, err := RunCommand(command, CommandOption{
+		Env:             []string{"TEST_RUN_COMMAND_ENV=mackerel-agent"},
+		TimeoutDuration: 1 * time.Second,
+	})
 	if runtime.GOOS == "windows" {
 		stdout = strings.Replace(stdout, "\r\n", "\n", -1)
 		stderr = strings.Replace(stderr, "\r\n", "\n", -1)

diff --git a/config/config.go b/config/config.go
@@ -153,9 +153,10 @@ type PluginConfig struct {
 
 // CommandConfig represents an executable command configuration.
 type CommandConfig struct {
-	Raw  interface{} `toml:"command"`
-	User string
-	Env  Env `toml:"env"`
+	Raw            interface{} `toml:"command"`
+	User           string
+	Env            Env `toml:"env"`
+	TimeoutSeconds int64
 }
 
 // Env represents environments.
@@ -182,25 +183,28 @@ type Command struct {
 	cmdutil.CommandOption
 	Cmd  string
 	Args []string
-	User string
-	Env  []string
 }
 
 // Run the Command.
 func (cmd *Command) Run() (stdout, stderr string, exitCode int, err error) {
 	if len(cmd.Args) > 0 {
-		return cmdutil.RunCommandArgs(cmd.Args, cmd.User, cmd.Env, cmd.CommandOption)
+		return cmdutil.RunCommandArgs(cmd.Args, cmd.CommandOption)
 	}
-	return cmdutil.RunCommand(cmd.Cmd, cmd.User, cmd.Env, cmd.CommandOption)
+	return cmdutil.RunCommand(cmd.Cmd, cmd.CommandOption)
 }
 
 // RunWithEnv runs the Command with Environment.
 func (cmd *Command) RunWithEnv(env []string) (stdout, stderr string, exitCode int, err error) {
 	env = append(cmd.Env, env...)
+	opt := cmdutil.CommandOption{
+		User:            cmd.User,
+		Env:             env,
+		TimeoutDuration: cmd.TimeoutDuration,
+	}
 	if len(cmd.Args) > 0 {
-		return cmdutil.RunCommandArgs(cmd.Args, cmd.User, env, cmd.CommandOption)
+		return cmdutil.RunCommandArgs(cmd.Args, opt)
 	}
-	return cmdutil.RunCommand(cmd.Cmd, cmd.User, env, cmd.CommandOption)
+	return cmdutil.RunCommand(cmd.Cmd, opt)
 }
 
 // CommandString returns the command string for log messages
@@ -214,28 +218,21 @@ func (cmd *Command) CommandString() string {
 // MetricPlugin represents the configuration of a metric plugin
 // The User option is ignored on Windows
 type MetricPlugin struct {
-	Command          Command
+	Command          *Command
 	CustomIdentifier *string
 	IncludePattern   *regexp.Regexp
 	ExcludePattern   *regexp.Regexp
 }
 
 func (pconf *PluginConfig) buildMetricPlugin() (*MetricPlugin, error) {
-	cmd, err := parseCommand(pconf.CommandRaw, pconf.User)
+	cmd, err := parseCommand(pconf.CommandRaw, pconf.User, pconf.Env, pconf.TimeoutSeconds)
 	if err != nil {
 		return nil, err
 	}
 	if cmd == nil {
 		return nil, fmt.Errorf("failed to parse plugin command. A configuration value of `command` should be string or string slice, but %T", pconf.CommandRaw)
 	}
 
-	cmd.Env, err = pconf.Env.ConvertToStrings()
-	if err != nil {
-		return nil, err
-	}
-
-	cmd.TimeoutDuration = time.Duration(pconf.TimeoutSeconds * int64(time.Second))
-
 	var (
 		includePattern *regexp.Regexp
 		excludePattern *regexp.Regexp
@@ -254,7 +251,7 @@ func (pconf *PluginConfig) buildMetricPlugin() (*MetricPlugin, error) {
 	}
 
 	return &MetricPlugin{
-		Command:          *cmd,
+		Command:          cmd,
 		CustomIdentifier: pconf.CustomIdentifier,
 		IncludePattern:   includePattern,
 		ExcludePattern:   excludePattern,
@@ -273,34 +270,19 @@ type CheckPlugin struct {
 }
 
 func (pconf *PluginConfig) buildCheckPlugin(name string) (*CheckPlugin, error) {
-	cmd, err := parseCommand(pconf.CommandRaw, pconf.User)
+	cmd, err := parseCommand(pconf.CommandRaw, pconf.User, pconf.Env, pconf.TimeoutSeconds)
 	if err != nil {
 		return nil, err
 	}
 	if cmd == nil {
 		return nil, fmt.Errorf("failed to parse plugin command. A configuration value of `command` should be string or string slice, but %T", pconf.CommandRaw)
 	}
 
-	cmd.Env, err = pconf.Env.ConvertToStrings()
+	action, err := parseCommand(pconf.Action.Raw, pconf.Action.User, pconf.Action.Env, pconf.Action.TimeoutSeconds)
 	if err != nil {
 		return nil, err
 	}
 
-	cmd.TimeoutDuration = time.Duration(pconf.TimeoutSeconds * int64(time.Second))
-
-	action, err := parseCommand(pconf.Action.Raw, pconf.Action.User)
-	if err != nil {
-		return nil, err
-	}
-
-	if action != nil {
-		action.Env, err = pconf.Action.Env.ConvertToStrings()
-		if err != nil {
-			return nil, err
-		}
-		action.TimeoutDuration = cmd.TimeoutDuration
-	}
-
 	plugin := CheckPlugin{
 		Command:               *cmd,
 		NotificationInterval:  pconf.NotificationInterval,
@@ -324,35 +306,28 @@ type MetadataPlugin struct {
 }
 
 func (pconf *PluginConfig) buildMetadataPlugin() (*MetadataPlugin, error) {
-	cmd, err := parseCommand(pconf.CommandRaw, pconf.User)
+	cmd, err := parseCommand(pconf.CommandRaw, pconf.User, pconf.Env, pconf.TimeoutSeconds)
 	if err != nil {
 		return nil, err
 	}
 	if cmd == nil {
 		return nil, fmt.Errorf("failed to parse plugin command. A configuration value of `command` should be string or string slice, but %T", pconf.CommandRaw)
 	}
 
-	cmd.Env, err = pconf.Env.ConvertToStrings()
-	if err != nil {
-		return nil, err
-	}
-
-	cmd.TimeoutDuration = time.Duration(pconf.TimeoutSeconds * int64(time.Second))
-
 	return &MetadataPlugin{
 		Command:           *cmd,
 		ExecutionInterval: pconf.ExecutionInterval,
 	}, nil
 }
 
-func parseCommand(commandRaw interface{}, user string) (command *Command, err error) {
+func parseCommand(commandRaw interface{}, user string, env Env, timeoutSeconds int64) (command *Command, err error) {
 	const errFmt = "failed to parse plugin command. A configuration value of `command` should be string or string slice, but %T"
+	var cmd *Command
 	switch t := commandRaw.(type) {
 	case string:
-		return &Command{
-			Cmd:  t,
-			User: user,
-		}, nil
+		cmd = &Command{
+			Cmd: t,
+		}
 	case []interface{}:
 		if len(t) > 0 {
 			args := []string{}
@@ -361,25 +336,30 @@ func parseCommand(commandRaw interface{}, user string) (command *Command, err er
 				if !ok {
 					return nil, fmt.Errorf(errFmt, commandRaw)
 				}
-
 				args = append(args, str)
 			}
-			return &Command{
+			cmd = &Command{
 				Args: args,
-				User: user,
-			}, nil
+			}
 		}
 		return nil, fmt.Errorf(errFmt, commandRaw)
 	case []string:
-		return &Command{
+		cmd = &Command{
 			Args: t,
-			User: user,
-		}, nil
+		}
 	case nil:
 		return nil, nil
 	default:
 		return nil, fmt.Errorf(errFmt, commandRaw)
 	}
+	cmd.User = user
+	cmd.Env, err = env.ConvertToStrings()
+	if err != nil {
+		return nil, err
+	}
+	cmd.TimeoutDuration = time.Duration(timeoutSeconds * int64(time.Second))
+
+	return cmd, nil
 }
 
 const postMetricsDequeueDelaySecondsMax = 59   // max delay seconds for dequeuing from buffer queue

diff --git a/config/config_test.go b/config/config_test.go
@@ -437,8 +437,8 @@ func TestLoadConfigFile(t *testing.T) {
 	if checks.Command.TimeoutDuration != 60*time.Second {
 		t.Error("check timeout_seconds should be 60s")
 	}
-	if checks.Action.TimeoutDuration != 60*time.Second {
-		t.Error("check timeout_seconds in action should also be 60s")
+	if checks.Action.TimeoutDuration != 0*time.Second {
+		t.Error("check timeout_seconds in action should be 0s")
 	}
 	if *checks.NotificationInterval != 60 {
 		t.Error("notification_interval should be 60")