From 017d80a0e4d258454825a19a88ea5cc06dd25b43 Mon Sep 17 00:00:00 2001 From: Trey Dockendorf Date: Wed, 31 Mar 2021 05:59:48 -0400 Subject: [PATCH] Ensure errors in mmdf parsing return errors --- collectors/mmdf.go | 12 ++++++------ collectors/mmdf_test.go | 41 +++++++++++++++++++++++++++++------------ 2 files changed, 35 insertions(+), 18 deletions(-) diff --git a/collectors/mmdf.go b/collectors/mmdf.go index cf5c59a..eb9899d 100644 --- a/collectors/mmdf.go +++ b/collectors/mmdf.go @@ -174,8 +174,8 @@ func (c *MmdfCollector) mmdfCollect(fs string) (DFMetric, error) { if err != nil { return DFMetric{}, err } - dfMetric := parse_mmdf(out, c.logger) - return dfMetric, nil + dfMetric, err := parse_mmdf(out, c.logger) + return dfMetric, err } func mmlfsfsFilesystems(ctx context.Context, logger log.Logger) ([]string, error) { @@ -204,7 +204,7 @@ func mmdf(fs string, ctx context.Context) (string, error) { return out.String(), nil } -func parse_mmdf(out string, logger log.Logger) DFMetric { +func parse_mmdf(out string, logger log.Logger) (DFMetric, error) { var dfMetrics DFMetric headers := make(map[string][]string) values := make(map[string][]string) @@ -231,11 +231,11 @@ func parse_mmdf(out string, logger log.Logger) DFMetric { for k, vals := range headers { if _, ok := values[k]; !ok { level.Error(logger).Log("msg", "Header section missing from values", "header", k) - continue + return dfMetrics, fmt.Errorf("Header section missing from values: %s", k) } if len(vals) != len(values[k]) { level.Error(logger).Log("msg", "Length of headers does not equal length of values", "header", k, "values", len(values[k]), "headers", len(vals)) - continue + return dfMetrics, fmt.Errorf("Length of headers does not equal length of values: %s", k) } for i, v := range vals { mapKey := fmt.Sprintf("%s:%s", k, v) @@ -257,5 +257,5 @@ func parse_mmdf(out string, logger log.Logger) DFMetric { } } } - return dfMetrics + return dfMetrics, nil } diff --git a/collectors/mmdf_test.go b/collectors/mmdf_test.go index a716e1c..845dd4a 100644 --- a/collectors/mmdf_test.go +++ b/collectors/mmdf_test.go @@ -42,7 +42,7 @@ mmdf:metadata:0:1:::13891534848:6011299328:43:58139768:0: mmdf:fsTotal:0:1:::3661677723648:481202021888:14:12117655064:0: mmdf:inode:0:1:::430741822:484301506:915043328:1332164000: ` - mmdfStdoutErr = ` + mmdfStdoutErrValues = ` mmdf:nsd:HEADER:version:reserved:reserved:nsdName:storagePool:diskSize:failureGroup:metadata:data:freeBlocks:freeBlocksPct:freeFragments:freeFragmentsPct:diskAvailableForAlloc: mmdf:poolTotal:HEADER:version:reserved:reserved:poolName:poolSize:freeBlocks:freeBlocksPct:freeFragments:freeFragmentsPct:maxDiskSize: mmdf:data:HEADER:version:reserved:reserved:totalData:freeBlocks:freeBlocksPct:freeFragments:freeFragmentsPct: @@ -53,6 +53,21 @@ mmdf:nsd:0:1:::P_META_VD102:system:771751936:300:Yes:No:320274944:41:5005384:1:: mmdf:nsd:0:1:::P_DATA_VD02:data:46766489600:200:No:Yes:6092915712:13:154966272:0:: mmdf:poolTotal:0:1:::data:3647786188800:475190722560:13:12059515296:0:3860104580096: mmdf:data:0:1:::3647786188800:475190722560:13:12059515296:0: +mmdf:fsTotal:0:1:::3661677723648:481202021888:14:12117655064:0: +mmdf:inode:0:1:::430741822:484301506:915043328:1332164000: +` + mmdfStdoutErrLen = ` +mmdf:nsd:HEADER:version:reserved:reserved:nsdName:storagePool:diskSize:failureGroup:metadata:data:freeBlocks:freeBlocksPct:freeFragments:freeFragmentsPct:diskAvailableForAlloc: +mmdf:poolTotal:HEADER:version:reserved:reserved:poolName:poolSize:freeBlocks:freeBlocksPct:freeFragments:freeFragmentsPct:maxDiskSize: +mmdf:data:HEADER:version:reserved:reserved:totalData:freeBlocks:freeBlocksPct:freeFragments:freeFragmentsPct: +mmdf:metadata:HEADER:version:reserved:reserved:totalMetadata:freeBlocks:freeBlocksPct:freeFragments:freeFragmentsPct: +mmdf:fsTotal:HEADER:version:reserved:reserved:fsSize:freeBlocks:freeBlocksPct:freeFragments:freeFragmentsPct: +mmdf:inode:HEADER:version:reserved:reserved:usedInodes:freeInodes:allocatedInodes:maxInodes: +mmdf:nsd:0:1:::P_META_VD102:system:771751936:300:Yes:No:320274944:41:5005384:1:: +mmdf:nsd:0:1:::P_DATA_VD02:data:46766489600:200:No:Yes:6092915712:13:154966272:0:: +mmdf:poolTotal:0:1:::data:3647786188800:475190722560:13:12059515296:0:3860104580096: +mmdf:data:0:1:::3647786188800:475190722560:13:12059515296:0: +mmdf:metadata:0:1:::13891534848:6011299328:43:58139768:0: mmdf:fsTotal:0:1:::3661677723648:481202021888:14:12117655064: mmdf:inode:0:1:::430741822:484301506:915043328:1332164000: ` @@ -107,7 +122,11 @@ func TestMmdfTimeout(t *testing.T) { } func TestParseMmdf(t *testing.T) { - dfmetrics := parse_mmdf(mmdfStdout, log.NewNopLogger()) + dfmetrics, err := parse_mmdf(mmdfStdout, log.NewNopLogger()) + if err != nil { + t.Errorf("Unexpected error: %s", err.Error()) + return + } if dfmetrics.InodesFree != 484301506 { t.Errorf("Unexpected value for InodesFree, got %v", dfmetrics.InodesFree) } @@ -120,17 +139,15 @@ func TestParseMmdf(t *testing.T) { } func TestParseMmdfErrors(t *testing.T) { - dfmetrics := parse_mmdf(mmdfStdoutErr, log.NewNopLogger()) - if dfmetrics.InodesFree != 484301506 { - t.Errorf("Unexpected value for InodesFree, got %v", dfmetrics.InodesFree) - } - // Was skipped due to invalid length - if dfmetrics.FSTotal != 0 { - t.Errorf("Unexpected value for FSTotal, got %v", dfmetrics.FSTotal) + _, err := parse_mmdf(mmdfStdoutErrValues, log.NewNopLogger()) + if err == nil { + t.Errorf("Expected error") + return } - // Was skipped due to missing values - if dfmetrics.MetadataTotal != 0 { - t.Errorf("Unexpected value for MetadataTotal, got %v", dfmetrics.MetadataTotal) + _, err = parse_mmdf(mmdfStdoutErrLen, log.NewNopLogger()) + if err == nil { + t.Errorf("Expected error") + return } }