From 575cb09235a05d9045cacd78d53229a7dfe041a3 Mon Sep 17 00:00:00 2001 From: Juraj Michalek Date: Sat, 10 Jul 2021 22:54:30 +0200 Subject: [PATCH 1/7] rule: fix reload signal not working Signed-off-by: Juraj Michalek --- cmd/thanos/rule.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/thanos/rule.go b/cmd/thanos/rule.go index ff5ca1310c..1872e6e935 100644 --- a/cmd/thanos/rule.go +++ b/cmd/thanos/rule.go @@ -80,7 +80,6 @@ type ruleConfig struct { ruleFiles []string objStoreConfig *extflag.PathOrContent dataDir string - reloadSignal <-chan struct{} lset labels.Labels } @@ -195,6 +194,7 @@ func registerRule(app *extkingpin.App) { tracer, comp, *conf, + reload, getFlagsMap(cmd.Flags()), httpLogOpts, grpcLogOpts, @@ -257,6 +257,7 @@ func runRule( tracer opentracing.Tracer, comp component.Component, conf ruleConfig, + reloadSignal <-chan struct{}, flagsMap map[string]string, httpLogOpts []logging.Option, grpcLogOpts []grpc_logging.Option, @@ -483,7 +484,7 @@ func runRule( } for { select { - case <-conf.reloadSignal: + case <-reloadSignal: if err := reloadRules(logger, conf.ruleFiles, ruleMgr, conf.evalInterval, metrics); err != nil { level.Error(logger).Log("msg", "reload rules by sighup failed", "err", err) } From ddec89951f489db01713053c8ea92fcfdb314ea6 Mon Sep 17 00:00:00 2001 From: Juraj Michalek Date: Tue, 13 Jul 2021 22:26:00 +0200 Subject: [PATCH 2/7] CHANGELOG: added PR Signed-off-by: Juraj Michalek --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3321088fd5..cfabd3cf3d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,8 @@ We use *breaking :warning:* to mark changes that are not backward compatible (re ### Fixed +- [#4442](https://github.com/thanos-io/thanos/pull/4442) rule: fix reload signal not working + ### Changed ## [v0.22.0 - in progress](https://github.com/thanos-io/thanos/tree/release-0.22) From 541cacc561d234de7bcba8382b16ce871cdf28ef Mon Sep 17 00:00:00 2001 From: Juraj Michalek Date: Thu, 15 Jul 2021 15:32:22 +0200 Subject: [PATCH 3/7] rule: added test for realoding using sighup signal Signed-off-by: Juraj Michalek --- test/e2e/rule_test.go | 43 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 38 insertions(+), 5 deletions(-) diff --git a/test/e2e/rule_test.go b/test/e2e/rule_test.go index e2ee815132..e97202d6e0 100644 --- a/test/e2e/rule_test.go +++ b/test/e2e/rule_test.go @@ -58,7 +58,7 @@ groups: annotations: summary: "I always complain and allow partial response in query." ` - testAlertRuleAddedLaterWebHandler = ` + testAlertRuleAddedLater = ` groups: - name: example interval: 1s @@ -97,6 +97,34 @@ func reloadRulesHTTP(t *testing.T, ctx context.Context, endpoint string) { testutil.Equals(t, 200, resp.StatusCode) } +func reloadRulesSignal(t *testing.T, r *e2ethanos.Service) { + c := e2e.NewCommand("kill", "-1", "1") + _, _, err := r.Exec(c) + testutil.Ok(t, err) +} + +func checkReloadSuccessful(t *testing.T, ctx context.Context, endpoint string) { + req, err := http.NewRequestWithContext(ctx, "GET", "http://"+endpoint+"/api/v1/rules", 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) + defer resp.Body.Close() + + body, err := ioutil.ReadAll(resp.Body) + testutil.Ok(t, err) + + var data struct { + Status string + Data *rulespb.RuleGroups + } + + testutil.Ok(t, json.Unmarshal(body, &data)) + testutil.Equals(t, "success", data.Status) + + testutil.Assert(t, len(data.Data.Groups) == 3, "expected there to be 3 rule groups") +} + func rulegroupCorrectData(t *testing.T, ctx context.Context, endpoint string) { req, err := http.NewRequestWithContext(ctx, "GET", "http://"+endpoint+"/api/v1/rules", ioutil.NopCloser(bytes.NewReader(nil))) testutil.Ok(t, err) @@ -317,11 +345,16 @@ func TestRule(t *testing.T) { rulegroupCorrectData(t, ctx, r.HTTPEndpoint()) }) - 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. + t.Run("signal reload works", func(t *testing.T) { + // Add a new rule via sending sighup + createRuleFile(t, fmt.Sprintf("%s/newrule.yaml", rulesPath), testAlertRuleAddedLater) + reloadRulesSignal(t, r) + checkReloadSuccessful(t, ctx, r.HTTPEndpoint()) + }) - createRuleFile(t, fmt.Sprintf("%s/newrule.yaml", rulesPath), testAlertRuleAddedLaterWebHandler) + t.Run("http reload works", func(t *testing.T) { + // Add a new rule via /-/reload. + createRuleFile(t, fmt.Sprintf("%s/newrule.yaml", rulesPath), testAlertRuleAddedLater) reloadRulesHTTP(t, ctx, r.HTTPEndpoint()) }) From 5ac9691d3e828e880eed200ff67a98686cfafbf3 Mon Sep 17 00:00:00 2001 From: Juraj Michalek Date: Fri, 16 Jul 2021 15:27:36 +0200 Subject: [PATCH 4/7] rule: adressed comments for test of realoding using sighup Signed-off-by: Juraj Michalek --- test/e2e/rule_test.go | 56 ++++++++++++++++++++++++++++++++----------- 1 file changed, 42 insertions(+), 14 deletions(-) diff --git a/test/e2e/rule_test.go b/test/e2e/rule_test.go index e97202d6e0..b4de47cc37 100644 --- a/test/e2e/rule_test.go +++ b/test/e2e/rule_test.go @@ -58,7 +58,7 @@ groups: annotations: summary: "I always complain and allow partial response in query." ` - testAlertRuleAddedLater = ` + testAlertRuleAddedLaterWebHandler = ` groups: - name: example interval: 1s @@ -71,9 +71,42 @@ groups: severity: page annotations: summary: "I always complain and I have been loaded via /-/reload." +` + testAlertRuleAddedLaterSignal = ` +groups: +- name: example + interval: 1s + 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 sighup signal." +- name: example2 + interval: 1s + 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 sighup signal." ` ) + + + +type rulesResp struct { + Status string + Data *rulespb.RuleGroups +} + func createRuleFile(t *testing.T, path, content string) { t.Helper() err := ioutil.WriteFile(path, []byte(content), 0666) @@ -103,7 +136,7 @@ func reloadRulesSignal(t *testing.T, r *e2ethanos.Service) { testutil.Ok(t, err) } -func checkReloadSuccessful(t *testing.T, ctx context.Context, endpoint string) { +func checkReloadSuccessful(t *testing.T, ctx context.Context, endpoint string, expectedRulegroupCount int) { req, err := http.NewRequestWithContext(ctx, "GET", "http://"+endpoint+"/api/v1/rules", ioutil.NopCloser(bytes.NewReader(nil))) testutil.Ok(t, err) resp, err := http.DefaultClient.Do(req) @@ -114,15 +147,12 @@ func checkReloadSuccessful(t *testing.T, ctx context.Context, endpoint string) { body, err := ioutil.ReadAll(resp.Body) testutil.Ok(t, err) - var data struct { - Status string - Data *rulespb.RuleGroups - } + var data = rulesResp{} testutil.Ok(t, json.Unmarshal(body, &data)) testutil.Equals(t, "success", data.Status) - testutil.Assert(t, len(data.Data.Groups) == 3, "expected there to be 3 rule groups") + testutil.Assert(t, len(data.Data.Groups) == expectedRulegroupCount, fmt.Sprintf("expected there to be %d rule groups", expectedRulegroupCount)) } func rulegroupCorrectData(t *testing.T, ctx context.Context, endpoint string) { @@ -136,10 +166,7 @@ func rulegroupCorrectData(t *testing.T, ctx context.Context, endpoint string) { body, err := ioutil.ReadAll(resp.Body) testutil.Ok(t, err) - var data struct { - Status string - Data *rulespb.RuleGroups - } + var data = rulesResp{} testutil.Ok(t, json.Unmarshal(body, &data)) testutil.Equals(t, "success", data.Status) @@ -347,15 +374,16 @@ func TestRule(t *testing.T) { t.Run("signal reload works", func(t *testing.T) { // Add a new rule via sending sighup - createRuleFile(t, fmt.Sprintf("%s/newrule.yaml", rulesPath), testAlertRuleAddedLater) + createRuleFile(t, fmt.Sprintf("%s/newrule.yaml", rulesPath), testAlertRuleAddedLaterSignal) reloadRulesSignal(t, r) - checkReloadSuccessful(t, ctx, r.HTTPEndpoint()) + checkReloadSuccessful(t, ctx, r.HTTPEndpoint(), 4) }) t.Run("http reload works", func(t *testing.T) { // Add a new rule via /-/reload. - createRuleFile(t, fmt.Sprintf("%s/newrule.yaml", rulesPath), testAlertRuleAddedLater) + createRuleFile(t, fmt.Sprintf("%s/newrule.yaml", rulesPath), testAlertRuleAddedLaterWebHandler) reloadRulesHTTP(t, ctx, r.HTTPEndpoint()) + checkReloadSuccessful(t, ctx, r.HTTPEndpoint(), 3) }) queryAndAssertSeries(t, ctx, q.HTTPEndpoint(), "ALERTS", promclient.QueryOptions{ From 3f4a4ade85d4c9fd90a768b205026833df094331 Mon Sep 17 00:00:00 2001 From: Juraj Michalek Date: Fri, 16 Jul 2021 15:30:17 +0200 Subject: [PATCH 5/7] ran make format Signed-off-by: Juraj Michalek --- test/e2e/rule_test.go | 3 --- 1 file changed, 3 deletions(-) diff --git a/test/e2e/rule_test.go b/test/e2e/rule_test.go index b4de47cc37..fb40d84788 100644 --- a/test/e2e/rule_test.go +++ b/test/e2e/rule_test.go @@ -99,9 +99,6 @@ groups: ` ) - - - type rulesResp struct { Status string Data *rulespb.RuleGroups From f96b9bb90a67b690b4cbfcb2fc601ba5dc9691f5 Mon Sep 17 00:00:00 2001 From: Juraj Michalek Date: Mon, 19 Jul 2021 13:23:41 +0200 Subject: [PATCH 6/7] addressed comment for test of realoding using sighup Signed-off-by: Juraj Michalek --- test/e2e/rule_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/test/e2e/rule_test.go b/test/e2e/rule_test.go index fb40d84788..bc3f09520d 100644 --- a/test/e2e/rule_test.go +++ b/test/e2e/rule_test.go @@ -139,10 +139,9 @@ func checkReloadSuccessful(t *testing.T, ctx context.Context, endpoint string, e resp, err := http.DefaultClient.Do(req) testutil.Ok(t, err) testutil.Equals(t, 200, resp.StatusCode) - defer resp.Body.Close() body, err := ioutil.ReadAll(resp.Body) - testutil.Ok(t, err) + testutil.Ok(t, resp.Body.Close()) var data = rulesResp{} From c20d14a675ec19bb30953b1870fadf30e0658ade Mon Sep 17 00:00:00 2001 From: Juraj Michalek Date: Mon, 19 Jul 2021 13:31:58 +0200 Subject: [PATCH 7/7] make linter happy Signed-off-by: Juraj Michalek --- test/e2e/rule_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/e2e/rule_test.go b/test/e2e/rule_test.go index bc3f09520d..3e7f6a2069 100644 --- a/test/e2e/rule_test.go +++ b/test/e2e/rule_test.go @@ -140,7 +140,7 @@ func checkReloadSuccessful(t *testing.T, ctx context.Context, endpoint string, e testutil.Ok(t, err) testutil.Equals(t, 200, resp.StatusCode) - body, err := ioutil.ReadAll(resp.Body) + body, _ := ioutil.ReadAll(resp.Body) testutil.Ok(t, resp.Body.Close()) var data = rulesResp{}