cmd: rule: do not wrap reload endpoint with prefix twice (#2533)

* cmd: rule: do not wrap reload endpoint with '/' Do not wrap the router with `/` on the `/-/reload` endpoint. Otherwise, it is inaccessible when no prefix has been specified by the user. Signed-off-by: Giedrius Statkevičius <[email protected]> * CHANGELOG: update Signed-off-by: Giedrius Statkevičius <[email protected]> * e2e: rule: add test for reloading rules via /-/reload Add a test-case to the e2e tests for testing whether reloading rules via /-/reload works. Signed-off-by: Giedrius Statkevičius <[email protected]>
thanos-io · Apr 30, 2020 · 4d4ff50 · 4d4ff50
1 parent 1a10b94
commit 4d4ff50
Show file tree

Hide file tree

Showing 3 changed files with 56 additions and 8 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -9,11 +9,12 @@ NOTE: As semantic versioning states all 0.y.z releases can contain breaking chan
 
 We use *breaking* word for marking changes that are not backward compatible (relates only to v0.y.z releases.)
 
-## Unreleased
+## [v0.12.2](https://github.com/thanos-io/thanos/releases/tag/v0.12.2) - 2020.04.XX
 
 ### Fixed
 
 - [#2459](https://github.com/thanos-io/thanos/issues/2459) Compact: Fixed issue with old blocks being marked and deleted in a (slow) loop.
+- [#2533](https://github.com/thanos-io/thanos/pull/2515) Rule: do not wrap reload endpoint with `/`. Makes `/-/reload` accessible again when no prefix has been specified.
 
 ## [v0.12.1](https://github.com/thanos-io/thanos/releases/tag/v0.12.1) - 2020.04.20
 

diff --git a/cmd/thanos/rule.go b/cmd/thanos/rule.go
@@ -561,7 +561,7 @@ func runRule(
 			router = router.WithPrefix(webRoutePrefix)
 		}
 
-		router.WithPrefix(webRoutePrefix).Post("/-/reload", func(w http.ResponseWriter, r *http.Request) {
+		router.Post("/-/reload", func(w http.ResponseWriter, r *http.Request) {
 			reloadMsg := make(chan error)
 			reloadWebhandler <- reloadMsg
 			if err := <-reloadMsg; err != nil {

diff --git a/test/e2e/rule_test.go b/test/e2e/rule_test.go
@@ -59,18 +59,44 @@ groups:
       severity: page
     annotations:
       summary: "I always complain and allow partial response in query."
+`
+	testAlertRuleAddedLaterWebHandler = `
+groups:
+- name: example
+  partial_response_strategy: "WARN"
+  rules:
+  - alert: TestAlert_HasBeenLoadedViaWebHandler
+    # It must be based on actual metric, otherwise call to StoreAPI would be not involved.
+    expr: absent(some_metric)
+    labels:
+      severity: page
+    annotations:
+      summary: "I always complain and I have been loaded via /-/reload."
 `
 )
 
+func createRuleFile(t *testing.T, path, content string) {
+	t.Helper()
+	err := ioutil.WriteFile(path, []byte(content), 0666)
+	testutil.Ok(t, err)
+}
+
 func createRuleFiles(t *testing.T, dir string) {
 	t.Helper()
 
 	for i, rule := range []string{testAlertRuleAbortOnPartialResponse, testAlertRuleWarnOnPartialResponse} {
-		err := ioutil.WriteFile(filepath.Join(dir, fmt.Sprintf("rules-%d.yaml", i)), []byte(rule), 0666)
-		testutil.Ok(t, err)
+		createRuleFile(t, filepath.Join(dir, fmt.Sprintf("rules-%d.yaml", i)), rule)
 	}
 }
 
+func reloadRulesHTTP(t *testing.T, ctx context.Context, endpoint string) {
+	req, err := http.NewRequestWithContext(ctx, "POST", "http://"+endpoint+"/-/reload", ioutil.NopCloser(bytes.NewReader(nil)))
+	testutil.Ok(t, err)
+	resp, err := http.DefaultClient.Do(req)
+	testutil.Ok(t, err)
+	testutil.Equals(t, 200, resp.StatusCode)
+}
+
 func writeTargets(t *testing.T, path string, addrs ...string) {
 	t.Helper()
 
@@ -269,10 +295,14 @@ func TestRule(t *testing.T) {
 	testutil.Ok(t, err)
 	defer s.Close()
 
+	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
+	defer cancel()
+
 	// Prepare work dirs.
 	rulesSubDir := filepath.Join("rules")
-	testutil.Ok(t, os.MkdirAll(filepath.Join(s.SharedDir(), rulesSubDir), os.ModePerm))
-	createRuleFiles(t, filepath.Join(s.SharedDir(), rulesSubDir))
+	rulesPath := filepath.Join(s.SharedDir(), rulesSubDir)
+	testutil.Ok(t, os.MkdirAll(rulesPath, os.ModePerm))
+	createRuleFiles(t, rulesPath)
 	amTargetsSubDir := filepath.Join("rules_am_targets")
 	testutil.Ok(t, os.MkdirAll(filepath.Join(s.SharedDir(), amTargetsSubDir), os.ModePerm))
 	queryTargetsSubDir := filepath.Join("rules_query_targets")
@@ -433,8 +463,13 @@ func TestRule(t *testing.T) {
 		testutil.Ok(t, r.WaitSumMetrics(e2e.Equals(1), "thanos_ruler_alertmanagers_dns_provider_results"))
 	})
 
-	ctx, cancel := context.WithTimeout(context.Background(), 3*time.Minute)
-	defer cancel()
+	t.Run("reload works", func(t *testing.T) {
+		// Add a new rule via /-/reload.
+		// TODO(GiedriusS): add a test for reloading via SIGHUP. Need to extend e2e framework to expose PIDs.
+
+		createRuleFile(t, fmt.Sprintf("%s/newrule.yaml", rulesPath), testAlertRuleAddedLaterWebHandler)
+		reloadRulesHTTP(t, ctx, r.HTTPEndpoint())
+	})
 
 	queryAndAssertSeries(t, ctx, q.HTTPEndpoint(), "ALERTS", promclient.QueryOptions{
 		Deduplicate: false,
@@ -446,6 +481,13 @@ func TestRule(t *testing.T) {
 			"alertstate": "firing",
 			"replica":    "1",
 		},
+		{
+			"__name__":   "ALERTS",
+			"severity":   "page",
+			"alertname":  "TestAlert_HasBeenLoadedViaWebHandler",
+			"alertstate": "firing",
+			"replica":    "1",
+		},
 		{
 			"__name__":   "ALERTS",
 			"severity":   "page",
@@ -461,6 +503,11 @@ func TestRule(t *testing.T) {
 			"alertname": "TestAlert_AbortOnPartialResponse",
 			"replica":   "1",
 		},
+		{
+			"severity":  "page",
+			"alertname": "TestAlert_HasBeenLoadedViaWebHandler",
+			"replica":   "1",
+		},
 		{
 			"severity":  "page",
 			"alertname": "TestAlert_WarnOnPartialResponse",