Unistore: Fix dualwriter mode 3 (#108092)

* Unistore: Fix dualwriter mode 3

Fallback to legacy if Unified is the main store and we get an error when
reading from it

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


---------

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

pkg/storage/legacysql/dualwrite/dualwriter.go

@@ -39,7 +39,16 @@ type dualWriter struct {
 func (d *dualWriter) Get(ctx context.Context, name string, options *metav1.GetOptions) (runtime.Object, error) {
 	// If we read from unified, we can just do that and return.
 	if d.readUnified {
-		return d.unified.Get(ctx, name, options)
+		unifiedGet, unifiedErr := d.unified.Get(ctx, name, options)
+		if apierrors.IsNotFound(unifiedErr) {
+			// If resource is not found in unified storage, fallback to legacy.
+			// This fixes cases in where records (stored in multiple tables, including permissions)
+			// are inserted first in legacy and then on Unified.
+			log := logging.FromContext(ctx).With("method", "Get")
+			log.Error("resource not found in Unified Storage, trying to GET from legacy", "err", unifiedErr)
+			return d.legacy.Get(ctx, name, options)
+		}
+		return unifiedGet, unifiedErr
 	}
 	// If legacy is still our main store, lets first read from it.
 	legacyGet, err := d.legacy.Get(ctx, name, options)

pkg/storage/legacysql/dualwrite/dualwriter_mode3_test.go

@@ -109,12 +109,31 @@ func TestMode3_Get(t *testing.T) {
 				},
 			},
 			{
-				name: "should return an error when getting an object in the unified store fails, and should not go to legacy",
+				name: "should return an error when getting an object in the unified store fails",
 				setupStorageFn: func(m *mock.Mock, name string) {
 					m.On("Get", mock.Anything, name, mock.Anything).Return(nil, errors.New("error"))
 				},
 				wantErr: true,
 			},
+			{
+				name: "should return an error when getting an object in the unified store fails with not found error, and legacy fails as well",
+				setupLegacyFn: func(m *mock.Mock, name string) {
+					m.On("Get", mock.Anything, name, mock.Anything).Return(nil, errors.New("error"))
+				},
+				setupStorageFn: func(m *mock.Mock, name string) {
+					m.On("Get", mock.Anything, name, mock.Anything).Return(nil, apierrors.NewNotFound(schema.GroupResource{Group: "dashboards.dashboard.grafana.app", Resource: "dashboard"}, "uid"))
+				},
+				wantErr: true,
+			},
+			{
+				name: "should succeed when getting an object in the UnifiedStorage fails with NotFound, but Legacy succeeds",
+				setupLegacyFn: func(m *mock.Mock, name string) {
+					m.On("Get", mock.Anything, name, mock.Anything).Return(exampleObj, nil)
+				},
+				setupStorageFn: func(m *mock.Mock, name string) {
+					m.On("Get", mock.Anything, name, mock.Anything).Return(nil, apierrors.NewNotFound(schema.GroupResource{Group: "dashboards.dashboard.grafana.app", Resource: "dashboard"}, "uid"))
+				},
+			},
 			{
 				name: "should succeed when getting an object in the LegacyStorage fails",
 				setupLegacyFn: func(m *mock.Mock, name string) {

pkg/tests/apis/dashboard/integration/api_validation_test.go

@@ -58,11 +58,10 @@ type TestContext struct {
 // TestIntegrationValidation tests the dashboard K8s API
 func TestIntegrationValidation(t *testing.T) {
 	if testing.Short() {
-		t.Skip("skipping integration test2")
+		t.Skip("skipping integration test")
 	}
 
-	// TODO: Skip mode3 - borken due to race conditions while setting default permissions across storage backends
-	dualWriterModes := []rest.DualWriterMode{rest.Mode0, rest.Mode1, rest.Mode2}
+	dualWriterModes := []rest.DualWriterMode{rest.Mode0, rest.Mode1, rest.Mode2, rest.Mode3, rest.Mode4, rest.Mode5}
 	for _, dualWriterMode := range dualWriterModes {
 		t.Run(fmt.Sprintf("DualWriterMode %d", dualWriterMode), func(t *testing.T) {
 			// Create a K8sTestHelper which will set up a real API server