From 4d4ff5036e3f0e1f96ce530ef4a7caa6970f8f15 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Giedrius=20Statkevi=C4=8Dius?= Date: Thu, 30 Apr 2020 18:44:45 +0300 Subject: [PATCH] cmd: rule: do not wrap reload endpoint with prefix twice (#2533) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 * CHANGELOG: update Signed-off-by: Giedrius Statkevičius * 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 --- CHANGELOG.md | 3 ++- cmd/thanos/rule.go | 2 +- test/e2e/rule_test.go | 59 ++++++++++++++++++++++++++++++++++++++----- 3 files changed, 56 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 00ed757c56..23d09c5714 100644 --- 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 index 33889f7c99..d49409627b 100644 --- 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 index 3a601608b8..0e87b4a9dd 100644 --- 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",