From 654467401159ec91bad7efa9b1a939354e69a93d Mon Sep 17 00:00:00 2001
From: Will Assis <35489495+gassiss@users.noreply.github.com>
Date: Fri, 7 Mar 2025 09:04:59 -0300
Subject: [PATCH] fix(unified-storage): Fix dualwriter DELETE mode3 not
 returning error from legacy (#101728)

* Fix dualwrite package not returning error when getting a non-not found error from legacy storage in mode
---
 pkg/storage/legacysql/dualwrite/dualwriter.go          |  7 ++++---
 .../legacysql/dualwrite/dualwriter_mode3_test.go       | 10 ++++++++++
 2 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/pkg/storage/legacysql/dualwrite/dualwriter.go b/pkg/storage/legacysql/dualwrite/dualwriter.go
index 242b401b856..7523d549e59 100644
--- a/pkg/storage/legacysql/dualwrite/dualwriter.go
+++ b/pkg/storage/legacysql/dualwrite/dualwriter.go
@@ -95,7 +95,7 @@ func (d *dualWriter) Create(ctx context.Context, in runtime.Object, createValida
 
 	storageObj, errObjectSt := d.unified.Create(ctx, createdCopy, createValidation, options)
 	if errObjectSt != nil {
-		log.Error("unable to create object in unified storage", "err", err)
+		log.Error("unable to create object in unified storage", "err", errObjectSt)
 		if d.errorIsOK {
 			return createdFromLegacy, nil
 		}
@@ -105,12 +105,13 @@ func (d *dualWriter) Create(ctx context.Context, in runtime.Object, createValida
 		if err != nil {
 			log.Error("unable to cleanup object in legacy storage", "err", err)
 		}
+		return storageObj, errObjectSt
 	}
 
 	if d.readUnified {
 		return storageObj, errObjectSt
 	}
-	return createdFromLegacy, errObjectSt
+	return createdFromLegacy, err
 }
 
 func (d *dualWriter) Delete(ctx context.Context, name string, deleteValidation rest.ValidateObjectFunc, options *metav1.DeleteOptions) (runtime.Object, bool, error) {
@@ -121,7 +122,7 @@ func (d *dualWriter) Delete(ctx context.Context, name string, deleteValidation r
 	// but legacy failed, the user would get a failure, but not be able to retry the delete
 	// as they would not be able to see the object in unistore anymore.
 	objFromLegacy, asyncLegacy, err := d.legacy.Delete(ctx, name, deleteValidation, options)
-	if err != nil && !d.readUnified {
+	if err != nil && (!d.readUnified || !d.errorIsOK && !apierrors.IsNotFound(err)) {
 		return objFromLegacy, asyncLegacy, err
 	}
 
diff --git a/pkg/storage/legacysql/dualwrite/dualwriter_mode3_test.go b/pkg/storage/legacysql/dualwrite/dualwriter_mode3_test.go
index c53be55b64c..bd55753c949 100644
--- a/pkg/storage/legacysql/dualwrite/dualwriter_mode3_test.go
+++ b/pkg/storage/legacysql/dualwrite/dualwriter_mode3_test.go
@@ -256,6 +256,16 @@ func TestMode3_Delete(t *testing.T) {
 				},
 				wantErr: true,
 			},
+			{
+				name: "should return an error when deleting an object in the LegacyStorage fails",
+				setupLegacyFn: func(m *mock.Mock, input string) {
+					m.On("Delete", mock.Anything, input, mock.Anything, mock.Anything).Return(nil, false, apierrors.NewInternalError(errors.New("error")))
+				},
+				setupStorageFn: func(m *mock.Mock, input string) {
+					m.On("Delete", mock.Anything, input, mock.Anything, mock.Anything).Panic("i should not be called")
+				},
+				wantErr: true,
+			},
 		}
 
 	name := "foo"