From 540c62876f3aaa55bc1c738a9a7e13092a692057 Mon Sep 17 00:00:00 2001 From: someshkoli Date: Sat, 24 Apr 2021 23:17:04 +0530 Subject: [PATCH 1/5] Adds wildcard filepath option to tools command - Adds glob check - Adds test and test files Signed-off-by: someshkoli --- cmd/thanos/testdata/rules-files/valid_1.yaml | 18 +++++++++++ cmd/thanos/testdata/rules-files/valid_2.yaml | 18 +++++++++++ cmd/thanos/tools.go | 33 +++++++++++++------- cmd/thanos/tools_test.go | 15 +++++++++ 4 files changed, 72 insertions(+), 12 deletions(-) create mode 100644 cmd/thanos/testdata/rules-files/valid_1.yaml create mode 100644 cmd/thanos/testdata/rules-files/valid_2.yaml diff --git a/cmd/thanos/testdata/rules-files/valid_1.yaml b/cmd/thanos/testdata/rules-files/valid_1.yaml new file mode 100644 index 0000000000..fb75e799e2 --- /dev/null +++ b/cmd/thanos/testdata/rules-files/valid_1.yaml @@ -0,0 +1,18 @@ +groups: + - name: test-alert-group + partial_response_strategy: "warn" + interval: 2m + rules: + - alert: TestAlert + expr: 1 + labels: + key: value + annotations: + key: value + + - name: test-rule-group + partial_response_strategy: "warn" + interval: 2m + rules: + - record: test_metric + expr: 1 diff --git a/cmd/thanos/testdata/rules-files/valid_2.yaml b/cmd/thanos/testdata/rules-files/valid_2.yaml new file mode 100644 index 0000000000..fb75e799e2 --- /dev/null +++ b/cmd/thanos/testdata/rules-files/valid_2.yaml @@ -0,0 +1,18 @@ +groups: + - name: test-alert-group + partial_response_strategy: "warn" + interval: 2m + rules: + - alert: TestAlert + expr: 1 + labels: + key: value + annotations: + key: value + + - name: test-rule-group + partial_response_strategy: "warn" + interval: 2m + rules: + - record: test_metric + expr: 1 diff --git a/cmd/thanos/tools.go b/cmd/thanos/tools.go index c0c9b04ed8..68575a30bf 100644 --- a/cmd/thanos/tools.go +++ b/cmd/thanos/tools.go @@ -4,7 +4,9 @@ package main import ( + "errors" "os" + "path/filepath" "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" @@ -25,7 +27,7 @@ func registerTools(app *extkingpin.App) { func registerCheckRules(app extkingpin.AppClause) { cmd := app.Command("rules-check", "Check if the rule files are valid or not.") - ruleFiles := cmd.Flag("rules", "The rule files glob to check (repeated).").Required().ExistingFiles() + ruleFiles := cmd.Flag("rules", "The rule files glob to check (repeated).").Required().Strings() cmd.Setup(func(g *run.Group, logger log.Logger, reg *prometheus.Registry, _ opentracing.Tracer, _ <-chan struct{}, _ bool) error { // Dummy actor to immediately kill the group after the run function returns. @@ -39,26 +41,33 @@ func checkRulesFiles(logger log.Logger, files *[]string) error { for _, fn := range *files { level.Info(logger).Log("msg", "checking", "filename", fn) - f, err := os.Open(fn) + matches, err := filepath.Glob(fn) + if matches == nil { + err = errors.New("Matching file not found") + } if err != nil { level.Error(logger).Log("result", "FAILED", "error", err) level.Info(logger).Log() failed.Add(err) continue } - defer func() { _ = f.Close() }() + for _, fn1 := range matches { + // no need to check path error + f, _ := os.Open(fn1) + defer func() { _ = f.Close() }() - n, errs := rules.ValidateAndCount(f) - if errs.Err() != nil { - level.Error(logger).Log("result", "FAILED") - for _, e := range errs { - level.Error(logger).Log("error", e.Error()) - failed.Add(e) + n, errs := rules.ValidateAndCount(f) + if errs.Err() != nil { + level.Error(logger).Log("result", "FAILED") + for _, e := range errs { + level.Error(logger).Log("error", e.Error()) + failed.Add(e) + } + level.Info(logger).Log() + continue } - level.Info(logger).Log() - continue + level.Info(logger).Log("result", "SUCCESS", "rules found", n) } - level.Info(logger).Log("result", "SUCCESS", "rules found", n) } return failed.Err() } diff --git a/cmd/thanos/tools_test.go b/cmd/thanos/tools_test.go index 8a5792f180..512f788c90 100644 --- a/cmd/thanos/tools_test.go +++ b/cmd/thanos/tools_test.go @@ -29,3 +29,18 @@ func Test_CheckRules(t *testing.T) { testutil.NotOk(t, checkRulesFiles(logger, &fn), "expected err for file %s", fn) } } + +func Test_CheckRules_Glob(t *testing.T) { + // regex path + files := &[]string{"./testdata/rules-files/valid*.yaml"} + logger := log.NewNopLogger() + testutil.Ok(t, checkRulesFiles(logger, files)) + + // direct path + files = &[]string{"./testdata/rules-files/valid.yaml"} + testutil.Ok(t, checkRulesFiles(logger, files)) + + // invalid path + files = &[]string{"./testdata/rules-files/*.yamlaaa"} + testutil.NotOk(t, checkRulesFiles(logger, files), "expected err for file %s", files) +} From 46964d7c326b143c741ef72754ceed2087ffa039 Mon Sep 17 00:00:00 2001 From: someshkoli Date: Sun, 25 Apr 2021 15:52:44 +0530 Subject: [PATCH 2/5] improve error check for `filepath.Glob` Signed-off-by: someshkoli --- cmd/thanos/tools.go | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/cmd/thanos/tools.go b/cmd/thanos/tools.go index 68575a30bf..1d4f0e7f24 100644 --- a/cmd/thanos/tools.go +++ b/cmd/thanos/tools.go @@ -38,17 +38,22 @@ func registerCheckRules(app extkingpin.AppClause) { func checkRulesFiles(logger log.Logger, files *[]string) error { var failed errutil.MultiError + errhandle := func(e error) { + level.Error(logger).Log("result", "FAILED", "error", e) + level.Info(logger).Log() + failed.Add(e) + } for _, fn := range *files { level.Info(logger).Log("msg", "checking", "filename", fn) matches, err := filepath.Glob(fn) - if matches == nil { - err = errors.New("Matching file not found") - } if err != nil { - level.Error(logger).Log("result", "FAILED", "error", err) - level.Info(logger).Log() - failed.Add(err) + errhandle(err) + continue + } + if matches == nil { + err = errors.New("matching file not found") + errhandle(err) continue } for _, fn1 := range matches { From c159f3192427501de2ad5a34388b4a5e1269ab55 Mon Sep 17 00:00:00 2001 From: someshkoli Date: Sun, 25 Apr 2021 15:58:40 +0530 Subject: [PATCH 3/5] update errors import Signed-off-by: someshkoli --- cmd/thanos/tools.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cmd/thanos/tools.go b/cmd/thanos/tools.go index 1d4f0e7f24..b2002b824b 100644 --- a/cmd/thanos/tools.go +++ b/cmd/thanos/tools.go @@ -4,7 +4,6 @@ package main import ( - "errors" "os" "path/filepath" @@ -12,6 +11,7 @@ import ( "github.com/go-kit/kit/log/level" "github.com/oklog/run" "github.com/opentracing/opentracing-go" + "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" "github.com/thanos-io/thanos/pkg/errutil" "github.com/thanos-io/thanos/pkg/extkingpin" From 60fcc00f407688267a9b70bf6f9f52d53cac76c9 Mon Sep 17 00:00:00 2001 From: someshkoli Date: Mon, 26 Apr 2021 23:21:44 +0530 Subject: [PATCH 4/5] minor code refactor Signed-off-by: someshkoli --- cmd/thanos/tools.go | 28 +++++++++++----------------- 1 file changed, 11 insertions(+), 17 deletions(-) diff --git a/cmd/thanos/tools.go b/cmd/thanos/tools.go index b2002b824b..698d66e285 100644 --- a/cmd/thanos/tools.go +++ b/cmd/thanos/tools.go @@ -36,29 +36,23 @@ func registerCheckRules(app extkingpin.AppClause) { }) } -func checkRulesFiles(logger log.Logger, files *[]string) error { +func checkRulesFiles(logger log.Logger, patterns *[]string) error { var failed errutil.MultiError - errhandle := func(e error) { - level.Error(logger).Log("result", "FAILED", "error", e) - level.Info(logger).Log() - failed.Add(e) - } - for _, fn := range *files { - level.Info(logger).Log("msg", "checking", "filename", fn) - matches, err := filepath.Glob(fn) - if err != nil { - errhandle(err) - continue - } - if matches == nil { + for _, p := range *patterns { + level.Info(logger).Log("msg", "checking", "pattern", p) + matches, err := filepath.Glob(p) + if err != nil || matches == nil { err = errors.New("matching file not found") - errhandle(err) + level.Error(logger).Log("result", "FAILED", "error", err) + level.Info(logger).Log() + failed.Add(err) continue } - for _, fn1 := range matches { + for _, fn := range matches { + level.Info(logger).Log("msg", "checking", "filename", fn) // no need to check path error - f, _ := os.Open(fn1) + f, _ := os.Open(fn) defer func() { _ = f.Close() }() n, errs := rules.ValidateAndCount(f) From 3dbaedf83029400541e8360194c65edf84776da7 Mon Sep 17 00:00:00 2001 From: someshkoli Date: Tue, 27 Apr 2021 19:02:15 +0530 Subject: [PATCH 5/5] add err check for file open Signed-off-by: someshkoli --- cmd/thanos/tools.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/cmd/thanos/tools.go b/cmd/thanos/tools.go index 698d66e285..e916da8ed0 100644 --- a/cmd/thanos/tools.go +++ b/cmd/thanos/tools.go @@ -51,8 +51,13 @@ func checkRulesFiles(logger log.Logger, patterns *[]string) error { } for _, fn := range matches { level.Info(logger).Log("msg", "checking", "filename", fn) - // no need to check path error - f, _ := os.Open(fn) + f, er := os.Open(fn) + if er != nil { + level.Error(logger).Log("result", "FAILED", "error", er) + level.Info(logger).Log() + failed.Add(err) + continue + } defer func() { _ = f.Close() }() n, errs := rules.ValidateAndCount(f)