Merge pull request #15 from treydock/timeouts

Simplified timeout and error handling

collectors/mmces.go

@@ -112,25 +112,13 @@ func (c *MmcesCollector) collect(nodename string) ([]CESMetric, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Duration(*mmcesTimeout)*time.Second)
 	defer cancel()
 	mmces_state_out, err = mmcesExec(nodename, ctx)
-	if ctx.Err() == context.DeadlineExceeded {
-		if c.useCache {
-			metrics = mmcesCache
-		}
-		return metrics, ctx.Err()
-	}
-	if err != nil {
-		if c.useCache {
-			metrics = mmcesCache
-		}
-		return metrics, err
-	}
-	metrics, err = mmces_state_show_parse(mmces_state_out)
 	if err != nil {
 		if c.useCache {
 			metrics = mmcesCache
 		}
 		return metrics, err
 	}
+	metrics = mmces_state_show_parse(mmces_state_out)
 	if c.useCache {
 		mmcesCache = metrics
 	}
@@ -142,13 +130,15 @@ func mmces(nodename string, ctx context.Context) (string, error) {
 	var out bytes.Buffer
 	cmd.Stdout = &out
 	err := cmd.Run()
-	if err != nil {
+	if ctx.Err() == context.DeadlineExceeded {
+		return "", ctx.Err()
+	} else if err != nil {
 		return "", err
 	}
 	return out.String(), nil
 }
 
-func mmces_state_show_parse(out string) ([]CESMetric, error) {
+func mmces_state_show_parse(out string) []CESMetric {
 	var metrics []CESMetric
 	lines := strings.Split(out, "\n")
 	var headers []string
@@ -176,7 +166,7 @@ func mmces_state_show_parse(out string) ([]CESMetric, error) {
 		metric.State = values[i]
 		metrics = append(metrics, metric)
 	}
-	return metrics, nil
+	return metrics
 }
 
 func parseMmcesState(status string) float64 {

collectors/mmces_test.go

@@ -49,11 +49,40 @@ func TestMmces(t *testing.T) {
 	}
 }
 
-func TestParseMmcesStateShow(t *testing.T) {
-	metrics, err := mmces_state_show_parse(mmcesStdout)
-	if err != nil {
-		t.Errorf("Unexpected error: %s", err.Error())
+func TestMmcesError(t *testing.T) {
+	execCommand = fakeExecCommand
+	mockedExitStatus = 1
+	mockedStdout = "foo"
+	defer func() { execCommand = exec.CommandContext }()
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	out, err := mmces("ib-protocol01.domain", ctx)
+	if err == nil {
+		t.Errorf("Expected error")
+	}
+	if out != "" {
+		t.Errorf("Unexpected out: %s", out)
+	}
+}
+
+func TestMmcesTimeout(t *testing.T) {
+	execCommand = fakeExecCommand
+	mockedExitStatus = 1
+	mockedStdout = "foo"
+	defer func() { execCommand = exec.CommandContext }()
+	ctx, cancel := context.WithTimeout(context.Background(), 0*time.Second)
+	defer cancel()
+	out, err := mmces("ib-protocol01.domain", ctx)
+	if err != context.DeadlineExceeded {
+		t.Errorf("Expected DeadlineExceeded")
+	}
+	if out != "" {
+		t.Errorf("Unexpected out: %s", out)
 	}
+}
+
+func TestParseMmcesStateShow(t *testing.T) {
+	metrics := mmces_state_show_parse(mmcesStdout)
 	if len(metrics) != 8 {
 		t.Errorf("Expected 8 metrics returned, got %d", len(metrics))
 		return

collectors/mmdf.go

@@ -193,19 +193,11 @@ func (c *MmdfCollector) mmdfCollect(fs string) (DFMetric, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Duration(*mmdfTimeout)*time.Second)
 	defer cancel()
 	out, err = MmdfExec(fs, ctx)
-	if ctx.Err() == context.DeadlineExceeded {
-		dfMetric = c.mmdfReadCache(fs)
-		return dfMetric, ctx.Err()
-	}
-	if err != nil {
-		dfMetric = c.mmdfReadCache(fs)
-		return dfMetric, err
-	}
-	dfMetric, err = parse_mmdf(out, c.logger)
 	if err != nil {
 		dfMetric = c.mmdfReadCache(fs)
 		return dfMetric, err
 	}
+	dfMetric = parse_mmdf(out, c.logger)
 	c.mmdfWriteCache(fs, dfMetric)
 	return dfMetric, nil
 }
@@ -231,13 +223,15 @@ func mmdf(fs string, ctx context.Context) (string, error) {
 	var out bytes.Buffer
 	cmd.Stdout = &out
 	err := cmd.Run()
-	if err != nil {
+	if ctx.Err() == context.DeadlineExceeded {
+		return "", ctx.Err()
+	} else if err != nil {
 		return "", err
 	}
 	return out.String(), nil
 }
 
-func parse_mmdf(out string, logger log.Logger) (DFMetric, error) {
+func parse_mmdf(out string, logger log.Logger) DFMetric {
 	var dfMetrics DFMetric
 	headers := make(map[string][]string)
 	values := make(map[string][]string)
@@ -282,7 +276,7 @@ func parse_mmdf(out string, logger log.Logger) (DFMetric, error) {
 			}
 		}
 	}
-	return dfMetrics, nil
+	return dfMetrics
 }
 
 func (c *MmdfCollector) mmdfReadCache(fs string) DFMetric {

collectors/mmdf_test.go

@@ -60,11 +60,40 @@ func TestMmdf(t *testing.T) {
 	}
 }
 
-func TestParseMmdf(t *testing.T) {
-	dfmetrics, err := parse_mmdf(mmdfStdout, log.NewNopLogger())
-	if err != nil {
-		t.Errorf("Unexpected error: %s", err.Error())
+func TestMmdfError(t *testing.T) {
+	execCommand = fakeExecCommand
+	mockedExitStatus = 1
+	mockedStdout = "foo"
+	defer func() { execCommand = exec.CommandContext }()
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	out, err := mmdf("test", ctx)
+	if err == nil {
+		t.Errorf("Expected error")
+	}
+	if out != "" {
+		t.Errorf("Unexpected out: %s", out)
+	}
+}
+
+func TestMmdfTimeout(t *testing.T) {
+	execCommand = fakeExecCommand
+	mockedExitStatus = 1
+	mockedStdout = "foo"
+	defer func() { execCommand = exec.CommandContext }()
+	ctx, cancel := context.WithTimeout(context.Background(), 0*time.Second)
+	defer cancel()
+	out, err := mmdf("test", ctx)
+	if err != context.DeadlineExceeded {
+		t.Errorf("Expected DeadlineExceeded")
+	}
+	if out != "" {
+		t.Errorf("Unexpected out: %s", out)
 	}
+}
+
+func TestParseMmdf(t *testing.T) {
+	dfmetrics := parse_mmdf(mmdfStdout, log.NewNopLogger())
 	if dfmetrics.InodesFree != 484301506 {
 		t.Errorf("Unexpected value for InodesFree, got %d", dfmetrics.InodesFree)
 	}

collectors/mmdiag.go

@@ -97,25 +97,13 @@ func (c *MmdiagCollector) collect() (DiagMetric, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Duration(*mmdiagTimeout)*time.Second)
 	defer cancel()
 	out, err = mmdiagExec("--waiters", ctx)
-	if ctx.Err() == context.DeadlineExceeded {
-		if c.useCache {
-			diagMetric = mmdiagCache
-		}
-		return diagMetric, ctx.Err()
-	}
-	if err != nil {
-		if c.useCache {
-			diagMetric = mmdiagCache
-		}
-		return diagMetric, err
-	}
-	err = parse_mmdiag_waiters(out, &diagMetric, c.logger)
 	if err != nil {
 		if c.useCache {
 			diagMetric = mmdiagCache
 		}
 		return diagMetric, err
 	}
+	parse_mmdiag_waiters(out, &diagMetric, c.logger)
 	if c.useCache {
 		mmdiagCache = diagMetric
 	}
@@ -127,13 +115,15 @@ func mmdiag(arg string, ctx context.Context) (string, error) {
 	var out bytes.Buffer
 	cmd.Stdout = &out
 	err := cmd.Run()
-	if err != nil {
+	if ctx.Err() == context.DeadlineExceeded {
+		return "", ctx.Err()
+	} else if err != nil {
 		return "", err
 	}
 	return out.String(), nil
 }
 
-func parse_mmdiag_waiters(out string, diagMetric *DiagMetric, logger log.Logger) error {
+func parse_mmdiag_waiters(out string, diagMetric *DiagMetric, logger log.Logger) {
 	lines := strings.Split(out, "\n")
 	waitersPattern := regexp.MustCompile(`^Waiting ([0-9.]+) sec.*thread ([0-9]+)`)
 	excludePattern := regexp.MustCompile(*configWaiterExclude)
@@ -156,5 +146,4 @@ func parse_mmdiag_waiters(out string, diagMetric *DiagMetric, logger log.Logger)
 			diagMetric.Waiters = append(diagMetric.Waiters, waiter)
 		}
 	}
-	return nil
 }

collectors/mmdiag_test.go

@@ -71,15 +71,44 @@ func TestMmdiag(t *testing.T) {
 	}
 }
 
+func TestMmdiagError(t *testing.T) {
+	execCommand = fakeExecCommand
+	mockedExitStatus = 1
+	mockedStdout = "foo"
+	defer func() { execCommand = exec.CommandContext }()
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	out, err := mmdiag("--waiters", ctx)
+	if err == nil {
+		t.Errorf("Expected error")
+	}
+	if out != "" {
+		t.Errorf("Unexpected out: %s", out)
+	}
+}
+
+func TestMmdiagTimeout(t *testing.T) {
+	execCommand = fakeExecCommand
+	mockedExitStatus = 1
+	mockedStdout = "foo"
+	defer func() { execCommand = exec.CommandContext }()
+	ctx, cancel := context.WithTimeout(context.Background(), 0*time.Second)
+	defer cancel()
+	out, err := mmdiag("--waiters", ctx)
+	if err != context.DeadlineExceeded {
+		t.Errorf("Expected DeadlineExceeded")
+	}
+	if out != "" {
+		t.Errorf("Unexpected out: %s", out)
+	}
+}
+
 func TestParseMmdiagWaiters(t *testing.T) {
 	threshold := 30
 	configWaiterThreshold = &threshold
 	configWaiterExclude = &defWaiterExclude
 	var metric DiagMetric
-	err := parse_mmdiag_waiters(waitersStdout, &metric, log.NewNopLogger())
-	if err != nil {
-		t.Errorf("Unexpected error: %s", err.Error())
-	}
+	parse_mmdiag_waiters(waitersStdout, &metric, log.NewNopLogger())
 	if val := len(metric.Waiters); val != 2 {
 		t.Errorf("Unexpected Waiters len got %v", val)
 		return

collectors/mmgetstate.go

@@ -96,25 +96,13 @@ func (c *MmgetstateCollector) collect() (MmgetstateMetrics, error) {
 	ctx, cancel := context.WithTimeout(context.Background(), time.Duration(*mmgetstateTimeout)*time.Second)
 	defer cancel()
 	out, err = MmgetstateExec(ctx)
-	if ctx.Err() == context.DeadlineExceeded {
-		if c.useCache {
-			metric = mmgetstateCache
-		}
-		return metric, ctx.Err()
-	}
-	if err != nil {
-		if c.useCache {
-			metric = mmgetstateCache
-		}
-		return metric, err
-	}
-	metric, err = mmgetstate_parse(out)
 	if err != nil {
 		if c.useCache {
 			metric = mmgetstateCache
 		}
 		return metric, err
 	}
+	metric = mmgetstate_parse(out)
 	if c.useCache {
 		mmgetstateCache = metric
 	}
@@ -126,13 +114,15 @@ func mmgetstate(ctx context.Context) (string, error) {
 	var out bytes.Buffer
 	cmd.Stdout = &out
 	err := cmd.Run()
-	if err != nil {
+	if ctx.Err() == context.DeadlineExceeded {
+		return "", ctx.Err()
+	} else if err != nil {
 		return "", err
 	}
 	return out.String(), nil
 }
 
-func mmgetstate_parse(out string) (MmgetstateMetrics, error) {
+func mmgetstate_parse(out string) MmgetstateMetrics {
 	metric := MmgetstateMetrics{}
 	lines := strings.Split(out, "\n")
 	var headers []string
@@ -158,5 +148,5 @@ func mmgetstate_parse(out string) (MmgetstateMetrics, error) {
 			}
 		}
 	}
-	return metric, nil
+	return metric
 }

collectors/mmgetstate_test.go

@@ -49,11 +49,40 @@ func TestMmgetstate(t *testing.T) {
 	}
 }
 
-func TestParseMmgetstate(t *testing.T) {
-	metric, err := mmgetstate_parse(mmgetstateStdout)
-	if err != nil {
-		t.Errorf("Unexpected error: %s", err.Error())
+func TestMmgetstateError(t *testing.T) {
+	execCommand = fakeExecCommand
+	mockedExitStatus = 1
+	mockedStdout = "foo"
+	defer func() { execCommand = exec.CommandContext }()
+	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
+	defer cancel()
+	out, err := mmgetstate(ctx)
+	if err == nil {
+		t.Errorf("Expected error")
+	}
+	if out != "" {
+		t.Errorf("Unexpected out: %s", out)
+	}
+}
+
+func TestMmgetstateTimeout(t *testing.T) {
+	execCommand = fakeExecCommand
+	mockedExitStatus = 1
+	mockedStdout = "foo"
+	defer func() { execCommand = exec.CommandContext }()
+	ctx, cancel := context.WithTimeout(context.Background(), 0*time.Second)
+	defer cancel()
+	out, err := mmgetstate(ctx)
+	if err != context.DeadlineExceeded {
+		t.Errorf("Expected DeadlineExceeded")
+	}
+	if out != "" {
+		t.Errorf("Unexpected out: %s", out)
 	}
+}
+
+func TestParseMmgetstate(t *testing.T) {
+	metric := mmgetstate_parse(mmgetstateStdout)
 	if val := metric.state; val != "active" {
 		t.Errorf("Unexpected state got %s", val)
 	}