pkg/rule: Fix bugs where rules were out of sync.

CHANGELOG.md

@@ -44,6 +44,7 @@ Compactor now properly handles partial block uploads for all operation like rete
 - [#1882](https://github.com/thanos-io/thanos/pull/1882) Receive: upload to object storage as 'receive' rather than 'sidecar'.
 - [#1907](https://github.com/thanos-io/thanos/pull/1907) Store: Fixed the duration unit for the metric `thanos_bucket_store_series_gate_duration_seconds`.
 - [#1931](https://github.com/thanos-io/thanos/pull/1931) Compact: Fixed the compactor successfully exiting when actually an error occurred while compacting a blocks group.
+- [#1439](https://github.com/thanos-io/thanos/pull/1439) Rule: Fix bugs where rules were out of sync.
 - [#1872](https://github.com/thanos-io/thanos/pull/1872) Ruler: `/api/v1/rules` now shows a properly formatted value
 - [#1945](https://github.com/thanos-io/thanos/pull/1945) `master` container images are now built with Go 1.13
 - [#1956](https://github.com/thanos-io/thanos/pull/1956) Ruler: now properly ignores duplicated query addresses

cmd/thanos/rule.go

@@ -484,16 +484,24 @@ func runRule(
 				}
 
 				level.Debug(logger).Log("msg", "configured rule files", "files", strings.Join(ruleFiles, ","))
-				var files []string
+				var (
+					files     []string
+					seenFiles = make(map[string]struct{})
+				)
 				for _, pat := range ruleFiles {
 					fs, err := filepath.Glob(pat)
 					if err != nil {
 						// The only error can be a bad pattern.
 						level.Error(logger).Log("msg", "retrieving rule files failed. Ignoring file.", "pattern", pat, "err", err)
 						continue
 					}
-
-					files = append(files, fs...)
+					for _, fp := range fs {
+						if _, ok := seenFiles[fp]; ok {
+							continue
+						}
+						files = append(files, fp)
+						seenFiles[fp] = struct{}{}
+					}
 				}
 
 				level.Info(logger).Log("msg", "reload rule files", "numFiles", len(files))

pkg/rule/rule.go

@@ -67,17 +67,17 @@ func (m *Manager) SetRuleManager(s storepb.PartialResponseStrategy, mgr *rules.M
 func (m *Manager) RuleGroups() []Group {
 	m.mtx.RLock()
 	defer m.mtx.RUnlock()
-	var res []Group
+	var groups []Group
 	for s, r := range m.mgrs {
 		for _, group := range r.RuleGroups() {
-			res = append(res, Group{
+			groups = append(groups, Group{
 				Group:                   group,
 				PartialResponseStrategy: s,
 				originalFile:            m.ruleFiles[group.File()],
 			})
 		}
 	}
-	return res
+	return groups
 }
 
 func (m *Manager) AlertingRules() []AlertingRule {
@@ -213,6 +213,21 @@ func (m *Manager) Update(evalInterval time.Duration, files []string) error {
 			continue
 		}
 	}
+
+	for s, mgr := range m.mgrs {
+		if _, ok := filesByStrategy[s]; ok {
+			continue
+		}
+
+		if len(mgr.RuleGroups()) == 0 {
+			continue
+		}
+
+		if err := mgr.Update(evalInterval, []string{}, nil); err != nil {
+			errs = append(errs, err)
+		}
+	}
+
 	m.ruleFiles = ruleFiles
 	m.mtx.Unlock()
 

pkg/rule/rule_test.go

@@ -63,14 +63,17 @@ groups:
 		Appendable: nopAppendable{},
 	}
 	thanosRuleMgr := NewManager(dir)
-	ruleMgr := rules.NewManager(&opts)
-	thanosRuleMgr.SetRuleManager(storepb.PartialResponseStrategy_ABORT, ruleMgr)
-	thanosRuleMgr.SetRuleManager(storepb.PartialResponseStrategy_WARN, ruleMgr)
+	ruleMgrAbort := rules.NewManager(&opts)
+	ruleMgrWarn := rules.NewManager(&opts)
+	thanosRuleMgr.SetRuleManager(storepb.PartialResponseStrategy_ABORT, ruleMgrAbort)
+	thanosRuleMgr.SetRuleManager(storepb.PartialResponseStrategy_WARN, ruleMgrWarn)
 
-	testutil.Ok(t, thanosRuleMgr.Update(10*time.Second, []string{filepath.Join(dir, "rule.yaml")}))
+	ruleMgrAbort.Run()
+	ruleMgrWarn.Run()
+	defer ruleMgrAbort.Stop()
+	defer ruleMgrWarn.Stop()
 
-	ruleMgr.Run()
-	defer ruleMgr.Stop()
+	testutil.Ok(t, thanosRuleMgr.Update(10*time.Second, []string{filepath.Join(dir, "rule.yaml")}))
 
 	select {
 	case <-time.After(2 * time.Minute):
@@ -222,6 +225,44 @@ groups:
 	}
 }
 
+func TestUpdateAfterClear(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))
+
+	opts := rules.ManagerOptions{
+		Logger: log.NewLogfmtLogger(os.Stderr),
+	}
+	m := NewManager(dir)
+	ruleMgrAbort := rules.NewManager(&opts)
+	ruleMgrWarn := rules.NewManager(&opts)
+	m.SetRuleManager(storepb.PartialResponseStrategy_ABORT, ruleMgrAbort)
+	m.SetRuleManager(storepb.PartialResponseStrategy_WARN, ruleMgrWarn)
+
+	ruleMgrAbort.Run()
+	ruleMgrWarn.Run()
+	defer ruleMgrAbort.Stop()
+	defer ruleMgrWarn.Stop()
+
+	err = m.Update(1*time.Second, []string{
+		filepath.Join(dir, "no_strategy.yaml"),
+	})
+	testutil.Ok(t, err)
+	testutil.Equals(t, 1, len(m.RuleGroups()))
+
+	err = m.Update(1*time.Second, []string{})
+	testutil.Ok(t, err)
+	testutil.Equals(t, 0, len(m.RuleGroups()))
+}
+
 func TestRuleGroupMarshalYAML(t *testing.T) {
 	const expected = `groups:
 - name: something1