app/vmauth: follow-up for 3a45bbb4e0

- Move the test for SRV discovery into a separate function. This allows verifying round-robin discovery across SRV records.
- Restore the original netutil.Resolver after the test finishes, so it doesn't interfere with other tests.
- Move the description of the bugfix into the correct place at docs/CHANGELOG.md - it should be placed under v1.102.0-rc2
  instead of v1.102.0-rc1.
- Remove unneeded code in URLPrefix.sanitizeAndInitialize(), since it is expected this function is called only once
  for finishing URLPrefix initializiation. In this case URLPrefix.nextDiscoveryDeadline and URLPrefix.n are equal to 0
  according to https://pkg.go.dev/sync/atomic#Uint64
- Properly fix the bug at URLPrefix.discoverBackendAddrsIfNeeded() - it is expected that hostToAddrs map uses
  the original hostname keys, including 'srv+' prefix, so it shouldn't be removed when looping over up.busOriginal.
  Instead, the 'srv+' prefix must be removed from the hostname only locally before passing the hostname to netutil.Resolver.LookupSRV.

Updates https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6401
This commit is contained in:
Aliaksandr Valialkin 2024-07-16 10:31:21 +02:00
parent fe33626b0d
commit 590aeccd7d
No known key found for this signature in database
GPG key ID: 52C003EE2BCDB9EB
3 changed files with 44 additions and 13 deletions

View file

@ -354,11 +354,12 @@ func (up *URLPrefix) discoverBackendAddrsIfNeeded() {
// ips for the given host have been already discovered
continue
}
var resolvedAddrs []string
if strings.HasPrefix(host, "srv+") {
// The host has the format 'srv+realhost'. Strip 'srv+' prefix before performing the lookup.
host = strings.TrimPrefix(host, "srv+")
_, addrs, err := netutil.Resolver.LookupSRV(ctx, "", "", host)
srvHost := strings.TrimPrefix(host, "srv+")
_, addrs, err := netutil.Resolver.LookupSRV(ctx, "", "", srvHost)
if err != nil {
logger.Warnf("cannot discover backend SRV records for %s: %s; use it literally", bu, err)
resolvedAddrs = []string{host}
@ -390,7 +391,6 @@ func (up *URLPrefix) discoverBackendAddrsIfNeeded() {
var busNew []*backendURL
for _, bu := range up.busOriginal {
host := bu.Hostname()
host = strings.TrimPrefix(host, "srv+")
port := bu.Port()
for _, addr := range hostToAddrs[host] {
buCopy := *bu
@ -1017,8 +1017,6 @@ func (up *URLPrefix) sanitizeAndInitialize() error {
}
}
up.bus.Store(&bus)
up.nextDiscoveryDeadline.Store(0)
up.n.Store(0)
return nil
}

View file

@ -86,6 +86,7 @@ func TestCreateTargetURLSuccess(t *testing.T) {
f := func(ui *UserInfo, requestURI, expectedTarget, expectedRequestHeaders, expectedResponseHeaders string,
expectedRetryStatusCodes []int, expectedLoadBalancingPolicy string, expectedDropSrcPathPrefixParts int) {
t.Helper()
if err := ui.initURLs(); err != nil {
t.Fatalf("cannot initialize urls inside UserInfo: %s", err)
}
@ -122,6 +123,7 @@ func TestCreateTargetURLSuccess(t *testing.T) {
t.Fatalf("unexpected dropSrcPathPrefixParts; got %d; want %d", up.dropSrcPathPrefixParts, expectedDropSrcPathPrefixParts)
}
}
// Simple routing with `url_prefix`
f(&UserInfo{
URLPrefix: mustParseURL("http://foo.bar"),
@ -260,7 +262,32 @@ func TestCreateTargetURLSuccess(t *testing.T) {
f(ui, `/api/v1/query?query=up{env="prod"}`, `http://vmselect/1/prometheus/api/v1/query?query=up%7Benv%3D%22prod%22%7D`, "", "", nil, "least_loaded", 0)
f(ui, `/api/v1/query?query=up{foo="bar",env="dev",pod!=""}`, `http://vmselect/0/prometheus/api/v1/query?query=up%7Bfoo%3D%22bar%22%2Cenv%3D%22dev%22%2Cpod%21%3D%22%22%7D`, "", "", nil, "least_loaded", 0)
f(ui, `/api/v1/query?query=up{foo="bar"}`, `http://default-server/api/v1/query?query=up%7Bfoo%3D%22bar%22%7D`, "", "", nil, "least_loaded", 0)
}
func TestUserInfoGetBackendURL_SRV(t *testing.T) {
f := func(ui *UserInfo, requestURI, expectedTarget string) {
t.Helper()
u, err := url.Parse(requestURI)
if err != nil {
t.Fatalf("cannot parse %q: %s", requestURI, err)
}
u = normalizeURL(u)
up, _ := ui.getURLPrefixAndHeaders(u, nil)
if up == nil {
t.Fatalf("cannot match available backend: %s", err)
}
bu := up.getBackendURL()
target := mergeURLs(bu.url, u, up.dropSrcPathPrefixParts)
bu.put()
gotTarget := target.String()
if gotTarget != expectedTarget {
t.Fatalf("unexpected target\ngot:\n%q\nwant\n%q", gotTarget, expectedTarget)
}
}
// Discover backendURL with SRV hostnames
customResolver := &fakeResolver{
Resolver: &net.Resolver{},
lookupSRVResults: map[string][]*net.SRV{
@ -283,11 +310,14 @@ func TestCreateTargetURLSuccess(t *testing.T) {
},
},
}
origResolver := netutil.Resolver
netutil.Resolver = customResolver
defer func() {
netutil.Resolver = origResolver
}()
// Discover backendURL
allowed := true
ui = &UserInfo{
ui := &UserInfo{
URLMaps: []URLMap{
{
SrcPaths: getRegexs([]string{"/select/.+"}),
@ -301,12 +331,15 @@ func TestCreateTargetURLSuccess(t *testing.T) {
DiscoverBackendIPs: &allowed,
URLPrefix: mustParseURL("http://non-exist-dns-addr"),
}
f(ui, `/select/0/prometheus/api/v1/query?query=up`, "http://10.6.142.51:8481/select/0/prometheus/api/v1/query?query=up", "", "", nil, "least_loaded", 0)
// url_prefix counter will be reset, still go to 10.6.142.51
f(ui, `/select/0/prometheus/api/v1/query?query=up`, "http://10.6.142.51:8481/select/0/prometheus/api/v1/query?query=up", "", "", nil, "least_loaded", 0)
f(ui, `/insert/0/prometheus/api/v1/write`, "http://10.6.142.52:8480/insert/0/prometheus/api/v1/write", "", "", nil, "least_loaded", 0)
if err := ui.initURLs(); err != nil {
t.Fatalf("cannot initialize urls inside UserInfo: %s", err)
}
f(ui, `/select/0/prometheus/api/v1/query?query=up`, "http://10.6.142.51:8481/select/0/prometheus/api/v1/query?query=up")
f(ui, `/select/0/prometheus/api/v1/query?query=up`, "http://10.6.142.50:8481/select/0/prometheus/api/v1/query?query=up")
f(ui, `/insert/0/prometheus/api/v1/write`, "http://10.6.142.52:8480/insert/0/prometheus/api/v1/write")
// unsuccessful dns resolve
f(ui, `/test`, "http://non-exist-dns-addr/test", "", "", nil, "least_loaded", 0)
f(ui, `/test`, "http://non-exist-dns-addr/test")
}
func TestCreateTargetURLFailure(t *testing.T) {

View file

@ -99,6 +99,7 @@ Released at 2024-06-24
* BUGFIX: [Single-node VictoriaMetrics](https://docs.victoriametrics.com/) and `vmstorage` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/cluster-victoriametrics/): add validation for the max value specified for `-retentionPeriod`. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6330) for details.
* BUGFIX: [vmui](https://docs.victoriametrics.com/#vmui): **copy row** button in Table view produces unexpected result. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6421) and [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6495).
* BUGFIX: [vmalert-tool](https://docs.victoriametrics.com/vmalert-tool/): prevent hanging when processing groups without rules. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6500).
* BUGFIX: [vmauth](https://docs.victoriametrics.com/vmauth/): fix discovering backend IPs when `url_prefix` contains hostname with srv+ prefix. Thanks to @shichanglin5 for the [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6401).
## [v1.102.0-rc1](https://github.com/VictoriaMetrics/VictoriaMetrics/releases/tag/v1.102.0-rc1)
@ -151,7 +152,6 @@ Released at 2024-06-07
* BUGFIX: [Single-node VictoriaMetrics](https://docs.victoriametrics.com/) and `vmstorage` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/cluster-victoriametrics/): correctly apply `-inmemoryDataFlushInterval` when it's set to minimum supported value 1s.
* BUGFIX: [Single-node VictoriaMetrics](https://docs.victoriametrics.com/) and `vminsert` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/cluster-victoriametrics/): reduce the default value for `-maxLabelValueLen` command-line flag from `16KiB` to `1KiB`. This should prevent from issues like [this one](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6176) when time series with too long labels are ingested into VictoriaMetrics.
* BUGFIX: [vmauth](https://docs.victoriametrics.com/vmauth/): properly release memory used for metrics during config reload. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6247).
* BUGFIX: [vmauth](https://docs.victoriametrics.com/vmauth/): fix discovering backend IPs when `url_prefix` contains hostname with srv+ prefix. Thanks to @shichanglin5 for the [pull request](https://github.com/VictoriaMetrics/VictoriaMetrics/pull/6401).
* BUGFIX: [dashboards](https://grafana.com/orgs/victoriametrics): fix `AnnotationQueryRunner` error in Grafana when executing annotations query against Prometheus backend. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6309) for details.
* BUGFIX: [Single-node VictoriaMetrics](https://docs.victoriametrics.com/) and `vmselect` in [VictoriaMetrics cluster](https://docs.victoriametrics.com/cluster-victoriametrics/): filter deleted label names and values from [`/api/v1/labels`](https://docs.victoriametrics.com/url-examples/#apiv1labels) and [`/api/v1/label/.../values`](https://docs.victoriametrics.com/url-examples/#apiv1labelvalues) responses when `match[]` filter matches small number of time series. The issue was introduced [v1.81.0](https://docs.victoriametrics.com/changelog_2022/#v1810).
* BUGFIX: [vmalert-tool](https://docs.victoriametrics.com/vmalert-tool/): fix float values template in `input_series`. See [this issue](https://github.com/VictoriaMetrics/VictoriaMetrics/issues/6391).