Skip to content

Commit d6154d5

Browse files
aknuds1aknuds1
committed
Revert "Update cortex to use runtime config from dskit (#4440)"
This reverts commit a635a1e. Signed-off-by: Arve Knudsen <[email protected]>
1 parent 84f240e commit d6154d5

File tree

6 files changed

+283
-6
lines changed

6 files changed

+283
-6
lines changed

‎pkg/cortex/cortex.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import (
1616
"github.com/grafana/dskit/grpcutil"
1717
"github.com/grafana/dskit/kv/memberlist"
1818
"github.com/grafana/dskit/modules"
19-
"github.com/grafana/dskit/runtimeconfig"
2019
"github.com/grafana/dskit/services"
2120
"github.com/pkg/errors"
2221
"github.com/prometheus/client_golang/prometheus"
@@ -61,6 +60,7 @@ import (
6160
"github.com/cortexproject/cortex/pkg/util/fakeauth"
6261
util_log "github.com/cortexproject/cortex/pkg/util/log"
6362
"github.com/cortexproject/cortex/pkg/util/process"
63+
"github.com/cortexproject/cortex/pkg/util/runtimeconfig"
6464
"github.com/cortexproject/cortex/pkg/util/validation"
6565
)
6666

‎pkg/cortex/modules.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,6 @@ import (
1212
"github.com/grafana/dskit/kv/codec"
1313
"github.com/grafana/dskit/kv/memberlist"
1414
"github.com/grafana/dskit/modules"
15-
"github.com/grafana/dskit/runtimeconfig"
1615
"github.com/grafana/dskit/services"
1716
"github.com/opentracing-contrib/go-stdlib/nethttp"
1817
"github.com/opentracing/opentracing-go"
@@ -48,6 +47,7 @@ import (
4847
"github.com/cortexproject/cortex/pkg/scheduler"
4948
"github.com/cortexproject/cortex/pkg/storegateway"
5049
util_log "github.com/cortexproject/cortex/pkg/util/log"
50+
"github.com/cortexproject/cortex/pkg/util/runtimeconfig"
5151
"github.com/cortexproject/cortex/pkg/util/validation"
5252
)
5353

‎pkg/cortex/runtime_config.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,11 @@ import (
66
"net/http"
77

88
"github.com/grafana/dskit/kv"
9-
"github.com/grafana/dskit/runtimeconfig"
109
"gopkg.in/yaml.v2"
1110

1211
"github.com/cortexproject/cortex/pkg/ingester"
1312
"github.com/cortexproject/cortex/pkg/util"
13+
"github.com/cortexproject/cortex/pkg/util/runtimeconfig"
1414
"github.com/cortexproject/cortex/pkg/util/validation"
1515
)
1616

‎vendor/github.com/grafana/dskit/runtimeconfig/manager.go renamed to ‎pkg/util/runtimeconfig/manager.go

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,11 +13,10 @@ import (
1313

1414
"github.com/go-kit/log"
1515
"github.com/go-kit/log/level"
16+
"github.com/grafana/dskit/services"
1617
"github.com/pkg/errors"
1718
"github.com/prometheus/client_golang/prometheus"
1819
"github.com/prometheus/client_golang/prometheus/promauto"
19-
20-
"github.com/grafana/dskit/services"
2120
)
2221

2322
// Loader loads the configuration from file.

‎pkg/util/runtimeconfig/manager_test.go

Lines changed: 279 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,279 @@
1+
package runtimeconfig
2+
3+
import (
4+
"context"
5+
"crypto/sha256"
6+
"fmt"
7+
"io"
8+
"os"
9+
"strings"
10+
"testing"
11+
"time"
12+
13+
"github.com/go-kit/log"
14+
"github.com/grafana/dskit/services"
15+
"github.com/prometheus/client_golang/prometheus"
16+
"github.com/prometheus/client_golang/prometheus/testutil"
17+
"github.com/stretchr/testify/assert"
18+
"github.com/stretchr/testify/require"
19+
"go.uber.org/atomic"
20+
"gopkg.in/yaml.v2"
21+
)
22+
23+
type TestLimits struct {
24+
Limit1 int `json:"limit1"`
25+
Limit2 int `json:"limit2"`
26+
}
27+
28+
// WARNING: THIS GLOBAL VARIABLE COULD LEAD TO UNEXPECTED BEHAVIOUR WHEN RUNNING MULTIPLE DIFFERENT TESTS
29+
var defaultTestLimits *TestLimits
30+
31+
type testOverrides struct {
32+
Overrides map[string]*TestLimits `yaml:"overrides"`
33+
}
34+
35+
// UnmarshalYAML implements the yaml.Unmarshaler interface.
36+
func (l *TestLimits) UnmarshalYAML(unmarshal func(interface{}) error) error {
37+
if defaultTestLimits != nil {
38+
*l = *defaultTestLimits
39+
}
40+
type plain TestLimits
41+
return unmarshal((*plain)(l))
42+
}
43+
44+
func testLoadOverrides(r io.Reader) (interface{}, error) {
45+
var overrides = &testOverrides{}
46+
47+
decoder := yaml.NewDecoder(r)
48+
decoder.SetStrict(true)
49+
if err := decoder.Decode(&overrides); err != nil {
50+
return nil, err
51+
}
52+
return overrides, nil
53+
}
54+
55+
func newTestOverridesManagerConfig(t *testing.T, i int32) (*atomic.Int32, Config) {
56+
var config = atomic.NewInt32(i)
57+
58+
// create empty file
59+
tempFile, err := os.CreateTemp("", "test-validation")
60+
require.NoError(t, err)
61+
62+
t.Cleanup(func() {
63+
tempFile.Close()
64+
os.Remove(tempFile.Name())
65+
})
66+
67+
// testing NewRuntimeConfigManager with overrides reload config set
68+
return config, Config{
69+
ReloadPeriod: 5 * time.Second,
70+
LoadPath: tempFile.Name(),
71+
Loader: func(_ io.Reader) (i interface{}, err error) {
72+
val := int(config.Load())
73+
return val, nil
74+
},
75+
}
76+
}
77+
78+
func TestNewOverridesManager(t *testing.T) {
79+
tempFile, err := os.CreateTemp("", "test-validation")
80+
require.NoError(t, err)
81+
82+
defer func() {
83+
// Clean up
84+
require.NoError(t, tempFile.Close())
85+
require.NoError(t, os.Remove(tempFile.Name()))
86+
}()
87+
88+
_, err = tempFile.WriteString(`overrides:
89+
user1:
90+
limit2: 150`)
91+
require.NoError(t, err)
92+
93+
defaultTestLimits = &TestLimits{Limit1: 100}
94+
95+
// testing NewRuntimeConfigManager with overrides reload config set
96+
overridesManagerConfig := Config{
97+
ReloadPeriod: time.Second,
98+
LoadPath: tempFile.Name(),
99+
Loader: testLoadOverrides,
100+
}
101+
102+
overridesManager, err := New(overridesManagerConfig, nil, log.NewNopLogger())
103+
require.NoError(t, err)
104+
require.NoError(t, services.StartAndAwaitRunning(context.Background(), overridesManager))
105+
106+
// Cleaning up
107+
require.NoError(t, services.StopAndAwaitTerminated(context.Background(), overridesManager))
108+
109+
// Make sure test limits were loaded.
110+
require.NotNil(t, overridesManager.GetConfig())
111+
}
112+
113+
func TestManager_ListenerWithDefaultLimits(t *testing.T) {
114+
tempFile, err := os.CreateTemp("", "test-validation")
115+
require.NoError(t, err)
116+
require.NoError(t, tempFile.Close())
117+
118+
defer func() {
119+
// Clean up
120+
require.NoError(t, os.Remove(tempFile.Name()))
121+
}()
122+
123+
config := []byte(`overrides:
124+
user1:
125+
limit2: 150`)
126+
err = os.WriteFile(tempFile.Name(), config, 0600)
127+
require.NoError(t, err)
128+
129+
defaultTestLimits = &TestLimits{Limit1: 100}
130+
131+
// testing NewRuntimeConfigManager with overrides reload config set
132+
overridesManagerConfig := Config{
133+
ReloadPeriod: time.Second,
134+
LoadPath: tempFile.Name(),
135+
Loader: testLoadOverrides,
136+
}
137+
138+
reg := prometheus.NewPedanticRegistry()
139+
140+
overridesManager, err := New(overridesManagerConfig, reg, log.NewNopLogger())
141+
require.NoError(t, err)
142+
require.NoError(t, services.StartAndAwaitRunning(context.Background(), overridesManager))
143+
144+
// check if the metrics is set to the config map value before
145+
assert.NoError(t, testutil.GatherAndCompare(reg, strings.NewReader(fmt.Sprintf(`
146+
# HELP runtime_config_hash Hash of the currently active runtime config file.
147+
# TYPE runtime_config_hash gauge
148+
runtime_config_hash{sha256="%s"} 1
149+
# HELP runtime_config_last_reload_successful Whether the last runtime-config reload attempt was successful.
150+
# TYPE runtime_config_last_reload_successful gauge
151+
runtime_config_last_reload_successful 1
152+
`, fmt.Sprintf("%x", sha256.Sum256(config))))))
153+
154+
// need to use buffer, otherwise loadConfig will throw away update
155+
ch := overridesManager.CreateListenerChannel(1)
156+
157+
// rewrite file
158+
config = []byte(`overrides:
159+
user2:
160+
limit2: 200`)
161+
err = os.WriteFile(tempFile.Name(), config, 0600)
162+
require.NoError(t, err)
163+
164+
// reload
165+
err = overridesManager.loadConfig()
166+
require.NoError(t, err)
167+
168+
var newValue interface{}
169+
select {
170+
case newValue = <-ch:
171+
// ok
172+
case <-time.After(time.Second):
173+
t.Fatal("listener was not called")
174+
}
175+
176+
to := newValue.(*testOverrides)
177+
require.Equal(t, 200, to.Overrides["user2"].Limit2) // new overrides
178+
require.Equal(t, 100, to.Overrides["user2"].Limit1) // from defaults
179+
180+
// check if the metrics have been updated
181+
assert.NoError(t, testutil.GatherAndCompare(reg, strings.NewReader(fmt.Sprintf(`
182+
# HELP runtime_config_hash Hash of the currently active runtime config file.
183+
# TYPE runtime_config_hash gauge
184+
runtime_config_hash{sha256="%s"} 1
185+
# HELP runtime_config_last_reload_successful Whether the last runtime-config reload attempt was successful.
186+
# TYPE runtime_config_last_reload_successful gauge
187+
runtime_config_last_reload_successful 1
188+
`, fmt.Sprintf("%x", sha256.Sum256(config))))))
189+
190+
// Cleaning up
191+
require.NoError(t, services.StopAndAwaitTerminated(context.Background(), overridesManager))
192+
193+
// Make sure test limits were loaded.
194+
require.NotNil(t, overridesManager.GetConfig())
195+
}
196+
197+
func TestManager_ListenerChannel(t *testing.T) {
198+
config, overridesManagerConfig := newTestOverridesManagerConfig(t, 555)
199+
200+
overridesManager, err := New(overridesManagerConfig, nil, log.NewNopLogger())
201+
require.NoError(t, err)
202+
require.NoError(t, services.StartAndAwaitRunning(context.Background(), overridesManager))
203+
204+
// need to use buffer, otherwise loadConfig will throw away update
205+
ch := overridesManager.CreateListenerChannel(1)
206+
207+
err = overridesManager.loadConfig()
208+
require.NoError(t, err)
209+
210+
select {
211+
case newValue := <-ch:
212+
require.Equal(t, 555, newValue)
213+
case <-time.After(time.Second):
214+
t.Fatal("listener was not called")
215+
}
216+
217+
config.Store(1111)
218+
err = overridesManager.loadConfig()
219+
require.NoError(t, err)
220+
221+
select {
222+
case newValue := <-ch:
223+
require.Equal(t, 1111, newValue)
224+
case <-time.After(time.Second):
225+
t.Fatal("listener was not called")
226+
}
227+
228+
overridesManager.CloseListenerChannel(ch)
229+
select {
230+
case _, ok := <-ch:
231+
require.False(t, ok)
232+
case <-time.After(time.Second):
233+
t.Fatal("channel not closed")
234+
}
235+
}
236+
237+
func TestManager_StopClosesListenerChannels(t *testing.T) {
238+
_, overridesManagerConfig := newTestOverridesManagerConfig(t, 555)
239+
240+
overridesManager, err := New(overridesManagerConfig, nil, log.NewNopLogger())
241+
require.NoError(t, err)
242+
require.NoError(t, services.StartAndAwaitRunning(context.Background(), overridesManager))
243+
244+
// need to use buffer, otherwise loadConfig will throw away update
245+
ch := overridesManager.CreateListenerChannel(0)
246+
247+
require.NoError(t, services.StopAndAwaitTerminated(context.Background(), overridesManager))
248+
249+
select {
250+
case _, ok := <-ch:
251+
require.False(t, ok)
252+
case <-time.After(time.Second):
253+
t.Fatal("channel not closed")
254+
}
255+
}
256+
257+
func TestManager_ShouldFastFailOnInvalidConfigAtStartup(t *testing.T) {
258+
// Create an invalid runtime config file.
259+
tempFile, err := os.CreateTemp("", "invalid-config")
260+
require.NoError(t, err)
261+
t.Cleanup(func() {
262+
require.NoError(t, os.Remove(tempFile.Name()))
263+
})
264+
265+
_, err = tempFile.Write([]byte("!invalid!"))
266+
require.NoError(t, err)
267+
require.NoError(t, tempFile.Close())
268+
269+
// Create the config manager and start it.
270+
cfg := Config{
271+
ReloadPeriod: time.Second,
272+
LoadPath: tempFile.Name(),
273+
Loader: testLoadOverrides,
274+
}
275+
276+
m, err := New(cfg, nil, log.NewNopLogger())
277+
require.NoError(t, err)
278+
require.Error(t, services.StartAndAwaitRunning(context.Background(), m))
279+
}

‎vendor/modules.txt

Lines changed: 0 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)