From e809ccb90a5d93cdd42e806bf691a221c6ca8e21 Mon Sep 17 00:00:00 2001 From: machine424 Date: Tue, 6 May 2025 16:46:21 +0200 Subject: [PATCH] test(cmd/main/TestRuntimeGOGCConfig): add checks on reloads as well Signed-off-by: machine424 --- cmd/prometheus/main_test.go | 80 ++++++++++++++++------- cmd/prometheus/query_log_test.go | 16 ++--- cmd/prometheus/scrape_failure_log_test.go | 13 +--- 3 files changed, 65 insertions(+), 44 deletions(-) diff --git a/cmd/prometheus/main_test.go b/cmd/prometheus/main_test.go index 34b026be91..e35585cb86 100644 --- a/cmd/prometheus/main_test.go +++ b/cmd/prometheus/main_test.go @@ -650,6 +650,16 @@ func TestRwProtoMsgFlagParser(t *testing.T) { } } +// reloadPrometheusConfig sends a reload request to the Prometheus server to apply +// updated configurations. +func reloadPrometheusConfig(t *testing.T, reloadURL string) { + t.Helper() + + r, err := http.Post(reloadURL, "text/plain", nil) + require.NoError(t, err, "Failed to reload Prometheus") + require.Equal(t, http.StatusOK, r.StatusCode, "Unexpected status code when reloading Prometheus") +} + func getGaugeValue(t *testing.T, body io.ReadCloser, metricName string) (float64, error) { t.Helper() @@ -679,7 +689,7 @@ func TestRuntimeGOGCConfig(t *testing.T) { name string config string gogcEnvVar string - expectedGOGC int + expectedGOGC float64 }{ { name: "empty config file", @@ -695,7 +705,7 @@ func TestRuntimeGOGCConfig(t *testing.T) { config: ` runtime: gogc: 77`, - expectedGOGC: 77, + expectedGOGC: 77.0, }, { name: "gogc set through config and env var", @@ -703,20 +713,20 @@ runtime: runtime: gogc: 77`, gogcEnvVar: "88", - expectedGOGC: 77, + expectedGOGC: 77.0, }, { name: "incomplete runtime block", config: ` runtime:`, - expectedGOGC: 75, + expectedGOGC: 75.0, }, { name: "incomplete runtime block and GOGC env var set", config: ` runtime:`, gogcEnvVar: "88", - expectedGOGC: 88, + expectedGOGC: 88.0, }, { name: "unrelated config and GOGC env var set", @@ -735,7 +745,13 @@ global: port := testutil.RandomUnprivilegedPort(t) os.WriteFile(configFile, []byte(tc.config), 0o777) - prom := prometheusCommandWithLogging(t, configFile, port, fmt.Sprintf("--storage.tsdb.path=%s", tmpDir)) + prom := prometheusCommandWithLogging( + t, + configFile, + port, + fmt.Sprintf("--storage.tsdb.path=%s", tmpDir), + "--web.enable-lifecycle", + ) // Inject GOGC when set. prom.Env = os.Environ() if tc.gogcEnvVar != "" { @@ -743,24 +759,42 @@ global: } require.NoError(t, prom.Start()) - var ( - r *http.Response - err error - ) - // Wait for the /metrics endpoint to be ready. - require.Eventually(t, func() bool { - r, err = http.Get(fmt.Sprintf("http://127.0.0.1:%d/metrics", port)) - if err != nil { - return false - } - return r.StatusCode == http.StatusOK - }, 5*time.Second, 50*time.Millisecond) - defer r.Body.Close() + ensureGOGCValue := func(val float64) { + var ( + r *http.Response + err error + ) + // Wait for the /metrics endpoint to be ready. + require.Eventually(t, func() bool { + r, err = http.Get(fmt.Sprintf("http://127.0.0.1:%d/metrics", port)) + if err != nil { + return false + } + return r.StatusCode == http.StatusOK + }, 5*time.Second, 50*time.Millisecond) + defer r.Body.Close() - // Check the final GOGC that's set, consider go_gc_gogc_percent from /metrics as source of truth. - gogc, err := getGaugeValue(t, r.Body, "go_gc_gogc_percent") - require.NoError(t, err) - require.Equal(t, float64(tc.expectedGOGC), gogc) + // Check the final GOGC that's set, consider go_gc_gogc_percent from /metrics as source of truth. + gogc, err := getGaugeValue(t, r.Body, "go_gc_gogc_percent") + require.NoError(t, err) + require.Equal(t, val, gogc) + } + + // The value is applied on startup. + ensureGOGCValue(tc.expectedGOGC) + + // After a reload with the same config, the value stays the same. + reloadURL := fmt.Sprintf("http://127.0.0.1:%d/-/reload", port) + reloadPrometheusConfig(t, reloadURL) + ensureGOGCValue(tc.expectedGOGC) + + // After a reload with different config, the value gets updated. + newConfig := ` +runtime: + gogc: 99` + os.WriteFile(configFile, []byte(newConfig), 0o777) + reloadPrometheusConfig(t, reloadURL) + ensureGOGCValue(99.0) }) } } diff --git a/cmd/prometheus/query_log_test.go b/cmd/prometheus/query_log_test.go index c63be02d4d..7c073b59d0 100644 --- a/cmd/prometheus/query_log_test.go +++ b/cmd/prometheus/query_log_test.go @@ -95,13 +95,6 @@ func (p *queryLogTest) setQueryLog(t *testing.T, queryLogFile string) { require.NoError(t, err) } -// reloadConfig reloads the configuration using POST. -func (p *queryLogTest) reloadConfig(t *testing.T) { - r, err := http.Post(fmt.Sprintf("http://%s:%d%s/-/reload", p.host, p.port, p.prefix), "text/plain", nil) - require.NoError(t, err) - require.Equal(t, 200, r.StatusCode) -} - // query runs a query according to the test origin. func (p *queryLogTest) query(t *testing.T) { switch p.origin { @@ -308,6 +301,7 @@ func (p *queryLogTest) run(t *testing.T) { }, p.params()...) prom := exec.Command(promPath, params...) + reloadURL := fmt.Sprintf("http://%s:%d%s/-/reload", p.host, p.port, p.prefix) // Log stderr in case of failure. stderr, err := prom.StderrPipe() @@ -335,7 +329,7 @@ func (p *queryLogTest) run(t *testing.T) { p.query(t) require.Empty(t, readQueryLog(t, queryLogFile.Name())) p.setQueryLog(t, queryLogFile.Name()) - p.reloadConfig(t) + reloadPrometheusConfig(t, reloadURL) } p.query(t) @@ -350,7 +344,7 @@ func (p *queryLogTest) run(t *testing.T) { p.validateLastQuery(t, ql) p.setQueryLog(t, "") - p.reloadConfig(t) + reloadPrometheusConfig(t, reloadURL) if !p.exactQueryCount() { qc = len(readQueryLog(t, queryLogFile.Name())) } @@ -362,7 +356,7 @@ func (p *queryLogTest) run(t *testing.T) { qc = len(ql) p.setQueryLog(t, queryLogFile.Name()) - p.reloadConfig(t) + reloadPrometheusConfig(t, reloadURL) p.query(t) qc++ @@ -406,7 +400,7 @@ func (p *queryLogTest) run(t *testing.T) { } p.validateLastQuery(t, ql) - p.reloadConfig(t) + reloadPrometheusConfig(t, reloadURL) p.query(t) diff --git a/cmd/prometheus/scrape_failure_log_test.go b/cmd/prometheus/scrape_failure_log_test.go index e04b1aaa2d..f35cb7bee6 100644 --- a/cmd/prometheus/scrape_failure_log_test.go +++ b/cmd/prometheus/scrape_failure_log_test.go @@ -112,7 +112,8 @@ scrape_configs: require.NoError(t, err, "Failed to update Prometheus configuration file") // Reload Prometheus with the updated configuration. - reloadPrometheus(t, port) + reloadURL := fmt.Sprintf("http://127.0.0.1:%d/-/reload", port) + reloadPrometheusConfig(t, reloadURL) // Count the number of lines in the scrape failure log file before any // further requests. @@ -146,7 +147,7 @@ scrape_configs: require.NoError(t, err, "Failed to update Prometheus configuration file") // Reload Prometheus with the updated configuration. - reloadPrometheus(t, port) + reloadPrometheusConfig(t, reloadURL) // Wait for at least two more requests to the mock server and verify that // new log entries are created. @@ -160,14 +161,6 @@ scrape_configs: require.Greater(t, countLinesInFile(scrapeFailureLogFile), postReloadLogLineCount, "New lines should be added to the scrape failure log file after re-adding the log setting") } -// reloadPrometheus sends a reload request to the Prometheus server to apply -// updated configurations. -func reloadPrometheus(t *testing.T, port int) { - resp, err := http.Post(fmt.Sprintf("http://127.0.0.1:%d/-/reload", port), "", nil) - require.NoError(t, err, "Failed to reload Prometheus") - require.Equal(t, http.StatusOK, resp.StatusCode, "Unexpected status code when reloading Prometheus") -} - // startGarbageServer sets up a mock server that returns a 500 Internal Server Error // for all requests. It also increments the request count each time it's hit. func startGarbageServer(t *testing.T, requestCount *atomic.Int32) string {