From 34116d77b2b051840233cdc9ccdf062567ba6f66 Mon Sep 17 00:00:00 2001 From: Arve Knudsen Date: Fri, 27 Aug 2021 12:10:15 +0200 Subject: [PATCH] Use kv package from github.com/grafana/dskit (#4436) Signed-off-by: Arve Knudsen Signed-off-by: Alvin Lin --- docs/configuration/config-file-reference.md | 4 +- go.mod | 10 +- go.sum | 4 +- ...getting_started_with_gossiped_ring_test.go | 6 + integration/kv_test.go | 25 +- pkg/alertmanager/alertmanager_ring.go | 2 +- pkg/alertmanager/api_test.go | 5 +- pkg/alertmanager/distributor_test.go | 21 +- pkg/alertmanager/multitenant.go | 5 +- pkg/alertmanager/multitenant_test.go | 39 +- pkg/api/api_test.go | 2 - pkg/chunk/purger/purger_test.go | 3 +- pkg/compactor/compactor_ring.go | 2 +- pkg/compactor/compactor_test.go | 18 +- pkg/cortex/cortex.go | 2 +- pkg/cortex/cortex_test.go | 2 +- pkg/cortex/modules.go | 18 +- pkg/cortex/runtime_config.go | 2 +- pkg/distributor/distributor_ring.go | 2 +- pkg/distributor/distributor_test.go | 18 +- pkg/distributor/ha_tracker.go | 7 +- pkg/distributor/ha_tracker_test.go | 36 +- pkg/ingester/flush_test.go | 2 +- pkg/ingester/ingester_test.go | 30 +- pkg/ingester/ingester_v2_test.go | 82 +- pkg/ingester/lifecycle_test.go | 20 +- pkg/ingester/user_state_test.go | 2 +- pkg/ingester/wal_test.go | 6 +- pkg/querier/blocks_store_queryable.go | 5 +- .../blocks_store_replicated_set_test.go | 10 +- pkg/querier/worker/scheduler_processor.go | 7 +- pkg/ring/basic_lifecycler.go | 2 +- pkg/ring/basic_lifecycler_delegates_test.go | 12 +- pkg/ring/basic_lifecycler_test.go | 34 +- pkg/ring/kv/client_test.go | 149 -- pkg/ring/kv/consul/client_test.go | 164 --- pkg/ring/kv/kv_test.go | 276 ---- pkg/ring/kv/memberlist/broadcast_test.go | 52 - .../kv/memberlist/kv_init_service_test.go | 60 - .../kv/memberlist/memberlist_client_test.go | 1270 ----------------- pkg/ring/kv/multi_test.go | 32 - pkg/ring/lifecycler.go | 7 +- pkg/ring/lifecycler_test.go | 57 +- pkg/ring/model.go | 5 +- pkg/ring/ring.go | 5 +- pkg/ring/ring_test.go | 13 +- pkg/ring/testutils/testutils.go | 2 +- pkg/ruler/api_test.go | 14 +- pkg/ruler/lifecycle_test.go | 14 +- pkg/ruler/ruler.go | 5 +- pkg/ruler/ruler_ring.go | 2 +- pkg/ruler/ruler_test.go | 22 +- pkg/storegateway/gateway.go | 5 +- pkg/storegateway/gateway_ring.go | 2 +- pkg/storegateway/gateway_test.go | 30 +- pkg/storegateway/sharding_strategy_test.go | 8 +- tools/doc-generator/main.go | 6 +- .../github.com/grafana/dskit/closer/closer.go | 9 + .../github.com/grafana/dskit}/kv/client.go | 33 +- .../grafana/dskit}/kv/codec/codec.go | 0 .../grafana/dskit}/kv/consul/client.go | 44 +- .../grafana/dskit}/kv/consul/metrics.go | 7 +- .../grafana/dskit}/kv/consul/mock.go | 74 +- .../github.com/grafana/dskit}/kv/etcd/etcd.go | 56 +- .../github.com/grafana/dskit}/kv/etcd/mock.go | 30 +- .../github.com/grafana/dskit/kv/kvtls/tls.go | 87 ++ .../grafana/dskit}/kv/memberlist/broadcast.go | 7 +- .../dskit/kv/memberlist/dnsprovider.go | 15 + .../grafana/dskit}/kv/memberlist/kv.pb.go | 0 .../grafana/dskit}/kv/memberlist/kv.proto | 0 .../dskit}/kv/memberlist/kv_init_service.go | 63 +- .../dskit}/kv/memberlist/memberlist_client.go | 39 +- .../dskit}/kv/memberlist/memberlist_logger.go | 2 +- .../grafana/dskit}/kv/memberlist/mergeable.go | 0 .../grafana/dskit}/kv/memberlist/metrics.go | 7 +- .../dskit}/kv/memberlist/tcp_transport.go | 8 +- .../github.com/grafana/dskit}/kv/metrics.go | 7 +- .../github.com/grafana/dskit}/kv/mock.go | 7 +- .../github.com/grafana/dskit}/kv/multi.go | 17 +- .../github.com/grafana/dskit}/kv/prefix.go | 0 .../grafana/dskit/services/basic_service.go | 4 +- .../grafana/dskit/services/failure_watcher.go | 2 +- .../grafana/dskit/services/manager.go | 26 +- .../grafana/dskit/services/service.go | 23 +- .../grafana/dskit/services/services.go | 6 +- vendor/modules.txt | 17 +- 86 files changed, 756 insertions(+), 2479 deletions(-) delete mode 100644 pkg/ring/kv/client_test.go delete mode 100644 pkg/ring/kv/consul/client_test.go delete mode 100644 pkg/ring/kv/kv_test.go delete mode 100644 pkg/ring/kv/memberlist/broadcast_test.go delete mode 100644 pkg/ring/kv/memberlist/kv_init_service_test.go delete mode 100644 pkg/ring/kv/memberlist/memberlist_client_test.go delete mode 100644 pkg/ring/kv/multi_test.go create mode 100644 vendor/github.com/grafana/dskit/closer/closer.go rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/client.go (89%) rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/codec/codec.go (100%) rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/consul/client.go (88%) rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/consul/metrics.go (94%) rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/consul/mock.go (71%) rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/etcd/etcd.go (82%) rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/etcd/mock.go (72%) create mode 100644 vendor/github.com/grafana/dskit/kv/kvtls/tls.go rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/memberlist/broadcast.go (83%) create mode 100644 vendor/github.com/grafana/dskit/kv/memberlist/dnsprovider.go rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/memberlist/kv.pb.go (100%) rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/memberlist/kv.proto (100%) rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/memberlist/kv_init_service.go (85%) rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/memberlist/memberlist_client.go (97%) rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/memberlist/memberlist_logger.go (98%) rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/memberlist/mergeable.go (100%) rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/memberlist/metrics.go (97%) rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/memberlist/tcp_transport.go (99%) rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/metrics.go (95%) rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/mock.go (82%) rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/multi.go (96%) rename {pkg/ring => vendor/github.com/grafana/dskit}/kv/prefix.go (100%) diff --git a/docs/configuration/config-file-reference.md b/docs/configuration/config-file-reference.md index 0190fe90ea..2964ecc07e 100644 --- a/docs/configuration/config-file-reference.md +++ b/docs/configuration/config-file-reference.md @@ -3810,9 +3810,7 @@ The `memberlist_config` configures the Gossip memberlist. [compression_enabled: | default = true] # Other cluster members to join. Can be specified multiple times. It can be an -# IP, hostname or an entry specified in the DNS Service Discovery format (see -# https://cortexmetrics.io/docs/configuration/arguments/#dns-service-discovery -# for more details). +# IP, hostname or an entry specified in the DNS Service Discovery format. # CLI flag: -memberlist.join [join_members: | default = []] diff --git a/go.mod b/go.mod index 81a65494d3..24d328ff0d 100644 --- a/go.mod +++ b/go.mod @@ -11,7 +11,6 @@ require ( github.com/NYTimes/gziphandler v1.1.1 github.com/alecthomas/units v0.0.0-20210208195552-ff826a37aa15 github.com/alicebob/miniredis/v2 v2.14.3 - github.com/armon/go-metrics v0.3.6 github.com/aws/aws-sdk-go v1.38.68 github.com/bradfitz/gomemcache v0.0.0-20190913173617-a41fca850d0b github.com/cespare/xxhash v1.1.0 @@ -30,12 +29,8 @@ require ( github.com/golang/protobuf v1.5.2 github.com/golang/snappy v0.0.4 github.com/gorilla/mux v1.8.0 - github.com/grafana/dskit v0.0.0-20210824090727-039d9afd9208 + github.com/grafana/dskit v0.0.0-20210827060659-9daca2f00327 github.com/grpc-ecosystem/go-grpc-middleware v1.3.0 - github.com/hashicorp/consul/api v1.9.1 - github.com/hashicorp/go-cleanhttp v0.5.1 - github.com/hashicorp/go-sockaddr v1.0.2 - github.com/hashicorp/memberlist v0.2.3 github.com/json-iterator/go v1.1.11 github.com/lib/pq v1.3.0 github.com/minio/minio-go/v7 v7.0.10 @@ -59,9 +54,6 @@ require ( github.com/uber/jaeger-client-go v2.29.1+incompatible github.com/weaveworks/common v0.0.0-20210722103813-e649eff5ab4a go.etcd.io/bbolt v1.3.6 - go.etcd.io/etcd v3.3.25+incompatible - go.etcd.io/etcd/client/v3 v3.5.0 - go.etcd.io/etcd/server/v3 v3.5.0 go.uber.org/atomic v1.9.0 golang.org/x/net v0.0.0-20210610132358-84b48f89b13b golang.org/x/sync v0.0.0-20210220032951-036812b2e83c diff --git a/go.sum b/go.sum index df09449056..2ee420f83e 100644 --- a/go.sum +++ b/go.sum @@ -962,8 +962,8 @@ github.com/gorilla/websocket v0.0.0-20170926233335-4201258b820c/go.mod h1:E7qHFY github.com/gorilla/websocket v1.4.0/go.mod h1:E7qHFY5m1UJ88s3WnNqhKjPHQ0heANvMoAMk2YaljkQ= github.com/gorilla/websocket v1.4.2 h1:+/TMaTYc4QFitKJxsQ7Yye35DkWvkdLcvGKqM+x0Ufc= github.com/gorilla/websocket v1.4.2/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE= -github.com/grafana/dskit v0.0.0-20210824090727-039d9afd9208 h1:Yc+q7s/wcyd8bvPTQygY1iQTxxr931cLISsRhYltrJM= -github.com/grafana/dskit v0.0.0-20210824090727-039d9afd9208/go.mod h1:uF46UNN1/feB1egpq8UGbBBKvJjGgZauW7pcVbeFLLM= +github.com/grafana/dskit v0.0.0-20210827060659-9daca2f00327 h1:THdW9RnugPdLwW8RmHB/xOcKf267QunSH1mDuaJkhWk= +github.com/grafana/dskit v0.0.0-20210827060659-9daca2f00327/go.mod h1:+T2iuDOzx/BSQJSvli9FUvLM5HnV8aDPmXM8KWuVj3M= github.com/grafana/gocql v0.0.0-20200605141915-ba5dc39ece85 h1:xLuzPoOzdfNb/RF/IENCw+oLVdZB4G21VPhkHBgwSHY= github.com/grafana/gocql v0.0.0-20200605141915-ba5dc39ece85/go.mod h1:crI9WX6p0IhrqB+DqIUHulRW853PaNFf7o4UprV//3I= github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7/go.mod h1:FecbI9+v66THATjSRHfNgh1IVFe/9kFxbXtjV0ctIMA= diff --git a/integration/getting_started_with_gossiped_ring_test.go b/integration/getting_started_with_gossiped_ring_test.go index 24be49ccbd..90e17925e5 100644 --- a/integration/getting_started_with_gossiped_ring_test.go +++ b/integration/getting_started_with_gossiped_ring_test.go @@ -125,4 +125,10 @@ func TestGettingStartedWithGossipedRing(t *testing.T) { // single ingester and so we have 1 block shipped from ingesters and loaded by both store-gateways. require.NoError(t, cortex1.WaitSumMetrics(e2e.Equals(1), "cortex_bucket_store_blocks_loaded")) require.NoError(t, cortex2.WaitSumMetrics(e2e.Equals(1), "cortex_bucket_store_blocks_loaded")) + + // Make sure that no DNS failures occurred. + // No actual DNS lookups are necessarily performed, so we can't really assert on that. + mlMatcher := labels.MustNewMatcher(labels.MatchEqual, "name", "memberlist") + require.NoError(t, cortex1.WaitSumMetricsWithOptions(e2e.Equals(0), []string{"cortex_dns_failures_total"}, e2e.WithLabelMatchers(mlMatcher))) + require.NoError(t, cortex2.WaitSumMetricsWithOptions(e2e.Equals(0), []string{"cortex_dns_failures_total"}, e2e.WithLabelMatchers(mlMatcher))) } diff --git a/integration/kv_test.go b/integration/kv_test.go index c72548cd61..7a3793cd1a 100644 --- a/integration/kv_test.go +++ b/integration/kv_test.go @@ -10,6 +10,10 @@ import ( "testing" "time" + "github.com/go-kit/kit/log" + "github.com/grafana/dskit/kv" + "github.com/grafana/dskit/kv/consul" + "github.com/grafana/dskit/kv/etcd" "github.com/prometheus/client_golang/prometheus" dto "github.com/prometheus/client_model/go" "github.com/stretchr/testify/assert" @@ -17,9 +21,6 @@ import ( "github.com/cortexproject/cortex/integration/e2e" e2edb "github.com/cortexproject/cortex/integration/e2e/db" - "github.com/cortexproject/cortex/pkg/ring/kv" - "github.com/cortexproject/cortex/pkg/ring/kv/consul" - "github.com/cortexproject/cortex/pkg/ring/kv/etcd" ) func TestKVList(t *testing.T) { @@ -117,7 +118,9 @@ func TestKVWatchAndDelete(t *testing.T) { }) } -func setupEtcd(t *testing.T, scenario *e2e.Scenario, reg prometheus.Registerer) kv.Client { +func setupEtcd(t *testing.T, scenario *e2e.Scenario, reg prometheus.Registerer, logger log.Logger) kv.Client { + t.Helper() + etcdSvc := e2edb.NewETCD() require.NoError(t, scenario.StartAndWaitReady(etcdSvc)) @@ -131,13 +134,15 @@ func setupEtcd(t *testing.T, scenario *e2e.Scenario, reg prometheus.Registerer) MaxRetries: 5, }, }, - }, stringCodec{}, reg) + }, stringCodec{}, reg, logger) require.NoError(t, err) return etcdKv } -func setupConsul(t *testing.T, scenario *e2e.Scenario, reg prometheus.Registerer) kv.Client { +func setupConsul(t *testing.T, scenario *e2e.Scenario, reg prometheus.Registerer, logger log.Logger) kv.Client { + t.Helper() + consulSvc := e2edb.NewConsul() require.NoError(t, scenario.StartAndWaitReady(consulSvc)) @@ -152,14 +157,14 @@ func setupConsul(t *testing.T, scenario *e2e.Scenario, reg prometheus.Registerer WatchKeyRateLimit: 1, }, }, - }, stringCodec{}, reg) + }, stringCodec{}, reg, logger) require.NoError(t, err) return consulKv } func testKVs(t *testing.T, testFn func(t *testing.T, client kv.Client, reg *prometheus.Registry)) { - setupFns := map[string]func(t *testing.T, scenario *e2e.Scenario, reg prometheus.Registerer) kv.Client{ + setupFns := map[string]func(t *testing.T, scenario *e2e.Scenario, reg prometheus.Registerer, logger log.Logger) kv.Client{ "etcd": setupEtcd, "consul": setupConsul, } @@ -171,13 +176,13 @@ func testKVs(t *testing.T, testFn func(t *testing.T, client kv.Client, reg *prom } } -func testKVScenario(t *testing.T, kvSetupFn func(t *testing.T, scenario *e2e.Scenario, reg prometheus.Registerer) kv.Client, testFn func(t *testing.T, client kv.Client, reg *prometheus.Registry)) { +func testKVScenario(t *testing.T, kvSetupFn func(t *testing.T, scenario *e2e.Scenario, reg prometheus.Registerer, logger log.Logger) kv.Client, testFn func(t *testing.T, client kv.Client, reg *prometheus.Registry)) { s, err := e2e.NewScenario(networkName) require.NoError(t, err) defer s.Close() reg := prometheus.NewRegistry() - client := kvSetupFn(t, s, reg) + client := kvSetupFn(t, s, prometheus.WrapRegistererWithPrefix("cortex_", reg), log.NewNopLogger()) testFn(t, client, reg) } diff --git a/pkg/alertmanager/alertmanager_ring.go b/pkg/alertmanager/alertmanager_ring.go index 4cd28de6b0..f2f8fa9cb5 100644 --- a/pkg/alertmanager/alertmanager_ring.go +++ b/pkg/alertmanager/alertmanager_ring.go @@ -8,9 +8,9 @@ import ( "github.com/go-kit/kit/log/level" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv" util_log "github.com/cortexproject/cortex/pkg/util/log" ) diff --git a/pkg/alertmanager/api_test.go b/pkg/alertmanager/api_test.go index ac617b89e5..fe54bb2f4e 100644 --- a/pkg/alertmanager/api_test.go +++ b/pkg/alertmanager/api_test.go @@ -18,15 +18,14 @@ import ( "github.com/prometheus/client_golang/prometheus" commoncfg "github.com/prometheus/common/config" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "github.com/thanos-io/thanos/pkg/objstore" + "github.com/weaveworks/common/user" "gopkg.in/yaml.v2" "github.com/cortexproject/cortex/pkg/alertmanager/alertspb" "github.com/cortexproject/cortex/pkg/alertmanager/alertstore/bucketclient" util_log "github.com/cortexproject/cortex/pkg/util/log" - - "github.com/stretchr/testify/require" - "github.com/weaveworks/common/user" ) func TestAMConfigValidationAPI(t *testing.T) { diff --git a/pkg/alertmanager/distributor_test.go b/pkg/alertmanager/distributor_test.go index d99575091e..248749aa63 100644 --- a/pkg/alertmanager/distributor_test.go +++ b/pkg/alertmanager/distributor_test.go @@ -13,22 +13,23 @@ import ( "testing" "time" + "github.com/go-kit/kit/log" "github.com/grafana/dskit/flagext" - - "github.com/cortexproject/cortex/pkg/alertmanager/alertmanagerpb" - "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv" - "github.com/cortexproject/cortex/pkg/ring/kv/consul" - util_log "github.com/cortexproject/cortex/pkg/util/log" - "github.com/cortexproject/cortex/pkg/util/test" - + "github.com/grafana/dskit/kv" + "github.com/grafana/dskit/kv/consul" "github.com/grafana/dskit/services" "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/weaveworks/common/httpgrpc" "github.com/weaveworks/common/user" "google.golang.org/grpc" "google.golang.org/grpc/health/grpc_health_v1" + + "github.com/cortexproject/cortex/pkg/alertmanager/alertmanagerpb" + "github.com/cortexproject/cortex/pkg/ring" + util_log "github.com/cortexproject/cortex/pkg/util/log" + "github.com/cortexproject/cortex/pkg/util/test" ) func TestDistributor_DistributeRequest(t *testing.T) { @@ -336,7 +337,9 @@ func prepare(t *testing.T, numAM, numHappyAM, replicationFactor int, responseBod amByAddr[a.myAddr] = ams[i] } - kvStore := consul.NewInMemoryClient(ring.GetCodec()) + kvStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + err := kvStore.CAS(context.Background(), RingKey, func(_ interface{}) (interface{}, bool, error) { return &ring.Desc{ diff --git a/pkg/alertmanager/multitenant.go b/pkg/alertmanager/multitenant.go index 43e5687b5b..25f6dcfc25 100644 --- a/pkg/alertmanager/multitenant.go +++ b/pkg/alertmanager/multitenant.go @@ -16,6 +16,7 @@ import ( "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv" "github.com/grafana/dskit/services" "github.com/pkg/errors" "github.com/prometheus/alertmanager/cluster" @@ -34,7 +35,6 @@ import ( "github.com/cortexproject/cortex/pkg/alertmanager/alertstore" "github.com/cortexproject/cortex/pkg/ring" "github.com/cortexproject/cortex/pkg/ring/client" - "github.com/cortexproject/cortex/pkg/ring/kv" "github.com/cortexproject/cortex/pkg/tenant" "github.com/cortexproject/cortex/pkg/util" "github.com/cortexproject/cortex/pkg/util/concurrency" @@ -344,7 +344,8 @@ func NewMultitenantAlertmanager(cfg *MultitenantAlertmanagerConfig, store alerts ringStore, err = kv.NewClient( cfg.ShardingRing.KVStore, ring.GetCodec(), - kv.RegistererWithKVName(registerer, "alertmanager"), + kv.RegistererWithKVName(prometheus.WrapRegistererWithPrefix("cortex_", registerer), "alertmanager"), + logger, ) if err != nil { return nil, errors.Wrap(err, "create KV store client") diff --git a/pkg/alertmanager/multitenant_test.go b/pkg/alertmanager/multitenant_test.go index 5bc4938e9c..777907716d 100644 --- a/pkg/alertmanager/multitenant_test.go +++ b/pkg/alertmanager/multitenant_test.go @@ -21,6 +21,7 @@ import ( "github.com/go-kit/kit/log" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv/consul" "github.com/grafana/dskit/services" "github.com/prometheus/alertmanager/cluster/clusterpb" "github.com/prometheus/alertmanager/notify" @@ -43,7 +44,6 @@ import ( "github.com/cortexproject/cortex/pkg/alertmanager/alertstore" "github.com/cortexproject/cortex/pkg/alertmanager/alertstore/bucketclient" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv/consul" "github.com/cortexproject/cortex/pkg/storage/bucket" "github.com/cortexproject/cortex/pkg/util" "github.com/cortexproject/cortex/pkg/util/concurrency" @@ -611,7 +611,9 @@ func TestMultitenantAlertmanager_deleteUnusedLocalUserState(t *testing.T) { func TestMultitenantAlertmanager_zoneAwareSharding(t *testing.T) { ctx := context.Background() alertStore := prepareInMemoryAlertStore() - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + const ( user1 = "user1" user2 = "user2" @@ -689,7 +691,8 @@ func TestMultitenantAlertmanager_deleteUnusedRemoteUserState(t *testing.T) { ) alertStore := prepareInMemoryAlertStore() - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) createInstance := func(i int) *MultitenantAlertmanager { reg := prometheus.NewPedanticRegistry() @@ -1005,7 +1008,8 @@ func TestMultitenantAlertmanager_InitialSyncWithSharding(t *testing.T) { ctx := context.Background() amConfig := mockAlertmanagerConfig(t) amConfig.ShardingEnabled = true - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) // Use an alert store with a mocked backend. bkt := &bucket.ClientMock{} @@ -1109,7 +1113,9 @@ func TestMultitenantAlertmanager_PerTenantSharding(t *testing.T) { for _, tt := range tc { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + alertStore := prepareInMemoryAlertStore() var instances []*MultitenantAlertmanager @@ -1296,7 +1302,9 @@ func TestMultitenantAlertmanager_SyncOnRingTopologyChanges(t *testing.T) { amConfig.ShardingRing.RingCheckPeriod = 100 * time.Millisecond amConfig.PollInterval = time.Hour // Don't trigger the periodic check. - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + alertStore := prepareInMemoryAlertStore() reg := prometheus.NewPedanticRegistry() @@ -1347,7 +1355,9 @@ func TestMultitenantAlertmanager_RingLifecyclerShouldAutoForgetUnhealthyInstance amConfig.ShardingRing.HeartbeatPeriod = 100 * time.Millisecond amConfig.ShardingRing.HeartbeatTimeout = heartbeatTimeout - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + alertStore := prepareInMemoryAlertStore() am, err := createMultitenantAlertmanager(amConfig, nil, nil, alertStore, ringStore, nil, log.NewNopLogger(), nil) @@ -1379,7 +1389,8 @@ func TestMultitenantAlertmanager_InitialSyncFailureWithSharding(t *testing.T) { ctx := context.Background() amConfig := mockAlertmanagerConfig(t) amConfig.ShardingEnabled = true - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) // Mock the store to fail listing configs. bkt := &bucket.ClientMock{} @@ -1401,7 +1412,9 @@ func TestMultitenantAlertmanager_InitialSyncFailureWithSharding(t *testing.T) { func TestAlertmanager_ReplicasPosition(t *testing.T) { ctx := context.Background() - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + mockStore := prepareInMemoryAlertStore() require.NoError(t, mockStore.SetAlertConfig(ctx, alertspb.AlertConfigDesc{ User: "user-1", @@ -1500,7 +1513,9 @@ func TestAlertmanager_StateReplicationWithSharding(t *testing.T) { for _, tt := range tc { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + mockStore := prepareInMemoryAlertStore() clientPool := newPassthroughAlertmanagerClientPool() externalURL := flagext.URLValue{} @@ -1693,7 +1708,9 @@ func TestAlertmanager_StateReplicationWithSharding_InitialSyncFromPeers(t *testi for _, tt := range tc { t.Run(tt.name, func(t *testing.T) { ctx := context.Background() - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + mockStore := prepareInMemoryAlertStore() clientPool := newPassthroughAlertmanagerClientPool() externalURL := flagext.URLValue{} diff --git a/pkg/api/api_test.go b/pkg/api/api_test.go index 3bfbd221de..925cc8709e 100644 --- a/pkg/api/api_test.go +++ b/pkg/api/api_test.go @@ -24,7 +24,6 @@ func TestNewApiWithoutSourceIPExtractor(t *testing.T) { require.NoError(t, err) api, err := New(cfg, serverCfg, server, &FakeLogger{}) - require.NoError(t, err) require.Nil(t, api.sourceIPs) } @@ -40,7 +39,6 @@ func TestNewApiWithSourceIPExtractor(t *testing.T) { require.NoError(t, err) api, err := New(cfg, serverCfg, server, &FakeLogger{}) - require.NoError(t, err) require.NotNil(t, api.sourceIPs) } diff --git a/pkg/chunk/purger/purger_test.go b/pkg/chunk/purger/purger_test.go index 5b6e7ec5e5..5141bb508d 100644 --- a/pkg/chunk/purger/purger_test.go +++ b/pkg/chunk/purger/purger_test.go @@ -8,11 +8,10 @@ import ( "testing" "time" - "github.com/prometheus/client_golang/prometheus/testutil" - "github.com/grafana/dskit/flagext" "github.com/grafana/dskit/services" "github.com/prometheus/client_golang/prometheus" + "github.com/prometheus/client_golang/prometheus/testutil" "github.com/prometheus/common/model" "github.com/prometheus/prometheus/promql/parser" "github.com/stretchr/testify/require" diff --git a/pkg/compactor/compactor_ring.go b/pkg/compactor/compactor_ring.go index cb98a55b25..a6a4d12402 100644 --- a/pkg/compactor/compactor_ring.go +++ b/pkg/compactor/compactor_ring.go @@ -7,9 +7,9 @@ import ( "github.com/go-kit/kit/log/level" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv" util_log "github.com/cortexproject/cortex/pkg/util/log" ) diff --git a/pkg/compactor/compactor_test.go b/pkg/compactor/compactor_test.go index 29a1dfe751..75523eac36 100644 --- a/pkg/compactor/compactor_test.go +++ b/pkg/compactor/compactor_test.go @@ -19,6 +19,7 @@ import ( "github.com/go-kit/kit/log" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv/consul" "github.com/grafana/dskit/services" "github.com/oklog/ulid" "github.com/pkg/errors" @@ -35,7 +36,6 @@ import ( "gopkg.in/yaml.v2" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv/consul" "github.com/cortexproject/cortex/pkg/storage/bucket" cortex_tsdb "github.com/cortexproject/cortex/pkg/storage/tsdb" "github.com/cortexproject/cortex/pkg/util/concurrency" @@ -813,11 +813,14 @@ func TestCompactor_ShouldCompactAllUsersOnShardingEnabledButOnlyOneInstanceRunni bucketClient.MockUpload("user-1/bucket-index.json.gz", nil) bucketClient.MockUpload("user-2/bucket-index.json.gz", nil) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + cfg := prepareConfig() cfg.ShardingEnabled = true cfg.ShardingRing.InstanceID = "compactor-1" cfg.ShardingRing.InstanceAddr = "1.2.3.4" - cfg.ShardingRing.KVStore.Mock = consul.NewInMemoryClient(ring.GetCodec()) + cfg.ShardingRing.KVStore.Mock = ringStore c, _, tsdbPlanner, logs, _ := prepare(t, cfg, bucketClient) @@ -890,7 +893,8 @@ func TestCompactor_ShouldCompactOnlyUsersOwnedByTheInstanceOnShardingEnabledAndM } // Create a shared KV Store - kvstore := consul.NewInMemoryClient(ring.GetCodec()) + kvstore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) // Create two compactors var compactors []*Compactor @@ -1212,7 +1216,8 @@ func TestCompactor_DeleteLocalSyncFiles(t *testing.T) { } // Create a shared KV Store - kvstore := consul.NewInMemoryClient(ring.GetCodec()) + kvstore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) // Create two compactors var compactors []*Compactor @@ -1286,11 +1291,14 @@ func TestCompactor_ShouldFailCompactionOnTimeout(t *testing.T) { bucketClient := &bucket.ClientMock{} bucketClient.MockIter("", []string{}, nil) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + cfg := prepareConfig() cfg.ShardingEnabled = true cfg.ShardingRing.InstanceID = "compactor-1" cfg.ShardingRing.InstanceAddr = "1.2.3.4" - cfg.ShardingRing.KVStore.Mock = consul.NewInMemoryClient(ring.GetCodec()) + cfg.ShardingRing.KVStore.Mock = ringStore // Set ObservePeriod to longer than the timeout period to mock a timeout while waiting on ring to become ACTIVE cfg.ShardingRing.ObservePeriod = time.Second * 10 diff --git a/pkg/cortex/cortex.go b/pkg/cortex/cortex.go index 48e63cc76f..c98e079a20 100644 --- a/pkg/cortex/cortex.go +++ b/pkg/cortex/cortex.go @@ -13,6 +13,7 @@ import ( "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv/memberlist" "github.com/grafana/dskit/modules" "github.com/grafana/dskit/runtimeconfig" "github.com/grafana/dskit/services" @@ -49,7 +50,6 @@ import ( "github.com/cortexproject/cortex/pkg/querier/tenantfederation" querier_worker "github.com/cortexproject/cortex/pkg/querier/worker" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv/memberlist" "github.com/cortexproject/cortex/pkg/ruler" "github.com/cortexproject/cortex/pkg/ruler/rulestore" "github.com/cortexproject/cortex/pkg/scheduler" diff --git a/pkg/cortex/cortex_test.go b/pkg/cortex/cortex_test.go index e19e9e91e3..7c7c2c615c 100644 --- a/pkg/cortex/cortex_test.go +++ b/pkg/cortex/cortex_test.go @@ -13,6 +13,7 @@ import ( "time" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv" "github.com/grafana/dskit/services" "github.com/prometheus/client_golang/prometheus" "github.com/stretchr/testify/assert" @@ -26,7 +27,6 @@ import ( "github.com/cortexproject/cortex/pkg/frontend/v1/frontendv1pb" "github.com/cortexproject/cortex/pkg/ingester" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv" "github.com/cortexproject/cortex/pkg/ruler" "github.com/cortexproject/cortex/pkg/scheduler/schedulerpb" "github.com/cortexproject/cortex/pkg/storage/bucket" diff --git a/pkg/cortex/modules.go b/pkg/cortex/modules.go index 557066e72c..562dec0d89 100644 --- a/pkg/cortex/modules.go +++ b/pkg/cortex/modules.go @@ -9,6 +9,8 @@ import ( "time" "github.com/go-kit/kit/log/level" + "github.com/grafana/dskit/kv/codec" + "github.com/grafana/dskit/kv/memberlist" "github.com/grafana/dskit/modules" "github.com/grafana/dskit/runtimeconfig" "github.com/grafana/dskit/services" @@ -19,6 +21,7 @@ import ( "github.com/prometheus/prometheus/promql" "github.com/prometheus/prometheus/rules" prom_storage "github.com/prometheus/prometheus/storage" + "github.com/thanos-io/thanos/pkg/discovery/dns" httpgrpc_server "github.com/weaveworks/common/httpgrpc/server" "github.com/weaveworks/common/server" @@ -41,8 +44,6 @@ import ( "github.com/cortexproject/cortex/pkg/querier/tenantfederation" querier_worker "github.com/cortexproject/cortex/pkg/querier/worker" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv/codec" - "github.com/cortexproject/cortex/pkg/ring/kv/memberlist" "github.com/cortexproject/cortex/pkg/ruler" "github.com/cortexproject/cortex/pkg/scheduler" "github.com/cortexproject/cortex/pkg/storegateway" @@ -763,11 +764,20 @@ func (t *Cortex) initStoreGateway() (serv services.Service, err error) { } func (t *Cortex) initMemberlistKV() (services.Service, error) { - t.Cfg.MemberlistKV.MetricsRegisterer = prometheus.DefaultRegisterer + reg := prometheus.DefaultRegisterer + t.Cfg.MemberlistKV.MetricsRegisterer = reg t.Cfg.MemberlistKV.Codecs = []codec.Codec{ ring.GetCodec(), } - t.MemberlistKV = memberlist.NewKVInitService(&t.Cfg.MemberlistKV, util_log.Logger) + dnsProviderReg := prometheus.WrapRegistererWithPrefix( + "cortex_", + prometheus.WrapRegistererWith( + prometheus.Labels{"name": "memberlist"}, + reg, + ), + ) + dnsProvider := dns.NewProvider(util_log.Logger, dnsProviderReg, dns.GolangResolverType) + t.MemberlistKV = memberlist.NewKVInitService(&t.Cfg.MemberlistKV, util_log.Logger, dnsProvider) t.API.RegisterMemberlistKV(t.MemberlistKV) // Update the config. diff --git a/pkg/cortex/runtime_config.go b/pkg/cortex/runtime_config.go index c45caa0c13..34101aaacc 100644 --- a/pkg/cortex/runtime_config.go +++ b/pkg/cortex/runtime_config.go @@ -5,11 +5,11 @@ import ( "io" "net/http" + "github.com/grafana/dskit/kv" "github.com/grafana/dskit/runtimeconfig" "gopkg.in/yaml.v2" "github.com/cortexproject/cortex/pkg/ingester" - "github.com/cortexproject/cortex/pkg/ring/kv" "github.com/cortexproject/cortex/pkg/util" "github.com/cortexproject/cortex/pkg/util/validation" ) diff --git a/pkg/distributor/distributor_ring.go b/pkg/distributor/distributor_ring.go index f6cdae01ad..dc1ce7c1c2 100644 --- a/pkg/distributor/distributor_ring.go +++ b/pkg/distributor/distributor_ring.go @@ -7,9 +7,9 @@ import ( "github.com/go-kit/kit/log/level" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv" util_log "github.com/cortexproject/cortex/pkg/util/log" ) diff --git a/pkg/distributor/distributor_test.go b/pkg/distributor/distributor_test.go index badb64f9ec..921701ca01 100644 --- a/pkg/distributor/distributor_test.go +++ b/pkg/distributor/distributor_test.go @@ -16,6 +16,8 @@ import ( "github.com/go-kit/kit/log" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv" + "github.com/grafana/dskit/kv/consul" "github.com/grafana/dskit/services" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/testutil" @@ -36,8 +38,6 @@ import ( "github.com/cortexproject/cortex/pkg/prom1/storage/metric" "github.com/cortexproject/cortex/pkg/ring" ring_client "github.com/cortexproject/cortex/pkg/ring/client" - "github.com/cortexproject/cortex/pkg/ring/kv" - "github.com/cortexproject/cortex/pkg/ring/kv/consul" "github.com/cortexproject/cortex/pkg/tenant" "github.com/cortexproject/cortex/pkg/util" "github.com/cortexproject/cortex/pkg/util/chunkcompat" @@ -685,7 +685,11 @@ func TestDistributor_PushHAInstances(t *testing.T) { }) defer stopAll(ds, r) codec := GetReplicaDescCodec() - mock := kv.PrefixClient(consul.NewInMemoryClient(codec), "prefix") + + ringStore, closer := consul.NewInMemoryClient(codec, log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + + mock := kv.PrefixClient(ringStore, "prefix") d := ds[0] if tc.enableTracker { @@ -1582,7 +1586,9 @@ func BenchmarkDistributor_Push(b *testing.B) { b.Run(testName, func(b *testing.B) { // Create an in-memory KV store for the ring with 1 ingester registered. - kvStore := consul.NewInMemoryClient(ring.GetCodec()) + kvStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + b.Cleanup(func() { assert.NoError(b, closer.Close()) }) + err := kvStore.CAS(context.Background(), ring.IngesterRingKey, func(_ interface{}) (interface{}, bool, error) { d := &ring.Desc{} @@ -1931,7 +1937,9 @@ func prepare(t *testing.T, cfg prepConfig) ([]*Distributor, []mockIngester, *rin ingestersByAddr[addr] = &ingesters[i] } - kvStore := consul.NewInMemoryClient(ring.GetCodec()) + kvStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + err := kvStore.CAS(context.Background(), ring.IngesterRingKey, func(_ interface{}) (interface{}, bool, error) { return &ring.Desc{ diff --git a/pkg/distributor/ha_tracker.go b/pkg/distributor/ha_tracker.go index 12891270a2..504b24deea 100644 --- a/pkg/distributor/ha_tracker.go +++ b/pkg/distributor/ha_tracker.go @@ -13,14 +13,14 @@ import ( "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" "github.com/gogo/protobuf/proto" + "github.com/grafana/dskit/kv" + "github.com/grafana/dskit/kv/codec" "github.com/grafana/dskit/services" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "github.com/prometheus/prometheus/pkg/timestamp" "github.com/cortexproject/cortex/pkg/cortexpb" - "github.com/cortexproject/cortex/pkg/ring/kv" - "github.com/cortexproject/cortex/pkg/ring/kv/codec" "github.com/cortexproject/cortex/pkg/util" ) @@ -177,7 +177,8 @@ func newHATracker(cfg HATrackerConfig, limits haTrackerLimits, reg prometheus.Re client, err := kv.NewClient( cfg.KVStore, GetReplicaDescCodec(), - kv.RegistererWithKVName(reg, "distributor-hatracker"), + kv.RegistererWithKVName(prometheus.WrapRegistererWithPrefix("cortex_", reg), "distributor-hatracker"), + logger, ) if err != nil { return nil, err diff --git a/pkg/distributor/ha_tracker_test.go b/pkg/distributor/ha_tracker_test.go index 920c531c18..3ce52c2125 100644 --- a/pkg/distributor/ha_tracker_test.go +++ b/pkg/distributor/ha_tracker_test.go @@ -9,6 +9,8 @@ import ( "github.com/go-kit/kit/log" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv" + "github.com/grafana/dskit/kv/consul" "github.com/grafana/dskit/services" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" @@ -21,8 +23,6 @@ import ( "github.com/cortexproject/cortex/pkg/cortexpb" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv" - "github.com/cortexproject/cortex/pkg/ring/kv/consul" "github.com/cortexproject/cortex/pkg/util" util_log "github.com/cortexproject/cortex/pkg/util/log" "github.com/cortexproject/cortex/pkg/util/test" @@ -119,7 +119,10 @@ func TestWatchPrefixAssignment(t *testing.T) { replica := "r1" codec := GetReplicaDescCodec() - mock := kv.PrefixClient(consul.NewInMemoryClient(codec), "prefix") + kvStore, closer := consul.NewInMemoryClient(codec, log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + + mock := kv.PrefixClient(kvStore, "prefix") c, err := newHATracker(HATrackerConfig{ EnableHATracker: true, KVStore: kv.Config{Mock: mock}, @@ -303,7 +306,10 @@ func TestCheckReplicaUpdateTimeout(t *testing.T) { user := "user" codec := GetReplicaDescCodec() - mock := kv.PrefixClient(consul.NewInMemoryClient(codec), "prefix") + kvStore, closer := consul.NewInMemoryClient(codec, log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + + mock := kv.PrefixClient(kvStore, "prefix") c, err := newHATracker(HATrackerConfig{ EnableHATracker: true, KVStore: kv.Config{Mock: mock}, @@ -349,7 +355,10 @@ func TestCheckReplicaMultiUser(t *testing.T) { cluster := "c1" codec := GetReplicaDescCodec() - mock := kv.PrefixClient(consul.NewInMemoryClient(codec), "prefix") + kvStore, closer := consul.NewInMemoryClient(codec, log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + + mock := kv.PrefixClient(kvStore, "prefix") c, err := newHATracker(HATrackerConfig{ EnableHATracker: true, KVStore: kv.Config{Mock: mock}, @@ -426,7 +435,10 @@ func TestCheckReplicaUpdateTimeoutJitter(t *testing.T) { t.Run(testName, func(t *testing.T) { // Init HA tracker codec := GetReplicaDescCodec() - mock := kv.PrefixClient(consul.NewInMemoryClient(codec), "prefix") + kvStore, closer := consul.NewInMemoryClient(codec, log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + + mock := kv.PrefixClient(kvStore, "prefix") c, err := newHATracker(HATrackerConfig{ EnableHATracker: true, KVStore: kv.Config{Mock: mock}, @@ -522,7 +534,10 @@ func TestHAClustersLimit(t *testing.T) { const userID = "user" codec := GetReplicaDescCodec() - mock := kv.PrefixClient(consul.NewInMemoryClient(codec), "prefix") + kvStore, closer := consul.NewInMemoryClient(codec, log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + + mock := kv.PrefixClient(kvStore, "prefix") limits := trackerLimits{maxClusters: 2} t1, err := newHATracker(HATrackerConfig{ @@ -655,7 +670,7 @@ func TestHATracker_MetricsCleanup(t *testing.T) { cortex_ha_tracker_elected_replica_timestamp_seconds{cluster="cluster",user="userB"} 10 cortex_ha_tracker_elected_replica_timestamp_seconds{cluster="cluster1",user="userA"} 5 cortex_ha_tracker_elected_replica_timestamp_seconds{cluster="cluster2",user="userA"} 8 - + # HELP cortex_ha_tracker_kv_store_cas_total The total number of CAS calls to the KV store for a user ID/cluster. # TYPE cortex_ha_tracker_kv_store_cas_total counter cortex_ha_tracker_kv_store_cas_total{cluster="cluster",user="userB"} 10 @@ -688,7 +703,10 @@ func TestCheckReplicaCleanup(t *testing.T) { reg := prometheus.NewPedanticRegistry() - mock := kv.PrefixClient(consul.NewInMemoryClient(GetReplicaDescCodec()), "prefix") + kvStore, closer := consul.NewInMemoryClient(GetReplicaDescCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + + mock := kv.PrefixClient(kvStore, "prefix") c, err := newHATracker(HATrackerConfig{ EnableHATracker: true, KVStore: kv.Config{Mock: mock}, diff --git a/pkg/ingester/flush_test.go b/pkg/ingester/flush_test.go index 8b1fb8d458..ed1090c61c 100644 --- a/pkg/ingester/flush_test.go +++ b/pkg/ingester/flush_test.go @@ -9,6 +9,7 @@ import ( "time" "github.com/go-kit/kit/log" + "github.com/grafana/dskit/kv" "github.com/grafana/dskit/services" "github.com/prometheus/common/model" "github.com/prometheus/prometheus/pkg/labels" @@ -20,7 +21,6 @@ import ( "github.com/cortexproject/cortex/pkg/cortexpb" "github.com/cortexproject/cortex/pkg/ingester/client" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv" "github.com/cortexproject/cortex/pkg/util" "github.com/cortexproject/cortex/pkg/util/validation" ) diff --git a/pkg/ingester/ingester_test.go b/pkg/ingester/ingester_test.go index b09530838e..2cc58146c1 100644 --- a/pkg/ingester/ingester_test.go +++ b/pkg/ingester/ingester_test.go @@ -58,9 +58,11 @@ func newTestStore(t require.TestingT, cfg Config, clientConfig client.Config, li return store, ing } -func newDefaultTestStore(t require.TestingT) (*testStore, *Ingester) { +func newDefaultTestStore(t testing.TB) (*testStore, *Ingester) { + t.Helper() + return newTestStore(t, - defaultIngesterTestConfig(), + defaultIngesterTestConfig(t), defaultClientTestConfig(), defaultLimitsTestConfig(), nil) } @@ -259,7 +261,7 @@ func TestIngesterMetadataAppend(t *testing.T) { t.Run(tc.desc, func(t *testing.T) { limits := defaultLimitsTestConfig() limits.MaxLocalMetadataPerMetric = 50 - _, ing := newTestStore(t, defaultIngesterTestConfig(), defaultClientTestConfig(), limits, nil) + _, ing := newTestStore(t, defaultIngesterTestConfig(t), defaultClientTestConfig(), limits, nil) userIDs, _ := pushTestMetadata(t, ing, tc.numMetadata, tc.metadataPerMetric) for _, userID := range userIDs { @@ -289,7 +291,7 @@ func TestIngesterMetadataAppend(t *testing.T) { } func TestIngesterPurgeMetadata(t *testing.T) { - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.MetadataRetainPeriod = 20 * time.Millisecond _, ing := newTestStore(t, cfg, defaultClientTestConfig(), defaultLimitsTestConfig(), nil) userIDs, _ := pushTestMetadata(t, ing, 10, 3) @@ -307,7 +309,7 @@ func TestIngesterPurgeMetadata(t *testing.T) { func TestIngesterMetadataMetrics(t *testing.T) { reg := prometheus.NewPedanticRegistry() - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.MetadataRetainPeriod = 20 * time.Millisecond _, ing := newTestStore(t, cfg, defaultClientTestConfig(), defaultLimitsTestConfig(), reg) _, _ = pushTestMetadata(t, ing, 10, 3) @@ -379,7 +381,7 @@ func TestIngesterSendsOnlySeriesWithData(t *testing.T) { func TestIngesterIdleFlush(t *testing.T) { // Create test ingester with short flush cycle - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.FlushCheckPeriod = 20 * time.Millisecond cfg.MaxChunkIdle = 100 * time.Millisecond cfg.RetainPeriod = 500 * time.Millisecond @@ -416,7 +418,7 @@ func TestIngesterIdleFlush(t *testing.T) { func TestIngesterSpreadFlush(t *testing.T) { // Create test ingester with short flush cycle - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.SpreadFlushes = true cfg.FlushCheckPeriod = 20 * time.Millisecond store, ing := newTestStore(t, cfg, defaultClientTestConfig(), defaultLimitsTestConfig(), nil) @@ -528,7 +530,7 @@ func TestIngesterUserLimitExceeded(t *testing.T) { require.NoError(t, os.Mkdir(blocksDir, os.ModePerm)) chunksIngesterGenerator := func() *Ingester { - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.WALConfig.WALEnabled = true cfg.WALConfig.Recover = true cfg.WALConfig.Dir = chunksDir @@ -539,7 +541,7 @@ func TestIngesterUserLimitExceeded(t *testing.T) { } blocksIngesterGenerator := func() *Ingester { - ing, err := prepareIngesterWithBlocksStorageAndLimits(t, defaultIngesterTestConfig(), limits, blocksDir, nil) + ing, err := prepareIngesterWithBlocksStorageAndLimits(t, defaultIngesterTestConfig(t), limits, blocksDir, nil) require.NoError(t, err) require.NoError(t, services.StartAndAwaitRunning(context.Background(), ing)) // Wait until it's ACTIVE @@ -651,7 +653,7 @@ func TestIngesterMetricLimitExceeded(t *testing.T) { require.NoError(t, os.Mkdir(blocksDir, os.ModePerm)) chunksIngesterGenerator := func() *Ingester { - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.WALConfig.WALEnabled = true cfg.WALConfig.Recover = true cfg.WALConfig.Dir = chunksDir @@ -662,7 +664,7 @@ func TestIngesterMetricLimitExceeded(t *testing.T) { } blocksIngesterGenerator := func() *Ingester { - ing, err := prepareIngesterWithBlocksStorageAndLimits(t, defaultIngesterTestConfig(), limits, blocksDir, nil) + ing, err := prepareIngesterWithBlocksStorageAndLimits(t, defaultIngesterTestConfig(t), limits, blocksDir, nil) require.NoError(t, err) require.NoError(t, services.StartAndAwaitRunning(context.Background(), ing)) // Wait until it's ACTIVE @@ -870,7 +872,7 @@ func benchmarkData(nSeries int) (allLabels []labels.Labels, allSamples []cortexp } func benchmarkIngesterPush(b *testing.B, limits validation.Limits, errorsExpected bool) { - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(b) clientCfg := defaultClientTestConfig() const ( @@ -913,7 +915,7 @@ func benchmarkIngesterPush(b *testing.B, limits validation.Limits, errorsExpecte } func BenchmarkIngester_QueryStream(b *testing.B) { - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(b) clientCfg := defaultClientTestConfig() limits := defaultLimitsTestConfig() _, ing := newTestStore(b, cfg, clientCfg, limits, nil) @@ -1012,7 +1014,7 @@ func TestIngesterActiveSeries(t *testing.T) { registry := prometheus.NewRegistry() // Create a mocked ingester - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.LifecyclerConfig.JoinAfter = 0 cfg.ActiveSeriesMetricsEnabled = !testData.disableActiveSeries diff --git a/pkg/ingester/ingester_v2_test.go b/pkg/ingester/ingester_v2_test.go index 4ff09e3425..f5374fcee4 100644 --- a/pkg/ingester/ingester_v2_test.go +++ b/pkg/ingester/ingester_v2_test.go @@ -508,7 +508,7 @@ func TestIngester_v2Push(t *testing.T) { validation.DiscardedSamples.Reset() // Create a mocked ingester - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.LifecyclerConfig.JoinAfter = 0 cfg.ActiveSeriesMetricsEnabled = !testData.disableActiveSeries cfg.BlocksStorageConfig.TSDB.MaxExemplars = testData.maxExemplars @@ -602,7 +602,7 @@ func TestIngester_v2Push_ShouldCorrectlyTrackMetricsInMultiTenantScenario(t *tes registry := prometheus.NewRegistry() // Create a mocked ingester - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.LifecyclerConfig.JoinAfter = 0 i, err := prepareIngesterWithBlocksStorage(t, cfg, registry) @@ -683,7 +683,7 @@ func TestIngester_v2Push_DecreaseInactiveSeries(t *testing.T) { registry := prometheus.NewRegistry() // Create a mocked ingester - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.ActiveSeriesMetricsIdleTimeout = 100 * time.Millisecond cfg.LifecyclerConfig.JoinAfter = 0 @@ -754,7 +754,7 @@ func benchmarkIngesterV2Push(b *testing.B, limits validation.Limits, errorsExpec ctx := user.InjectOrgID(context.Background(), userID) // Create a mocked ingester - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(b) cfg.LifecyclerConfig.JoinAfter = 0 ingester, err := prepareIngesterWithBlocksStorage(b, cfg, registry) @@ -1039,7 +1039,7 @@ func Benchmark_Ingester_v2PushOnError(b *testing.B) { } // Create a mocked ingester - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(b) cfg.LifecyclerConfig.JoinAfter = 0 limits := defaultLimitsTestConfig() @@ -1111,7 +1111,7 @@ func Test_Ingester_v2LabelNames(t *testing.T) { expected := []string{"__name__", "status", "route"} // Create ingester - i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(), nil) + i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(t), nil) require.NoError(t, err) require.NoError(t, services.StartAndAwaitRunning(context.Background(), i)) defer services.StopAndAwaitTerminated(context.Background(), i) //nolint:errcheck @@ -1155,7 +1155,7 @@ func Test_Ingester_v2LabelValues(t *testing.T) { } // Create ingester - i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(), nil) + i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(t), nil) require.NoError(t, err) require.NoError(t, services.StartAndAwaitRunning(context.Background(), i)) defer services.StopAndAwaitTerminated(context.Background(), i) //nolint:errcheck @@ -1274,7 +1274,7 @@ func Test_Ingester_v2Query(t *testing.T) { } // Create ingester - i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(), nil) + i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(t), nil) require.NoError(t, err) require.NoError(t, services.StartAndAwaitRunning(context.Background(), i)) defer services.StopAndAwaitTerminated(context.Background(), i) //nolint:errcheck @@ -1309,7 +1309,7 @@ func Test_Ingester_v2Query(t *testing.T) { } } func TestIngester_v2Query_ShouldNotCreateTSDBIfDoesNotExists(t *testing.T) { - i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(), nil) + i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(t), nil) require.NoError(t, err) require.NoError(t, services.StartAndAwaitRunning(context.Background(), i)) defer services.StopAndAwaitTerminated(context.Background(), i) //nolint:errcheck @@ -1329,7 +1329,7 @@ func TestIngester_v2Query_ShouldNotCreateTSDBIfDoesNotExists(t *testing.T) { } func TestIngester_v2LabelValues_ShouldNotCreateTSDBIfDoesNotExists(t *testing.T) { - i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(), nil) + i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(t), nil) require.NoError(t, err) require.NoError(t, services.StartAndAwaitRunning(context.Background(), i)) defer services.StopAndAwaitTerminated(context.Background(), i) //nolint:errcheck @@ -1349,7 +1349,7 @@ func TestIngester_v2LabelValues_ShouldNotCreateTSDBIfDoesNotExists(t *testing.T) } func TestIngester_v2LabelNames_ShouldNotCreateTSDBIfDoesNotExists(t *testing.T) { - i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(), nil) + i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(t), nil) require.NoError(t, err) require.NoError(t, services.StartAndAwaitRunning(context.Background(), i)) defer services.StopAndAwaitTerminated(context.Background(), i) //nolint:errcheck @@ -1371,7 +1371,7 @@ func TestIngester_v2LabelNames_ShouldNotCreateTSDBIfDoesNotExists(t *testing.T) func TestIngester_v2Push_ShouldNotCreateTSDBIfNotInActiveState(t *testing.T) { // Configure the lifecycler to not immediately join the ring, to make sure // the ingester will NOT be in the ACTIVE state when we'll push samples. - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.LifecyclerConfig.JoinAfter = 10 * time.Second i, err := prepareIngesterWithBlocksStorage(t, cfg, nil) @@ -1419,7 +1419,7 @@ func TestIngester_getOrCreateTSDB_ShouldNotAllowToCreateTSDBIfIngesterStateIsNot for testName, testData := range tests { t.Run(testName, func(t *testing.T) { - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.LifecyclerConfig.JoinAfter = 60 * time.Second i, err := prepareIngesterWithBlocksStorage(t, cfg, nil) @@ -1568,7 +1568,7 @@ func Test_Ingester_v2MetricsForLabelMatchers(t *testing.T) { } // Create ingester - i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(), nil) + i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(t), nil) require.NoError(t, err) require.NoError(t, services.StartAndAwaitRunning(context.Background(), i)) defer services.StopAndAwaitTerminated(context.Background(), i) //nolint:errcheck @@ -1673,7 +1673,7 @@ func createIngesterWithSeries(t testing.TB, userID string, numSeries, numSamples const maxBatchSize = 1000 // Create ingester. - i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(), nil) + i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(t), nil) require.NoError(t, err) require.NoError(t, services.StartAndAwaitRunning(context.Background(), i)) t.Cleanup(func() { @@ -1719,7 +1719,7 @@ func createIngesterWithSeries(t testing.TB, userID string, numSeries, numSamples func TestIngester_v2QueryStream(t *testing.T) { // Create ingester. - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) // change stream type in runtime. var streamType QueryStreamType @@ -1823,7 +1823,7 @@ func TestIngester_v2QueryStream(t *testing.T) { func TestIngester_v2QueryStreamManySamples(t *testing.T) { // Create ingester. - i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(), nil) + i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(t), nil) require.NoError(t, err) require.NoError(t, services.StartAndAwaitRunning(context.Background(), i)) defer services.StopAndAwaitTerminated(context.Background(), i) //nolint:errcheck @@ -1917,7 +1917,7 @@ func TestIngester_v2QueryStreamManySamples(t *testing.T) { func TestIngester_v2QueryStreamManySamplesChunks(t *testing.T) { // Create ingester. - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.StreamChunksWhenUsingBlocks = true i, err := prepareIngesterWithBlocksStorage(t, cfg, nil) @@ -2053,7 +2053,7 @@ func BenchmarkIngester_v2QueryStream_Chunks(b *testing.B) { } func benchmarkV2QueryStream(b *testing.B, streamChunks bool) { - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(b) cfg.StreamChunksWhenUsingBlocks = streamChunks // Create ingester. @@ -2334,7 +2334,7 @@ func TestIngester_v2OpenExistingTSDBOnStartup(t *testing.T) { require.NoError(t, err) defer os.RemoveAll(tempDir) - ingesterCfg := defaultIngesterTestConfig() + ingesterCfg := defaultIngesterTestConfig(t) ingesterCfg.BlocksStorageEnabled = true ingesterCfg.BlocksStorageConfig.TSDB.Dir = tempDir ingesterCfg.BlocksStorageConfig.TSDB.MaxTSDBOpeningConcurrencyOnStartup = testData.concurrency @@ -2362,7 +2362,7 @@ func TestIngester_v2OpenExistingTSDBOnStartup(t *testing.T) { } func TestIngester_shipBlocks(t *testing.T) { - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.LifecyclerConfig.JoinAfter = 0 cfg.BlocksStorageConfig.TSDB.ShipConcurrency = 2 @@ -2400,7 +2400,7 @@ func TestIngester_shipBlocks(t *testing.T) { } func TestIngester_dontShipBlocksWhenTenantDeletionMarkerIsPresent(t *testing.T) { - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.LifecyclerConfig.JoinAfter = 0 cfg.BlocksStorageConfig.TSDB.ShipConcurrency = 2 @@ -2449,7 +2449,7 @@ func TestIngester_dontShipBlocksWhenTenantDeletionMarkerIsPresent(t *testing.T) } func TestIngester_seriesCountIsCorrectAfterClosingTSDBForDeletedTenant(t *testing.T) { - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.LifecyclerConfig.JoinAfter = 0 cfg.BlocksStorageConfig.TSDB.ShipConcurrency = 2 @@ -2493,7 +2493,7 @@ func TestIngester_seriesCountIsCorrectAfterClosingTSDBForDeletedTenant(t *testin func TestIngester_closeAndDeleteUserTSDBIfIdle_shouldNotCloseTSDBIfShippingIsInProgress(t *testing.T) { ctx := context.Background() - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.LifecyclerConfig.JoinAfter = 0 cfg.BlocksStorageConfig.TSDB.ShipConcurrency = 2 @@ -2536,7 +2536,7 @@ func TestIngester_closeAndDeleteUserTSDBIfIdle_shouldNotCloseTSDBIfShippingIsInP func TestIngester_closingAndOpeningTsdbConcurrently(t *testing.T) { ctx := context.Background() - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.BlocksStorageConfig.TSDB.CloseIdleTSDBTimeout = 0 // Will not run the loop, but will allow us to close any TSDB fast. // Create ingester @@ -2587,7 +2587,7 @@ func TestIngester_closingAndOpeningTsdbConcurrently(t *testing.T) { func TestIngester_idleCloseEmptyTSDB(t *testing.T) { ctx := context.Background() - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.BlocksStorageConfig.TSDB.ShipInterval = 1 * time.Minute cfg.BlocksStorageConfig.TSDB.HeadCompactionInterval = 1 * time.Minute cfg.BlocksStorageConfig.TSDB.CloseIdleTSDBTimeout = 0 // Will not run the loop, but will allow us to close any TSDB fast. @@ -2636,7 +2636,7 @@ func (m *shipperMock) Sync(ctx context.Context) (uploaded int, err error) { } func TestIngester_invalidSamplesDontChangeLastUpdateTime(t *testing.T) { - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.LifecyclerConfig.JoinAfter = 0 // Create ingester @@ -2864,7 +2864,7 @@ func TestIngester_flushing(t *testing.T) { }, } { t.Run(name, func(t *testing.T) { - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.LifecyclerConfig.JoinAfter = 0 cfg.BlocksStorageConfig.TSDB.ShipConcurrency = 1 cfg.BlocksStorageConfig.TSDB.ShipInterval = 1 * time.Minute // Long enough to not be reached during the test. @@ -2895,7 +2895,7 @@ func TestIngester_flushing(t *testing.T) { } func TestIngester_ForFlush(t *testing.T) { - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.LifecyclerConfig.JoinAfter = 0 cfg.BlocksStorageConfig.TSDB.ShipConcurrency = 1 cfg.BlocksStorageConfig.TSDB.ShipInterval = 10 * time.Minute // Long enough to not be reached during the test. @@ -2973,7 +2973,7 @@ func Test_Ingester_v2UserStats(t *testing.T) { } // Create ingester - i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(), nil) + i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(t), nil) require.NoError(t, err) require.NoError(t, services.StartAndAwaitRunning(context.Background(), i)) defer services.StopAndAwaitTerminated(context.Background(), i) //nolint:errcheck @@ -3021,7 +3021,7 @@ func Test_Ingester_v2AllUserStats(t *testing.T) { } // Create ingester - i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(), nil) + i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(t), nil) require.NoError(t, err) require.NoError(t, services.StartAndAwaitRunning(context.Background(), i)) defer services.StopAndAwaitTerminated(context.Background(), i) //nolint:errcheck @@ -3071,7 +3071,7 @@ func Test_Ingester_v2AllUserStats(t *testing.T) { } func TestIngesterCompactIdleBlock(t *testing.T) { - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.LifecyclerConfig.JoinAfter = 0 cfg.BlocksStorageConfig.TSDB.ShipConcurrency = 1 cfg.BlocksStorageConfig.TSDB.HeadCompactionInterval = 1 * time.Hour // Long enough to not be reached during the test. @@ -3150,7 +3150,7 @@ func TestIngesterCompactIdleBlock(t *testing.T) { } func TestIngesterCompactAndCloseIdleTSDB(t *testing.T) { - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.LifecyclerConfig.JoinAfter = 0 cfg.BlocksStorageConfig.TSDB.ShipInterval = 1 * time.Second // Required to enable shipping. cfg.BlocksStorageConfig.TSDB.ShipConcurrency = 1 @@ -3346,7 +3346,7 @@ func TestHeadCompactionOnStartup(t *testing.T) { overrides, err := validation.NewOverrides(limits, nil) require.NoError(t, err) - ingesterCfg := defaultIngesterTestConfig() + ingesterCfg := defaultIngesterTestConfig(t) ingesterCfg.BlocksStorageEnabled = true ingesterCfg.BlocksStorageConfig.TSDB.Dir = tempDir ingesterCfg.BlocksStorageConfig.Bucket.Backend = "s3" @@ -3370,7 +3370,7 @@ func TestHeadCompactionOnStartup(t *testing.T) { } func TestIngester_CloseTSDBsOnShutdown(t *testing.T) { - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.LifecyclerConfig.JoinAfter = 0 // Create ingester @@ -3404,7 +3404,7 @@ func TestIngester_CloseTSDBsOnShutdown(t *testing.T) { func TestIngesterNotDeleteUnshippedBlocks(t *testing.T) { chunkRange := 2 * time.Hour chunkRangeMilliSec := chunkRange.Milliseconds() - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.BlocksStorageConfig.TSDB.BlockRanges = []time.Duration{chunkRange} cfg.BlocksStorageConfig.TSDB.Retention = time.Millisecond // Which means delete all but first block. cfg.LifecyclerConfig.JoinAfter = 0 @@ -3512,7 +3512,7 @@ func TestIngesterNotDeleteUnshippedBlocks(t *testing.T) { } func TestIngesterPushErrorDuringForcedCompaction(t *testing.T) { - i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(), nil) + i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(t), nil) require.NoError(t, err) require.NoError(t, services.StartAndAwaitRunning(context.Background(), i)) @@ -3546,7 +3546,7 @@ func TestIngesterPushErrorDuringForcedCompaction(t *testing.T) { func TestIngesterNoFlushWithInFlightRequest(t *testing.T) { registry := prometheus.NewRegistry() - i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(), registry) + i, err := prepareIngesterWithBlocksStorage(t, defaultIngesterTestConfig(t), registry) require.NoError(t, err) require.NoError(t, services.StartAndAwaitRunning(context.Background(), i)) @@ -3696,7 +3696,7 @@ func TestIngester_v2PushInstanceLimits(t *testing.T) { for testName, testData := range tests { t.Run(testName, func(t *testing.T) { // Create a mocked ingester - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.LifecyclerConfig.JoinAfter = 0 cfg.InstanceLimitsFn = func() *InstanceLimits { return &testData.limits @@ -3762,7 +3762,7 @@ func TestIngester_instanceLimitsMetrics(t *testing.T) { MaxInMemorySeries: 30, } - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.InstanceLimitsFn = func() *InstanceLimits { return &l } @@ -3797,7 +3797,7 @@ func TestIngester_inflightPushRequests(t *testing.T) { limits := InstanceLimits{MaxInflightPushRequests: 1} // Create a mocked ingester - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.InstanceLimitsFn = func() *InstanceLimits { return &limits } cfg.LifecyclerConfig.JoinAfter = 0 diff --git a/pkg/ingester/lifecycle_test.go b/pkg/ingester/lifecycle_test.go index 68c7ba1687..7c3cea01ee 100644 --- a/pkg/ingester/lifecycle_test.go +++ b/pkg/ingester/lifecycle_test.go @@ -12,6 +12,7 @@ import ( "github.com/go-kit/kit/log" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv/consul" "github.com/grafana/dskit/services" "github.com/prometheus/common/model" "github.com/prometheus/prometheus/pkg/labels" @@ -25,7 +26,6 @@ import ( "github.com/cortexproject/cortex/pkg/cortexpb" "github.com/cortexproject/cortex/pkg/ingester/client" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv/consul" "github.com/cortexproject/cortex/pkg/ring/testutils" "github.com/cortexproject/cortex/pkg/util/test" "github.com/cortexproject/cortex/pkg/util/validation" @@ -33,8 +33,12 @@ import ( const userID = "1" -func defaultIngesterTestConfig() Config { - consul := consul.NewInMemoryClient(ring.GetCodec()) +func defaultIngesterTestConfig(t testing.TB) Config { + t.Helper() + + consul, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + cfg := Config{} flagext.DefaultValues(&cfg) flagext.DefaultValues(&cfg.BlocksStorageConfig) @@ -66,7 +70,7 @@ func defaultLimitsTestConfig() validation.Limits { // TestIngesterRestart tests a restarting ingester doesn't keep adding more tokens. func TestIngesterRestart(t *testing.T) { - config := defaultIngesterTestConfig() + config := defaultIngesterTestConfig(t) clientConfig := defaultClientTestConfig() limits := defaultLimitsTestConfig() config.LifecyclerConfig.UnregisterOnShutdown = false @@ -99,7 +103,7 @@ func TestIngesterRestart(t *testing.T) { func TestIngester_ShutdownHandler(t *testing.T) { for _, unregister := range []bool{false, true} { t.Run(fmt.Sprintf("unregister=%t", unregister), func(t *testing.T) { - config := defaultIngesterTestConfig() + config := defaultIngesterTestConfig(t) clientConfig := defaultClientTestConfig() limits := defaultLimitsTestConfig() config.LifecyclerConfig.UnregisterOnShutdown = unregister @@ -127,7 +131,7 @@ func TestIngesterChunksTransfer(t *testing.T) { require.NoError(t, err) // Start the first ingester, and get it into ACTIVE state. - cfg1 := defaultIngesterTestConfig() + cfg1 := defaultIngesterTestConfig(t) cfg1.LifecyclerConfig.ID = "ingester1" cfg1.LifecyclerConfig.Addr = "ingester1" cfg1.LifecyclerConfig.JoinAfter = 0 * time.Second @@ -147,7 +151,7 @@ func TestIngesterChunksTransfer(t *testing.T) { require.NoError(t, err) // Start a second ingester, but let it go into PENDING - cfg2 := defaultIngesterTestConfig() + cfg2 := defaultIngesterTestConfig(t) cfg2.LifecyclerConfig.RingConfig.KVStore.Mock = cfg1.LifecyclerConfig.RingConfig.KVStore.Mock cfg2.LifecyclerConfig.ID = "ingester2" cfg2.LifecyclerConfig.Addr = "ingester2" @@ -195,7 +199,7 @@ func TestIngesterBadTransfer(t *testing.T) { require.NoError(t, err) // Start ingester in PENDING. - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.LifecyclerConfig.ID = "ingester1" cfg.LifecyclerConfig.Addr = "ingester1" cfg.LifecyclerConfig.JoinAfter = 100 * time.Second diff --git a/pkg/ingester/user_state_test.go b/pkg/ingester/user_state_test.go index f50da94df4..68c2c7b3fd 100644 --- a/pkg/ingester/user_state_test.go +++ b/pkg/ingester/user_state_test.go @@ -84,7 +84,7 @@ func TestForSeriesMatchingBatching(t *testing.T) { func TestTeardown(t *testing.T) { reg := prometheus.NewPedanticRegistry() _, ing := newTestStore(t, - defaultIngesterTestConfig(), + defaultIngesterTestConfig(t), defaultClientTestConfig(), defaultLimitsTestConfig(), reg) diff --git a/pkg/ingester/wal_test.go b/pkg/ingester/wal_test.go index ad1495203b..030c6cb72d 100644 --- a/pkg/ingester/wal_test.go +++ b/pkg/ingester/wal_test.go @@ -28,7 +28,7 @@ func TestWAL(t *testing.T) { require.NoError(t, os.RemoveAll(dirname)) }() - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.WALConfig.WALEnabled = true cfg.WALConfig.CheckpointEnabled = true cfg.WALConfig.Recover = true @@ -108,7 +108,7 @@ func TestWAL(t *testing.T) { } func TestCheckpointRepair(t *testing.T) { - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(t) cfg.WALConfig.WALEnabled = true cfg.WALConfig.CheckpointEnabled = true cfg.WALConfig.Recover = true @@ -290,7 +290,7 @@ func BenchmarkWALReplay(b *testing.B) { require.NoError(b, os.RemoveAll(dirname)) }() - cfg := defaultIngesterTestConfig() + cfg := defaultIngesterTestConfig(b) cfg.WALConfig.WALEnabled = true cfg.WALConfig.CheckpointEnabled = true cfg.WALConfig.Recover = true diff --git a/pkg/querier/blocks_store_queryable.go b/pkg/querier/blocks_store_queryable.go index 85869afe9a..2094cfecdc 100644 --- a/pkg/querier/blocks_store_queryable.go +++ b/pkg/querier/blocks_store_queryable.go @@ -12,6 +12,7 @@ import ( "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" "github.com/gogo/protobuf/types" + "github.com/grafana/dskit/kv" "github.com/grafana/dskit/services" "github.com/oklog/ulid" "github.com/pkg/errors" @@ -32,7 +33,6 @@ import ( "github.com/cortexproject/cortex/pkg/querier/series" "github.com/cortexproject/cortex/pkg/querier/stats" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv" "github.com/cortexproject/cortex/pkg/storage/bucket" cortex_tsdb "github.com/cortexproject/cortex/pkg/storage/tsdb" "github.com/cortexproject/cortex/pkg/storage/tsdb/bucketindex" @@ -210,7 +210,8 @@ func NewBlocksStoreQueryableFromConfig(querierCfg Config, gatewayCfg storegatewa storesRingBackend, err := kv.NewClient( storesRingCfg.KVStore, ring.GetCodec(), - kv.RegistererWithKVName(reg, "querier-store-gateway"), + kv.RegistererWithKVName(prometheus.WrapRegistererWithPrefix("cortex_", reg), "querier-store-gateway"), + logger, ) if err != nil { return nil, errors.Wrap(err, "failed to create store-gateway ring backend") diff --git a/pkg/querier/blocks_store_replicated_set_test.go b/pkg/querier/blocks_store_replicated_set_test.go index 7772b681d7..e71bfbdb9c 100644 --- a/pkg/querier/blocks_store_replicated_set_test.go +++ b/pkg/querier/blocks_store_replicated_set_test.go @@ -9,6 +9,7 @@ import ( "github.com/go-kit/kit/log" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv/consul" "github.com/grafana/dskit/services" "github.com/oklog/ulid" "github.com/prometheus/client_golang/prometheus" @@ -17,7 +18,6 @@ import ( "github.com/stretchr/testify/require" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv/consul" cortex_tsdb "github.com/cortexproject/cortex/pkg/storage/tsdb" "github.com/cortexproject/cortex/pkg/util" "github.com/cortexproject/cortex/pkg/util/test" @@ -328,7 +328,9 @@ func TestBlocksStoreReplicationSet_GetClientsFor(t *testing.T) { ctx := context.Background() // Setup the ring state. - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + require.NoError(t, ringStore.CAS(ctx, "test", func(in interface{}) (interface{}, bool, error) { d := ring.NewDesc() testData.setup(d) @@ -386,7 +388,9 @@ func TestBlocksStoreReplicationSet_GetClientsFor_ShouldSupportRandomLoadBalancin block1 := ulid.MustNew(1, nil) // Create a ring. - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + require.NoError(t, ringStore.CAS(ctx, "test", func(in interface{}) (interface{}, bool, error) { d := ring.NewDesc() for n := 1; n <= numInstances; n++ { diff --git a/pkg/querier/worker/scheduler_processor.go b/pkg/querier/worker/scheduler_processor.go index f4f7371091..201fa27df2 100644 --- a/pkg/querier/worker/scheduler_processor.go +++ b/pkg/querier/worker/scheduler_processor.go @@ -6,18 +6,17 @@ import ( "net/http" "time" - "github.com/grafana/dskit/backoff" - otgrpc "github.com/opentracing-contrib/go-grpc" - "github.com/weaveworks/common/user" - "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" + "github.com/grafana/dskit/backoff" "github.com/grafana/dskit/services" + otgrpc "github.com/opentracing-contrib/go-grpc" "github.com/opentracing/opentracing-go" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "github.com/weaveworks/common/httpgrpc" "github.com/weaveworks/common/middleware" + "github.com/weaveworks/common/user" "google.golang.org/grpc" "google.golang.org/grpc/health/grpc_health_v1" diff --git a/pkg/ring/basic_lifecycler.go b/pkg/ring/basic_lifecycler.go index bf90235257..215032895a 100644 --- a/pkg/ring/basic_lifecycler.go +++ b/pkg/ring/basic_lifecycler.go @@ -9,11 +9,11 @@ import ( "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" + "github.com/grafana/dskit/kv" "github.com/grafana/dskit/services" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" - "github.com/cortexproject/cortex/pkg/ring/kv" "github.com/cortexproject/cortex/pkg/util" util_log "github.com/cortexproject/cortex/pkg/util/log" ) diff --git a/pkg/ring/basic_lifecycler_delegates_test.go b/pkg/ring/basic_lifecycler_delegates_test.go index 5aa12e9abd..52f1d8d4ae 100644 --- a/pkg/ring/basic_lifecycler_delegates_test.go +++ b/pkg/ring/basic_lifecycler_delegates_test.go @@ -31,7 +31,7 @@ func TestLeaveOnStoppingDelegate(t *testing.T) { } leaveDelegate := NewLeaveOnStoppingDelegate(testDelegate, log.NewNopLogger()) - lifecycler, _, err := prepareBasicLifecyclerWithDelegate(cfg, leaveDelegate) + lifecycler, _, err := prepareBasicLifecyclerWithDelegate(t, cfg, leaveDelegate) require.NoError(t, err) require.NoError(t, services.StartAndAwaitRunning(ctx, lifecycler)) @@ -58,7 +58,7 @@ func TestTokensPersistencyDelegate_ShouldSkipTokensLoadingIfFileDoesNotExist(t * ctx := context.Background() cfg := prepareBasicLifecyclerConfig() - lifecycler, _, err := prepareBasicLifecyclerWithDelegate(cfg, persistencyDelegate) + lifecycler, _, err := prepareBasicLifecyclerWithDelegate(t, cfg, persistencyDelegate) require.NoError(t, err) defer services.StopAndAwaitTerminated(ctx, lifecycler) //nolint:errcheck @@ -102,7 +102,7 @@ func TestTokensPersistencyDelegate_ShouldLoadTokensFromFileIfFileExist(t *testin ctx := context.Background() cfg := prepareBasicLifecyclerConfig() - lifecycler, _, err := prepareBasicLifecyclerWithDelegate(cfg, persistencyDelegate) + lifecycler, _, err := prepareBasicLifecyclerWithDelegate(t, cfg, persistencyDelegate) require.NoError(t, err) require.NoError(t, services.StartAndAwaitRunning(ctx, lifecycler)) @@ -166,7 +166,7 @@ func TestTokensPersistencyDelegate_ShouldHandleTheCaseTheInstanceIsAlreadyInTheR ctx := context.Background() cfg := prepareBasicLifecyclerConfig() - lifecycler, store, err := prepareBasicLifecyclerWithDelegate(cfg, persistencyDelegate) + lifecycler, store, err := prepareBasicLifecyclerWithDelegate(t, cfg, persistencyDelegate) require.NoError(t, err) defer services.StopAndAwaitTerminated(ctx, lifecycler) //nolint:errcheck @@ -214,7 +214,7 @@ func TestDelegatesChain(t *testing.T) { ctx := context.Background() cfg := prepareBasicLifecyclerConfig() - lifecycler, _, err := prepareBasicLifecyclerWithDelegate(cfg, chain) + lifecycler, _, err := prepareBasicLifecyclerWithDelegate(t, cfg, chain) require.NoError(t, err) defer services.StopAndAwaitTerminated(ctx, lifecycler) //nolint:errcheck @@ -273,7 +273,7 @@ func TestAutoForgetDelegate(t *testing.T) { testDelegate := &mockDelegate{} autoForgetDelegate := NewAutoForgetDelegate(forgetPeriod, testDelegate, log.NewNopLogger()) - lifecycler, store, err := prepareBasicLifecyclerWithDelegate(cfg, autoForgetDelegate) + lifecycler, store, err := prepareBasicLifecyclerWithDelegate(t, cfg, autoForgetDelegate) require.NoError(t, err) // Setup the initial state of the ring. diff --git a/pkg/ring/basic_lifecycler_test.go b/pkg/ring/basic_lifecycler_test.go index d69c5be2de..1c1eeef6a8 100644 --- a/pkg/ring/basic_lifecycler_test.go +++ b/pkg/ring/basic_lifecycler_test.go @@ -6,13 +6,13 @@ import ( "time" "github.com/go-kit/kit/log" + "github.com/grafana/dskit/kv" + "github.com/grafana/dskit/kv/consul" "github.com/grafana/dskit/services" "github.com/prometheus/client_golang/prometheus/testutil" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/cortexproject/cortex/pkg/ring/kv" - "github.com/cortexproject/cortex/pkg/ring/kv/consul" "github.com/cortexproject/cortex/pkg/util/test" ) @@ -83,7 +83,7 @@ func TestBasicLifecycler_RegisterOnStart(t *testing.T) { t.Run(testName, func(t *testing.T) { ctx := context.Background() cfg := prepareBasicLifecyclerConfig() - lifecycler, delegate, store, err := prepareBasicLifecycler(cfg) + lifecycler, delegate, store, err := prepareBasicLifecycler(t, cfg) require.NoError(t, err) defer services.StopAndAwaitTerminated(ctx, lifecycler) //nolint:errcheck @@ -159,7 +159,7 @@ func TestBasicLifecycler_RegisterOnStart(t *testing.T) { func TestBasicLifecycler_UnregisterOnStop(t *testing.T) { ctx := context.Background() cfg := prepareBasicLifecyclerConfig() - lifecycler, delegate, store, err := prepareBasicLifecycler(cfg) + lifecycler, delegate, store, err := prepareBasicLifecycler(t, cfg) require.NoError(t, err) delegate.onRegister = func(_ *BasicLifecycler, _ Desc, _ bool, _ string, _ InstanceDesc) (InstanceState, Tokens) { @@ -195,7 +195,7 @@ func TestBasicLifecycler_HeartbeatWhileRunning(t *testing.T) { cfg := prepareBasicLifecyclerConfig() cfg.HeartbeatPeriod = 10 * time.Millisecond - lifecycler, _, store, err := prepareBasicLifecycler(cfg) + lifecycler, _, store, err := prepareBasicLifecycler(t, cfg) require.NoError(t, err) defer services.StopAndAwaitTerminated(ctx, lifecycler) //nolint:errcheck require.NoError(t, services.StartAndAwaitRunning(ctx, lifecycler)) @@ -219,7 +219,7 @@ func TestBasicLifecycler_HeartbeatWhileStopping(t *testing.T) { cfg := prepareBasicLifecyclerConfig() cfg.HeartbeatPeriod = 10 * time.Millisecond - lifecycler, delegate, store, err := prepareBasicLifecycler(cfg) + lifecycler, delegate, store, err := prepareBasicLifecycler(t, cfg) require.NoError(t, err) require.NoError(t, services.StartAndAwaitRunning(ctx, lifecycler)) @@ -257,7 +257,7 @@ func TestBasicLifecycler_HeartbeatAfterBackendRest(t *testing.T) { cfg := prepareBasicLifecyclerConfig() cfg.HeartbeatPeriod = 10 * time.Millisecond - lifecycler, delegate, store, err := prepareBasicLifecycler(cfg) + lifecycler, delegate, store, err := prepareBasicLifecycler(t, cfg) require.NoError(t, err) defer services.StopAndAwaitTerminated(ctx, lifecycler) //nolint:errcheck @@ -291,7 +291,7 @@ func TestBasicLifecycler_HeartbeatAfterBackendRest(t *testing.T) { func TestBasicLifecycler_ChangeState(t *testing.T) { ctx := context.Background() cfg := prepareBasicLifecyclerConfig() - lifecycler, delegate, store, err := prepareBasicLifecycler(cfg) + lifecycler, delegate, store, err := prepareBasicLifecycler(t, cfg) require.NoError(t, err) defer services.StopAndAwaitTerminated(ctx, lifecycler) //nolint:errcheck @@ -319,7 +319,7 @@ func TestBasicLifecycler_TokensObservePeriod(t *testing.T) { cfg.NumTokens = 5 cfg.TokensObservePeriod = time.Second - lifecycler, delegate, store, err := prepareBasicLifecycler(cfg) + lifecycler, delegate, store, err := prepareBasicLifecycler(t, cfg) require.NoError(t, err) delegate.onRegister = func(_ *BasicLifecycler, _ Desc, _ bool, _ string, _ InstanceDesc) (InstanceState, Tokens) { @@ -358,7 +358,7 @@ func TestBasicLifecycler_updateInstance_ShouldAddInstanceToTheRingIfDoesNotExist cfg := prepareBasicLifecyclerConfig() cfg.HeartbeatPeriod = time.Hour // No heartbeat during the test. - lifecycler, delegate, store, err := prepareBasicLifecycler(cfg) + lifecycler, delegate, store, err := prepareBasicLifecycler(t, cfg) require.NoError(t, err) defer services.StopAndAwaitTerminated(ctx, lifecycler) //nolint:errcheck @@ -403,14 +403,20 @@ func prepareBasicLifecyclerConfig() BasicLifecyclerConfig { } } -func prepareBasicLifecycler(cfg BasicLifecyclerConfig) (*BasicLifecycler, *mockDelegate, kv.Client, error) { +func prepareBasicLifecycler(t testing.TB, cfg BasicLifecyclerConfig) (*BasicLifecycler, *mockDelegate, kv.Client, error) { + t.Helper() + delegate := &mockDelegate{} - lifecycler, store, err := prepareBasicLifecyclerWithDelegate(cfg, delegate) + lifecycler, store, err := prepareBasicLifecyclerWithDelegate(t, cfg, delegate) return lifecycler, delegate, store, err } -func prepareBasicLifecyclerWithDelegate(cfg BasicLifecyclerConfig, delegate BasicLifecyclerDelegate) (*BasicLifecycler, kv.Client, error) { - store := consul.NewInMemoryClient(GetCodec()) +func prepareBasicLifecyclerWithDelegate(t testing.TB, cfg BasicLifecyclerConfig, delegate BasicLifecyclerDelegate) (*BasicLifecycler, kv.Client, error) { + t.Helper() + + store, closer := consul.NewInMemoryClient(GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + lifecycler, err := NewBasicLifecycler(cfg, testRingName, testRingKey, store, delegate, log.NewNopLogger(), nil) return lifecycler, store, err } diff --git a/pkg/ring/kv/client_test.go b/pkg/ring/kv/client_test.go deleted file mode 100644 index c0043efe3f..0000000000 --- a/pkg/ring/kv/client_test.go +++ /dev/null @@ -1,149 +0,0 @@ -package kv - -import ( - "context" - "testing" - "time" - - "github.com/gogo/protobuf/proto" - "github.com/prometheus/client_golang/prometheus" - "github.com/stretchr/testify/require" - "gopkg.in/yaml.v2" - - "github.com/cortexproject/cortex/pkg/ring/kv/codec" -) - -func TestParseConfig(t *testing.T) { - conf := ` -store: consul -consul: - host: "consul:8500" - consistentreads: true -prefix: "test/" -multi: - primary: consul - secondary: etcd -` - - cfg := Config{} - - err := yaml.Unmarshal([]byte(conf), &cfg) - require.NoError(t, err) - require.Equal(t, "consul", cfg.Store) - require.Equal(t, "test/", cfg.Prefix) - require.Equal(t, "consul:8500", cfg.Consul.Host) - require.Equal(t, "consul", cfg.Multi.Primary) - require.Equal(t, "etcd", cfg.Multi.Secondary) -} - -func Test_createClient_multiBackend_withSingleRing(t *testing.T) { - storeCfg, testCodec := newConfigsForTest() - require.NotPanics(t, func() { - _, err := createClient("multi", "/collector", storeCfg, testCodec, Primary, prometheus.NewRegistry()) - require.NoError(t, err) - }) -} - -func Test_createClient_multiBackend_withMultiRing(t *testing.T) { - storeCfg1, testCodec := newConfigsForTest() - storeCfg2 := StoreConfig{} - reg := prometheus.NewRegistry() - - require.NotPanics(t, func() { - _, err := createClient("multi", "/test", storeCfg1, testCodec, Primary, reg) - require.NoError(t, err) - }, "First client for KV store must not panic") - require.NotPanics(t, func() { - _, err := createClient("mock", "/test", storeCfg2, testCodec, Primary, reg) - require.NoError(t, err) - }, "Second client for KV store must not panic") -} - -func Test_createClient_singleBackend_mustContainRoleAndTypeLabels(t *testing.T) { - storeCfg, testCodec := newConfigsForTest() - reg := prometheus.NewRegistry() - client, err := createClient("mock", "/test1", storeCfg, testCodec, Primary, reg) - require.NoError(t, err) - require.NoError(t, client.CAS(context.Background(), "/test", func(_ interface{}) (out interface{}, retry bool, err error) { - out = &mockMessage{id: "inCAS"} - retry = false - return - })) - - actual := typeToRoleMap(t, reg) - require.Len(t, actual, 1) - require.Equal(t, "primary", actual["mock"]) -} - -func Test_createClient_multiBackend_mustContainRoleAndTypeLabels(t *testing.T) { - storeCfg, testCodec := newConfigsForTest() - storeCfg.Multi.MirrorEnabled = true - storeCfg.Multi.MirrorTimeout = 10 * time.Second - reg := prometheus.NewRegistry() - client, err := createClient("multi", "/test1", storeCfg, testCodec, Primary, reg) - require.NoError(t, err) - require.NoError(t, client.CAS(context.Background(), "/test", func(_ interface{}) (out interface{}, retry bool, err error) { - out = &mockMessage{id: "inCAS"} - retry = false - return - })) - - actual := typeToRoleMap(t, reg) - // expected multi-primary, inmemory-primary and mock-secondary - require.Len(t, actual, 3) - require.Equal(t, "primary", actual["multi"]) - require.Equal(t, "primary", actual["inmemory"]) - require.Equal(t, "secondary", actual["mock"]) - -} - -func typeToRoleMap(t *testing.T, reg prometheus.Gatherer) map[string]string { - mfs, err := reg.Gather() - require.NoError(t, err) - result := map[string]string{} - for _, mf := range mfs { - for _, m := range mf.GetMetric() { - backendType := "" - role := "" - for _, l := range m.GetLabel() { - if l.GetName() == "role" { - role = l.GetValue() - } else if l.GetName() == "type" { - backendType = l.GetValue() - } - } - require.NotEmpty(t, backendType) - require.NotEmpty(t, role) - result[backendType] = role - } - } - return result -} -func newConfigsForTest() (cfg StoreConfig, c codec.Codec) { - cfg = StoreConfig{ - Multi: MultiConfig{ - Primary: "inmemory", - Secondary: "mock", - }, - } - c = codec.NewProtoCodec("test", func() proto.Message { - return &mockMessage{id: "inCodec"} - }) - return -} - -type mockMessage struct { - id string -} - -func (m *mockMessage) Reset() { - panic("do not use") -} - -func (m *mockMessage) String() string { - panic("do not use") -} - -func (m *mockMessage) ProtoMessage() { - panic("do not use") -} diff --git a/pkg/ring/kv/consul/client_test.go b/pkg/ring/kv/consul/client_test.go deleted file mode 100644 index 135a87e0c3..0000000000 --- a/pkg/ring/kv/consul/client_test.go +++ /dev/null @@ -1,164 +0,0 @@ -package consul - -import ( - "context" - "fmt" - "strconv" - "testing" - "time" - - "github.com/go-kit/kit/log/level" - consul "github.com/hashicorp/consul/api" - "github.com/stretchr/testify/require" - - "github.com/cortexproject/cortex/pkg/ring/kv/codec" - util_log "github.com/cortexproject/cortex/pkg/util/log" -) - -func writeValuesToKV(client *Client, key string, start, end int, sleep time.Duration) <-chan struct{} { - ch := make(chan struct{}) - go func() { - defer close(ch) - for i := start; i <= end; i++ { - level.Debug(util_log.Logger).Log("ts", time.Now(), "msg", "writing value", "val", i) - _, _ = client.kv.Put(&consul.KVPair{Key: key, Value: []byte(fmt.Sprintf("%d", i))}, nil) - time.Sleep(sleep) - } - }() - return ch -} - -func TestWatchKeyWithRateLimit(t *testing.T) { - c := NewInMemoryClientWithConfig(codec.String{}, Config{ - WatchKeyRateLimit: 5.0, - WatchKeyBurstSize: 1, - }) - - const key = "test" - const max = 100 - - ch := writeValuesToKV(c, key, 0, max, 10*time.Millisecond) - - observed := observeValueForSomeTime(c, key, 1200*time.Millisecond) // little over 1 second - - // wait until updater finishes - <-ch - - if testing.Verbose() { - t.Log(observed) - } - // Let's see how many updates we have observed. Given the rate limit and our observing time, it should be 6 - // We should also have seen one of the later values, as we're observing for longer than a second, so rate limit should allow - // us to see it. - if len(observed) < 5 || len(observed) > 10 { - t.Error("Expected ~6 observed values, got", observed) - } - last := observed[len(observed)-1] - n, _ := strconv.Atoi(last) - if n < max/2 { - t.Error("Expected to see high last observed value, got", observed) - } -} - -func TestWatchKeyNoRateLimit(t *testing.T) { - c := NewInMemoryClientWithConfig(codec.String{}, Config{ - WatchKeyRateLimit: 0, - }) - - const key = "test" - const max = 100 - - ch := writeValuesToKV(c, key, 0, max, time.Millisecond) - observed := observeValueForSomeTime(c, key, 500*time.Millisecond) - - // wait until updater finishes - <-ch - - // With no limit, we should see most written values (we can lose some values if watching - // code is busy while multiple new values are written) - if len(observed) < 3*max/4 { - t.Error("Expected at least 3/4 of all values, got", observed) - } -} - -func TestReset(t *testing.T) { - c := NewInMemoryClient(codec.String{}) - - const key = "test" - const max = 5 - - ch := make(chan error) - go func() { - defer close(ch) - for i := 0; i <= max; i++ { - level.Debug(util_log.Logger).Log("ts", time.Now(), "msg", "writing value", "val", i) - _, _ = c.kv.Put(&consul.KVPair{Key: key, Value: []byte(fmt.Sprintf("%d", i))}, nil) - if i == 1 { - c.kv.(*mockKV).ResetIndex() - } - if i == 2 { - c.kv.(*mockKV).ResetIndexForKey(key) - } - time.Sleep(10 * time.Millisecond) - } - }() - - observed := observeValueForSomeTime(c, key, 25*max*time.Millisecond) - - // wait until updater finishes - <-ch - - // Let's see how many updates we have observed. Given the rate limit and our observing time, we should see all numeric values - if testing.Verbose() { - t.Log(observed) - } - if len(observed) < max { - t.Error("Expected all values, got", observed) - } else if observed[len(observed)-1] != fmt.Sprintf("%d", max) { - t.Error("Expected to see last written value, got", observed) - } -} - -func observeValueForSomeTime(client *Client, key string, timeout time.Duration) []string { - observed := []string(nil) - ctx, cancel := context.WithTimeout(context.Background(), timeout) - defer cancel() - client.WatchKey(ctx, key, func(i interface{}) bool { - s, ok := i.(string) - if !ok { - return false - } - level.Debug(util_log.Logger).Log("ts", time.Now(), "msg", "observed value", "val", s) - observed = append(observed, s) - return true - }) - return observed -} - -func TestWatchKeyWithNoStartValue(t *testing.T) { - c := NewInMemoryClient(codec.String{}) - - const key = "test" - - go func() { - time.Sleep(100 * time.Millisecond) - _, err := c.kv.Put(&consul.KVPair{Key: key, Value: []byte("start")}, nil) - require.NoError(t, err) - - time.Sleep(100 * time.Millisecond) - _, err = c.kv.Put(&consul.KVPair{Key: key, Value: []byte("end")}, nil) - require.NoError(t, err) - }() - - ctx, fn := context.WithTimeout(context.Background(), 300*time.Millisecond) - defer fn() - - reported := 0 - c.WatchKey(ctx, key, func(i interface{}) bool { - reported++ - return reported != 2 - }) - - // we should see both start and end values. - require.Equal(t, 2, reported) -} diff --git a/pkg/ring/kv/kv_test.go b/pkg/ring/kv/kv_test.go deleted file mode 100644 index 67720d5417..0000000000 --- a/pkg/ring/kv/kv_test.go +++ /dev/null @@ -1,276 +0,0 @@ -package kv - -import ( - "context" - "fmt" - "io" - "sort" - "strconv" - "sync" - "testing" - "time" - - "github.com/stretchr/testify/require" - - "github.com/cortexproject/cortex/pkg/ring/kv/codec" - "github.com/cortexproject/cortex/pkg/ring/kv/consul" - "github.com/cortexproject/cortex/pkg/ring/kv/etcd" -) - -func withFixtures(t *testing.T, f func(*testing.T, Client)) { - for _, fixture := range []struct { - name string - factory func() (Client, io.Closer, error) - }{ - {"consul", func() (Client, io.Closer, error) { - return consul.NewInMemoryClient(codec.String{}), etcd.NopCloser, nil - }}, - {"etcd", func() (Client, io.Closer, error) { - return etcd.Mock(codec.String{}) - }}, - } { - t.Run(fixture.name, func(t *testing.T) { - client, closer, err := fixture.factory() - require.NoError(t, err) - defer closer.Close() - f(t, client) - }) - } -} - -var ( - ctx = context.Background() - key = "/key" -) - -func TestCAS(t *testing.T) { - withFixtures(t, func(t *testing.T, client Client) { - // Blindly set key to "0". - err := client.CAS(ctx, key, func(in interface{}) (interface{}, bool, error) { - return "0", true, nil - }) - require.NoError(t, err) - - // Swap key to i+1 iff its i. - for i := 0; i < 10; i++ { - err = client.CAS(ctx, key, func(in interface{}) (interface{}, bool, error) { - require.EqualValues(t, strconv.Itoa(i), in) - return strconv.Itoa(i + 1), true, nil - }) - require.NoError(t, err) - } - - // Make sure the CASes left the right value - "10". - value, err := client.Get(ctx, key) - require.NoError(t, err) - require.EqualValues(t, "10", value) - }) -} - -// TestNilCAS ensures we can return nil from the CAS callback when we don't -// want to modify the value. -func TestNilCAS(t *testing.T) { - withFixtures(t, func(t *testing.T, client Client) { - // Blindly set key to "0". - err := client.CAS(ctx, key, func(in interface{}) (interface{}, bool, error) { - return "0", true, nil - }) - require.NoError(t, err) - - // Ensure key is "0" and don't set it. - err = client.CAS(ctx, key, func(in interface{}) (interface{}, bool, error) { - require.EqualValues(t, "0", in) - return nil, false, nil - }) - require.NoError(t, err) - - // Make sure value is still 0. - value, err := client.Get(ctx, key) - require.NoError(t, err) - require.EqualValues(t, "0", value) - }) -} - -func TestWatchKey(t *testing.T) { - const key = "test" - const max = 100 - const sleep = 15 * time.Millisecond - const totalTestTimeout = 3 * max * sleep - const expectedFactor = 0.75 // we may not see every single value - - withFixtures(t, func(t *testing.T, client Client) { - observedValuesCh := make(chan string, max) - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - go func() { - // Start watching before we even start generating values. - // Values will be buffered in the channel. - client.WatchKey(ctx, key, func(value interface{}) bool { - observedValuesCh <- value.(string) - return true - }) - }() - - // update value for the key - go func() { - for i := 0; i < max; i++ { - // Start with sleeping, so that watching client see empty KV store at the beginning. - time.Sleep(sleep) - - err := client.CAS(ctx, key, func(in interface{}) (out interface{}, retry bool, err error) { - return fmt.Sprintf("%d", i), true, nil - }) - - if ctx.Err() != nil { - break - } - require.NoError(t, err) - } - }() - - lastObservedValue := -1 - observedCount := 0 - - totalDeadline := time.After(totalTestTimeout) - - for watching := true; watching; { - select { - case <-totalDeadline: - watching = false - case valStr := <-observedValuesCh: - val, err := strconv.Atoi(valStr) - if err != nil { - t.Fatal("Unexpected value observed:", valStr) - } - - if val <= lastObservedValue { - t.Fatal("Unexpected value observed:", val, "previous:", lastObservedValue) - } - lastObservedValue = val - observedCount++ - - if observedCount >= expectedFactor*max { - watching = false - } - } - } - - if observedCount < expectedFactor*max { - t.Errorf("expected at least %.0f%% observed values, got %.0f%% (observed count: %d)", 100*expectedFactor, 100*float64(observedCount)/max, observedCount) - } - }) -} - -func TestWatchPrefix(t *testing.T) { - withFixtures(t, func(t *testing.T, client Client) { - const prefix = "test/" - const prefix2 = "ignore/" - - // We are going to generate this number of updates, sleeping between each update. - const max = 100 - const sleep = time.Millisecond * 10 - // etcd seems to be quite slow. If we finish faster, test will end sooner. - // (We regularly see generators taking up to 5 seconds to produce all messages on some platforms!) - const totalTestTimeout = 10 * time.Second - - observedKeysCh := make(chan string, max) - - ctx, cancel := context.WithCancel(context.Background()) - defer cancel() - - wg := sync.WaitGroup{} - - wg.Add(1) - go func() { - defer wg.Done() - - // start watching before we even start generating values. values will be buffered - client.WatchPrefix(ctx, prefix, func(key string, val interface{}) bool { - observedKeysCh <- key - return true - }) - }() - - gen := func(p string) { - defer wg.Done() - - start := time.Now() - for i := 0; i < max && ctx.Err() == nil; i++ { - // Start with sleeping, so that watching client can see empty KV store at the beginning. - time.Sleep(sleep) - - key := fmt.Sprintf("%s%d", p, i) - err := client.CAS(ctx, key, func(in interface{}) (out interface{}, retry bool, err error) { - return key, true, nil - }) - - if ctx.Err() != nil { - break - } - require.NoError(t, err) - } - t.Log("Generator finished in", time.Since(start)) - } - - wg.Add(2) - go gen(prefix) - go gen(prefix2) // we don't want to see these keys reported - - observedKeys := map[string]int{} - - totalDeadline := time.After(totalTestTimeout) - - start := time.Now() - for watching := true; watching; { - select { - case <-totalDeadline: - watching = false - case key := <-observedKeysCh: - observedKeys[key]++ - if len(observedKeys) == max { - watching = false - } - } - } - t.Log("Watching finished in", time.Since(start)) - - // Stop all goroutines and wait until terminated. - cancel() - wg.Wait() - - // verify that each key was reported once, and keys outside prefix were not reported - for i := 0; i < max; i++ { - key := fmt.Sprintf("%s%d", prefix, i) - - if observedKeys[key] != 1 { - t.Errorf("key %s has incorrect value %d", key, observedKeys[key]) - } - delete(observedKeys, key) - } - - if len(observedKeys) > 0 { - t.Errorf("unexpected keys reported: %v", observedKeys) - } - }) -} - -// TestList makes sure stored keys are listed back. -func TestList(t *testing.T) { - keysToCreate := []string{"a", "b", "c"} - - withFixtures(t, func(t *testing.T, client Client) { - for _, key := range keysToCreate { - err := client.CAS(context.Background(), key, func(in interface{}) (out interface{}, retry bool, err error) { - return key, false, nil - }) - require.NoError(t, err) - } - - storedKeys, err := client.List(context.Background(), "") - require.NoError(t, err) - sort.Strings(storedKeys) - - require.Equal(t, keysToCreate, storedKeys) - }) -} diff --git a/pkg/ring/kv/memberlist/broadcast_test.go b/pkg/ring/kv/memberlist/broadcast_test.go deleted file mode 100644 index 0ef681093c..0000000000 --- a/pkg/ring/kv/memberlist/broadcast_test.go +++ /dev/null @@ -1,52 +0,0 @@ -package memberlist - -import "testing" - -func TestInvalidates(t *testing.T) { - const key = "ring" - - messages := map[string]ringBroadcast{ - "b1": {key: key, content: []string{"A", "B", "C"}, version: 1}, - "b2": {key: key, content: []string{"A", "B", "C"}, version: 2}, - "b3": {key: key, content: []string{"A"}, version: 3}, - "b4": {key: key, content: []string{"A", "B"}, version: 4}, - "b5": {key: key, content: []string{"A", "B", "D"}, version: 5}, - "b6": {key: key, content: []string{"A", "B", "C", "D"}, version: 6}, - } - - checkInvalidate(t, messages, "b2", "b1", true, false) - checkInvalidate(t, messages, "b3", "b1", false, false) - checkInvalidate(t, messages, "b3", "b2", false, false) - checkInvalidate(t, messages, "b4", "b1", false, false) - checkInvalidate(t, messages, "b4", "b2", false, false) - checkInvalidate(t, messages, "b4", "b3", true, false) - checkInvalidate(t, messages, "b5", "b1", false, false) - checkInvalidate(t, messages, "b5", "b2", false, false) - checkInvalidate(t, messages, "b5", "b3", true, false) - checkInvalidate(t, messages, "b5", "b4", true, false) - checkInvalidate(t, messages, "b6", "b1", true, false) - checkInvalidate(t, messages, "b6", "b2", true, false) - checkInvalidate(t, messages, "b6", "b3", true, false) - checkInvalidate(t, messages, "b6", "b4", true, false) - checkInvalidate(t, messages, "b6", "b5", true, false) -} - -func checkInvalidate(t *testing.T, messages map[string]ringBroadcast, key1, key2 string, firstInvalidatesSecond, secondInvalidatesFirst bool) { - b1, ok := messages[key1] - if !ok { - t.Fatal("cannot find", key1) - } - - b2, ok := messages[key2] - if !ok { - t.Fatal("cannot find", key2) - } - - if b1.Invalidates(b2) != firstInvalidatesSecond { - t.Errorf("%s.Invalidates(%s) returned %t. %s={%v, %d}, %s={%v, %d}", key1, key2, !firstInvalidatesSecond, key1, b1.content, b1.version, key2, b2.content, b2.version) - } - - if b2.Invalidates(b1) != secondInvalidatesFirst { - t.Errorf("%s.Invalidates(%s) returned %t. %s={%v, %d}, %s={%v, %d}", key2, key1, !secondInvalidatesFirst, key2, b2.content, b2.version, key1, b1.content, b1.version) - } -} diff --git a/pkg/ring/kv/memberlist/kv_init_service_test.go b/pkg/ring/kv/memberlist/kv_init_service_test.go deleted file mode 100644 index 2a01d93a94..0000000000 --- a/pkg/ring/kv/memberlist/kv_init_service_test.go +++ /dev/null @@ -1,60 +0,0 @@ -package memberlist - -import ( - "bytes" - "testing" - "time" - - "github.com/grafana/dskit/flagext" - "github.com/hashicorp/memberlist" - "github.com/stretchr/testify/require" -) - -func TestPage(t *testing.T) { - conf := memberlist.DefaultLANConfig() - ml, err := memberlist.Create(conf) - require.NoError(t, err) - - t.Cleanup(func() { - _ = ml.Shutdown() - }) - - require.NoError(t, pageTemplate.Execute(&bytes.Buffer{}, pageData{ - Now: time.Now(), - Memberlist: ml, - SortedMembers: ml.Members(), - Store: nil, - ReceivedMessages: []message{{ - ID: 10, - Time: time.Now(), - Size: 50, - Pair: KeyValuePair{ - Key: "hello", - Value: []byte("world"), - Codec: "codec", - }, - Version: 20, - Changes: []string{"A", "B", "C"}, - }}, - - SentMessages: []message{{ - ID: 10, - Time: time.Now(), - Size: 50, - Pair: KeyValuePair{ - Key: "hello", - Value: []byte("world"), - Codec: "codec", - }, - Version: 20, - Changes: []string{"A", "B", "C"}, - }}, - })) -} - -func TestStop(t *testing.T) { - var cfg KVConfig - flagext.DefaultValues(&cfg) - kvinit := NewKVInitService(&cfg, nil) - require.NoError(t, kvinit.stopping(nil)) -} diff --git a/pkg/ring/kv/memberlist/memberlist_client_test.go b/pkg/ring/kv/memberlist/memberlist_client_test.go deleted file mode 100644 index 7a88197d27..0000000000 --- a/pkg/ring/kv/memberlist/memberlist_client_test.go +++ /dev/null @@ -1,1270 +0,0 @@ -package memberlist - -import ( - "bytes" - "context" - "encoding/gob" - "errors" - "fmt" - "math" - "math/rand" - "net" - "sort" - "sync" - "testing" - "time" - - "github.com/go-kit/kit/log" - "github.com/grafana/dskit/flagext" - "github.com/grafana/dskit/services" - "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" - - "github.com/cortexproject/cortex/pkg/ring/kv/codec" - "github.com/cortexproject/cortex/pkg/util/test" -) - -const ACTIVE = 1 -const JOINING = 2 -const LEFT = 3 - -// Simple mergeable data structure, used for gossiping -type member struct { - Timestamp int64 - Tokens []uint32 - State int -} - -type data struct { - Members map[string]member -} - -func (d *data) Merge(mergeable Mergeable, localCAS bool) (Mergeable, error) { - if mergeable == nil { - return nil, nil - } - - od, ok := mergeable.(*data) - if !ok || od == nil { - return nil, fmt.Errorf("invalid thing to merge: %T", od) - } - - updated := map[string]member{} - - for k, v := range od.Members { - if v.Timestamp > d.Members[k].Timestamp { - d.Members[k] = v - updated[k] = v - } - } - - if localCAS { - for k, v := range d.Members { - if _, ok := od.Members[k]; !ok && v.State != LEFT { - v.State = LEFT - v.Tokens = nil - d.Members[k] = v - updated[k] = v - } - } - } - - if len(updated) == 0 { - return nil, nil - } - return &data{Members: updated}, nil -} - -func (d *data) MergeContent() []string { - // return list of keys - out := []string(nil) - for k := range d.Members { - out = append(out, k) - } - return out -} - -// This method deliberately ignores zero limit, so that tests can observe LEFT state as well. -func (d *data) RemoveTombstones(limit time.Time) (total, removed int) { - for n, m := range d.Members { - if m.State == LEFT { - if time.Unix(m.Timestamp, 0).Before(limit) { - // remove it - delete(d.Members, n) - removed++ - } else { - total++ - } - } - } - return -} - -func (m member) clone() member { - out := member{ - Timestamp: m.Timestamp, - Tokens: make([]uint32, len(m.Tokens)), - State: m.State, - } - copy(out.Tokens, m.Tokens) - return out -} - -func (d *data) Clone() Mergeable { - out := &data{ - Members: make(map[string]member, len(d.Members)), - } - for k, v := range d.Members { - out.Members[k] = v.clone() - } - return out -} - -func (d *data) getAllTokens() []uint32 { - out := []uint32(nil) - for _, m := range d.Members { - out = append(out, m.Tokens...) - } - - sort.Sort(sortableUint32(out)) - return out -} - -type dataCodec struct{} - -func (d dataCodec) CodecID() string { - return "testDataCodec" -} - -func (d dataCodec) Decode(b []byte) (interface{}, error) { - dec := gob.NewDecoder(bytes.NewBuffer(b)) - out := &data{} - err := dec.Decode(out) - return out, err -} - -func (d dataCodec) Encode(val interface{}) ([]byte, error) { - buf := bytes.Buffer{} - enc := gob.NewEncoder(&buf) - err := enc.Encode(val) - return buf.Bytes(), err -} - -var _ codec.Codec = &dataCodec{} - -type sortableUint32 []uint32 - -func (ts sortableUint32) Len() int { return len(ts) } -func (ts sortableUint32) Swap(i, j int) { ts[i], ts[j] = ts[j], ts[i] } -func (ts sortableUint32) Less(i, j int) bool { return ts[i] < ts[j] } - -const key = "test" - -func updateFn(name string) func(*data) (*data, bool, error) { - return func(in *data) (out *data, retry bool, err error) { - // Modify value that was passed as a parameter. - // Client takes care of concurrent modifications. - var r *data = in - if r == nil { - r = &data{Members: map[string]member{}} - } - - m, ok := r.Members[name] - if !ok { - r.Members[name] = member{ - Timestamp: time.Now().Unix(), - Tokens: generateTokens(128), - State: JOINING, - } - } else { - // We need to update timestamp, otherwise CAS will fail - m.Timestamp = time.Now().Unix() - m.State = ACTIVE - r.Members[name] = m - } - - return r, true, nil - } -} - -func get(t *testing.T, kv *Client, key string) interface{} { - val, err := kv.Get(context.Background(), key) - if err != nil { - t.Fatalf("Failed to get value for key %s: %v", key, err) - } - return val -} - -func getData(t *testing.T, kv *Client, key string) *data { - t.Helper() - val := get(t, kv, key) - if val == nil { - return nil - } - if r, ok := val.(*data); ok { - return r - } - t.Fatalf("Expected ring, got: %T", val) - return nil -} - -func cas(t *testing.T, kv *Client, key string, updateFn func(*data) (*data, bool, error)) { - t.Helper() - - if err := casWithErr(context.Background(), t, kv, key, updateFn); err != nil { - t.Fatal(err) - } -} - -func casWithErr(ctx context.Context, t *testing.T, kv *Client, key string, updateFn func(*data) (*data, bool, error)) error { - t.Helper() - fn := func(in interface{}) (out interface{}, retry bool, err error) { - var r *data = nil - if in != nil { - r = in.(*data) - } - - d, rt, e := updateFn(r) - if d == nil { - // translate nil pointer to nil interface value - return nil, rt, e - } - return d, rt, e - } - - return kv.CAS(ctx, key, fn) -} - -func TestBasicGetAndCas(t *testing.T) { - c := dataCodec{} - - name := "Ing 1" - var cfg KVConfig - flagext.DefaultValues(&cfg) - cfg.TCPTransport = TCPTransportConfig{ - BindAddrs: []string{"localhost"}, - } - cfg.Codecs = []codec.Codec{c} - - mkv := NewKV(cfg, log.NewNopLogger()) - require.NoError(t, services.StartAndAwaitRunning(context.Background(), mkv)) - defer services.StopAndAwaitTerminated(context.Background(), mkv) //nolint:errcheck - - kv, err := NewClient(mkv, c) - require.NoError(t, err) - - const key = "test" - - val := get(t, kv, key) - if val != nil { - t.Error("Expected nil, got:", val) - } - - // Create member in PENDING state, with some tokens - cas(t, kv, key, updateFn(name)) - - r := getData(t, kv, key) - if r == nil || r.Members[name].Timestamp == 0 || len(r.Members[name].Tokens) <= 0 { - t.Fatalf("Expected ring with tokens, got %v", r) - } - - val = get(t, kv, "other key") - if val != nil { - t.Errorf("Expected nil, got: %v", val) - } - - // Update member into ACTIVE state - cas(t, kv, key, updateFn(name)) - r = getData(t, kv, key) - if r.Members[name].State != ACTIVE { - t.Errorf("Expected member to be active after second update, got %v", r) - } - - // Delete member - cas(t, kv, key, func(r *data) (*data, bool, error) { - delete(r.Members, name) - return r, true, nil - }) - - r = getData(t, kv, key) - if r.Members[name].State != LEFT { - t.Errorf("Expected member to be LEFT, got %v", r) - } -} - -func withFixtures(t *testing.T, testFN func(t *testing.T, kv *Client)) { - t.Helper() - - c := dataCodec{} - - var cfg KVConfig - flagext.DefaultValues(&cfg) - cfg.TCPTransport = TCPTransportConfig{} - cfg.Codecs = []codec.Codec{c} - - mkv := NewKV(cfg, log.NewNopLogger()) - require.NoError(t, services.StartAndAwaitRunning(context.Background(), mkv)) - defer services.StopAndAwaitTerminated(context.Background(), mkv) //nolint:errcheck - - kv, err := NewClient(mkv, c) - require.NoError(t, err) - - testFN(t, kv) -} - -func TestCASNoOutput(t *testing.T) { - withFixtures(t, func(t *testing.T, kv *Client) { - // should succeed with single call - calls := 0 - cas(t, kv, key, func(d *data) (*data, bool, error) { - calls++ - return nil, true, nil - }) - - require.Equal(t, 1, calls) - }) -} - -func TestCASErrorNoRetry(t *testing.T) { - withFixtures(t, func(t *testing.T, kv *Client) { - calls := 0 - err := casWithErr(context.Background(), t, kv, key, func(d *data) (*data, bool, error) { - calls++ - return nil, false, errors.New("don't worry, be happy") - }) - require.EqualError(t, err, "failed to CAS-update key test: fn returned error: don't worry, be happy") - require.Equal(t, 1, calls) - }) -} - -func TestCASErrorWithRetries(t *testing.T) { - withFixtures(t, func(t *testing.T, kv *Client) { - calls := 0 - err := casWithErr(context.Background(), t, kv, key, func(d *data) (*data, bool, error) { - calls++ - return nil, true, errors.New("don't worry, be happy") - }) - require.EqualError(t, err, "failed to CAS-update key test: fn returned error: don't worry, be happy") - require.Equal(t, 10, calls) // hard-coded in CAS function. - }) -} - -func TestCASNoChange(t *testing.T) { - withFixtures(t, func(t *testing.T, kv *Client) { - cas(t, kv, key, func(in *data) (*data, bool, error) { - if in == nil { - in = &data{Members: map[string]member{}} - } - - in.Members["hello"] = member{ - Timestamp: time.Now().Unix(), - Tokens: generateTokens(128), - State: JOINING, - } - - return in, true, nil - }) - - startTime := time.Now() - calls := 0 - err := casWithErr(context.Background(), t, kv, key, func(d *data) (*data, bool, error) { - calls++ - return d, true, nil - }) - require.EqualError(t, err, "failed to CAS-update key test: no change detected") - require.Equal(t, maxCasRetries, calls) - // if there was no change, CAS sleeps before every retry - require.True(t, time.Since(startTime) >= (maxCasRetries-1)*noChangeDetectedRetrySleep) - }) -} - -func TestCASNoChangeShortTimeout(t *testing.T) { - withFixtures(t, func(t *testing.T, kv *Client) { - cas(t, kv, key, func(in *data) (*data, bool, error) { - if in == nil { - in = &data{Members: map[string]member{}} - } - - in.Members["hello"] = member{ - Timestamp: time.Now().Unix(), - Tokens: generateTokens(128), - State: JOINING, - } - - return in, true, nil - }) - - ctx, cancel := context.WithTimeout(context.Background(), 500*time.Millisecond) - defer cancel() - - calls := 0 - err := casWithErr(ctx, t, kv, key, func(d *data) (*data, bool, error) { - calls++ - return d, true, nil - }) - require.EqualError(t, err, "failed to CAS-update key test: context deadline exceeded") - require.Equal(t, 1, calls) // hard-coded in CAS function. - }) -} - -func TestCASFailedBecauseOfVersionChanges(t *testing.T) { - withFixtures(t, func(t *testing.T, kv *Client) { - cas(t, kv, key, func(in *data) (*data, bool, error) { - return &data{Members: map[string]member{"nonempty": {Timestamp: time.Now().Unix()}}}, true, nil - }) - - calls := 0 - // outer cas - err := casWithErr(context.Background(), t, kv, key, func(d *data) (*data, bool, error) { - // outer CAS logic - calls++ - - // run inner-CAS that succeeds, and that will make outer cas to fail - cas(t, kv, key, func(d *data) (*data, bool, error) { - // to avoid delays due to merging, we update different ingester each time. - d.Members[fmt.Sprintf("%d", calls)] = member{ - Timestamp: time.Now().Unix(), - } - return d, true, nil - }) - - d.Members["world"] = member{ - Timestamp: time.Now().Unix(), - } - return d, true, nil - }) - - require.EqualError(t, err, "failed to CAS-update key test: too many retries") - require.Equal(t, maxCasRetries, calls) - }) -} - -func TestMultipleCAS(t *testing.T) { - c := dataCodec{} - - var cfg KVConfig - flagext.DefaultValues(&cfg) - cfg.Codecs = []codec.Codec{c} - - mkv := NewKV(cfg, log.NewNopLogger()) - mkv.maxCasRetries = 20 - require.NoError(t, services.StartAndAwaitRunning(context.Background(), mkv)) - defer services.StopAndAwaitTerminated(context.Background(), mkv) //nolint:errcheck - - kv, err := NewClient(mkv, c) - require.NoError(t, err) - - wg := &sync.WaitGroup{} - start := make(chan struct{}) - - const members = 10 - const namePattern = "Member-%d" - - for i := 0; i < members; i++ { - wg.Add(1) - go func(name string) { - defer wg.Done() - <-start - up := updateFn(name) - cas(t, kv, "test", up) // JOINING state - cas(t, kv, "test", up) // ACTIVE state - }(fmt.Sprintf(namePattern, i)) - } - - close(start) // start all CAS updates - wg.Wait() // wait until all CAS updates are finished - - // Now let's test that all members are in ACTIVE state - r := getData(t, kv, "test") - require.True(t, r != nil, "nil ring") - - for i := 0; i < members; i++ { - n := fmt.Sprintf(namePattern, i) - - if r.Members[n].State != ACTIVE { - t.Errorf("Expected member %s to be ACTIVE got %v", n, r.Members[n].State) - } - } - - // Make all members leave - start = make(chan struct{}) - - for i := 0; i < members; i++ { - wg.Add(1) - go func(name string) { - defer wg.Done() - - <-start - up := func(in *data) (out *data, retry bool, err error) { - delete(in.Members, name) - return in, true, nil - } - cas(t, kv, "test", up) // PENDING state - }(fmt.Sprintf(namePattern, i)) - } - - close(start) // start all CAS updates - wg.Wait() // wait until all CAS updates are finished - - r = getData(t, kv, "test") - require.True(t, r != nil, "nil ring") - - for i := 0; i < members; i++ { - n := fmt.Sprintf(namePattern, i) - - if r.Members[n].State != LEFT { - t.Errorf("Expected member %s to be ACTIVE got %v", n, r.Members[n].State) - } - } -} - -func TestMultipleClients(t *testing.T) { - c := dataCodec{} - - const members = 10 - const key = "ring" - - var clients []*Client - - stop := make(chan struct{}) - start := make(chan struct{}) - - port := 0 - - for i := 0; i < members; i++ { - id := fmt.Sprintf("Member-%d", i) - var cfg KVConfig - flagext.DefaultValues(&cfg) - cfg.NodeName = id - - cfg.GossipInterval = 100 * time.Millisecond - cfg.GossipNodes = 3 - cfg.PushPullInterval = 5 * time.Second - - cfg.TCPTransport = TCPTransportConfig{ - BindAddrs: []string{"localhost"}, - BindPort: 0, // randomize ports - } - - cfg.Codecs = []codec.Codec{c} - - mkv := NewKV(cfg, log.NewNopLogger()) - require.NoError(t, services.StartAndAwaitRunning(context.Background(), mkv)) - - kv, err := NewClient(mkv, c) - require.NoError(t, err) - - clients = append(clients, kv) - - go runClient(t, kv, id, key, port, start, stop) - - // next KV will connect to this one - port = kv.kv.GetListeningPort() - } - - println("Waiting before start") - time.Sleep(2 * time.Second) - close(start) - - println("Observing ring ...") - - startTime := time.Now() - firstKv := clients[0] - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - updates := 0 - firstKv.WatchKey(ctx, key, func(in interface{}) bool { - updates++ - - r := in.(*data) - - minTimestamp, maxTimestamp, avgTimestamp := getTimestamps(r.Members) - - now := time.Now() - t.Log("Update", now.Sub(startTime).String(), ": Ring has", len(r.Members), "members, and", len(r.getAllTokens()), - "tokens, oldest timestamp:", now.Sub(time.Unix(minTimestamp, 0)).String(), - "avg timestamp:", now.Sub(time.Unix(avgTimestamp, 0)).String(), - "youngest timestamp:", now.Sub(time.Unix(maxTimestamp, 0)).String()) - return true // yes, keep watching - }) - cancel() // make linter happy - - t.Logf("Ring updates observed: %d", updates) - - if updates < members { - // in general, at least one update from each node. (although that's not necessarily true... - // but typically we get more updates than that anyway) - t.Errorf("expected to see updates, got %d", updates) - } - - // Let's check all the clients to see if they have relatively up-to-date information - // All of them should at least have all the clients - // And same tokens. - allTokens := []uint32(nil) - - for i := 0; i < members; i++ { - kv := clients[i] - - r := getData(t, kv, key) - t.Logf("KV %d: number of known members: %d\n", i, len(r.Members)) - if len(r.Members) != members { - t.Errorf("Member %d has only %d members in the ring", i, len(r.Members)) - } - - minTimestamp, maxTimestamp, avgTimestamp := getTimestamps(r.Members) - for n, ing := range r.Members { - if ing.State != ACTIVE { - t.Errorf("Member %d: invalid state of member %s in the ring: %v ", i, n, ing.State) - } - } - now := time.Now() - t.Logf("Member %d: oldest: %v, avg: %v, youngest: %v", i, - now.Sub(time.Unix(minTimestamp, 0)).String(), - now.Sub(time.Unix(avgTimestamp, 0)).String(), - now.Sub(time.Unix(maxTimestamp, 0)).String()) - - tokens := r.getAllTokens() - if allTokens == nil { - allTokens = tokens - t.Logf("Found tokens: %d", len(allTokens)) - } else { - if len(allTokens) != len(tokens) { - t.Errorf("Member %d: Expected %d tokens, got %d", i, len(allTokens), len(tokens)) - } else { - for ix, tok := range allTokens { - if tok != tokens[ix] { - t.Errorf("Member %d: Tokens at position %d differ: %v, %v", i, ix, tok, tokens[ix]) - break - } - } - } - } - } - - // We cannot shutdown the KV until now in order for Get() to work reliably. - close(stop) -} - -func TestJoinMembersWithRetryBackoff(t *testing.T) { - c := dataCodec{} - - const members = 3 - const key = "ring" - - var clients []*Client - - stop := make(chan struct{}) - start := make(chan struct{}) - - ports, err := getFreePorts(members) - require.NoError(t, err) - - watcher := services.NewFailureWatcher() - go func() { - for { - select { - case err := <-watcher.Chan(): - t.Errorf("service reported error: %v", err) - case <-stop: - return - } - } - }() - - for i, port := range ports { - id := fmt.Sprintf("Member-%d", i) - var cfg KVConfig - flagext.DefaultValues(&cfg) - cfg.NodeName = id - - cfg.GossipInterval = 100 * time.Millisecond - cfg.GossipNodes = 3 - cfg.PushPullInterval = 5 * time.Second - - cfg.MinJoinBackoff = 100 * time.Millisecond - cfg.MaxJoinBackoff = 1 * time.Minute - cfg.MaxJoinRetries = 10 - cfg.AbortIfJoinFails = true - - cfg.TCPTransport = TCPTransportConfig{ - BindAddrs: []string{"localhost"}, - BindPort: port, - } - - cfg.Codecs = []codec.Codec{c} - - if i == 0 { - // Add members to first KV config to join immediately on initialization. - // This will enforce backoff as each next members listener is not open yet. - cfg.JoinMembers = []string{fmt.Sprintf("localhost:%d", ports[1])} - } else { - // Add delay to each next member to force backoff - time.Sleep(1 * time.Second) - } - - mkv := NewKV(cfg, log.NewNopLogger()) // Not started yet. - watcher.WatchService(mkv) - - kv, err := NewClient(mkv, c) - require.NoError(t, err) - - clients = append(clients, kv) - - startKVAndRunClient := func(kv *Client, id string, port int) { - err = services.StartAndAwaitRunning(context.Background(), mkv) - if err != nil { - t.Errorf("failed to start KV: %v", err) - } - runClient(t, kv, id, key, port, start, stop) - } - - if i == 0 { - go startKVAndRunClient(kv, id, 0) - } else { - go startKVAndRunClient(kv, id, ports[i-1]) - } - } - - t.Log("Waiting all members to join memberlist cluster") - close(start) - time.Sleep(2 * time.Second) - - t.Log("Observing ring ...") - - startTime := time.Now() - firstKv := clients[0] - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) - observedMembers := 0 - firstKv.WatchKey(ctx, key, func(in interface{}) bool { - r := in.(*data) - observedMembers = len(r.Members) - - now := time.Now() - t.Log("Update", now.Sub(startTime).String(), ": Ring has", len(r.Members), "members, and", len(r.getAllTokens()), - "tokens") - return true // yes, keep watching - }) - cancel() // make linter happy - - // Let clients exchange messages for a while - close(stop) - - if observedMembers < members { - t.Errorf("expected to see %d but saw %d", members, observedMembers) - } -} - -func TestMemberlistFailsToJoin(t *testing.T) { - c := dataCodec{} - - ports, err := getFreePorts(1) - require.NoError(t, err) - - var cfg KVConfig - flagext.DefaultValues(&cfg) - cfg.MinJoinBackoff = 100 * time.Millisecond - cfg.MaxJoinBackoff = 100 * time.Millisecond - cfg.MaxJoinRetries = 2 - cfg.AbortIfJoinFails = true - - cfg.TCPTransport = TCPTransportConfig{ - BindAddrs: []string{"localhost"}, - BindPort: 0, - } - - cfg.JoinMembers = []string{fmt.Sprintf("127.0.0.1:%d", ports[0])} - - cfg.Codecs = []codec.Codec{c} - - mkv := NewKV(cfg, log.NewNopLogger()) - require.NoError(t, services.StartAndAwaitRunning(context.Background(), mkv)) - - ctxTimeout, cancel := context.WithTimeout(context.Background(), 5*time.Second) - defer cancel() - - // Service should fail soon after starting, since it cannot join the cluster. - _ = mkv.AwaitTerminated(ctxTimeout) - - // We verify service state here. - require.Equal(t, mkv.FailureCase(), errFailedToJoinCluster) -} - -func getFreePorts(count int) ([]int, error) { - var ports []int - for i := 0; i < count; i++ { - addr, err := net.ResolveTCPAddr("tcp", "127.0.0.1:0") - if err != nil { - return nil, err - } - - l, err := net.ListenTCP("tcp", addr) - if err != nil { - return nil, err - } - defer l.Close() - ports = append(ports, l.Addr().(*net.TCPAddr).Port) - } - return ports, nil -} - -func getTimestamps(members map[string]member) (min int64, max int64, avg int64) { - min = int64(math.MaxInt64) - - for _, ing := range members { - if ing.Timestamp < min { - min = ing.Timestamp - } - - if ing.Timestamp > max { - max = ing.Timestamp - } - - avg += ing.Timestamp - } - if len(members) > 0 { - avg /= int64(len(members)) - } - return -} - -func runClient(t *testing.T, kv *Client, name string, ringKey string, portToConnect int, start <-chan struct{}, stop <-chan struct{}) { - // stop gossipping about the ring(s) - defer services.StopAndAwaitTerminated(context.Background(), kv.kv) //nolint:errcheck - - for { - select { - case <-start: - start = nil - - // let's join the first member - if portToConnect > 0 { - _, err := kv.kv.JoinMembers([]string{fmt.Sprintf("127.0.0.1:%d", portToConnect)}) - if err != nil { - t.Errorf("%s failed to join the cluster: %v", name, err) - return - } - } - case <-stop: - return - case <-time.After(1 * time.Second): - cas(t, kv, ringKey, updateFn(name)) - } - } -} - -// avoid dependency on ring package -func generateTokens(numTokens int) []uint32 { - var tokens []uint32 - for i := 0; i < numTokens; { - candidate := rand.Uint32() - tokens = append(tokens, candidate) - i++ - } - return tokens -} - -type distributedCounter map[string]int - -func (dc distributedCounter) Merge(mergeable Mergeable, localCAS bool) (Mergeable, error) { - if mergeable == nil { - return nil, nil - } - - odc, ok := mergeable.(distributedCounter) - if !ok || odc == nil { - return nil, fmt.Errorf("invalid thing to merge: %T", mergeable) - } - - updated := distributedCounter{} - - for k, v := range odc { - if v > dc[k] { - dc[k] = v - updated[k] = v - } - } - - if len(updated) == 0 { - return nil, nil - } - return updated, nil -} - -func (dc distributedCounter) MergeContent() []string { - // return list of keys - out := []string(nil) - for k := range dc { - out = append(out, k) - } - return out -} - -func (dc distributedCounter) RemoveTombstones(limit time.Time) (_, _ int) { - // nothing to do - return -} - -func (dc distributedCounter) Clone() Mergeable { - out := make(distributedCounter, len(dc)) - for k, v := range dc { - out[k] = v - } - return out -} - -type distributedCounterCodec struct{} - -func (d distributedCounterCodec) CodecID() string { - return "distributedCounter" -} - -func (d distributedCounterCodec) Decode(b []byte) (interface{}, error) { - dec := gob.NewDecoder(bytes.NewBuffer(b)) - out := &distributedCounter{} - err := dec.Decode(out) - return *out, err -} - -func (d distributedCounterCodec) Encode(val interface{}) ([]byte, error) { - buf := bytes.Buffer{} - enc := gob.NewEncoder(&buf) - err := enc.Encode(val) - return buf.Bytes(), err -} - -var _ codec.Codec = &distributedCounterCodec{} - -func TestMultipleCodecs(t *testing.T) { - var cfg KVConfig - flagext.DefaultValues(&cfg) - cfg.TCPTransport = TCPTransportConfig{ - BindAddrs: []string{"localhost"}, - BindPort: 0, // randomize - } - - cfg.Codecs = []codec.Codec{ - dataCodec{}, - distributedCounterCodec{}, - } - - mkv1 := NewKV(cfg, log.NewNopLogger()) - require.NoError(t, services.StartAndAwaitRunning(context.Background(), mkv1)) - defer services.StopAndAwaitTerminated(context.Background(), mkv1) //nolint:errcheck - - kv1, err := NewClient(mkv1, dataCodec{}) - require.NoError(t, err) - - kv2, err := NewClient(mkv1, distributedCounterCodec{}) - require.NoError(t, err) - - err = kv1.CAS(context.Background(), "data", func(in interface{}) (out interface{}, retry bool, err error) { - var d *data = nil - if in != nil { - d = in.(*data) - } - if d == nil { - d = &data{} - } - if d.Members == nil { - d.Members = map[string]member{} - } - d.Members["test"] = member{ - Timestamp: time.Now().Unix(), - State: ACTIVE, - } - return d, true, nil - }) - require.NoError(t, err) - - err = kv2.CAS(context.Background(), "counter", func(in interface{}) (out interface{}, retry bool, err error) { - var dc distributedCounter = nil - if in != nil { - dc = in.(distributedCounter) - } - if dc == nil { - dc = distributedCounter{} - } - dc["test"] = 5 - return dc, true, err - }) - require.NoError(t, err) - - // We will read values from second KV, which will join the first one - mkv2 := NewKV(cfg, log.NewNopLogger()) - require.NoError(t, services.StartAndAwaitRunning(context.Background(), mkv2)) - defer services.StopAndAwaitTerminated(context.Background(), mkv2) //nolint:errcheck - - // Join second KV to first one. That will also trigger state transfer. - _, err = mkv2.JoinMembers([]string{fmt.Sprintf("127.0.0.1:%d", mkv1.GetListeningPort())}) - require.NoError(t, err) - - // Now read both values from second KV. It should have both values by now. - - // fetch directly from single KV, to see that both are stored in the same one - val, err := mkv2.Get("data", dataCodec{}) - require.NoError(t, err) - require.NotNil(t, val) - require.NotZero(t, val.(*data).Members["test"].Timestamp) - require.Equal(t, ACTIVE, val.(*data).Members["test"].State) - - val, err = mkv2.Get("counter", distributedCounterCodec{}) - require.NoError(t, err) - require.NotNil(t, val) - require.Equal(t, 5, val.(distributedCounter)["test"]) -} - -func TestGenerateRandomSuffix(t *testing.T) { - h1 := generateRandomSuffix() - h2 := generateRandomSuffix() - h3 := generateRandomSuffix() - - require.NotEqual(t, h1, h2) - require.NotEqual(t, h2, h3) -} - -func TestRejoin(t *testing.T) { - ports, err := getFreePorts(2) - require.NoError(t, err) - - var cfg1 KVConfig - flagext.DefaultValues(&cfg1) - cfg1.TCPTransport = TCPTransportConfig{ - BindAddrs: []string{"localhost"}, - BindPort: ports[0], - } - - cfg1.RandomizeNodeName = true - cfg1.Codecs = []codec.Codec{dataCodec{}} - cfg1.AbortIfJoinFails = false - - cfg2 := cfg1 - cfg2.TCPTransport.BindPort = ports[1] - cfg2.JoinMembers = []string{fmt.Sprintf("localhost:%d", ports[0])} - cfg2.RejoinInterval = 1 * time.Second - - mkv1 := NewKV(cfg1, log.NewNopLogger()) - require.NoError(t, services.StartAndAwaitRunning(context.Background(), mkv1)) - defer services.StopAndAwaitTerminated(context.Background(), mkv1) //nolint:errcheck - - mkv2 := NewKV(cfg2, log.NewNopLogger()) - require.NoError(t, services.StartAndAwaitRunning(context.Background(), mkv2)) - defer services.StopAndAwaitTerminated(context.Background(), mkv2) //nolint:errcheck - - membersFunc := func() interface{} { - return mkv2.memberlist.NumMembers() - } - - test.Poll(t, 5*time.Second, 2, membersFunc) - - // Shutdown first KV - require.NoError(t, services.StopAndAwaitTerminated(context.Background(), mkv1)) - - // Second KV should see single member now. - test.Poll(t, 5*time.Second, 1, membersFunc) - - // Let's start first KV again. It is not configured to join the cluster, but KV2 is rejoining. - mkv1 = NewKV(cfg1, log.NewNopLogger()) - require.NoError(t, services.StartAndAwaitRunning(context.Background(), mkv1)) - defer services.StopAndAwaitTerminated(context.Background(), mkv1) //nolint:errcheck - - test.Poll(t, 5*time.Second, 2, membersFunc) -} - -func TestMessageBuffer(t *testing.T) { - buf := []message(nil) - size := 0 - - buf, size = addMessageToBuffer(buf, size, 100, message{Size: 50}) - assert.Len(t, buf, 1) - assert.Equal(t, size, 50) - - buf, size = addMessageToBuffer(buf, size, 100, message{Size: 50}) - assert.Len(t, buf, 2) - assert.Equal(t, size, 100) - - buf, size = addMessageToBuffer(buf, size, 100, message{Size: 25}) - assert.Len(t, buf, 2) - assert.Equal(t, size, 75) -} - -func TestNotifyMsgResendsOnlyChanges(t *testing.T) { - codec := dataCodec{} - - cfg := KVConfig{} - // We will be checking for number of messages in the broadcast queue, so make sure to use known retransmit factor. - cfg.RetransmitMult = 1 - cfg.Codecs = append(cfg.Codecs, codec) - - kv := NewKV(cfg, log.NewNopLogger()) - require.NoError(t, services.StartAndAwaitRunning(context.Background(), kv)) - defer services.StopAndAwaitTerminated(context.Background(), kv) //nolint:errcheck - - client, err := NewClient(kv, codec) - require.NoError(t, err) - - // No broadcast messages from KV at the beginning. - require.Equal(t, 0, len(kv.GetBroadcasts(0, math.MaxInt32))) - - now := time.Now() - - require.NoError(t, client.CAS(context.Background(), key, func(in interface{}) (out interface{}, retry bool, err error) { - d := getOrCreateData(in) - d.Members["a"] = member{Timestamp: now.Unix(), State: JOINING} - d.Members["b"] = member{Timestamp: now.Unix(), State: JOINING} - return d, true, nil - })) - - // Check that new instance is broadcasted about just once. - assert.Equal(t, 1, len(kv.GetBroadcasts(0, math.MaxInt32))) - require.Equal(t, 0, len(kv.GetBroadcasts(0, math.MaxInt32))) - - kv.NotifyMsg(marshalKeyValuePair(t, key, codec, &data{ - Members: map[string]member{ - "a": {Timestamp: now.Unix() - 5, State: ACTIVE}, - "b": {Timestamp: now.Unix() + 5, State: ACTIVE, Tokens: []uint32{1, 2, 3}}, - "c": {Timestamp: now.Unix(), State: ACTIVE}, - }})) - - // Check two things here: - // 1) state of value in KV store - // 2) broadcast message only has changed members - - d := getData(t, client, key) - assert.Equal(t, &data{ - Members: map[string]member{ - "a": {Timestamp: now.Unix(), State: JOINING, Tokens: []uint32{}}, // unchanged, timestamp too old - "b": {Timestamp: now.Unix() + 5, State: ACTIVE, Tokens: []uint32{1, 2, 3}}, - "c": {Timestamp: now.Unix(), State: ACTIVE, Tokens: []uint32{}}, - }}, d) - - bs := kv.GetBroadcasts(0, math.MaxInt32) - require.Equal(t, 1, len(bs)) - - d = decodeDataFromMarshalledKeyValuePair(t, bs[0], key, codec) - assert.Equal(t, &data{ - Members: map[string]member{ - // "a" is not here, because it wasn't changed by the message. - "b": {Timestamp: now.Unix() + 5, State: ACTIVE, Tokens: []uint32{1, 2, 3}}, - "c": {Timestamp: now.Unix(), State: ACTIVE}, - }}, d) -} - -func TestSendingOldTombstoneShouldNotForwardMessage(t *testing.T) { - codec := dataCodec{} - - cfg := KVConfig{} - // We will be checking for number of messages in the broadcast queue, so make sure to use known retransmit factor. - cfg.RetransmitMult = 1 - cfg.LeftIngestersTimeout = 5 * time.Minute - cfg.Codecs = append(cfg.Codecs, codec) - - kv := NewKV(cfg, log.NewNopLogger()) - require.NoError(t, services.StartAndAwaitRunning(context.Background(), kv)) - defer services.StopAndAwaitTerminated(context.Background(), kv) //nolint:errcheck - - client, err := NewClient(kv, codec) - require.NoError(t, err) - - now := time.Now() - - // No broadcast messages from KV at the beginning. - require.Equal(t, 0, len(kv.GetBroadcasts(0, math.MaxInt32))) - - for _, tc := range []struct { - name string - valueBeforeSend *data // value in KV store before sending messsage - msgToSend *data - valueAfterSend *data // value in KV store after sending message - broadcastMessage *data // broadcasted change, if not nil - }{ - // These tests follow each other (end state of KV in state is starting point in the next state). - { - name: "old tombstone, empty KV", - valueBeforeSend: nil, - msgToSend: &data{Members: map[string]member{"instance": {Timestamp: now.Unix() - int64(2*cfg.LeftIngestersTimeout.Seconds()), State: LEFT}}}, - valueAfterSend: nil, // no change to KV - broadcastMessage: nil, // no new message - }, - - { - name: "recent tombstone, empty KV", - valueBeforeSend: nil, - msgToSend: &data{Members: map[string]member{"instance": {Timestamp: now.Unix(), State: LEFT}}}, - broadcastMessage: &data{Members: map[string]member{"instance": {Timestamp: now.Unix(), State: LEFT}}}, - valueAfterSend: &data{Members: map[string]member{"instance": {Timestamp: now.Unix(), State: LEFT, Tokens: []uint32{}}}}, - }, - - { - name: "old tombstone, KV contains tombstone already", - valueBeforeSend: &data{Members: map[string]member{"instance": {Timestamp: now.Unix(), State: LEFT, Tokens: []uint32{}}}}, - msgToSend: &data{Members: map[string]member{"instance": {Timestamp: now.Unix() - 10, State: LEFT}}}, - broadcastMessage: nil, - valueAfterSend: &data{Members: map[string]member{"instance": {Timestamp: now.Unix(), State: LEFT, Tokens: []uint32{}}}}, - }, - - { - name: "fresh tombstone, KV contains tombstone already", - valueBeforeSend: &data{Members: map[string]member{"instance": {Timestamp: now.Unix(), State: LEFT, Tokens: []uint32{}}}}, - msgToSend: &data{Members: map[string]member{"instance": {Timestamp: now.Unix() + 10, State: LEFT}}}, - broadcastMessage: &data{Members: map[string]member{"instance": {Timestamp: now.Unix() + 10, State: LEFT}}}, - valueAfterSend: &data{Members: map[string]member{"instance": {Timestamp: now.Unix() + 10, State: LEFT, Tokens: []uint32{}}}}, - }, - } { - t.Run(tc.name, func(t *testing.T) { - d := getData(t, client, key) - if tc.valueBeforeSend == nil { - require.True(t, d == nil || len(d.Members) == 0) - } else { - require.Equal(t, tc.valueBeforeSend, d, "valueBeforeSend") - } - - kv.NotifyMsg(marshalKeyValuePair(t, key, codec, tc.msgToSend)) - - bs := kv.GetBroadcasts(0, math.MaxInt32) - if tc.broadcastMessage == nil { - require.Equal(t, 0, len(bs), "expected no broadcast message") - } else { - require.Equal(t, 1, len(bs), "expected broadcast message") - require.Equal(t, tc.broadcastMessage, decodeDataFromMarshalledKeyValuePair(t, bs[0], key, codec)) - } - - d = getData(t, client, key) - if tc.valueAfterSend == nil { - require.True(t, d == nil || len(d.Members) == 0) - } else { - require.Equal(t, tc.valueAfterSend, d, "valueAfterSend") - } - }) - } -} - -func decodeDataFromMarshalledKeyValuePair(t *testing.T, marshalledKVP []byte, key string, codec dataCodec) *data { - kvp := KeyValuePair{} - require.NoError(t, kvp.Unmarshal(marshalledKVP)) - require.Equal(t, key, kvp.Key) - - val, err := codec.Decode(kvp.Value) - require.NoError(t, err) - d, ok := val.(*data) - require.True(t, ok) - return d -} - -func marshalKeyValuePair(t *testing.T, key string, codec codec.Codec, value interface{}) []byte { - data, err := codec.Encode(value) - require.NoError(t, err) - - kvp := KeyValuePair{Key: key, Codec: codec.CodecID(), Value: data} - data, err = kvp.Marshal() - require.NoError(t, err) - return data -} - -func getOrCreateData(in interface{}) *data { - // Modify value that was passed as a parameter. - // Client takes care of concurrent modifications. - r, ok := in.(*data) - if !ok || r == nil { - return &data{Members: map[string]member{}} - } - return r -} diff --git a/pkg/ring/kv/multi_test.go b/pkg/ring/kv/multi_test.go deleted file mode 100644 index 9385ed5ff8..0000000000 --- a/pkg/ring/kv/multi_test.go +++ /dev/null @@ -1,32 +0,0 @@ -package kv - -import ( - "testing" - - "github.com/stretchr/testify/assert" - "gopkg.in/yaml.v2" -) - -func boolPtr(b bool) *bool { - return &b -} - -func TestMultiRuntimeConfigWithVariousEnabledValues(t *testing.T) { - testcases := map[string]struct { - yaml string - expected *bool - }{ - "nil": {"primary: test", nil}, - "true": {"primary: test\nmirror_enabled: true", boolPtr(true)}, - "false": {"mirror_enabled: false", boolPtr(false)}, - } - - for name, tc := range testcases { - t.Run(name, func(t *testing.T) { - c := MultiRuntimeConfig{} - err := yaml.Unmarshal([]byte(tc.yaml), &c) - assert.NoError(t, err, tc.yaml) - assert.Equal(t, tc.expected, c.Mirroring, tc.yaml) - }) - } -} diff --git a/pkg/ring/lifecycler.go b/pkg/ring/lifecycler.go index 19d1d5299b..0c1982955d 100644 --- a/pkg/ring/lifecycler.go +++ b/pkg/ring/lifecycler.go @@ -2,7 +2,6 @@ package ring import ( "context" - "errors" "flag" "fmt" "os" @@ -12,13 +11,14 @@ import ( "github.com/go-kit/kit/log/level" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv" "github.com/grafana/dskit/services" + "github.com/pkg/errors" perrors "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" "github.com/prometheus/client_golang/prometheus/promauto" "go.uber.org/atomic" - "github.com/cortexproject/cortex/pkg/ring/kv" "github.com/cortexproject/cortex/pkg/util" "github.com/cortexproject/cortex/pkg/util/log" ) @@ -157,7 +157,8 @@ func NewLifecycler(cfg LifecyclerConfig, flushTransferer FlushTransferer, ringNa store, err := kv.NewClient( cfg.RingConfig.KVStore, codec, - kv.RegistererWithKVName(reg, ringName+"-lifecycler"), + kv.RegistererWithKVName(prometheus.WrapRegistererWithPrefix("cortex_", reg), ringName+"-lifecycler"), + log.Logger, ) if err != nil { return nil, err diff --git a/pkg/ring/lifecycler_test.go b/pkg/ring/lifecycler_test.go index 9c869fad45..92d40fff14 100644 --- a/pkg/ring/lifecycler_test.go +++ b/pkg/ring/lifecycler_test.go @@ -9,12 +9,13 @@ import ( "testing" "time" + "github.com/go-kit/kit/log" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv/consul" "github.com/grafana/dskit/services" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/cortexproject/cortex/pkg/ring/kv/consul" "github.com/cortexproject/cortex/pkg/util/test" ) @@ -43,9 +44,12 @@ func checkNormalised(d interface{}, id string) bool { } func TestLifecycler_HealthyInstancesCount(t *testing.T) { + ringStore, closer := consul.NewInMemoryClient(GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + var ringConfig Config flagext.DefaultValues(&ringConfig) - ringConfig.KVStore.Mock = consul.NewInMemoryClient(GetCodec()) + ringConfig.KVStore.Mock = ringStore ctx := context.Background() @@ -90,9 +94,12 @@ func TestLifecycler_HealthyInstancesCount(t *testing.T) { } func TestLifecycler_ZonesCount(t *testing.T) { + ringStore, closer := consul.NewInMemoryClient(GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + var ringConfig Config flagext.DefaultValues(&ringConfig) - ringConfig.KVStore.Mock = consul.NewInMemoryClient(GetCodec()) + ringConfig.KVStore.Mock = ringStore events := []struct { zone string @@ -130,9 +137,12 @@ func TestLifecycler_ZonesCount(t *testing.T) { } func TestLifecycler_NilFlushTransferer(t *testing.T) { + ringStore, closer := consul.NewInMemoryClient(GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + var ringConfig Config flagext.DefaultValues(&ringConfig) - ringConfig.KVStore.Mock = consul.NewInMemoryClient(GetCodec()) + ringConfig.KVStore.Mock = ringStore lifecyclerConfig := testLifecyclerConfig(ringConfig, "ing1") // Create a lifecycler with nil FlushTransferer to make sure it operates correctly @@ -152,9 +162,12 @@ func TestLifecycler_NilFlushTransferer(t *testing.T) { func TestLifecycler_TwoRingsWithDifferentKeysOnTheSameKVStore(t *testing.T) { // Create a shared ring + ringStore, closer := consul.NewInMemoryClient(GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + var ringConfig Config flagext.DefaultValues(&ringConfig) - ringConfig.KVStore.Mock = consul.NewInMemoryClient(GetCodec()) + ringConfig.KVStore.Mock = ringStore // Create two lifecyclers, each on a separate ring lifecyclerConfig1 := testLifecyclerConfig(ringConfig, "instance-1") @@ -189,10 +202,12 @@ func (f *nopFlushTransferer) TransferOut(_ context.Context) error { } func TestLifecycler_ShouldHandleInstanceAbruptlyRestarted(t *testing.T) { + ringStore, closer := consul.NewInMemoryClient(GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + var ringConfig Config flagext.DefaultValues(&ringConfig) - c := GetCodec() - ringConfig.KVStore.Mock = consul.NewInMemoryClient(c) + ringConfig.KVStore.Mock = ringStore r, err := New(ringConfig, "ingester", IngesterRingKey, nil) require.NoError(t, err) @@ -322,9 +337,12 @@ func (f *noopFlushTransferer) Flush() {} func (f *noopFlushTransferer) TransferOut(ctx context.Context) error { return nil } func TestRestartIngester_DisabledHeartbeat_unregister_on_shutdown_false(t *testing.T) { + ringStore, closer := consul.NewInMemoryClient(GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + var ringConfig Config flagext.DefaultValues(&ringConfig) - ringConfig.KVStore.Mock = consul.NewInMemoryClient(GetCodec()) + ringConfig.KVStore.Mock = ringStore r, err := New(ringConfig, "ingester", IngesterRingKey, nil) require.NoError(t, err) @@ -422,9 +440,12 @@ func TestRestartIngester_DisabledHeartbeat_unregister_on_shutdown_false(t *testi } func TestTokensOnDisk(t *testing.T) { + ringStore, closer := consul.NewInMemoryClient(GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + var ringConfig Config flagext.DefaultValues(&ringConfig) - ringConfig.KVStore.Mock = consul.NewInMemoryClient(GetCodec()) + ringConfig.KVStore.Mock = ringStore r, err := New(ringConfig, "ingester", IngesterRingKey, nil) require.NoError(t, err) @@ -496,10 +517,12 @@ func TestTokensOnDisk(t *testing.T) { // JoinInLeavingState ensures that if the lifecycler starts up and the ring already has it in a LEAVING state that it still is able to auto join func TestJoinInLeavingState(t *testing.T) { + ringStore, closer := consul.NewInMemoryClient(GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + var ringConfig Config flagext.DefaultValues(&ringConfig) - c := GetCodec() - ringConfig.KVStore.Mock = consul.NewInMemoryClient(c) + ringConfig.KVStore.Mock = ringStore r, err := New(ringConfig, "ingester", IngesterRingKey, nil) require.NoError(t, err) @@ -548,10 +571,12 @@ func TestJoinInLeavingState(t *testing.T) { // JoinInJoiningState ensures that if the lifecycler starts up and the ring already has it in a JOINING state that it still is able to auto join func TestJoinInJoiningState(t *testing.T) { + ringStore, closer := consul.NewInMemoryClient(GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + var ringConfig Config flagext.DefaultValues(&ringConfig) - c := GetCodec() - ringConfig.KVStore.Mock = consul.NewInMemoryClient(c) + ringConfig.KVStore.Mock = ringStore r, err := New(ringConfig, "ingester", IngesterRingKey, nil) require.NoError(t, err) @@ -610,10 +635,12 @@ func TestRestoreOfZoneWhenOverwritten(t *testing.T) { // so it gets removed. The current version of the lifecylcer should // write it back on update during its next heartbeat. + ringStore, closer := consul.NewInMemoryClient(GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + var ringConfig Config flagext.DefaultValues(&ringConfig) - codec := GetCodec() - ringConfig.KVStore.Mock = consul.NewInMemoryClient(codec) + ringConfig.KVStore.Mock = ringStore r, err := New(ringConfig, "ingester", IngesterRingKey, nil) require.NoError(t, err) diff --git a/pkg/ring/model.go b/pkg/ring/model.go index 3a257c3952..26487c050b 100644 --- a/pkg/ring/model.go +++ b/pkg/ring/model.go @@ -7,9 +7,8 @@ import ( "time" "github.com/gogo/protobuf/proto" - - "github.com/cortexproject/cortex/pkg/ring/kv/codec" - "github.com/cortexproject/cortex/pkg/ring/kv/memberlist" + "github.com/grafana/dskit/kv/codec" + "github.com/grafana/dskit/kv/memberlist" ) // ByAddr is a sortable list of InstanceDesc. diff --git a/pkg/ring/ring.go b/pkg/ring/ring.go index ae43e40b76..01ef9d24fe 100644 --- a/pkg/ring/ring.go +++ b/pkg/ring/ring.go @@ -12,11 +12,11 @@ import ( "time" "github.com/go-kit/kit/log/level" + "github.com/grafana/dskit/kv" "github.com/grafana/dskit/services" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" - "github.com/cortexproject/cortex/pkg/ring/kv" "github.com/cortexproject/cortex/pkg/util" "github.com/cortexproject/cortex/pkg/util/log" util_math "github.com/cortexproject/cortex/pkg/util/math" @@ -206,7 +206,8 @@ func New(cfg Config, name, key string, reg prometheus.Registerer) (*Ring, error) store, err := kv.NewClient( cfg.KVStore, codec, - kv.RegistererWithKVName(reg, name+"-ring"), + kv.RegistererWithKVName(prometheus.WrapRegistererWithPrefix("cortex_", reg), name+"-ring"), + log.Logger, ) if err != nil { return nil, err diff --git a/pkg/ring/ring_test.go b/pkg/ring/ring_test.go index a0089ee814..0486c9d303 100644 --- a/pkg/ring/ring_test.go +++ b/pkg/ring/ring_test.go @@ -11,13 +11,14 @@ import ( "testing" "time" + "github.com/go-kit/kit/log" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv" + "github.com/grafana/dskit/kv/consul" "github.com/grafana/dskit/services" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" - "github.com/cortexproject/cortex/pkg/ring/kv" - "github.com/cortexproject/cortex/pkg/ring/kv/consul" "github.com/cortexproject/cortex/pkg/util" "github.com/cortexproject/cortex/pkg/util/test" ) @@ -1926,7 +1927,8 @@ func compareReplicationSets(first, second ReplicationSet) (added, removed []stri // This test verifies that ring is getting updates, even after extending check in the loop method. func TestRingUpdates(t *testing.T) { - inmem := consul.NewInMemoryClient(GetCodec()) + inmem, closer := consul.NewInMemoryClient(GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) cfg := Config{ KVStore: kv.Config{Mock: inmem}, @@ -2019,10 +2021,11 @@ func startLifecycler(t *testing.T, cfg Config, heartbeat time.Duration, lifecycl // This test checks if shuffle-sharded ring can be reused, and whether it receives // updates from "main" ring. func TestShuffleShardWithCaching(t *testing.T) { - inmem := consul.NewInMemoryClientWithConfig(GetCodec(), consul.Config{ + inmem, closer := consul.NewInMemoryClientWithConfig(GetCodec(), consul.Config{ MaxCasRetries: 20, CasRetryDelay: 500 * time.Millisecond, - }) + }, log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) cfg := Config{ KVStore: kv.Config{Mock: inmem}, diff --git a/pkg/ring/testutils/testutils.go b/pkg/ring/testutils/testutils.go index 24439a0cbd..f0161f7cb2 100644 --- a/pkg/ring/testutils/testutils.go +++ b/pkg/ring/testutils/testutils.go @@ -4,9 +4,9 @@ import ( "context" "github.com/go-kit/kit/log/level" + "github.com/grafana/dskit/kv" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv" util_log "github.com/cortexproject/cortex/pkg/util/log" ) diff --git a/pkg/ruler/api_test.go b/pkg/ruler/api_test.go index 0213a8ce0b..2fc6038682 100644 --- a/pkg/ruler/api_test.go +++ b/pkg/ruler/api_test.go @@ -21,7 +21,7 @@ import ( ) func TestRuler_rules(t *testing.T) { - cfg, cleanup := defaultRulerConfig(newMockRuleStore(mockRules)) + cfg, cleanup := defaultRulerConfig(t, newMockRuleStore(mockRules)) defer cleanup() r, rcleanup := newTestRuler(t, cfg) @@ -78,7 +78,7 @@ func TestRuler_rules(t *testing.T) { } func TestRuler_rules_special_characters(t *testing.T) { - cfg, cleanup := defaultRulerConfig(newMockRuleStore(mockSpecialCharRules)) + cfg, cleanup := defaultRulerConfig(t, newMockRuleStore(mockSpecialCharRules)) defer cleanup() r, rcleanup := newTestRuler(t, cfg) @@ -135,7 +135,7 @@ func TestRuler_rules_special_characters(t *testing.T) { } func TestRuler_alerts(t *testing.T) { - cfg, cleanup := defaultRulerConfig(newMockRuleStore(mockRules)) + cfg, cleanup := defaultRulerConfig(t, newMockRuleStore(mockRules)) defer cleanup() r, rcleanup := newTestRuler(t, cfg) @@ -171,7 +171,7 @@ func TestRuler_alerts(t *testing.T) { } func TestRuler_Create(t *testing.T) { - cfg, cleanup := defaultRulerConfig(newMockRuleStore(make(map[string]rulespb.RuleGroupList))) + cfg, cleanup := defaultRulerConfig(t, newMockRuleStore(make(map[string]rulespb.RuleGroupList))) defer cleanup() r, rcleanup := newTestRuler(t, cfg) @@ -262,7 +262,7 @@ rules: } func TestRuler_DeleteNamespace(t *testing.T) { - cfg, cleanup := defaultRulerConfig(newMockRuleStore(mockRulesNamespaces)) + cfg, cleanup := defaultRulerConfig(t, newMockRuleStore(mockRulesNamespaces)) defer cleanup() r, rcleanup := newTestRuler(t, cfg) @@ -301,7 +301,7 @@ func TestRuler_DeleteNamespace(t *testing.T) { } func TestRuler_LimitsPerGroup(t *testing.T) { - cfg, cleanup := defaultRulerConfig(newMockRuleStore(make(map[string]rulespb.RuleGroupList))) + cfg, cleanup := defaultRulerConfig(t, newMockRuleStore(make(map[string]rulespb.RuleGroupList))) defer cleanup() r, rcleanup := newTestRuler(t, cfg) @@ -356,7 +356,7 @@ rules: } func TestRuler_RulerGroupLimits(t *testing.T) { - cfg, cleanup := defaultRulerConfig(newMockRuleStore(make(map[string]rulespb.RuleGroupList))) + cfg, cleanup := defaultRulerConfig(t, newMockRuleStore(make(map[string]rulespb.RuleGroupList))) defer cleanup() r, rcleanup := newTestRuler(t, cfg) diff --git a/pkg/ruler/lifecycle_test.go b/pkg/ruler/lifecycle_test.go index 3ae50e1457..96bbea64d2 100644 --- a/pkg/ruler/lifecycle_test.go +++ b/pkg/ruler/lifecycle_test.go @@ -6,11 +6,13 @@ import ( "testing" "time" + "github.com/go-kit/kit/log" + "github.com/grafana/dskit/kv/consul" "github.com/grafana/dskit/services" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv/consul" "github.com/cortexproject/cortex/pkg/ring/testutils" "github.com/cortexproject/cortex/pkg/util/test" ) @@ -19,14 +21,15 @@ import ( func TestRulerShutdown(t *testing.T) { ctx := context.Background() - config, cleanup := defaultRulerConfig(newMockRuleStore(mockRules)) + config, cleanup := defaultRulerConfig(t, newMockRuleStore(mockRules)) defer cleanup() r, rcleanup := newRuler(t, config) defer rcleanup() r.cfg.EnableSharding = true - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) err := enableSharding(r, ringStore) require.NoError(t, err) @@ -54,7 +57,7 @@ func TestRuler_RingLifecyclerShouldAutoForgetUnhealthyInstances(t *testing.T) { const heartbeatTimeout = time.Minute ctx := context.Background() - config, cleanup := defaultRulerConfig(newMockRuleStore(mockRules)) + config, cleanup := defaultRulerConfig(t, newMockRuleStore(mockRules)) defer cleanup() r, rcleanup := newRuler(t, config) defer rcleanup() @@ -62,7 +65,8 @@ func TestRuler_RingLifecyclerShouldAutoForgetUnhealthyInstances(t *testing.T) { r.cfg.Ring.HeartbeatPeriod = 100 * time.Millisecond r.cfg.Ring.HeartbeatTimeout = heartbeatTimeout - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) err := enableSharding(r, ringStore) require.NoError(t, err) diff --git a/pkg/ruler/ruler.go b/pkg/ruler/ruler.go index 13e1b3d931..a4e5ad2efe 100644 --- a/pkg/ruler/ruler.go +++ b/pkg/ruler/ruler.go @@ -15,6 +15,7 @@ import ( "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv" "github.com/grafana/dskit/services" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" @@ -29,7 +30,6 @@ import ( "github.com/cortexproject/cortex/pkg/cortexpb" "github.com/cortexproject/cortex/pkg/ring" ring_client "github.com/cortexproject/cortex/pkg/ring/client" - "github.com/cortexproject/cortex/pkg/ring/kv" "github.com/cortexproject/cortex/pkg/ruler/rulespb" "github.com/cortexproject/cortex/pkg/ruler/rulestore" "github.com/cortexproject/cortex/pkg/tenant" @@ -282,7 +282,8 @@ func NewRuler(cfg Config, manager MultiTenantManager, reg prometheus.Registerer, ringStore, err := kv.NewClient( cfg.Ring.KVStore, ring.GetCodec(), - kv.RegistererWithKVName(reg, "ruler"), + kv.RegistererWithKVName(prometheus.WrapRegistererWithPrefix("cortex_", reg), "ruler"), + logger, ) if err != nil { return nil, errors.Wrap(err, "create KV store client") diff --git a/pkg/ruler/ruler_ring.go b/pkg/ruler/ruler_ring.go index 893963c680..2231858b03 100644 --- a/pkg/ruler/ruler_ring.go +++ b/pkg/ruler/ruler_ring.go @@ -7,9 +7,9 @@ import ( "time" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv" ) const ( diff --git a/pkg/ruler/ruler_test.go b/pkg/ruler/ruler_test.go index 8041125a59..5ba38161c5 100644 --- a/pkg/ruler/ruler_test.go +++ b/pkg/ruler/ruler_test.go @@ -19,6 +19,8 @@ import ( "github.com/go-kit/kit/log/level" "github.com/gorilla/mux" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv" + "github.com/grafana/dskit/kv/consul" "github.com/grafana/dskit/services" "github.com/prometheus/client_golang/prometheus" prom_testutil "github.com/prometheus/client_golang/prometheus/testutil" @@ -36,8 +38,6 @@ import ( "github.com/cortexproject/cortex/pkg/chunk" "github.com/cortexproject/cortex/pkg/cortexpb" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv" - "github.com/cortexproject/cortex/pkg/ring/kv/consul" "github.com/cortexproject/cortex/pkg/ruler/rulespb" "github.com/cortexproject/cortex/pkg/ruler/rulestore" "github.com/cortexproject/cortex/pkg/ruler/rulestore/objectclient" @@ -45,12 +45,17 @@ import ( "github.com/cortexproject/cortex/pkg/util" ) -func defaultRulerConfig(store rulestore.RuleStore) (Config, func()) { +func defaultRulerConfig(t testing.TB, store rulestore.RuleStore) (Config, func()) { + t.Helper() + // Create a new temporary directory for the rules, so that // each test will run in isolation. rulesDir, _ := ioutil.TempDir("/tmp", "ruler-tests") + codec := ring.GetCodec() - consul := consul.NewInMemoryClient(codec) + consul, closer := consul.NewInMemoryClient(codec, log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + cfg := Config{} flagext.DefaultValues(&cfg) cfg.RulePath = rulesDir @@ -179,7 +184,7 @@ func TestNotifierSendsUserIDHeader(t *testing.T) { defer ts.Close() // We create an empty rule store so that the ruler will not load any rule from it. - cfg, cleanup := defaultRulerConfig(newMockRuleStore(nil)) + cfg, cleanup := defaultRulerConfig(t, newMockRuleStore(nil)) defer cleanup() cfg.AlertmanagerURL = ts.URL @@ -211,7 +216,7 @@ func TestNotifierSendsUserIDHeader(t *testing.T) { } func TestRuler_Rules(t *testing.T) { - cfg, cleanup := defaultRulerConfig(newMockRuleStore(mockRules)) + cfg, cleanup := defaultRulerConfig(t, newMockRuleStore(mockRules)) defer cleanup() r, rcleanup := newTestRuler(t, cfg) @@ -639,7 +644,8 @@ func TestSharding(t *testing.T) { for name, tc := range testCases { t.Run(name, func(t *testing.T) { - kvStore := consul.NewInMemoryClient(ring.GetCodec()) + kvStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) setupRuler := func(id string, host string, port int, forceRing *ring.Ring) *Ruler { cfg := Config{ @@ -847,7 +853,7 @@ type ruleGroupKey struct { } func TestRuler_ListAllRules(t *testing.T) { - cfg, cleanup := defaultRulerConfig(newMockRuleStore(mockRules)) + cfg, cleanup := defaultRulerConfig(t, newMockRuleStore(mockRules)) defer cleanup() r, rcleanup := newTestRuler(t, cfg) diff --git a/pkg/storegateway/gateway.go b/pkg/storegateway/gateway.go index 447f6d615c..09819b6bf7 100644 --- a/pkg/storegateway/gateway.go +++ b/pkg/storegateway/gateway.go @@ -9,6 +9,7 @@ import ( "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" + "github.com/grafana/dskit/kv" "github.com/grafana/dskit/services" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" @@ -19,7 +20,6 @@ import ( "github.com/weaveworks/common/logging" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv" "github.com/cortexproject/cortex/pkg/storage/bucket" cortex_tsdb "github.com/cortexproject/cortex/pkg/storage/tsdb" "github.com/cortexproject/cortex/pkg/storegateway/storegatewaypb" @@ -113,7 +113,8 @@ func NewStoreGateway(gatewayCfg Config, storageCfg cortex_tsdb.BlocksStorageConf ringStore, err = kv.NewClient( gatewayCfg.ShardingRing.KVStore, ring.GetCodec(), - kv.RegistererWithKVName(reg, "store-gateway"), + kv.RegistererWithKVName(prometheus.WrapRegistererWithPrefix("cortex_", reg), "store-gateway"), + logger, ) if err != nil { return nil, errors.Wrap(err, "create KV store client") diff --git a/pkg/storegateway/gateway_ring.go b/pkg/storegateway/gateway_ring.go index 072d3662a8..ec076fc258 100644 --- a/pkg/storegateway/gateway_ring.go +++ b/pkg/storegateway/gateway_ring.go @@ -8,9 +8,9 @@ import ( "github.com/go-kit/kit/log/level" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv" util_log "github.com/cortexproject/cortex/pkg/util/log" ) diff --git a/pkg/storegateway/gateway_test.go b/pkg/storegateway/gateway_test.go index 40bc57eb4b..90ae1fe215 100644 --- a/pkg/storegateway/gateway_test.go +++ b/pkg/storegateway/gateway_test.go @@ -18,6 +18,7 @@ import ( "github.com/go-kit/kit/log" "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv/consul" "github.com/grafana/dskit/services" "github.com/oklog/ulid" "github.com/pkg/errors" @@ -37,7 +38,6 @@ import ( "google.golang.org/grpc/status" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv/consul" "github.com/cortexproject/cortex/pkg/storage/bucket" "github.com/cortexproject/cortex/pkg/storage/bucket/filesystem" cortex_tsdb "github.com/cortexproject/cortex/pkg/storage/tsdb" @@ -130,7 +130,9 @@ func TestStoreGateway_InitialSyncWithDefaultShardingEnabled(t *testing.T) { gatewayCfg := mockGatewayConfig() gatewayCfg.ShardingEnabled = true storageCfg := mockStorageConfig(t) - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + bucketClient := &bucket.ClientMock{} // Setup the initial instance state in the ring. @@ -199,7 +201,9 @@ func TestStoreGateway_InitialSyncFailure(t *testing.T) { gatewayCfg := mockGatewayConfig() gatewayCfg.ShardingEnabled = true storageCfg := mockStorageConfig(t) - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + bucketClient := &bucket.ClientMock{} g, err := newStoreGateway(gatewayCfg, storageCfg, bucketClient, ringStore, defaultLimitsOverrides(t), mockLoggingLevel(), log.NewNopLogger(), nil) @@ -299,10 +303,11 @@ func TestStoreGateway_InitialSyncWithWaitRingStability(t *testing.T) { t.Log("random generator seed:", seed) ctx := context.Background() - ringStore := consul.NewInMemoryClientWithConfig(ring.GetCodec(), consul.Config{ + ringStore, closer := consul.NewInMemoryClientWithConfig(ring.GetCodec(), consul.Config{ MaxCasRetries: 20, CasRetryDelay: 500 * time.Millisecond, - }) + }, log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) // Create the configured number of gateways. var gateways []*StoreGateway @@ -401,7 +406,8 @@ func TestStoreGateway_BlocksSyncWithDefaultSharding_RingTopologyChangedAfterScal t.Log("random generator seed:", seed) ctx := context.Background() - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) // Create the configured number of gateways. var initialGateways []*StoreGateway @@ -563,7 +569,9 @@ func TestStoreGateway_ShouldSupportLoadRingTokensFromFile(t *testing.T) { gatewayCfg.ShardingRing.TokensFilePath = tokensFile.Name() storageCfg := mockStorageConfig(t) - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + bucketClient := &bucket.ClientMock{} bucketClient.MockIter("", []string{}, nil) @@ -688,7 +696,9 @@ func TestStoreGateway_SyncOnRingTopologyChanged(t *testing.T) { storageCfg.BucketStore.SyncInterval = time.Hour // Do not trigger the periodic sync in this test. reg := prometheus.NewPedanticRegistry() - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + bucketClient := &bucket.ClientMock{} bucketClient.MockIter("", []string{}, nil) @@ -747,7 +757,9 @@ func TestStoreGateway_RingLifecyclerShouldAutoForgetUnhealthyInstances(t *testin storageCfg := mockStorageConfig(t) - ringStore := consul.NewInMemoryClient(ring.GetCodec()) + ringStore, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) + bucketClient := &bucket.ClientMock{} bucketClient.MockIter("", []string{}, nil) diff --git a/pkg/storegateway/sharding_strategy_test.go b/pkg/storegateway/sharding_strategy_test.go index 926381a54e..09552be717 100644 --- a/pkg/storegateway/sharding_strategy_test.go +++ b/pkg/storegateway/sharding_strategy_test.go @@ -6,6 +6,7 @@ import ( "time" "github.com/go-kit/kit/log" + "github.com/grafana/dskit/kv/consul" "github.com/grafana/dskit/services" "github.com/oklog/ulid" "github.com/prometheus/client_golang/prometheus" @@ -16,7 +17,6 @@ import ( "github.com/thanos-io/thanos/pkg/extprom" "github.com/cortexproject/cortex/pkg/ring" - "github.com/cortexproject/cortex/pkg/ring/kv/consul" cortex_tsdb "github.com/cortexproject/cortex/pkg/storage/tsdb" ) @@ -242,7 +242,8 @@ func TestDefaultShardingStrategy(t *testing.T) { t.Parallel() ctx := context.Background() - store := consul.NewInMemoryClient(ring.GetCodec()) + store, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) // Initialize the ring state. require.NoError(t, store.CAS(ctx, "test", func(in interface{}) (interface{}, bool, error) { @@ -599,7 +600,8 @@ func TestShuffleShardingStrategy(t *testing.T) { t.Parallel() ctx := context.Background() - store := consul.NewInMemoryClient(ring.GetCodec()) + store, closer := consul.NewInMemoryClient(ring.GetCodec(), log.NewNopLogger()) + t.Cleanup(func() { assert.NoError(t, closer.Close()) }) // Initialize the ring state. require.NoError(t, store.CAS(ctx, "test", func(in interface{}) (interface{}, bool, error) { diff --git a/tools/doc-generator/main.go b/tools/doc-generator/main.go index 946760de96..a6fb85d3d6 100644 --- a/tools/doc-generator/main.go +++ b/tools/doc-generator/main.go @@ -9,6 +9,9 @@ import ( "strings" "text/template" + "github.com/grafana/dskit/kv/consul" + "github.com/grafana/dskit/kv/etcd" + "github.com/grafana/dskit/kv/memberlist" "github.com/weaveworks/common/server" "github.com/cortexproject/cortex/pkg/alertmanager" @@ -29,9 +32,6 @@ import ( "github.com/cortexproject/cortex/pkg/querier" "github.com/cortexproject/cortex/pkg/querier/queryrange" querier_worker "github.com/cortexproject/cortex/pkg/querier/worker" - "github.com/cortexproject/cortex/pkg/ring/kv/consul" - "github.com/cortexproject/cortex/pkg/ring/kv/etcd" - "github.com/cortexproject/cortex/pkg/ring/kv/memberlist" "github.com/cortexproject/cortex/pkg/ruler" "github.com/cortexproject/cortex/pkg/ruler/rulestore" "github.com/cortexproject/cortex/pkg/storage/bucket/s3" diff --git a/vendor/github.com/grafana/dskit/closer/closer.go b/vendor/github.com/grafana/dskit/closer/closer.go new file mode 100644 index 0000000000..3402f5d579 --- /dev/null +++ b/vendor/github.com/grafana/dskit/closer/closer.go @@ -0,0 +1,9 @@ +package closer + +// Func is like http.HandlerFunc but for io.Closers. +type Func func() error + +// Close implements io.Closer. +func (f Func) Close() error { + return f() +} diff --git a/pkg/ring/kv/client.go b/vendor/github.com/grafana/dskit/kv/client.go similarity index 89% rename from pkg/ring/kv/client.go rename to vendor/github.com/grafana/dskit/kv/client.go index a7720434b2..e26efc5cd5 100644 --- a/pkg/ring/kv/client.go +++ b/vendor/github.com/grafana/dskit/kv/client.go @@ -6,12 +6,13 @@ import ( "fmt" "sync" + "github.com/go-kit/kit/log" "github.com/prometheus/client_golang/prometheus" - "github.com/cortexproject/cortex/pkg/ring/kv/codec" - "github.com/cortexproject/cortex/pkg/ring/kv/consul" - "github.com/cortexproject/cortex/pkg/ring/kv/etcd" - "github.com/cortexproject/cortex/pkg/ring/kv/memberlist" + "github.com/grafana/dskit/kv/codec" + "github.com/grafana/dskit/kv/consul" + "github.com/grafana/dskit/kv/etcd" + "github.com/grafana/dskit/kv/memberlist" ) const ( @@ -114,30 +115,30 @@ type Client interface { // NewClient creates a new Client (consul, etcd or inmemory) based on the config, // encodes and decodes data for storage using the codec. -func NewClient(cfg Config, codec codec.Codec, reg prometheus.Registerer) (Client, error) { +func NewClient(cfg Config, codec codec.Codec, reg prometheus.Registerer, logger log.Logger) (Client, error) { if cfg.Mock != nil { return cfg.Mock, nil } - return createClient(cfg.Store, cfg.Prefix, cfg.StoreConfig, codec, Primary, reg) + return createClient(cfg.Store, cfg.Prefix, cfg.StoreConfig, codec, Primary, reg, logger) } -func createClient(backend string, prefix string, cfg StoreConfig, codec codec.Codec, role role, reg prometheus.Registerer) (Client, error) { +func createClient(backend string, prefix string, cfg StoreConfig, codec codec.Codec, role role, reg prometheus.Registerer, logger log.Logger) (Client, error) { var client Client var err error switch backend { case "consul": - client, err = consul.NewClient(cfg.Consul, codec) + client, err = consul.NewClient(cfg.Consul, codec, logger) case "etcd": - client, err = etcd.New(cfg.Etcd, codec) + client, err = etcd.New(cfg.Etcd, codec, logger) case "inmemory": // If we use the in-memory store, make sure everyone gets the same instance // within the same process. inmemoryStoreInit.Do(func() { - inmemoryStore = consul.NewInMemoryClient(codec) + inmemoryStore, _ = consul.NewInMemoryClient(codec, logger) }) client = inmemoryStore @@ -152,11 +153,11 @@ func createClient(backend string, prefix string, cfg StoreConfig, codec codec.Co } case "multi": - client, err = buildMultiClient(cfg, codec, reg) + client, err = buildMultiClient(cfg, codec, reg, logger) // This case is for testing. The mock KV client does not do anything internally. case "mock": - client, err = buildMockClient() + client, err = buildMockClient(logger) default: return nil, fmt.Errorf("invalid KV store type: %s", backend) @@ -178,7 +179,7 @@ func createClient(backend string, prefix string, cfg StoreConfig, codec codec.Co return newMetricsClient(backend, client, prometheus.WrapRegistererWith(role.Labels(), reg)), nil } -func buildMultiClient(cfg StoreConfig, codec codec.Codec, reg prometheus.Registerer) (Client, error) { +func buildMultiClient(cfg StoreConfig, codec codec.Codec, reg prometheus.Registerer, logger log.Logger) (Client, error) { if cfg.Multi.Primary == "" || cfg.Multi.Secondary == "" { return nil, fmt.Errorf("primary or secondary store not set") } @@ -189,12 +190,12 @@ func buildMultiClient(cfg StoreConfig, codec codec.Codec, reg prometheus.Registe return nil, fmt.Errorf("primary and secondary stores must be different") } - primary, err := createClient(cfg.Multi.Primary, "", cfg, codec, Primary, reg) + primary, err := createClient(cfg.Multi.Primary, "", cfg, codec, Primary, reg, logger) if err != nil { return nil, err } - secondary, err := createClient(cfg.Multi.Secondary, "", cfg, codec, Secondary, reg) + secondary, err := createClient(cfg.Multi.Secondary, "", cfg, codec, Secondary, reg, logger) if err != nil { return nil, err } @@ -204,5 +205,5 @@ func buildMultiClient(cfg StoreConfig, codec codec.Codec, reg prometheus.Registe {client: secondary, name: cfg.Multi.Secondary}, } - return NewMultiClient(cfg.Multi, clients), nil + return NewMultiClient(cfg.Multi, clients, logger), nil } diff --git a/pkg/ring/kv/codec/codec.go b/vendor/github.com/grafana/dskit/kv/codec/codec.go similarity index 100% rename from pkg/ring/kv/codec/codec.go rename to vendor/github.com/grafana/dskit/kv/codec/codec.go diff --git a/pkg/ring/kv/consul/client.go b/vendor/github.com/grafana/dskit/kv/consul/client.go similarity index 88% rename from pkg/ring/kv/consul/client.go rename to vendor/github.com/grafana/dskit/kv/consul/client.go index 11c26df108..8117e37af3 100644 --- a/pkg/ring/kv/consul/client.go +++ b/vendor/github.com/grafana/dskit/kv/consul/client.go @@ -9,15 +9,15 @@ import ( "net/http" "time" + "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" - "github.com/grafana/dskit/backoff" consul "github.com/hashicorp/consul/api" "github.com/hashicorp/go-cleanhttp" "github.com/weaveworks/common/instrument" "golang.org/x/time/rate" - "github.com/cortexproject/cortex/pkg/ring/kv/codec" - util_log "github.com/cortexproject/cortex/pkg/util/log" + "github.com/grafana/dskit/backoff" + "github.com/grafana/dskit/kv/codec" ) const ( @@ -61,8 +61,9 @@ type kv interface { // Client is a KV.Client for Consul. type Client struct { kv - codec codec.Codec - cfg Config + codec codec.Codec + cfg Config + logger log.Logger } // RegisterFlags adds the flags required to config this to the given FlagSet @@ -77,7 +78,7 @@ func (cfg *Config) RegisterFlags(f *flag.FlagSet, prefix string) { } // NewClient returns a new Client. -func NewClient(cfg Config, codec codec.Codec) (*Client, error) { +func NewClient(cfg Config, codec codec.Codec, logger log.Logger) (*Client, error) { client, err := consul.NewClient(&consul.Config{ Address: cfg.Host, Token: cfg.ACLToken, @@ -92,9 +93,10 @@ func NewClient(cfg Config, codec codec.Codec) (*Client, error) { return nil, err } c := &Client{ - kv: consulMetrics{client.KV()}, - codec: codec, - cfg: cfg, + kv: consulMetrics{client.KV()}, + codec: codec, + cfg: cfg, + logger: logger, } return c, nil } @@ -144,14 +146,14 @@ func (c *Client) cas(ctx context.Context, key string, f func(in interface{}) (ou options := &consul.QueryOptions{} kvp, _, err := c.kv.Get(key, options.WithContext(ctx)) if err != nil { - level.Error(util_log.Logger).Log("msg", "error getting key", "key", key, "err", err) + level.Error(c.logger).Log("msg", "error getting key", "key", key, "err", err) continue } var intermediate interface{} if kvp != nil { out, err := c.codec.Decode(kvp.Value) if err != nil { - level.Error(util_log.Logger).Log("msg", "error decoding key", "key", key, "err", err) + level.Error(c.logger).Log("msg", "error decoding key", "key", key, "err", err) continue } // If key doesn't exist, index will be 0. @@ -175,7 +177,7 @@ func (c *Client) cas(ctx context.Context, key string, f func(in interface{}) (ou bytes, err := c.codec.Encode(intermediate) if err != nil { - level.Error(util_log.Logger).Log("msg", "error serialising value", "key", key, "err", err) + level.Error(c.logger).Log("msg", "error serialising value", "key", key, "err", err) continue } ok, _, err := c.kv.CAS(&consul.KVPair{ @@ -184,11 +186,11 @@ func (c *Client) cas(ctx context.Context, key string, f func(in interface{}) (ou ModifyIndex: index, }, writeOptions.WithContext(ctx)) if err != nil { - level.Error(util_log.Logger).Log("msg", "error CASing", "key", key, "err", err) + level.Error(c.logger).Log("msg", "error CASing", "key", key, "err", err) continue } if !ok { - level.Debug(util_log.Logger).Log("msg", "error CASing, trying again", "key", key, "index", index) + level.Debug(c.logger).Log("msg", "error CASing, trying again", "key", key, "index", index) continue } return nil @@ -214,7 +216,7 @@ func (c *Client) WatchKey(ctx context.Context, key string, f func(interface{}) b if errors.Is(err, context.Canceled) { break } - level.Error(util_log.Logger).Log("msg", "error while rate-limiting", "key", key, "err", err) + level.Error(c.logger).Log("msg", "error while rate-limiting", "key", key, "err", err) backoff.Wait() continue } @@ -231,7 +233,7 @@ func (c *Client) WatchKey(ctx context.Context, key string, f func(interface{}) b // Don't backoff if value is not found (kvp == nil). In that case, Consul still returns index value, // and next call to Get will block as expected. We handle missing value below. if err != nil { - level.Error(util_log.Logger).Log("msg", "error getting path", "key", key, "err", err) + level.Error(c.logger).Log("msg", "error getting path", "key", key, "err", err) backoff.Wait() continue } @@ -244,13 +246,13 @@ func (c *Client) WatchKey(ctx context.Context, key string, f func(interface{}) b } if kvp == nil { - level.Info(util_log.Logger).Log("msg", "value is nil", "key", key, "index", index) + level.Info(c.logger).Log("msg", "value is nil", "key", key, "index", index) continue } out, err := c.codec.Decode(kvp.Value) if err != nil { - level.Error(util_log.Logger).Log("msg", "error decoding key", "key", key, "err", err) + level.Error(c.logger).Log("msg", "error decoding key", "key", key, "err", err) continue } if !f(out) { @@ -274,7 +276,7 @@ func (c *Client) WatchPrefix(ctx context.Context, prefix string, f func(string, if errors.Is(err, context.Canceled) { break } - level.Error(util_log.Logger).Log("msg", "error while rate-limiting", "prefix", prefix, "err", err) + level.Error(c.logger).Log("msg", "error while rate-limiting", "prefix", prefix, "err", err) backoff.Wait() continue } @@ -290,7 +292,7 @@ func (c *Client) WatchPrefix(ctx context.Context, prefix string, f func(string, // kvps being nil here is not an error -- quite the opposite. Consul returns index, // which makes next query blocking, so there is no need to detect this and act on it. if err != nil { - level.Error(util_log.Logger).Log("msg", "error getting path", "prefix", prefix, "err", err) + level.Error(c.logger).Log("msg", "error getting path", "prefix", prefix, "err", err) backoff.Wait() continue } @@ -310,7 +312,7 @@ func (c *Client) WatchPrefix(ctx context.Context, prefix string, f func(string, out, err := c.codec.Decode(kvp.Value) if err != nil { - level.Error(util_log.Logger).Log("msg", "error decoding list of values for prefix:key", "prefix", prefix, "key", kvp.Key, "err", err) + level.Error(c.logger).Log("msg", "error decoding list of values for prefix:key", "prefix", prefix, "key", kvp.Key, "err", err) continue } if !f(kvp.Key, out) { diff --git a/pkg/ring/kv/consul/metrics.go b/vendor/github.com/grafana/dskit/kv/consul/metrics.go similarity index 94% rename from pkg/ring/kv/consul/metrics.go rename to vendor/github.com/grafana/dskit/kv/consul/metrics.go index 99910a16a3..0b29405677 100644 --- a/pkg/ring/kv/consul/metrics.go +++ b/vendor/github.com/grafana/dskit/kv/consul/metrics.go @@ -9,10 +9,9 @@ import ( ) var consulRequestDuration = instrument.NewHistogramCollector(prometheus.NewHistogramVec(prometheus.HistogramOpts{ - Namespace: "cortex", - Name: "consul_request_duration_seconds", - Help: "Time spent on consul requests.", - Buckets: prometheus.DefBuckets, + Name: "consul_request_duration_seconds", + Help: "Time spent on consul requests.", + Buckets: prometheus.DefBuckets, }, []string{"operation", "status_code"})) func init() { diff --git a/pkg/ring/kv/consul/mock.go b/vendor/github.com/grafana/dskit/kv/consul/mock.go similarity index 71% rename from pkg/ring/kv/consul/mock.go rename to vendor/github.com/grafana/dskit/kv/consul/mock.go index 5d1e455739..d22381bfe2 100644 --- a/pkg/ring/kv/consul/mock.go +++ b/vendor/github.com/grafana/dskit/kv/consul/mock.go @@ -2,15 +2,17 @@ package consul import ( "fmt" + "io" "strings" "sync" "time" + "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" consul "github.com/hashicorp/consul/api" - "github.com/cortexproject/cortex/pkg/ring/kv/codec" - util_log "github.com/cortexproject/cortex/pkg/util/log" + "github.com/grafana/dskit/closer" + "github.com/grafana/dskit/kv/codec" ) type mockKV struct { @@ -18,28 +20,49 @@ type mockKV struct { cond *sync.Cond kvps map[string]*consul.KVPair current uint64 // the current 'index in the log' + logger log.Logger + + // Channel closed once the in-memory consul mock should be closed. + close chan struct{} + closeWG sync.WaitGroup } // NewInMemoryClient makes a new mock consul client. -func NewInMemoryClient(codec codec.Codec) *Client { - return NewInMemoryClientWithConfig(codec, Config{}) +func NewInMemoryClient(codec codec.Codec, logger log.Logger) (*Client, io.Closer) { + return NewInMemoryClientWithConfig(codec, Config{}, logger) } // NewInMemoryClientWithConfig makes a new mock consul client with supplied Config. -func NewInMemoryClientWithConfig(codec codec.Codec, cfg Config) *Client { +func NewInMemoryClientWithConfig(codec codec.Codec, cfg Config, logger log.Logger) (*Client, io.Closer) { m := mockKV{ kvps: map[string]*consul.KVPair{}, // Always start from 1, we NEVER want to report back index 0 in the responses. // This is in line with Consul, and our new checks for index return value in client.go. current: 1, + logger: logger, + close: make(chan struct{}), } m.cond = sync.NewCond(&m.mtx) + + // Create a closer function used to close the main loop and wait until it's done. + // We need to wait until done, otherwise the goroutine leak finder used in tests + // may still report it as leaked. + closer := closer.Func(func() error { + close(m.close) + m.closeWG.Wait() + return nil + }) + + // Start the main loop in a dedicated goroutine. + m.closeWG.Add(1) go m.loop() + return &Client{ - kv: &m, - codec: codec, - cfg: cfg, - } + kv: &m, + codec: codec, + cfg: cfg, + logger: logger, + }, closer } func copyKVPair(in *consul.KVPair) *consul.KVPair { @@ -51,10 +74,17 @@ func copyKVPair(in *consul.KVPair) *consul.KVPair { // periodic loop to wake people up, so they can honour timeouts func (m *mockKV) loop() { - for range time.Tick(1 * time.Second) { - m.mtx.Lock() - m.cond.Broadcast() - m.mtx.Unlock() + defer m.closeWG.Done() + + for { + select { + case <-m.close: + return + case <-time.After(time.Second): + m.mtx.Lock() + m.cond.Broadcast() + m.mtx.Unlock() + } } } @@ -78,12 +108,12 @@ func (m *mockKV) Put(p *consul.KVPair, q *consul.WriteOptions) (*consul.WriteMet m.cond.Broadcast() - level.Debug(util_log.Logger).Log("msg", "Put", "key", p.Key, "value", fmt.Sprintf("%.40q", p.Value), "modify_index", m.current) + level.Debug(m.logger).Log("msg", "Put", "key", p.Key, "value", fmt.Sprintf("%.40q", p.Value), "modify_index", m.current) return nil, nil } func (m *mockKV) CAS(p *consul.KVPair, q *consul.WriteOptions) (bool, *consul.WriteMeta, error) { - level.Debug(util_log.Logger).Log("msg", "CAS", "key", p.Key, "modify_index", p.ModifyIndex, "value", fmt.Sprintf("%.40q", p.Value)) + level.Debug(m.logger).Log("msg", "CAS", "key", p.Key, "modify_index", p.ModifyIndex, "value", fmt.Sprintf("%.40q", p.Value)) m.mtx.Lock() defer m.mtx.Unlock() @@ -110,14 +140,14 @@ func (m *mockKV) CAS(p *consul.KVPair, q *consul.WriteOptions) (bool, *consul.Wr } func (m *mockKV) Get(key string, q *consul.QueryOptions) (*consul.KVPair, *consul.QueryMeta, error) { - level.Debug(util_log.Logger).Log("msg", "Get", "key", key, "wait_index", q.WaitIndex) + level.Debug(m.logger).Log("msg", "Get", "key", key, "wait_index", q.WaitIndex) m.mtx.Lock() defer m.mtx.Unlock() value := m.kvps[key] if value == nil && q.WaitIndex == 0 { - level.Debug(util_log.Logger).Log("msg", "Get - not found", "key", key) + level.Debug(m.logger).Log("msg", "Get - not found", "key", key) return nil, &consul.QueryMeta{LastIndex: m.current}, nil } @@ -146,17 +176,17 @@ func (m *mockKV) Get(key string, q *consul.QueryOptions) (*consul.KVPair, *consu } } if time.Now().After(deadline) { - level.Debug(util_log.Logger).Log("msg", "Get - deadline exceeded", "key", key) + level.Debug(m.logger).Log("msg", "Get - deadline exceeded", "key", key) return nil, &consul.QueryMeta{LastIndex: q.WaitIndex}, nil } } if value == nil { - level.Debug(util_log.Logger).Log("msg", "Get - not found", "key", key) + level.Debug(m.logger).Log("msg", "Get - not found", "key", key) return nil, &consul.QueryMeta{LastIndex: m.current}, nil } - level.Debug(util_log.Logger).Log("msg", "Get", "key", key, "modify_index", value.ModifyIndex, "value", fmt.Sprintf("%.40q", value.Value)) + level.Debug(m.logger).Log("msg", "Get", "key", key, "modify_index", value.ModifyIndex, "value", fmt.Sprintf("%.40q", value.Value)) return copyKVPair(value), &consul.QueryMeta{LastIndex: value.ModifyIndex}, nil } @@ -203,7 +233,7 @@ func (m *mockKV) ResetIndex() { m.current = 0 m.cond.Broadcast() - level.Debug(util_log.Logger).Log("msg", "Reset") + level.Debug(m.logger).Log("msg", "Reset") } func (m *mockKV) ResetIndexForKey(key string) { @@ -215,7 +245,7 @@ func (m *mockKV) ResetIndexForKey(key string) { } m.cond.Broadcast() - level.Debug(util_log.Logger).Log("msg", "ResetIndexForKey", "key", key) + level.Debug(m.logger).Log("msg", "ResetIndexForKey", "key", key) } // mockedMaxWaitTime returns the minimum duration between the input duration diff --git a/pkg/ring/kv/etcd/etcd.go b/vendor/github.com/grafana/dskit/kv/etcd/etcd.go similarity index 82% rename from pkg/ring/kv/etcd/etcd.go rename to vendor/github.com/grafana/dskit/kv/etcd/etcd.go index 35af81c1ed..6690f822fb 100644 --- a/pkg/ring/kv/etcd/etcd.go +++ b/vendor/github.com/grafana/dskit/kv/etcd/etcd.go @@ -7,25 +7,25 @@ import ( "fmt" "time" + "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" - "github.com/grafana/dskit/backoff" - "github.com/grafana/dskit/flagext" "github.com/pkg/errors" clientv3 "go.etcd.io/etcd/client/v3" "go.etcd.io/etcd/pkg/transport" - "github.com/cortexproject/cortex/pkg/ring/kv/codec" - util_log "github.com/cortexproject/cortex/pkg/util/log" - cortex_tls "github.com/cortexproject/cortex/pkg/util/tls" + "github.com/grafana/dskit/backoff" + "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv/codec" + "github.com/grafana/dskit/kv/kvtls" ) // Config for a new etcd.Client. type Config struct { - Endpoints []string `yaml:"endpoints"` - DialTimeout time.Duration `yaml:"dial_timeout"` - MaxRetries int `yaml:"max_retries"` - EnableTLS bool `yaml:"tls_enabled"` - TLS cortex_tls.ClientConfig `yaml:",inline"` + Endpoints []string `yaml:"endpoints"` + DialTimeout time.Duration `yaml:"dial_timeout"` + MaxRetries int `yaml:"max_retries"` + EnableTLS bool `yaml:"tls_enabled"` + TLS kvtls.ClientConfig `yaml:",inline"` UserName string `yaml:"username"` Password string `yaml:"password"` @@ -33,9 +33,10 @@ type Config struct { // Client implements ring.KVClient for etcd. type Client struct { - cfg Config - codec codec.Codec - cli *clientv3.Client + cfg Config + codec codec.Codec + cli *clientv3.Client + logger log.Logger } // RegisterFlagsWithPrefix adds the flags required to config this to the given FlagSet. @@ -66,7 +67,7 @@ func (cfg *Config) GetTLS() (*tls.Config, error) { } // New makes a new Client. -func New(cfg Config, codec codec.Codec) (*Client, error) { +func New(cfg Config, codec codec.Codec, logger log.Logger) (*Client, error) { tlsConfig, err := cfg.GetTLS() if err != nil { return nil, errors.Wrapf(err, "unable to initialise TLS configuration for etcd") @@ -100,9 +101,10 @@ func New(cfg Config, codec codec.Codec) (*Client, error) { } return &Client{ - cfg: cfg, - codec: codec, - cli: cli, + cfg: cfg, + codec: codec, + cli: cli, + logger: logger, }, nil } @@ -114,7 +116,7 @@ func (c *Client) CAS(ctx context.Context, key string, f func(in interface{}) (ou for i := 0; i < c.cfg.MaxRetries; i++ { resp, err := c.cli.Get(ctx, key) if err != nil { - level.Error(util_log.Logger).Log("msg", "error getting key", "key", key, "err", err) + level.Error(c.logger).Log("msg", "error getting key", "key", key, "err", err) lastErr = err continue } @@ -123,7 +125,7 @@ func (c *Client) CAS(ctx context.Context, key string, f func(in interface{}) (ou if len(resp.Kvs) > 0 { intermediate, err = c.codec.Decode(resp.Kvs[0].Value) if err != nil { - level.Error(util_log.Logger).Log("msg", "error decoding key", "key", key, "err", err) + level.Error(c.logger).Log("msg", "error decoding key", "key", key, "err", err) lastErr = err continue } @@ -147,7 +149,7 @@ func (c *Client) CAS(ctx context.Context, key string, f func(in interface{}) (ou buf, err := c.codec.Encode(intermediate) if err != nil { - level.Error(util_log.Logger).Log("msg", "error serialising value", "key", key, "err", err) + level.Error(c.logger).Log("msg", "error serialising value", "key", key, "err", err) lastErr = err continue } @@ -157,13 +159,13 @@ func (c *Client) CAS(ctx context.Context, key string, f func(in interface{}) (ou Then(clientv3.OpPut(key, string(buf))). Commit() if err != nil { - level.Error(util_log.Logger).Log("msg", "error CASing", "key", key, "err", err) + level.Error(c.logger).Log("msg", "error CASing", "key", key, "err", err) lastErr = err continue } // result is not Succeeded if the the comparison was false, meaning if the modify indexes did not match. if !result.Succeeded { - level.Debug(util_log.Logger).Log("msg", "failed to CAS, revision and version did not match in etcd", "key", key, "revision", revision) + level.Debug(c.logger).Log("msg", "failed to CAS, revision and version did not match in etcd", "key", key, "revision", revision) continue } @@ -191,7 +193,7 @@ outer: for backoff.Ongoing() { for resp := range c.cli.Watch(watchCtx, key) { if err := resp.Err(); err != nil { - level.Error(util_log.Logger).Log("msg", "watch error", "key", key, "err", err) + level.Error(c.logger).Log("msg", "watch error", "key", key, "err", err) continue outer } @@ -200,7 +202,7 @@ outer: for _, event := range resp.Events { out, err := c.codec.Decode(event.Kv.Value) if err != nil { - level.Error(util_log.Logger).Log("msg", "error decoding key", "key", key, "err", err) + level.Error(c.logger).Log("msg", "error decoding key", "key", key, "err", err) continue } @@ -227,7 +229,7 @@ outer: for backoff.Ongoing() { for resp := range c.cli.Watch(watchCtx, key, clientv3.WithPrefix()) { if err := resp.Err(); err != nil { - level.Error(util_log.Logger).Log("msg", "watch error", "key", key, "err", err) + level.Error(c.logger).Log("msg", "watch error", "key", key, "err", err) continue outer } @@ -235,13 +237,13 @@ outer: for _, event := range resp.Events { if event.Kv.Version == 0 && event.Kv.Value == nil { - // Delete notification. Since not all KV store clients (and Cortex codecs) support this, we ignore it. + // Delete notification. Since not all KV store clients (and codecs) support this, we ignore it. continue } out, err := c.codec.Decode(event.Kv.Value) if err != nil { - level.Error(util_log.Logger).Log("msg", "error decoding key", "key", key, "err", err) + level.Error(c.logger).Log("msg", "error decoding key", "key", key, "err", err) continue } diff --git a/pkg/ring/kv/etcd/mock.go b/vendor/github.com/grafana/dskit/kv/etcd/mock.go similarity index 72% rename from pkg/ring/kv/etcd/mock.go rename to vendor/github.com/grafana/dskit/kv/etcd/mock.go index 28ec7840f2..27ad71f2ef 100644 --- a/pkg/ring/kv/etcd/mock.go +++ b/vendor/github.com/grafana/dskit/kv/etcd/mock.go @@ -8,18 +8,20 @@ import ( "net/url" "time" - "github.com/grafana/dskit/flagext" + "github.com/go-kit/kit/log" "go.etcd.io/etcd/server/v3/embed" "go.etcd.io/etcd/server/v3/etcdserver/api/v3client" - "github.com/cortexproject/cortex/pkg/ring/kv/codec" + "github.com/grafana/dskit/closer" + "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv/codec" ) const etcdStartTimeout = 30 * time.Second // Mock returns a Mock Etcd client. // Inspired by https://github.com/ligato/cn-infra/blob/master/db/keyval/etcd/mocks/embeded_etcd.go. -func Mock(codec codec.Codec) (*Client, io.Closer, error) { +func Mock(codec codec.Codec, logger log.Logger) (*Client, io.Closer, error) { dir, err := ioutil.TempDir("", "etcd") if err != nil { return nil, nil, err @@ -45,7 +47,7 @@ func Mock(codec codec.Codec) (*Client, io.Closer, error) { return nil, nil, fmt.Errorf("server took too long to start") } - closer := CloserFunc(func() error { + closer := closer.Func(func() error { etcd.Server.Stop() return nil }) @@ -54,27 +56,15 @@ func Mock(codec codec.Codec) (*Client, io.Closer, error) { flagext.DefaultValues(&config) client := &Client{ - cfg: config, - codec: codec, - cli: v3client.New(etcd.Server), + cfg: config, + codec: codec, + cli: v3client.New(etcd.Server), + logger: logger, } return client, closer, nil } -// CloserFunc is like http.HandlerFunc but for io.Closers. -type CloserFunc func() error - -// Close implements io.Closer. -func (f CloserFunc) Close() error { - return f() -} - -// NopCloser does nothing. -var NopCloser = CloserFunc(func() error { - return nil -}) - // RegisterFlags adds the flags required to config this to the given FlagSet. func (cfg *Config) RegisterFlags(f *flag.FlagSet) { cfg.RegisterFlagsWithPrefix(f, "") diff --git a/vendor/github.com/grafana/dskit/kv/kvtls/tls.go b/vendor/github.com/grafana/dskit/kv/kvtls/tls.go new file mode 100644 index 0000000000..2338e02ba5 --- /dev/null +++ b/vendor/github.com/grafana/dskit/kv/kvtls/tls.go @@ -0,0 +1,87 @@ +package kvtls + +import ( + "crypto/tls" + "crypto/x509" + "flag" + "io/ioutil" + + "github.com/pkg/errors" + "google.golang.org/grpc" + "google.golang.org/grpc/credentials" +) + +// ClientConfig is the config for client TLS. +type ClientConfig struct { + CertPath string `yaml:"tls_cert_path"` + KeyPath string `yaml:"tls_key_path"` + CAPath string `yaml:"tls_ca_path"` + ServerName string `yaml:"tls_server_name"` + InsecureSkipVerify bool `yaml:"tls_insecure_skip_verify"` +} + +var ( + errKeyMissing = errors.New("certificate given but no key configured") + errCertMissing = errors.New("key given but no certificate configured") +) + +// RegisterFlagsWithPrefix registers flags with prefix. +func (cfg *ClientConfig) RegisterFlagsWithPrefix(prefix string, f *flag.FlagSet) { + f.StringVar(&cfg.CertPath, prefix+".tls-cert-path", "", "Path to the client certificate file, which will be used for authenticating with the server. Also requires the key path to be configured.") + f.StringVar(&cfg.KeyPath, prefix+".tls-key-path", "", "Path to the key file for the client certificate. Also requires the client certificate to be configured.") + f.StringVar(&cfg.CAPath, prefix+".tls-ca-path", "", "Path to the CA certificates file to validate server certificate against. If not set, the host's root CA certificates are used.") + f.StringVar(&cfg.ServerName, prefix+".tls-server-name", "", "Override the expected name on the server certificate.") + f.BoolVar(&cfg.InsecureSkipVerify, prefix+".tls-insecure-skip-verify", false, "Skip validating server certificate.") +} + +// GetTLSConfig initialises tls.Config from config options +func (cfg *ClientConfig) GetTLSConfig() (*tls.Config, error) { + config := &tls.Config{ + InsecureSkipVerify: cfg.InsecureSkipVerify, + ServerName: cfg.ServerName, + } + + // read ca certificates + if cfg.CAPath != "" { + var caCertPool *x509.CertPool + caCert, err := ioutil.ReadFile(cfg.CAPath) + if err != nil { + return nil, errors.Wrapf(err, "error loading ca cert: %s", cfg.CAPath) + } + caCertPool = x509.NewCertPool() + caCertPool.AppendCertsFromPEM(caCert) + + config.RootCAs = caCertPool + } + + // read client certificate + if cfg.CertPath != "" || cfg.KeyPath != "" { + if cfg.CertPath == "" { + return nil, errCertMissing + } + if cfg.KeyPath == "" { + return nil, errKeyMissing + } + clientCert, err := tls.LoadX509KeyPair(cfg.CertPath, cfg.KeyPath) + if err != nil { + return nil, errors.Wrapf(err, "failed to load TLS certificate %s,%s", cfg.CertPath, cfg.KeyPath) + } + config.Certificates = []tls.Certificate{clientCert} + } + + return config, nil +} + +// GetGRPCDialOptions creates GRPC DialOptions for TLS +func (cfg *ClientConfig) GetGRPCDialOptions(enabled bool) ([]grpc.DialOption, error) { + if !enabled { + return []grpc.DialOption{grpc.WithInsecure()}, nil + } + + tlsConfig, err := cfg.GetTLSConfig() + if err != nil { + return nil, errors.Wrap(err, "error creating grpc dial options") + } + + return []grpc.DialOption{grpc.WithTransportCredentials(credentials.NewTLS(tlsConfig))}, nil +} diff --git a/pkg/ring/kv/memberlist/broadcast.go b/vendor/github.com/grafana/dskit/kv/memberlist/broadcast.go similarity index 83% rename from pkg/ring/kv/memberlist/broadcast.go rename to vendor/github.com/grafana/dskit/kv/memberlist/broadcast.go index f739b67241..7c0a72cf8b 100644 --- a/pkg/ring/kv/memberlist/broadcast.go +++ b/vendor/github.com/grafana/dskit/kv/memberlist/broadcast.go @@ -3,10 +3,9 @@ package memberlist import ( "fmt" + "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" "github.com/hashicorp/memberlist" - - util_log "github.com/cortexproject/cortex/pkg/util/log" ) // ringBroadcast implements memberlist.Broadcast interface, which is used by memberlist.TransmitLimitedQueue. @@ -16,6 +15,7 @@ type ringBroadcast struct { version uint // local version of the value, generated by merging this change msg []byte // encoded key and value finished func(b ringBroadcast) + logger log.Logger } func (r ringBroadcast) Invalidates(old memberlist.Broadcast) bool { @@ -45,7 +45,7 @@ func (r ringBroadcast) Invalidates(old memberlist.Broadcast) bool { // otherwise, we may be invalidating some older messages, which however covered different // ingesters if r.version >= oldb.version { - level.Debug(util_log.Logger).Log("msg", "Invalidating forwarded broadcast", "key", r.key, "version", r.version, "oldVersion", oldb.version, "content", fmt.Sprintf("%v", r.content), "oldContent", fmt.Sprintf("%v", oldb.content)) + level.Debug(r.logger).Log("msg", "Invalidating forwarded broadcast", "key", r.key, "version", r.version, "oldVersion", oldb.version, "content", fmt.Sprintf("%v", r.content), "oldContent", fmt.Sprintf("%v", oldb.content)) return true } } @@ -60,5 +60,4 @@ func (r ringBroadcast) Finished() { if r.finished != nil { r.finished(r) } - r.msg = nil } diff --git a/vendor/github.com/grafana/dskit/kv/memberlist/dnsprovider.go b/vendor/github.com/grafana/dskit/kv/memberlist/dnsprovider.go new file mode 100644 index 0000000000..b51a5d0553 --- /dev/null +++ b/vendor/github.com/grafana/dskit/kv/memberlist/dnsprovider.go @@ -0,0 +1,15 @@ +package memberlist + +import ( + "context" +) + +// DNSProvider supports storing or resolving a list of addresses. +type DNSProvider interface { + // Resolve stores a list of provided addresses or their DNS records if requested. + // Implementations may have specific ways of interpreting addresses. + Resolve(ctx context.Context, addrs []string) error + + // Addresses returns the latest addresses present in the DNSProvider. + Addresses() []string +} diff --git a/pkg/ring/kv/memberlist/kv.pb.go b/vendor/github.com/grafana/dskit/kv/memberlist/kv.pb.go similarity index 100% rename from pkg/ring/kv/memberlist/kv.pb.go rename to vendor/github.com/grafana/dskit/kv/memberlist/kv.pb.go diff --git a/pkg/ring/kv/memberlist/kv.proto b/vendor/github.com/grafana/dskit/kv/memberlist/kv.proto similarity index 100% rename from pkg/ring/kv/memberlist/kv.proto rename to vendor/github.com/grafana/dskit/kv/memberlist/kv.proto diff --git a/pkg/ring/kv/memberlist/kv_init_service.go b/vendor/github.com/grafana/dskit/kv/memberlist/kv_init_service.go similarity index 85% rename from pkg/ring/kv/memberlist/kv_init_service.go rename to vendor/github.com/grafana/dskit/kv/memberlist/kv_init_service.go index 32427ed2a7..f1e053b13e 100644 --- a/pkg/ring/kv/memberlist/kv_init_service.go +++ b/vendor/github.com/grafana/dskit/kv/memberlist/kv_init_service.go @@ -13,21 +13,21 @@ import ( "time" "github.com/go-kit/kit/log" - "github.com/grafana/dskit/services" "github.com/hashicorp/memberlist" "go.uber.org/atomic" - "github.com/cortexproject/cortex/pkg/util" + "github.com/grafana/dskit/services" ) -// This service initialized memberlist.KV on first call to GetMemberlistKV, and starts it. On stop, +// KVInitService initializes a memberlist.KV on first call to GetMemberlistKV, and starts it. On stop, // KV is stopped too. If KV fails, error is reported from the service. type KVInitService struct { services.Service // config used for initialization - cfg *KVConfig - logger log.Logger + cfg *KVConfig + logger log.Logger + dnsProvider DNSProvider // init function, to avoid multiple initializations. init sync.Once @@ -38,20 +38,21 @@ type KVInitService struct { watcher *services.FailureWatcher } -func NewKVInitService(cfg *KVConfig, logger log.Logger) *KVInitService { +func NewKVInitService(cfg *KVConfig, logger log.Logger, dnsProvider DNSProvider) *KVInitService { kvinit := &KVInitService{ - cfg: cfg, - watcher: services.NewFailureWatcher(), - logger: logger, + cfg: cfg, + watcher: services.NewFailureWatcher(), + logger: logger, + dnsProvider: dnsProvider, } kvinit.Service = services.NewBasicService(nil, kvinit.running, kvinit.stopping).WithName("memberlist KV service") return kvinit } -// This method will initialize Memberlist.KV on first call, and add it to service failure watcher. +// GetMemberlistKV will initialize Memberlist.KV on first call, and add it to service failure watcher. func (kvs *KVInitService) GetMemberlistKV() (*KV, error) { kvs.init.Do(func() { - kv := NewKV(*kvs.cfg, kvs.logger) + kv := NewKV(*kvs.cfg, kvs.logger, kvs.dnsProvider) kvs.watcher.WatchService(kv) kvs.err = kv.StartAsync(context.Background()) @@ -92,7 +93,9 @@ func (kvs *KVInitService) stopping(_ error) error { func (kvs *KVInitService) ServeHTTP(w http.ResponseWriter, req *http.Request) { kv := kvs.getKV() if kv == nil { - util.WriteTextResponse(w, "This Cortex instance doesn't use memberlist.") + w.Header().Set("Content-Type", "text/plain") + // Ignore inactionable errors. + _, _ = w.Write([]byte("This instance doesn't use memberlist.")) return } @@ -151,14 +154,36 @@ func (kvs *KVInitService) ServeHTTP(w http.ResponseWriter, req *http.Request) { sent, received := kv.getSentAndReceivedMessages() - util.RenderHTTPResponse(w, pageData{ + v := pageData{ Now: time.Now(), Memberlist: kv.memberlist, SortedMembers: members, Store: kv.storeCopy(), SentMessages: sent, ReceivedMessages: received, - }, pageTemplate, req) + } + + accept := req.Header.Get("Accept") + if strings.Contains(accept, "application/json") { + w.Header().Set("Content-Type", "application/json") + + data, err := json.Marshal(v) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + return + } + + // We ignore errors here, because we cannot do anything about them. + // Write will trigger sending Status code, so we cannot send a different status code afterwards. + // Also this isn't internal error, but error communicating with client. + _, _ = w.Write(data) + return + } + + err := pageTemplate.Execute(w, v) + if err != nil { + http.Error(w, err.Error(), http.StatusInternalServerError) + } } func getFormat(req *http.Request) string { @@ -266,10 +291,10 @@ const pageContent = ` - Cortex Memberlist Status + Memberlist Status -

Cortex Memberlist Status

+

Memberlist Status

Current time: {{ .Now }}

    @@ -304,7 +329,7 @@ const pageContent = ` -

    Note that value "version" is node-specific. It starts with 0 (on restart), and increases on each received update. Size is in bytes.

    +

    Note that value "version" is node-specific. It starts with 0 (on restart), and increases on each received update. Size is in bytes.

    Memberlist Cluster Members

    @@ -355,7 +380,7 @@ const pageContent = ` {{ .Pair.Key }} size: {{ .Pair.Value | len }}, codec: {{ .Pair.Codec }} {{ .Version }} - {{ StringsJoin .Changes ", " }} + {{ StringsJoin .Changes ", " }} json | json-pretty @@ -391,7 +416,7 @@ const pageContent = ` {{ .Pair.Key }} size: {{ .Pair.Value | len }}, codec: {{ .Pair.Codec }} {{ .Version }} - {{ StringsJoin .Changes ", " }} + {{ StringsJoin .Changes ", " }} json | json-pretty diff --git a/pkg/ring/kv/memberlist/memberlist_client.go b/vendor/github.com/grafana/dskit/kv/memberlist/memberlist_client.go similarity index 97% rename from pkg/ring/kv/memberlist/memberlist_client.go rename to vendor/github.com/grafana/dskit/kv/memberlist/memberlist_client.go index 0d5afe9b58..4faad94f32 100644 --- a/pkg/ring/kv/memberlist/memberlist_client.go +++ b/vendor/github.com/grafana/dskit/kv/memberlist/memberlist_client.go @@ -15,16 +15,13 @@ import ( "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" - "github.com/grafana/dskit/backoff" - "github.com/grafana/dskit/flagext" - "github.com/grafana/dskit/services" "github.com/hashicorp/memberlist" "github.com/prometheus/client_golang/prometheus" - "github.com/thanos-io/thanos/pkg/discovery/dns" - "github.com/thanos-io/thanos/pkg/extprom" - "github.com/cortexproject/cortex/pkg/ring/kv/codec" - util_log "github.com/cortexproject/cortex/pkg/util/log" + "github.com/grafana/dskit/backoff" + "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv/codec" + "github.com/grafana/dskit/services" ) const ( @@ -165,7 +162,7 @@ type KVConfig struct { Codecs []codec.Codec `yaml:"-"` } -// RegisterFlags registers flags. +// RegisterFlagsWithPrefix registers flags. func (cfg *KVConfig) RegisterFlagsWithPrefix(f *flag.FlagSet, prefix string) { mlDefaults := defaultMemberlistConfig() @@ -174,7 +171,7 @@ func (cfg *KVConfig) RegisterFlagsWithPrefix(f *flag.FlagSet, prefix string) { f.BoolVar(&cfg.RandomizeNodeName, prefix+"memberlist.randomize-node-name", true, "Add random suffix to the node name.") f.DurationVar(&cfg.StreamTimeout, prefix+"memberlist.stream-timeout", mlDefaults.TCPTimeout, "The timeout for establishing a connection with a remote node, and for read/write operations.") f.IntVar(&cfg.RetransmitMult, prefix+"memberlist.retransmit-factor", mlDefaults.RetransmitMult, "Multiplication factor used when sending out messages (factor * log(N+1)).") - f.Var(&cfg.JoinMembers, prefix+"memberlist.join", "Other cluster members to join. Can be specified multiple times. It can be an IP, hostname or an entry specified in the DNS Service Discovery format (see https://cortexmetrics.io/docs/configuration/arguments/#dns-service-discovery for more details).") + f.Var(&cfg.JoinMembers, prefix+"memberlist.join", "Other cluster members to join. Can be specified multiple times. It can be an IP, hostname or an entry specified in the DNS Service Discovery format.") f.DurationVar(&cfg.MinJoinBackoff, prefix+"memberlist.min-join-backoff", 1*time.Second, "Min backoff duration to join other cluster members.") f.DurationVar(&cfg.MaxJoinBackoff, prefix+"memberlist.max-join-backoff", 1*time.Minute, "Max backoff duration to join other cluster members.") f.IntVar(&cfg.MaxJoinRetries, prefix+"memberlist.max-join-retries", 10, "Max number of retries to join other cluster members.") @@ -197,11 +194,11 @@ func (cfg *KVConfig) RegisterFlags(f *flag.FlagSet) { cfg.RegisterFlagsWithPrefix(f, "") } -func generateRandomSuffix() string { +func generateRandomSuffix(logger log.Logger) string { suffix := make([]byte, 4) _, err := rand.Read(suffix) if err != nil { - level.Error(util_log.Logger).Log("msg", "failed to generate random suffix", "err", err) + level.Error(logger).Log("msg", "failed to generate random suffix", "err", err) return "error" } return fmt.Sprintf("%2x", suffix) @@ -219,7 +216,7 @@ type KV struct { logger log.Logger // dns discovery provider - provider *dns.Provider + provider DNSProvider // Protects access to memberlist and broadcasts fields. initWG sync.WaitGroup @@ -329,21 +326,14 @@ var ( // gossiping part. Only after service is in Running state, it is really gossiping. Starting the service will also // trigger connecting to the existing memberlist cluster. If that fails and AbortIfJoinFails is true, error is returned // and service enters Failed state. -func NewKV(cfg KVConfig, logger log.Logger) *KV { +func NewKV(cfg KVConfig, logger log.Logger, dnsProvider DNSProvider) *KV { cfg.TCPTransport.MetricsRegisterer = cfg.MetricsRegisterer cfg.TCPTransport.MetricsNamespace = cfg.MetricsNamespace - mr := extprom.WrapRegistererWithPrefix("cortex_", - extprom.WrapRegistererWith( - prometheus.Labels{"name": "memberlist"}, - cfg.MetricsRegisterer, - ), - ) - mlkv := &KV{ cfg: cfg, logger: logger, - provider: dns.NewProvider(logger, mr, dns.GolangResolverType), + provider: dnsProvider, store: make(map[string]valueDesc), codecs: make(map[string]codec.Codec), watchers: make(map[string][]chan string), @@ -388,7 +378,7 @@ func (m *KV) buildMemberlistConfig() (*memberlist.Config, error) { mlCfg.Name = m.cfg.NodeName } if m.cfg.RandomizeNodeName { - mlCfg.Name = mlCfg.Name + "-" + generateRandomSuffix() + mlCfg.Name = mlCfg.Name + "-" + generateRandomSuffix(m.logger) level.Info(m.logger).Log("msg", "Using memberlist cluster node name", "name", mlCfg.Name) } @@ -450,7 +440,7 @@ func (m *KV) running(ctx context.Context) error { } } - var tickerChan <-chan time.Time = nil + var tickerChan <-chan time.Time if m.cfg.RejoinInterval > 0 && len(m.cfg.JoinMembers) > 0 { t := time.NewTicker(m.cfg.RejoinInterval) defer t.Stop() @@ -787,7 +777,7 @@ func (m *KV) notifyWatchers(key string) { // detect removals. If Merge doesn't result in any change (returns nil), then operation fails and is retried again. // After too many failed retries, this method returns error. func (m *KV) CAS(ctx context.Context, key string, codec codec.Codec, f func(in interface{}) (out interface{}, retry bool, err error)) error { - var lastError error = nil + var lastError error outer: for retries := m.maxCasRetries; retries > 0; retries-- { @@ -996,6 +986,7 @@ func (m *KV) queueBroadcast(key string, content []string, version uint, message finished: func(b ringBroadcast) { m.totalSizeOfBroadcastMessagesInQueue.Sub(float64(l)) }, + logger: m.logger, } m.totalSizeOfBroadcastMessagesInQueue.Add(float64(l)) diff --git a/pkg/ring/kv/memberlist/memberlist_logger.go b/vendor/github.com/grafana/dskit/kv/memberlist/memberlist_logger.go similarity index 98% rename from pkg/ring/kv/memberlist/memberlist_logger.go rename to vendor/github.com/grafana/dskit/kv/memberlist/memberlist_logger.go index 6b35a1e690..36e59306a1 100644 --- a/pkg/ring/kv/memberlist/memberlist_logger.go +++ b/vendor/github.com/grafana/dskit/kv/memberlist/memberlist_logger.go @@ -49,7 +49,7 @@ func (a loggerAdapter) Write(p []byte) (int, error) { } if lvl, ok := result["level"]; ok { lvl = strings.ToLower(lvl) - var lvlVal level.Value = nil + var lvlVal level.Value switch lvl { case "debug": diff --git a/pkg/ring/kv/memberlist/mergeable.go b/vendor/github.com/grafana/dskit/kv/memberlist/mergeable.go similarity index 100% rename from pkg/ring/kv/memberlist/mergeable.go rename to vendor/github.com/grafana/dskit/kv/memberlist/mergeable.go diff --git a/pkg/ring/kv/memberlist/metrics.go b/vendor/github.com/grafana/dskit/kv/memberlist/metrics.go similarity index 97% rename from pkg/ring/kv/memberlist/metrics.go rename to vendor/github.com/grafana/dskit/kv/memberlist/metrics.go index 16bd84300c..010288fbab 100644 --- a/pkg/ring/kv/memberlist/metrics.go +++ b/vendor/github.com/grafana/dskit/kv/memberlist/metrics.go @@ -6,10 +6,9 @@ import ( armonmetrics "github.com/armon/go-metrics" armonprometheus "github.com/armon/go-metrics/prometheus" "github.com/go-kit/kit/log/level" - "github.com/grafana/dskit/services" "github.com/prometheus/client_golang/prometheus" - util_log "github.com/cortexproject/cortex/pkg/util/log" + "github.com/grafana/dskit/services" ) func (m *KV) createAndRegisterMetrics() { @@ -190,7 +189,7 @@ func (m *KV) createAndRegisterMetrics() { } for _, c := range all { - m.cfg.MetricsRegisterer.MustRegister(c.(prometheus.Collector)) + m.cfg.MetricsRegisterer.MustRegister(c) } m.cfg.MetricsRegisterer.MustRegister(m) @@ -210,7 +209,7 @@ func (m *KV) createAndRegisterMetrics() { } if err != nil { - level.Error(util_log.Logger).Log("msg", "failed to register prometheus metrics for memberlist", "err", err) + level.Error(m.logger).Log("msg", "failed to register prometheus metrics for memberlist", "err", err) } } diff --git a/pkg/ring/kv/memberlist/tcp_transport.go b/vendor/github.com/grafana/dskit/kv/memberlist/tcp_transport.go similarity index 99% rename from pkg/ring/kv/memberlist/tcp_transport.go rename to vendor/github.com/grafana/dskit/kv/memberlist/tcp_transport.go index 4788e65d30..1d50032cdf 100644 --- a/pkg/ring/kv/memberlist/tcp_transport.go +++ b/vendor/github.com/grafana/dskit/kv/memberlist/tcp_transport.go @@ -14,14 +14,14 @@ import ( "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" - "github.com/grafana/dskit/flagext" "github.com/hashicorp/go-sockaddr" "github.com/hashicorp/memberlist" "github.com/pkg/errors" "github.com/prometheus/client_golang/prometheus" "go.uber.org/atomic" - tlsutil "github.com/cortexproject/cortex/pkg/util/tls" + "github.com/grafana/dskit/flagext" + "github.com/grafana/dskit/kv/kvtls" ) type messageType uint8 @@ -56,8 +56,8 @@ type TCPTransportConfig struct { MetricsRegisterer prometheus.Registerer `yaml:"-"` MetricsNamespace string `yaml:"-"` - TLSEnabled bool `yaml:"tls_enabled"` - TLS tlsutil.ClientConfig `yaml:",inline"` + TLSEnabled bool `yaml:"tls_enabled"` + TLS kvtls.ClientConfig `yaml:",inline"` } // RegisterFlags registers flags. diff --git a/pkg/ring/kv/metrics.go b/vendor/github.com/grafana/dskit/kv/metrics.go similarity index 95% rename from pkg/ring/kv/metrics.go rename to vendor/github.com/grafana/dskit/kv/metrics.go index 1dbd6e2edb..66fe9fa91b 100644 --- a/pkg/ring/kv/metrics.go +++ b/vendor/github.com/grafana/dskit/kv/metrics.go @@ -49,10 +49,9 @@ func newMetricsClient(backend string, c Client, reg prometheus.Registerer) Clien c: c, requestDuration: instrument.NewHistogramCollector( promauto.With(reg).NewHistogramVec(prometheus.HistogramOpts{ - Namespace: "cortex", - Name: "kv_request_duration_seconds", - Help: "Time spent on kv store requests.", - Buckets: prometheus.DefBuckets, + Name: "kv_request_duration_seconds", + Help: "Time spent on kv store requests.", + Buckets: prometheus.DefBuckets, ConstLabels: prometheus.Labels{ "type": backend, }, diff --git a/pkg/ring/kv/mock.go b/vendor/github.com/grafana/dskit/kv/mock.go similarity index 82% rename from pkg/ring/kv/mock.go rename to vendor/github.com/grafana/dskit/kv/mock.go index ac7ae011df..0854913fbf 100644 --- a/pkg/ring/kv/mock.go +++ b/vendor/github.com/grafana/dskit/kv/mock.go @@ -3,17 +3,16 @@ package kv import ( "context" + "github.com/go-kit/kit/log" "github.com/go-kit/kit/log/level" - - util_log "github.com/cortexproject/cortex/pkg/util/log" ) // The mockClient does not anything. // This is used for testing only. type mockClient struct{} -func buildMockClient() (Client, error) { - level.Warn(util_log.Logger).Log("msg", "created mockClient for testing only") +func buildMockClient(logger log.Logger) (Client, error) { + level.Warn(logger).Log("msg", "created mockClient for testing only") return mockClient{}, nil } diff --git a/pkg/ring/kv/multi.go b/vendor/github.com/grafana/dskit/kv/multi.go similarity index 96% rename from pkg/ring/kv/multi.go rename to vendor/github.com/grafana/dskit/kv/multi.go index 3bfb1bcdbb..e750e9aa4b 100644 --- a/pkg/ring/kv/multi.go +++ b/vendor/github.com/grafana/dskit/kv/multi.go @@ -8,32 +8,29 @@ import ( "time" "github.com/go-kit/kit/log" + "github.com/go-kit/kit/log/level" "github.com/prometheus/client_golang/prometheus" "go.uber.org/atomic" - - util_log "github.com/cortexproject/cortex/pkg/util/log" - - "github.com/go-kit/kit/log/level" ) var ( primaryStoreGauge = prometheus.NewGaugeVec(prometheus.GaugeOpts{ - Name: "cortex_multikv_primary_store", + Name: "multikv_primary_store", Help: "Selected primary KV store", }, []string{"store"}) mirrorEnabledGauge = prometheus.NewGauge(prometheus.GaugeOpts{ - Name: "cortex_multikv_mirror_enabled", + Name: "multikv_mirror_enabled", Help: "Is mirroring to secondary store enabled", }) mirrorWritesCounter = prometheus.NewCounter(prometheus.CounterOpts{ - Name: "cortex_multikv_mirror_writes_total", + Name: "multikv_mirror_writes_total", Help: "Number of mirror-writes to secondary store", }) mirrorFailuresCounter = prometheus.NewCounter(prometheus.CounterOpts{ - Name: "cortex_multikv_mirror_write_errors_total", + Name: "multikv_mirror_write_errors_total", Help: "Number of failures to mirror-write to secondary store", }) ) @@ -109,7 +106,7 @@ type MultiClient struct { // NewMultiClient creates new MultiClient with given KV Clients. // First client in the slice is the primary client. -func NewMultiClient(cfg MultiConfig, clients []kvclient) *MultiClient { +func NewMultiClient(cfg MultiConfig, clients []kvclient, logger log.Logger) *MultiClient { c := &MultiClient{ clients: clients, primaryID: atomic.NewInt32(0), @@ -118,7 +115,7 @@ func NewMultiClient(cfg MultiConfig, clients []kvclient) *MultiClient { mirrorTimeout: cfg.MirrorTimeout, mirroringEnabled: atomic.NewBool(cfg.MirrorEnabled), - logger: log.With(util_log.Logger, "component", "multikv"), + logger: log.With(logger, "component", "multikv"), } ctx, cancelFn := context.WithCancel(context.Background()) diff --git a/pkg/ring/kv/prefix.go b/vendor/github.com/grafana/dskit/kv/prefix.go similarity index 100% rename from pkg/ring/kv/prefix.go rename to vendor/github.com/grafana/dskit/kv/prefix.go diff --git a/vendor/github.com/grafana/dskit/services/basic_service.go b/vendor/github.com/grafana/dskit/services/basic_service.go index cb43eaa380..ead611a3f9 100644 --- a/vendor/github.com/grafana/dskit/services/basic_service.go +++ b/vendor/github.com/grafana/dskit/services/basic_service.go @@ -77,7 +77,7 @@ func invalidServiceStateWithFailureError(state, expected State, failure error) e return fmt.Errorf("invalid service state: %v, expected: %v, failure: %w", state, expected, failure) } -// Returns service built from three functions (using BasicService). +// NewBasicService returns service built from three functions (using BasicService). func NewBasicService(start StartingFn, run RunningFn, stop StoppingFn) *BasicService { return &BasicService{ startFn: start, @@ -246,7 +246,7 @@ func (b *BasicService) StopAsync() { } } -// Returns context that this service uses internally for controlling its lifecycle. It is the same context that +// ServiceContext returns context that this service uses internally for controlling its lifecycle. It is the same context that // is passed to Starting and Running functions, and is based on context passed to the service via StartAsync. // // Before service enters Starting state, there is no context. This context is stopped when service enters Stopping state. diff --git a/vendor/github.com/grafana/dskit/services/failure_watcher.go b/vendor/github.com/grafana/dskit/services/failure_watcher.go index 62cbe4561e..a44fd4495e 100644 --- a/vendor/github.com/grafana/dskit/services/failure_watcher.go +++ b/vendor/github.com/grafana/dskit/services/failure_watcher.go @@ -13,7 +13,7 @@ func NewFailureWatcher() *FailureWatcher { return &FailureWatcher{ch: make(chan error)} } -// Returns channel for this watcher. If watcher is nil, returns nil channel. +// Chan returns channel for this watcher. If watcher is nil, returns nil channel. // Errors returned on the channel include failure case and service description. func (w *FailureWatcher) Chan() <-chan error { if w == nil { diff --git a/vendor/github.com/grafana/dskit/services/manager.go b/vendor/github.com/grafana/dskit/services/manager.go index 6a556b1a78..4da481ec5e 100644 --- a/vendor/github.com/grafana/dskit/services/manager.go +++ b/vendor/github.com/grafana/dskit/services/manager.go @@ -17,18 +17,18 @@ const ( // ManagerListener listens for events from Manager. type ManagerListener interface { - // Called when Manager reaches Healthy state (all services Running) + // Healthy is called when Manager reaches Healthy state (all services Running) Healthy() - // Called when Manager reaches Stopped state (all services are either Terminated or Failed) + // Stopped is called when Manager reaches Stopped state (all services are either Terminated or Failed) Stopped() - // Called when service fails. + // Failure is called when service fails. Failure(service Service) } -// Service Manager is initialized with a collection of services. They all must be in New state. -// Service manager can start them, and observe their state as a group. +// Manager is initialized with a collection of services. They all must be in New state. +// Manager can start them, and observe their state as a group. // Once all services are running, Manager is said to be Healthy. It is possible for manager to never reach the Healthy state, if some services fail to start. // When all services are stopped (Terminated or Failed), manager is Stopped. type Manager struct { @@ -72,7 +72,7 @@ func NewManager(services ...Service) (*Manager, error) { return m, nil } -// Initiates service startup on all the services being managed. +// StartAsync initiates service startup on all the services being managed. // It is only valid to call this method if all of the services are New. func (m *Manager) StartAsync(ctx context.Context) error { for _, s := range m.services { @@ -84,7 +84,7 @@ func (m *Manager) StartAsync(ctx context.Context) error { return nil } -// Initiates service shutdown if necessary on all the services being managed. +// StopAsync initiates service shutdown if necessary on all the services being managed. func (m *Manager) StopAsync() { if m == nil { return @@ -95,7 +95,7 @@ func (m *Manager) StopAsync() { } } -// Returns true if all services are currently in the Running state. +// IsHealthy returns true if all services are currently in the Running state. func (m *Manager) IsHealthy() bool { m.mu.Lock() defer m.mu.Unlock() @@ -103,7 +103,7 @@ func (m *Manager) IsHealthy() bool { return m.state == healthy } -// Waits for the ServiceManager to become healthy. Returns nil, if manager is healthy, error otherwise (eg. manager +// AwaitHealthy waits for the ServiceManager to become healthy. Returns nil, if manager is healthy, error otherwise (eg. manager // is in a state in which it cannot get healthy anymore). func (m *Manager) AwaitHealthy(ctx context.Context) error { select { @@ -132,7 +132,7 @@ func (m *Manager) AwaitHealthy(ctx context.Context) error { return nil } -// Returns true if all services are in terminal state (Terminated or Failed) +// IsStopped returns true if all services are in terminal state (Terminated or Failed) func (m *Manager) IsStopped() bool { m.mu.Lock() defer m.mu.Unlock() @@ -140,7 +140,7 @@ func (m *Manager) IsStopped() bool { return m.state == stopped } -// Waits for the ServiceManager to become stopped. Returns nil, if manager is stopped, error when context finishes earlier. +// AwaitStopped waits for the ServiceManager to become stopped. Returns nil, if manager is stopped, error when context finishes earlier. func (m *Manager) AwaitStopped(ctx context.Context) error { select { case <-ctx.Done(): @@ -150,7 +150,7 @@ func (m *Manager) AwaitStopped(ctx context.Context) error { } } -// Provides a snapshot of the current state of all the services under management. +// ServicesByState provides a snapshot of the current state of all the services under management. func (m *Manager) ServicesByState() map[State][]Service { m.mu.Lock() defer m.mu.Unlock() @@ -219,7 +219,7 @@ func (m *Manager) serviceStateChanged(s Service, from State, to State) { } } -// Registers a ManagerListener to be run when this Manager changes state. +// AddListener registers a ManagerListener to be run when this Manager changes state. // The listener will not have previous state changes replayed, so it is suggested that listeners are added before any of the managed services are started. // // AddListener guarantees execution ordering across calls to a given listener but not across calls to multiple listeners. diff --git a/vendor/github.com/grafana/dskit/services/service.go b/vendor/github.com/grafana/dskit/services/service.go index 490cbcad8b..c559ef96a3 100644 --- a/vendor/github.com/grafana/dskit/services/service.go +++ b/vendor/github.com/grafana/dskit/services/service.go @@ -8,13 +8,14 @@ import ( // State of the service. See Service interface for full state diagram. type State int +// Possible states to represent the service State. const ( - New State = iota // Service is new, not running yet. Initial state. - Starting // Service is starting. If starting succeeds, service enters Running state. - Running // Service is fully running now. When service stops running, it enters Stopping state. - Stopping // Service is shutting down - Terminated // Service has stopped successfully. Terminal state. - Failed // Service has failed in Starting, Running or Stopping state. Terminal state. + New State = iota // New: Service is new, not running yet. Initial State. + Starting // Starting: Service is starting. If starting succeeds, service enters Running state. + Running // Running: Service is fully running now. When service stops running, it enters Stopping state. + Stopping // Stopping: Service is shutting down + Terminated // Terminated: Service has stopped successfully. Terminal state. + Failed // Failed: Service has failed in Starting, Running or Stopping state. Terminal state. ) func (s State) String() string { @@ -104,18 +105,18 @@ type NamedService interface { // Listener receives notifications about Service state changes. type Listener interface { - // Called when the service transitions from NEW to STARTING. + // Starting is called when the service transitions from NEW to STARTING. Starting() - // Called when the service transitions from STARTING to RUNNING. + // Running is called when the service transitions from STARTING to RUNNING. Running() - // Called when the service transitions to the STOPPING state. + // Stopping is called when the service transitions to the STOPPING state. Stopping(from State) - // Called when the service transitions to the TERMINATED state. + // Terminated is called when the service transitions to the TERMINATED state. Terminated(from State) - // Called when the service transitions to the FAILED state. + // Failed is called when the service transitions to the FAILED state. Failed(from State, failure error) } diff --git a/vendor/github.com/grafana/dskit/services/services.go b/vendor/github.com/grafana/dskit/services/services.go index f7a668789f..7bb91ae84b 100644 --- a/vendor/github.com/grafana/dskit/services/services.go +++ b/vendor/github.com/grafana/dskit/services/services.go @@ -6,7 +6,7 @@ import ( "time" ) -// Initializes basic service as an "idle" service -- it doesn't do anything in its Running state, +// NewIdleService initializes basic service as an "idle" service -- it doesn't do anything in its Running state, // but still supports all state transitions. func NewIdleService(up StartingFn, down StoppingFn) *BasicService { run := func(ctx context.Context) error { @@ -17,11 +17,11 @@ func NewIdleService(up StartingFn, down StoppingFn) *BasicService { return NewBasicService(up, run, down) } -// One iteration of the timer service. Called repeatedly until service is stopped, or this function returns error +// OneIteration is one iteration of the timer service. Called repeatedly until service is stopped, or this function returns error // in which case, service will fail. type OneIteration func(ctx context.Context) error -// Runs iteration function on every interval tick. When iteration returns error, service fails. +// NewTimerService runs iteration function on every interval tick. When iteration returns error, service fails. func NewTimerService(interval time.Duration, start StartingFn, iter OneIteration, stop StoppingFn) *BasicService { run := func(ctx context.Context) error { t := time.NewTicker(interval) diff --git a/vendor/modules.txt b/vendor/modules.txt index 1fdc59d689..8c76fc7b35 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -63,7 +63,6 @@ github.com/alicebob/miniredis/v2 github.com/alicebob/miniredis/v2/geohash github.com/alicebob/miniredis/v2/server # github.com/armon/go-metrics v0.3.6 -## explicit github.com/armon/go-metrics github.com/armon/go-metrics/prometheus # github.com/asaskevich/govalidator v0.0.0-20200907205600-7a23bdc65eef @@ -297,10 +296,17 @@ github.com/googleapis/gax-go/v2 github.com/gorilla/mux # github.com/gorilla/websocket v1.4.2 github.com/gorilla/websocket -# github.com/grafana/dskit v0.0.0-20210824090727-039d9afd9208 +# github.com/grafana/dskit v0.0.0-20210827060659-9daca2f00327 ## explicit github.com/grafana/dskit/backoff +github.com/grafana/dskit/closer github.com/grafana/dskit/flagext +github.com/grafana/dskit/kv +github.com/grafana/dskit/kv/codec +github.com/grafana/dskit/kv/consul +github.com/grafana/dskit/kv/etcd +github.com/grafana/dskit/kv/kvtls +github.com/grafana/dskit/kv/memberlist github.com/grafana/dskit/modules github.com/grafana/dskit/runtimeconfig github.com/grafana/dskit/services @@ -322,12 +328,10 @@ github.com/grpc-ecosystem/grpc-gateway/utilities # github.com/hailocab/go-hostpool v0.0.0-20160125115350-e80d13ce29ed github.com/hailocab/go-hostpool # github.com/hashicorp/consul/api v1.9.1 -## explicit github.com/hashicorp/consul/api # github.com/hashicorp/errwrap v1.0.0 github.com/hashicorp/errwrap # github.com/hashicorp/go-cleanhttp v0.5.1 -## explicit github.com/hashicorp/go-cleanhttp # github.com/hashicorp/go-hclog v0.12.2 github.com/hashicorp/go-hclog @@ -340,12 +344,10 @@ github.com/hashicorp/go-multierror # github.com/hashicorp/go-rootcerts v1.0.2 github.com/hashicorp/go-rootcerts # github.com/hashicorp/go-sockaddr v1.0.2 -## explicit github.com/hashicorp/go-sockaddr # github.com/hashicorp/golang-lru v0.5.4 github.com/hashicorp/golang-lru/simplelru # github.com/hashicorp/memberlist v0.2.3 -## explicit github.com/hashicorp/memberlist # github.com/hashicorp/serf v0.9.5 github.com/hashicorp/serf/coordinate @@ -699,7 +701,6 @@ github.com/yuin/gopher-lua/pm ## explicit go.etcd.io/bbolt # go.etcd.io/etcd v3.3.25+incompatible -## explicit go.etcd.io/etcd/pkg/transport # go.etcd.io/etcd/api/v3 v3.5.0 go.etcd.io/etcd/api/v3/authpb @@ -721,7 +722,6 @@ go.etcd.io/etcd/client/pkg/v3/types # go.etcd.io/etcd/client/v2 v2.305.0 go.etcd.io/etcd/client/v2 # go.etcd.io/etcd/client/v3 v3.5.0 -## explicit go.etcd.io/etcd/client/v3 go.etcd.io/etcd/client/v3/concurrency go.etcd.io/etcd/client/v3/credentials @@ -750,7 +750,6 @@ go.etcd.io/etcd/raft/v3/quorum go.etcd.io/etcd/raft/v3/raftpb go.etcd.io/etcd/raft/v3/tracker # go.etcd.io/etcd/server/v3 v3.5.0 -## explicit go.etcd.io/etcd/server/v3/auth go.etcd.io/etcd/server/v3/config go.etcd.io/etcd/server/v3/datadir