Rule: update manager when all rule files are removed (#3095)

This bug was already fixed in #2615 but it got lost when we merged #2200. Co-authored-by: johncming <[email protected]> Co-authored-by: Lili Cosic <[email protected]> Signed-off-by: Simon Pasquier <[email protected]> Co-authored-by: johncming <[email protected]> Co-authored-by: Lili Cosic <[email protected]>
thanos-io · Aug 28, 2020 · 905d7f1 · 905d7f1
1 parent 4ef1b8d
commit 905d7f1
Show file tree

Hide file tree

Showing 3 changed files with 57 additions and 0 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -11,6 +11,10 @@ We use *breaking :warning:* word for marking changes that are not backward compa
 
 ## Unreleased
 
+### Fixed
+
+* [#3095](https://github.com/thanos-io/thanos/pull/3095) Rule: update manager when all rule files are removed.
+
 ## [v0.15.0-rc.0](https://github.com/thanos-io/thanos/releases/tag/v0.15.0-rc.0) - 2020.08.26
 
 Highlights:

diff --git a/pkg/rules/manager.go b/pkg/rules/manager.go
@@ -314,6 +314,12 @@ func (m *Manager) Update(evalInterval time.Duration, files []string) error {
 		ruleFiles       = map[string]string{}
 	)
 
+	// Initialize filesByStrategy for existing managers' strategies to make
+	// sure that managers are updated when they have no rules configured.
+	for strategy := range m.mgrs {
+		filesByStrategy[strategy] = make([]string, 0)
+	}
+
 	if err := os.RemoveAll(m.workDir); err != nil {
 		return errors.Wrapf(err, "remove %s", m.workDir)
 	}

diff --git a/pkg/rules/manager_test.go b/pkg/rules/manager_test.go
@@ -320,3 +320,50 @@ func TestManager_Rules(t *testing.T) {
 	}()
 	testRulesAgainstExamples(t, filepath.Join(curr, "../../examples/alerts"), thanosRuleMgr)
 }
+
+func TestManagerUpdateWithNoRules(t *testing.T) {
+	dir, err := ioutil.TempDir("", "test_rule_rule_groups")
+	testutil.Ok(t, err)
+	defer func() { testutil.Ok(t, os.RemoveAll(dir)) }()
+
+	testutil.Ok(t, ioutil.WriteFile(filepath.Join(dir, "no_strategy.yaml"), []byte(`
+groups:
+- name: "something1"
+  rules:
+  - alert: "some"
+    expr: "up"
+`), os.ModePerm))
+
+	thanosRuleMgr := NewManager(
+		context.Background(),
+		nil,
+		dir,
+		rules.ManagerOptions{
+			Logger:    log.NewLogfmtLogger(os.Stderr),
+			Queryable: nopQueryable{},
+		},
+		func(partialResponseStrategy storepb.PartialResponseStrategy) rules.QueryFunc {
+			return func(ctx context.Context, q string, t time.Time) (promql.Vector, error) {
+				return nil, nil
+			}
+		},
+		nil,
+	)
+
+	// We need to run the underlying rule managers to update them more than
+	// once (otherwise there's a deadlock).
+	thanosRuleMgr.Run()
+	defer func() {
+		thanosRuleMgr.Stop()
+	}()
+
+	err = thanosRuleMgr.Update(1*time.Second, []string{
+		filepath.Join(dir, "no_strategy.yaml"),
+	})
+	testutil.Ok(t, err)
+	testutil.Equals(t, 1, len(thanosRuleMgr.RuleGroups()))
+
+	err = thanosRuleMgr.Update(1*time.Second, []string{})
+	testutil.Ok(t, err)
+	testutil.Equals(t, 0, len(thanosRuleMgr.RuleGroups()))
+}