From c505e015b3e045b82ca996486d0613973e8a5597 Mon Sep 17 00:00:00 2001
From: Matteo Cerutti <matteo.cerutti@hotmail.co.uk>
Date: Mon, 22 May 2017 21:41:34 +0100
Subject: [PATCH] Add timeout option to ipmi_sensor plugin - solves #2817
 (#2818)

---
 CHANGELOG.md                            |  1 +
 plugins/inputs/ipmi_sensor/README.md    |  7 +++++++
 plugins/inputs/ipmi_sensor/ipmi.go      | 11 ++++++++++-
 plugins/inputs/ipmi_sensor/ipmi_test.go |  6 +++++-
 4 files changed, 23 insertions(+), 2 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 2163a49f2..e95077d49 100644
--- 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
index 3a75d0c65..a1f6c8129 100644
--- 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
index 73f22b393..3b03e62ad 100644
--- 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
index 9e846065e..a6f5148c1 100644
--- 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)