From 7763bbd99316681bca87930fdde0f1739a2a4d74 Mon Sep 17 00:00:00 2001 From: Tobias Schmidt Date: Thu, 3 Mar 2016 12:19:35 -0500 Subject: [PATCH 1/3] Validate alertmanager URL --- cmd/prometheus/config.go | 25 ++++++++++++++++++++++--- cmd/prometheus/config_test.go | 12 ++++++++++++ 2 files changed, 34 insertions(+), 3 deletions(-) diff --git a/cmd/prometheus/config.go b/cmd/prometheus/config.go index 19858d1cd..35f8c0b8d 100644 --- a/cmd/prometheus/config.go +++ b/cmd/prometheus/config.go @@ -242,10 +242,12 @@ func parse(args []string) error { if err := parsePrometheusURL(); err != nil { return err } - if err := parseInfluxdbURL(); err != nil { return err } + if err := validateAlertmanagerURL(); err != nil { + return err + } cfg.remote.InfluxdbPassword = os.Getenv("INFLUXDB_PW") @@ -266,7 +268,7 @@ func parsePrometheusURL() error { } if ok := govalidator.IsURL(cfg.prometheusURL); !ok { - return fmt.Errorf("Invalid Prometheus URL: %s", cfg.prometheusURL) + return fmt.Errorf("invalid Prometheus URL: %s", cfg.prometheusURL) } promURL, err := url.Parse(cfg.prometheusURL) @@ -289,7 +291,7 @@ func parseInfluxdbURL() error { } if ok := govalidator.IsURL(cfg.influxdbURL); !ok { - return fmt.Errorf("Invalid InfluxDB URL: %s", cfg.influxdbURL) + return fmt.Errorf("invalid InfluxDB URL: %s", cfg.influxdbURL) } url, err := url.Parse(cfg.influxdbURL) @@ -301,6 +303,23 @@ func parseInfluxdbURL() error { return nil } +func validateAlertmanagerURL() error { + if cfg.notifier.AlertmanagerURL == "" { + return nil + } + if ok := govalidator.IsURL(cfg.notifier.AlertmanagerURL); !ok { + return fmt.Errorf("invalid Alertmanager URL: %s", cfg.notifier.AlertmanagerURL) + } + url, err := url.Parse(cfg.notifier.AlertmanagerURL) + if err != nil { + return err + } + if url.Scheme == "" { + return fmt.Errorf("missing scheme in Alertmanager URL: %s", cfg.notifier.AlertmanagerURL) + } + return nil +} + var helpTmpl = ` usage: prometheus [] {{ range $cat, $flags := . }}{{ if ne $cat "." }} == {{ $cat | upper }} =={{ end }} diff --git a/cmd/prometheus/config_test.go b/cmd/prometheus/config_test.go index 4f5e8c112..a77eaf207 100644 --- a/cmd/prometheus/config_test.go +++ b/cmd/prometheus/config_test.go @@ -48,6 +48,18 @@ func TestParse(t *testing.T) { input: []string{"-storage.remote.influxdb-url", "'https://some-url/'"}, valid: false, }, + { + input: []string{"-alertmanager.url", ""}, + valid: true, + }, + { + input: []string{"-alertmanager.url", "http://alertmanager.company.com"}, + valid: true, + }, + { + input: []string{"-alertmanager.url", "alertmanager.company.com"}, + valid: false, + }, } for i, test := range tests { From 56fc9bdff32a52179c75f7b525a9739cbe941acb Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Tue, 8 Mar 2016 15:49:03 +0100 Subject: [PATCH 2/3] Handle closed target provider channel This fixes the case where a target provider closes the update channel and exits before the context is canceled. This should only be true for the static provider but it's safer to generally handle this case. --- retrieval/targetmanager.go | 35 ++++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 15 deletions(-) diff --git a/retrieval/targetmanager.go b/retrieval/targetmanager.go index 6daa708fc..8eecc67d2 100644 --- a/retrieval/targetmanager.go +++ b/retrieval/targetmanager.go @@ -274,28 +274,28 @@ func (ts *targetSet) runProviders(ctx context.Context, providers map[string]Targ updates := make(chan []*config.TargetGroup) go func(name string, prov TargetProvider) { - var initial []*config.TargetGroup - select { case <-ctx.Done(): - wg.Done() - return - case initial = <-updates: + case initial, ok := <-updates: + // Handle the case that a target provider exits and closes the channel + // before the context is done. + if !ok { + break + } // First set of all targets the provider knows. + for _, tgroup := range initial { + targets, err := targetsFromGroup(tgroup, ts.config) + if err != nil { + log.With("target_group", tgroup).Errorf("Target update failed: %s", err) + continue + } + ts.tgroups[name+"/"+tgroup.Source] = targets + } case <-time.After(5 * time.Second): // Initial set didn't arrive. Act as if it was empty // and wait for updates later on. } - for _, tgroup := range initial { - targets, err := targetsFromGroup(tgroup, ts.config) - if err != nil { - log.With("target_group", tgroup).Errorf("Target update failed: %s", err) - continue - } - ts.tgroups[name+"/"+tgroup.Source] = targets - } - wg.Done() // Start listening for further updates. @@ -303,7 +303,12 @@ func (ts *targetSet) runProviders(ctx context.Context, providers map[string]Targ select { case <-ctx.Done(): return - case tgs := <-updates: + case tgs, ok := <-updates: + // Handle the case that a target provider exits and closes the channel + // before the context is done. + if !ok { + return + } for _, tg := range tgs { if err := ts.update(name, tg); err != nil { log.With("target_group", tg).Errorf("Target update failed: %s", err) From f2e359962c44edebd739f757d3b1294c10aacdac Mon Sep 17 00:00:00 2001 From: Fabian Reinartz Date: Tue, 8 Mar 2016 17:12:27 +0100 Subject: [PATCH 3/3] Sort exported targets --- retrieval/targetmanager.go | 8 ++++++-- web/web.go | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/retrieval/targetmanager.go b/retrieval/targetmanager.go index 8eecc67d2..5ce1e98f8 100644 --- a/retrieval/targetmanager.go +++ b/retrieval/targetmanager.go @@ -15,6 +15,7 @@ package retrieval import ( "fmt" + "sort" "strings" "sync" "time" @@ -134,11 +135,11 @@ func (tm *TargetManager) reload() { } // Pools returns the targets currently being scraped bucketed by their job name. -func (tm *TargetManager) Pools() map[string][]*Target { +func (tm *TargetManager) Pools() map[string]Targets { tm.mtx.RLock() defer tm.mtx.RUnlock() - pools := map[string][]*Target{} + pools := map[string]Targets{} // TODO(fabxc): this is just a hack to maintain compatibility for now. for _, ps := range tm.targetSets { @@ -151,6 +152,9 @@ func (tm *TargetManager) Pools() map[string][]*Target { ps.scrapePool.mtx.RUnlock() } + for _, targets := range pools { + sort.Sort(targets) + } return pools } diff --git a/web/web.go b/web/web.go index bfcbdb639..8ce45cb26 100644 --- a/web/web.go +++ b/web/web.go @@ -92,7 +92,7 @@ type PrometheusStatus struct { // A function that returns the current scrape targets pooled // by their job name. - TargetPools func() map[string][]*retrieval.Target + TargetPools func() map[string]retrieval.Targets // A function that returns all loaded rules. Rules func() []rules.Rule