From 2e82ac0cc10e953bb895ba32aa6b8e97c8f452f4 Mon Sep 17 00:00:00 2001
From: maicon <maiconscosta@gmail.com>
Date: Thu, 20 Feb 2025 17:43:36 -0300
Subject: [PATCH] Unistore: keep current dual writing mode when unable to run
 data syncer at bootstrap (#100852)

* Unistore: keep current dual writing mode when unable to run data syncer at bootstrap

Signed-off-by: Maicon Costa <maiconscosta@gmail.com>

---------

Signed-off-by: Maicon Costa <maiconscosta@gmail.com>
---
 pkg/apiserver/rest/dualwriter.go      |  4 ++--
 pkg/apiserver/rest/dualwriter_test.go | 28 +++++++++++++++++++++------
 2 files changed, 24 insertions(+), 8 deletions(-)

diff --git a/pkg/apiserver/rest/dualwriter.go b/pkg/apiserver/rest/dualwriter.go
index a4b6b9c8162..15be881b250 100644
--- a/pkg/apiserver/rest/dualwriter.go
+++ b/pkg/apiserver/rest/dualwriter.go
@@ -210,8 +210,8 @@ func SetDualWritingMode(
 		// Once we are done with running the syncer, we can change the mode back on the config to the desired one.
 		cfg.Mode = cfgModeTmp
 		if err != nil {
-			klog.Info("data syncer failed for mode:", m)
-			return Mode0, err
+			klog.Error("data syncer failed for mode:", m, "err", err)
+			return currentMode, nil
 		}
 		if !syncOk {
 			klog.Info("data syncer not ok for mode:", m)
diff --git a/pkg/apiserver/rest/dualwriter_test.go b/pkg/apiserver/rest/dualwriter_test.go
index 44f969351c1..13fc007dc53 100644
--- a/pkg/apiserver/rest/dualwriter_test.go
+++ b/pkg/apiserver/rest/dualwriter_test.go
@@ -2,6 +2,7 @@ package rest
 
 import (
 	"context"
+	"fmt"
 	"testing"
 	"time"
 
@@ -15,10 +16,11 @@ import (
 
 func TestSetDualWritingMode(t *testing.T) {
 	type testCase struct {
-		name         string
-		kvStore      *fakeNamespacedKV
-		desiredMode  DualWriterMode
-		expectedMode DualWriterMode
+		name            string
+		kvStore         *fakeNamespacedKV
+		desiredMode     DualWriterMode
+		expectedMode    DualWriterMode
+		serverLockError error
 	}
 	tests :=
 		[]testCase{
@@ -52,6 +54,13 @@ func TestSetDualWritingMode(t *testing.T) {
 				desiredMode:  Mode0,
 				expectedMode: Mode0,
 			},
+			{
+				name:            "should keep mode2 when trying to go from mode2 to mode3 and the server lock service returns an error",
+				kvStore:         &fakeNamespacedKV{data: map[string]string{"playlist.grafana.app/playlists": "2"}, namespace: "storage.dualwriting"},
+				desiredMode:     Mode3,
+				expectedMode:    Mode2,
+				serverLockError: fmt.Errorf("lock already exists"),
+			},
 		}
 
 	for _, tt := range tests {
@@ -68,12 +77,16 @@ func TestSetDualWritingMode(t *testing.T) {
 		lm.On("List", mock.Anything, mock.Anything).Return(exampleList, nil)
 		ls := legacyStoreMock{lm, l}
 
+		serverLockSvc := &fakeServerLock{
+			err: tt.serverLockError,
+		}
+
 		dwMode, err := SetDualWritingMode(context.Background(), tt.kvStore, &SyncerConfig{
 			LegacyStorage:     ls,
 			Storage:           us,
 			Kind:              "playlist.grafana.app/playlists",
 			Mode:              tt.desiredMode,
-			ServerLockService: &fakeServerLock{},
+			ServerLockService: serverLockSvc,
 			RequestInfo:       &request.RequestInfo{},
 			Reg:               p,
 
@@ -144,11 +157,14 @@ func (f *fakeNamespacedKV) Set(ctx context.Context, key, value string) error {
 	return nil
 }
 
-// Never lock in tests
 type fakeServerLock struct {
+	err error
 }
 
 func (f *fakeServerLock) LockExecuteAndRelease(ctx context.Context, actionName string, duration time.Duration, fn func(ctx context.Context)) error {
+	if f.err != nil {
+		return f.err
+	}
 	fn(ctx)
 	return nil
 }