Add timeout option to ipmi_sensor plugin - solves influxdata#2817 (in…

…fluxdata#2818)
jeichorn · Jul 24, 2017 · 4a8d18c · 4a8d18c
1 parent 0b91aea
commit 4a8d18c
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 2 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -5,6 +5,7 @@
 
 - [#2773](https://github.com/influxdata/telegraf/pull/2773): Add support for self-signed certs to InfluxDB input plugin
 - [#2581](https://github.com/influxdata/telegraf/pull/2581): Add Docker container environment variables as tags. Only whitelisted
+- [#2817](https://github.com/influxdata/telegraf/pull/2817): Added timeout option to IPMI sensor plugin
 
 ### Bugfixes
 

diff --git a/plugins/inputs/ipmi_sensor/README.md b/plugins/inputs/ipmi_sensor/README.md
@@ -43,6 +43,13 @@ The `server` tag will be made available when retrieving stats from remote server
   ## if no servers are specified, local machine sensor stats will be queried
   ##
   # servers = ["USERID:PASSW0RD@lan(192.168.1.1)"]
+
+  ## Recomended: use metric 'interval' that is a multiple of 'timeout' to avoid
+  ## gaps or overlap in pulled data
+  interval = "30s"
+
+  ## Timeout for the ipmitool command to complete. Default is 20 seconds.
+  timeout = "20s"
 ```
 
 ## Output

diff --git a/plugins/inputs/ipmi_sensor/ipmi.go b/plugins/inputs/ipmi_sensor/ipmi.go
@@ -19,6 +19,7 @@ var (
 type Ipmi struct {
 	Path    string
 	Servers []string
+	Timeout internal.Duration
 }
 
 var sampleConfig = `
@@ -33,6 +34,13 @@ var sampleConfig = `
   ## if no servers are specified, local machine sensor stats will be queried
   ##
   # servers = ["USERID:PASSW0RD@lan(192.168.1.1)"]
+
+  ## Recomended: use metric 'interval' that is a multiple of 'timeout' to avoid
+  ## gaps or overlap in pulled data
+  interval = "30s"
+
+  ## Timeout for the ipmitool command to complete
+  timeout = "20s"
 `
 
 func (m *Ipmi) SampleConfig() string {
@@ -78,7 +86,7 @@ func (m *Ipmi) parse(acc telegraf.Accumulator, server string) error {
 
 	opts = append(opts, "sdr")
 	cmd := execCommand(m.Path, opts...)
-	out, err := internal.CombinedOutputTimeout(cmd, time.Second*5)
+	out, err := internal.CombinedOutputTimeout(cmd, m.Timeout.Duration)
 	if err != nil {
 		return fmt.Errorf("failed to run command %s: %s - %s", strings.Join(cmd.Args, " "), err, string(out))
 	}
@@ -152,6 +160,7 @@ func init() {
 	if len(path) > 0 {
 		m.Path = path
 	}
+	m.Timeout = internal.Duration{Duration: time.Second * 20}
 	inputs.Add("ipmi_sensor", func() telegraf.Input {
 		m := m
 		return &m

diff --git a/plugins/inputs/ipmi_sensor/ipmi_test.go b/plugins/inputs/ipmi_sensor/ipmi_test.go
@@ -5,7 +5,9 @@ import (
 	"os"
 	"os/exec"
 	"testing"
+	"time"
 
+	"github.com/influxdata/telegraf/internal"
 	"github.com/influxdata/telegraf/testutil"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
@@ -15,6 +17,7 @@ func TestGather(t *testing.T) {
 	i := &Ipmi{
 		Servers: []string{"USERID:PASSW0RD@lan(192.168.1.1)"},
 		Path:    "ipmitool",
+		Timeout: internal.Duration{Duration: time.Second * 5},
 	}
 	// overwriting exec commands with mock commands
 	execCommand = fakeExecCommand
@@ -118,7 +121,8 @@ func TestGather(t *testing.T) {
 	}
 
 	i = &Ipmi{
-		Path: "ipmitool",
+		Path:    "ipmitool",
+		Timeout: internal.Duration{Duration: time.Second * 5},
 	}
 
 	err = acc.GatherError(i.Gather)