Skip to content

Commit 0ffc4c4

Browse files
suntalaDanCech
andauthored
Storage: Add mode reconciliation for modes 1 and 2 (grafana#87919)
* Add skeleton implementation for mode reconciliation between 1 and 2 * Track mode for each dual writer * Add test for setting dual writer * Include context when setting dual writing mode --------- Co-authored-by: Dan Cech <[email protected]>
1 parent cac4079 commit 0ffc4c4

File tree

8 files changed

+165
-5
lines changed

8 files changed

+165
-5
lines changed

.golangci.toml

+1
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ allow = [
7777
"github.com/grafana/grafana/pkg/apiserver",
7878
"github.com/grafana/grafana/pkg/services/apiserver/utils",
7979
"github.com/grafana/grafana/pkg/services/featuremgmt",
80+
"github.com/grafana/grafana/pkg/infra/kvstore",
8081
]
8182
deny = [
8283
{ pkg = "github.com/grafana/grafana/pkg", desc = "apiserver is not allowed to import grafana core" }

pkg/apiserver/rest/dualwriter.go

+74-1
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,15 @@ package rest
22

33
import (
44
"context"
5+
"errors"
6+
"fmt"
57

8+
"github.com/grafana/grafana/pkg/infra/kvstore"
9+
"github.com/grafana/grafana/pkg/services/featuremgmt"
610
metav1 "k8s.io/apimachinery/pkg/apis/meta/v1"
711
"k8s.io/apimachinery/pkg/runtime"
812
"k8s.io/apiserver/pkg/registry/rest"
13+
"k8s.io/klog/v2"
914
)
1015

1116
var (
@@ -69,12 +74,13 @@ type LegacyStorage interface {
6974
type DualWriter interface {
7075
Storage
7176
LegacyStorage
77+
Mode() DualWriterMode
7278
}
7379

7480
type DualWriterMode int
7581

7682
const (
77-
Mode1 DualWriterMode = iota
83+
Mode1 DualWriterMode = iota + 1
7884
Mode2
7985
Mode3
8086
Mode4
@@ -117,3 +123,70 @@ func (u *updateWrapper) Preconditions() *metav1.Preconditions {
117123
func (u *updateWrapper) UpdatedObject(ctx context.Context, oldObj runtime.Object) (newObj runtime.Object, err error) {
118124
return u.updated, nil
119125
}
126+
127+
func SetDualWritingMode(
128+
ctx context.Context,
129+
kvs *kvstore.NamespacedKVStore,
130+
features featuremgmt.FeatureToggles,
131+
entity string,
132+
legacy LegacyStorage,
133+
storage Storage,
134+
) (DualWriter, error) {
135+
toMode := map[string]DualWriterMode{
136+
"1": Mode1,
137+
"2": Mode2,
138+
"3": Mode3,
139+
"4": Mode4,
140+
}
141+
errDualWriterSetCurrentMode := errors.New("failed to set current dual writing mode")
142+
143+
// Use entity name as key
144+
m, ok, err := kvs.Get(ctx, entity)
145+
if err != nil {
146+
return nil, errors.New("failed to fetch current dual writing mode")
147+
}
148+
149+
currentMode, valid := toMode[m]
150+
151+
if !valid && ok {
152+
// Only log if "ok" because initially all instances will have mode unset for playlists.
153+
klog.Info("invalid dual writing mode for playlists mode:", m)
154+
}
155+
156+
if !valid || !ok {
157+
// Default to mode 1
158+
currentMode = Mode1
159+
160+
err := kvs.Set(ctx, entity, fmt.Sprint(currentMode))
161+
if err != nil {
162+
return nil, errDualWriterSetCurrentMode
163+
}
164+
}
165+
166+
// Desired mode is 2 and current mode is 1
167+
if features.IsEnabledGlobally(featuremgmt.FlagDualWritePlaylistsMode2) && (currentMode == Mode1) {
168+
// This is where we go through the different gates to allow the instance to migrate from mode 1 to mode 2.
169+
// There are none between mode 1 and mode 2
170+
currentMode = Mode2
171+
172+
err := kvs.Set(ctx, entity, fmt.Sprint(currentMode))
173+
if err != nil {
174+
return nil, errDualWriterSetCurrentMode
175+
}
176+
}
177+
// #TODO enable this check when we have a flag/config for setting mode 1 as the desired mode
178+
// if features.IsEnabledGlobally(featuremgmt.FlagDualWritePlaylistsMode1) && (currentMode == Mode2) {
179+
// // This is where we go through the different gates to allow the instance to migrate from mode 2 to mode 1.
180+
// // There are none between mode 1 and mode 2
181+
// currentMode = Mode1
182+
183+
// err := kvs.Set(ctx, entity, fmt.Sprint(currentMode))
184+
// if err != nil {
185+
// return nil, errDualWriterSetCurrentMode
186+
// }
187+
// }
188+
189+
// #TODO add support for other combinations of desired and current modes
190+
191+
return NewDualWriter(currentMode, legacy, storage), nil
192+
}

pkg/apiserver/rest/dualwriter_mode1.go

+5
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,11 @@ func NewDualWriterMode1(legacy LegacyStorage, storage Storage) *DualWriterMode1
3232
return &DualWriterMode1{Legacy: legacy, Storage: storage, Log: klog.NewKlogr().WithName("DualWriterMode1"), dualWriterMetrics: metrics}
3333
}
3434

35+
// Mode returns the mode of the dual writer.
36+
func (d *DualWriterMode1) Mode() DualWriterMode {
37+
return Mode1
38+
}
39+
3540
// Create overrides the behavior of the generic DualWriter and writes only to LegacyStorage.
3641
func (d *DualWriterMode1) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
3742
log := d.Log.WithValues("kind", options.Kind)

pkg/apiserver/rest/dualwriter_mode2.go

+5
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,11 @@ func NewDualWriterMode2(legacy LegacyStorage, storage Storage) *DualWriterMode2
3030
return &DualWriterMode2{Legacy: legacy, Storage: storage, Log: klog.NewKlogr().WithName("DualWriterMode2"), dualWriterMetrics: metrics}
3131
}
3232

33+
// Mode returns the mode of the dual writer.
34+
func (d *DualWriterMode2) Mode() DualWriterMode {
35+
return Mode2
36+
}
37+
3338
// Create overrides the behavior of the generic DualWriter and writes to LegacyStorage and Storage.
3439
func (d *DualWriterMode2) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
3540
log := d.Log.WithValues("kind", options.Kind)

pkg/apiserver/rest/dualwriter_mode3.go

+5
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,11 @@ func NewDualWriterMode3(legacy LegacyStorage, storage Storage) *DualWriterMode3
2626
return &DualWriterMode3{Legacy: legacy, Storage: storage, Log: klog.NewKlogr().WithName("DualWriterMode3"), dualWriterMetrics: metrics}
2727
}
2828

29+
// Mode returns the mode of the dual writer.
30+
func (d *DualWriterMode3) Mode() DualWriterMode {
31+
return Mode3
32+
}
33+
2934
// Create overrides the behavior of the generic DualWriter and writes to LegacyStorage and Storage.
3035
func (d *DualWriterMode3) Create(ctx context.Context, obj runtime.Object, createValidation rest.ValidateObjectFunc, options *metav1.CreateOptions) (runtime.Object, error) {
3136
log := klog.FromContext(ctx)

pkg/apiserver/rest/dualwriter_mode4.go

+5
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,11 @@ func NewDualWriterMode4(legacy LegacyStorage, storage Storage) *DualWriterMode4
2525
return &DualWriterMode4{Legacy: legacy, Storage: storage, Log: klog.NewKlogr().WithName("DualWriterMode4"), dualWriterMetrics: metrics}
2626
}
2727

28+
// Mode returns the mode of the dual writer.
29+
func (d *DualWriterMode4) Mode() DualWriterMode {
30+
return Mode4
31+
}
32+
2833
// #TODO remove all DualWriterMode4 methods once we remove the generic DualWriter implementation
2934

3035
// Create overrides the behavior of the generic DualWriter and writes only to Storage.

pkg/apiserver/rest/dualwriter_test.go

+61
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,61 @@
1+
package rest
2+
3+
import (
4+
"context"
5+
"fmt"
6+
"testing"
7+
8+
"github.com/grafana/grafana/pkg/infra/kvstore"
9+
"github.com/grafana/grafana/pkg/services/featuremgmt"
10+
"github.com/stretchr/testify/assert"
11+
"github.com/stretchr/testify/mock"
12+
)
13+
14+
func TestSetDualWritingMode(t *testing.T) {
15+
type testCase struct {
16+
name string
17+
features []any
18+
stackID string
19+
expectedMode DualWriterMode
20+
}
21+
tests :=
22+
// #TODO add test cases for kv store failures. Requires adding support in kvstore test_utils.go
23+
[]testCase{
24+
{
25+
name: "should return a mode 1 dual writer when no desired mode is set",
26+
features: []any{},
27+
stackID: "stack-1",
28+
expectedMode: Mode1,
29+
},
30+
{
31+
name: "should return a mode 2 dual writer when mode 2 is set as the desired mode",
32+
features: []any{featuremgmt.FlagDualWritePlaylistsMode2},
33+
stackID: "stack-1",
34+
expectedMode: Mode2,
35+
},
36+
}
37+
38+
for _, tt := range tests {
39+
l := (LegacyStorage)(nil)
40+
s := (Storage)(nil)
41+
m := &mock.Mock{}
42+
43+
ls := legacyStoreMock{m, l}
44+
us := storageMock{m, s}
45+
46+
f := featuremgmt.WithFeatures(tt.features...)
47+
kvStore := kvstore.WithNamespace(kvstore.NewFakeKVStore(), 0, "storage.dualwriting."+tt.stackID)
48+
49+
key := "playlist"
50+
51+
dw, err := SetDualWritingMode(context.Background(), kvStore, f, key, ls, us)
52+
assert.NoError(t, err)
53+
assert.Equal(t, tt.expectedMode, dw.Mode())
54+
55+
// check kv store
56+
val, ok, err := kvStore.Get(context.Background(), key)
57+
assert.True(t, ok)
58+
assert.NoError(t, err)
59+
assert.Equal(t, val, fmt.Sprint(tt.expectedMode))
60+
}
61+
}

pkg/registry/apis/playlist/register.go

+9-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package playlist
22

33
import (
4+
"context"
45
"fmt"
56
"time"
67

@@ -17,6 +18,7 @@ import (
1718
playlist "github.com/grafana/grafana/pkg/apis/playlist/v0alpha1"
1819
"github.com/grafana/grafana/pkg/apiserver/builder"
1920
grafanarest "github.com/grafana/grafana/pkg/apiserver/rest"
21+
"github.com/grafana/grafana/pkg/infra/kvstore"
2022
"github.com/grafana/grafana/pkg/services/apiserver/endpoints/request"
2123
"github.com/grafana/grafana/pkg/services/apiserver/utils"
2224
"github.com/grafana/grafana/pkg/services/featuremgmt"
@@ -32,18 +34,21 @@ type PlaylistAPIBuilder struct {
3234
namespacer request.NamespaceMapper
3335
gv schema.GroupVersion
3436
features featuremgmt.FeatureToggles
37+
kvStore *kvstore.NamespacedKVStore
3538
}
3639

3740
func RegisterAPIService(p playlistsvc.Service,
3841
apiregistration builder.APIRegistrar,
3942
cfg *setting.Cfg,
4043
features featuremgmt.FeatureToggles,
44+
kvStore kvstore.KVStore,
4145
) *PlaylistAPIBuilder {
4246
builder := &PlaylistAPIBuilder{
4347
service: p,
4448
namespacer: request.GetNamespaceMapper(cfg),
4549
gv: playlist.PlaylistResourceInfo.GroupVersion(),
4650
features: features,
51+
kvStore: kvstore.WithNamespace(kvStore, 0, "storage.dualwriting"),
4752
}
4853
apiregistration.RegisterAPI(builder)
4954
return builder
@@ -123,11 +128,11 @@ func (b *PlaylistAPIBuilder) GetAPIGroupInfo(
123128
return nil, err
124129
}
125130

126-
mode := grafanarest.Mode1
127-
if b.features.IsEnabledGlobally(featuremgmt.FlagDualWritePlaylistsMode2) {
128-
mode = grafanarest.Mode2
131+
dualWriter, err := grafanarest.SetDualWritingMode(context.Background(), b.kvStore, b.features, "playlist", legacyStore, store)
132+
if err != nil {
133+
return nil, err
129134
}
130-
storage[resource.StoragePath()] = grafanarest.NewDualWriter(mode, legacyStore, store)
135+
storage[resource.StoragePath()] = dualWriter
131136
}
132137

133138
apiGroupInfo.VersionedResourcesStorageMap[playlist.VERSION] = storage

0 commit comments

Comments
 (0)