From c54bb73867ae0024b5acb0bcaa611a129b012abc Mon Sep 17 00:00:00 2001 From: Aliaksandr Valialkin Date: Fri, 21 May 2021 16:34:03 +0300 Subject: [PATCH] all: do not skip SIGHUP signal during service initialization This can lead to stale or incomplete configs like in the https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1240 --- app/vmagent/remotewrite/remotewrite.go | 7 ++++++- app/vmalert/main.go | 9 +++++++-- app/vmauth/auth_config.go | 12 +++++++++--- app/vminsert/relabel/relabel.go | 6 +++++- docs/CHANGELOG.md | 1 + lib/promscrape/scraper.go | 7 +++++-- 6 files changed, 33 insertions(+), 9 deletions(-) diff --git a/app/vmagent/remotewrite/remotewrite.go b/app/vmagent/remotewrite/remotewrite.go index 17bb379b0..1b5b9b3f5 100644 --- a/app/vmagent/remotewrite/remotewrite.go +++ b/app/vmagent/remotewrite/remotewrite.go @@ -104,6 +104,12 @@ func Init() { *queues = 1 } initLabelsGlobal() + + // Register SIGHUP handler for config reload before loadRelabelConfigs. + // This guarantees that the config will be re-read if the signal arrives just after loadRelabelConfig. + // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1240 + sighupCh := procutil.NewSighupChan() + rcs, err := loadRelabelConfigs() if err != nil { logger.Fatalf("cannot load relabel configs: %s", err) @@ -130,7 +136,6 @@ func Init() { } // Start config reloader. - sighupCh := procutil.NewSighupChan() configReloaderWG.Add(1) go func() { defer configReloaderWG.Done() diff --git a/app/vmalert/main.go b/app/vmalert/main.go index 886dcb0b8..d11d72682 100644 --- a/app/vmalert/main.go +++ b/app/vmalert/main.go @@ -77,6 +77,12 @@ func main() { if err != nil { logger.Fatalf("failed to init: %s", err) } + + // Register SIGHUP handler for config re-read just before manager.start call. + // This guarantees that the config will be re-read if the signal arrives during manager.start call. + // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1240 + sighupCh := procutil.NewSighupChan() + if err := manager.start(ctx, *rulePath, *validateTemplates, *validateExpressions); err != nil { logger.Fatalf("failed to start: %s", err) } @@ -85,9 +91,8 @@ func main() { // init reload metrics with positive values to improve alerting conditions configSuccess.Set(1) configTimestamp.Set(fasttime.UnixTimestamp()) - sigHup := procutil.NewSighupChan() for { - <-sigHup + <-sighupCh configReloads.Inc() logger.Infof("SIGHUP received. Going to reload rules %q ...", *rulePath) if err := manager.update(ctx, *rulePath, *validateTemplates, *validateExpressions, false); err != nil { diff --git a/app/vmauth/auth_config.go b/app/vmauth/auth_config.go index 79bcde78c..a5ac3186d 100644 --- a/app/vmauth/auth_config.go +++ b/app/vmauth/auth_config.go @@ -6,6 +6,7 @@ import ( "fmt" "io/ioutil" "net/url" + "os" "regexp" "strings" "sync" @@ -109,6 +110,12 @@ func initAuthConfig() { if len(*authConfigPath) == 0 { logger.Fatalf("missing required `-auth.config` command-line flag") } + + // Register SIGHUP handler for config re-read just before readAuthConfig call. + // This guarantees that the config will be re-read if the signal arrives during readAuthConfig call. + // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1240 + sighupCh := procutil.NewSighupChan() + m, err := readAuthConfig(*authConfigPath) if err != nil { logger.Fatalf("cannot load auth config from `-auth.config=%s`: %s", *authConfigPath, err) @@ -118,7 +125,7 @@ func initAuthConfig() { authConfigWG.Add(1) go func() { defer authConfigWG.Done() - authConfigReloader() + authConfigReloader(sighupCh) }() } @@ -127,8 +134,7 @@ func stopAuthConfig() { authConfigWG.Wait() } -func authConfigReloader() { - sighupCh := procutil.NewSighupChan() +func authConfigReloader(sighupCh <-chan os.Signal) { for { select { case <-stopCh: diff --git a/app/vminsert/relabel/relabel.go b/app/vminsert/relabel/relabel.go index af1ca2800..60e2e7f3b 100644 --- a/app/vminsert/relabel/relabel.go +++ b/app/vminsert/relabel/relabel.go @@ -19,6 +19,11 @@ var relabelConfig = flag.String("relabelConfig", "", "Optional path to a file wi // Init must be called after flag.Parse and before using the relabel package. func Init() { + // Register SIGHUP handler for config re-read just before loadRelabelConfig call. + // This guarantees that the config will be re-read if the signal arrives during loadRelabelConfig call. + // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1240 + sighupCh := procutil.NewSighupChan() + pcs, err := loadRelabelConfig() if err != nil { logger.Fatalf("cannot load relabelConfig: %s", err) @@ -27,7 +32,6 @@ func Init() { if len(*relabelConfig) == 0 { return } - sighupCh := procutil.NewSighupChan() go func() { for range sighupCh { logger.Infof("received SIGHUP; reloading -relabelConfig=%q...", *relabelConfig) diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index c1b8e2265..4f4984d0e 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -31,6 +31,7 @@ sort: 15 * BUGFIX: vmauth, vmalert: properly re-use HTTP keep-alive connections to backends and datasources. Previously only 2 keep-alive connections per backend could be re-used. Other connections were closed after the first request. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1300) for details. * BUGFIX: vmalert: fix false positive error `result contains metrics with the same labelset after applying rule labels`, which could be triggered when recording rules generate unique metrics. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1293). * BUGFIX: vmctl: properly import InfluxDB rows if they have a field and a tag with identical names. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1299). +* BUGFIX: properly reload configs if `SIGHUP` signal arrives during service initialization. Previously such `SIGHUP` signal could be ingonred and configs weren't reloaded. ## [v1.59.0](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.59.0) diff --git a/lib/promscrape/scraper.go b/lib/promscrape/scraper.go index ee34079f7..89493be47 100644 --- a/lib/promscrape/scraper.go +++ b/lib/promscrape/scraper.go @@ -88,6 +88,11 @@ func runScraper(configFile string, pushData func(wr *prompbmarshal.WriteRequest) return } + // Register SIGHUP handler for config reload before loadConfig. + // This guarantees that the config will be re-read if the signal arrives just after loadConfig. + // See https://github.com/VictoriaMetrics/VictoriaMetrics/issues/1240 + sighupCh := procutil.NewSighupChan() + logger.Infof("reading Prometheus configs from %q", configFile) cfg, data, err := loadConfig(configFile) if err != nil { @@ -107,8 +112,6 @@ func runScraper(configFile string, pushData func(wr *prompbmarshal.WriteRequest) scs.add("gce_sd_configs", *gceSDCheckInterval, func(cfg *Config, swsPrev []*ScrapeWork) []*ScrapeWork { return cfg.getGCESDScrapeWork(swsPrev) }) scs.add("dockerswarm_sd_configs", *dockerswarmSDCheckInterval, func(cfg *Config, swsPrev []*ScrapeWork) []*ScrapeWork { return cfg.getDockerSwarmSDScrapeWork(swsPrev) }) - sighupCh := procutil.NewSighupChan() - var tickerCh <-chan time.Time if *configCheckInterval > 0 { ticker := time.NewTicker(*configCheckInterval)