Skip to content

Commit

Permalink
rule: fix query addr parsing (#2288)
Browse files Browse the repository at this point in the history
* rule: fix query addr parsing

Signed-off-by: Tobiasz Heller <tobiaszheller@gmail.com>

* CR: support different schemas

Signed-off-by: Tobiasz Heller <tobiaszheller@gmail.com>

* CR: docs and err

Signed-off-by: Tobiasz Heller <tobiaszheller@gmail.com>

* CR: improve error handling and more TC

Signed-off-by: Tobiasz Heller <tobiaszheller@gmail.com>
  • Loading branch information
tobiaszheller authored Apr 1, 2020
1 parent 8f492a9 commit fc754db
Show file tree
Hide file tree
Showing 4 changed files with 142 additions and 14 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ We use *breaking* word for marking changes that are not backward compatible (rel

### Fixed

- [#2288](https://github.com/thanos-io/thanos/pull/2288) Ruler: Fixes issue #2281 bug in ruler with parsing query addr with path prefix
- [#2238](https://github.com/thanos-io/thanos/pull/2238) Ruler: Fixed Issue #2204 bug in alert queue signalling filled up queue and alerts were dropped
- [#2231](https://github.com/thanos-io/thanos/pull/2231) Bucket Web - Sort chunks by thanos.downsample.resolution for better grouping
- [#2254](https://github.com/thanos-io/thanos/pull/2254) Bucket: Fix metrics registered multiple times in bucket replicate
Expand Down
26 changes: 12 additions & 14 deletions cmd/thanos/rule.go
Original file line number Diff line number Diff line change
Expand Up @@ -288,17 +288,16 @@ func runRule(
metrics := newRuleMetrics(reg)

var queryCfg []query.Config
var err error
if len(queryConfigYAML) > 0 {
var err error
queryCfg, err = query.LoadConfigs(queryConfigYAML)
if err != nil {
return err
}
} else {
for _, addr := range queryAddrs {
if addr == "" {
return errors.New("static querier address cannot be empty")
}
queryCfg, err = query.BuildQueryConfig(queryAddrs)
if err != nil {
return err
}

// Build the query configuration from the legacy query flags.
Expand All @@ -308,16 +307,15 @@ func runRule(
Files: querySDFiles,
RefreshInterval: model.Duration(querySDInterval),
})
}
queryCfg = append(queryCfg,
query.Config{
EndpointsConfig: http_util.EndpointsConfig{
Scheme: "http",
StaticAddresses: queryAddrs,
FileSDConfigs: fileSDConfigs,
queryCfg = append(queryCfg,
query.Config{
EndpointsConfig: http_util.EndpointsConfig{
Scheme: "http",
FileSDConfigs: fileSDConfigs,
},
},
},
)
)
}
}

queryProvider := dns.NewProvider(
Expand Down
34 changes: 34 additions & 0 deletions pkg/query/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,13 @@
package query

import (
"fmt"
"net/url"
"strings"

"gopkg.in/yaml.v2"

"github.com/pkg/errors"
http_util "github.com/thanos-io/thanos/pkg/http"
)

Expand Down Expand Up @@ -39,3 +44,32 @@ func LoadConfigs(confYAML []byte) ([]Config, error) {
}
return queryCfg, nil
}

// BuildQueryConfig returns a query client configuration from a static address.
func BuildQueryConfig(queryAddrs []string) ([]Config, error) {
configs := make([]Config, 0, len(queryAddrs))
for i, addr := range queryAddrs {
if addr == "" {
return nil, errors.Errorf("static querier address cannot be empty at index %d", i)
}
// If addr is missing schema, add http.
if !strings.Contains(addr, "://") {
addr = fmt.Sprintf("http://%s", addr)
}
u, err := url.Parse(addr)
if err != nil {
return nil, errors.Wrapf(err, "failed to parse addr %q", addr)
}
if u.Scheme != "http" && u.Scheme != "https" {
return nil, errors.Errorf("%q is not supported scheme for querier address", u.Scheme)
}
configs = append(configs, Config{
EndpointsConfig: http_util.EndpointsConfig{
Scheme: u.Scheme,
StaticAddresses: []string{u.Host},
PathPrefix: u.Path,
},
})
}
return configs, nil
}
95 changes: 95 additions & 0 deletions pkg/query/config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
// Copyright (c) The Thanos Authors.
// Licensed under the Apache License 2.0.

package query

import (
"testing"

"github.com/thanos-io/thanos/pkg/http"
"github.com/thanos-io/thanos/pkg/testutil"
)

func TestBuildQueryConfig(t *testing.T) {
for _, tc := range []struct {
desc string
addresses []string
err bool
expected []Config
}{
{
desc: "single addr without path",
addresses: []string{"localhost:9093"},
expected: []Config{{
EndpointsConfig: http.EndpointsConfig{
StaticAddresses: []string{"localhost:9093"},
Scheme: "http",
},
}},
},
{
desc: "1st addr without path, 2nd with",
addresses: []string{"localhost:9093", "localhost:9094/prefix"},
expected: []Config{
{
EndpointsConfig: http.EndpointsConfig{
StaticAddresses: []string{"localhost:9093"},
Scheme: "http",
},
},
{
EndpointsConfig: http.EndpointsConfig{
StaticAddresses: []string{"localhost:9094"},
Scheme: "http",
PathPrefix: "/prefix",
},
},
},
},
{
desc: "single addr with path and http scheme",
addresses: []string{"http://localhost:9093"},
expected: []Config{{
EndpointsConfig: http.EndpointsConfig{
StaticAddresses: []string{"localhost:9093"},
Scheme: "http",
},
}},
},
{
desc: "single addr with path and https scheme",
addresses: []string{"https://localhost:9093"},
expected: []Config{{
EndpointsConfig: http.EndpointsConfig{
StaticAddresses: []string{"localhost:9093"},
Scheme: "https",
},
}},
},
{
desc: "not supported scheme",
addresses: []string{"ttp://localhost:9093"},
err: true,
},
{
desc: "invalid addr",
addresses: []string{"this is not a valid addr"},
err: true,
},
{
desc: "empty addr",
addresses: []string{""},
err: true,
},
} {
t.Run(tc.desc, func(t *testing.T) {
cfg, err := BuildQueryConfig(tc.addresses)
if tc.err {
testutil.NotOk(t, err)
return
}

testutil.Equals(t, tc.expected, cfg)
})
}
}

0 comments on commit fc754db

Please sign in to comment.