From f79501ad09bece7c5feedc27a4ba2df4f06549ca Mon Sep 17 00:00:00 2001
From: Alan Protasio <alanprot@gmail.com>
Date: Mon, 21 Aug 2023 11:01:02 -0700
Subject: [PATCH] Fix sync bucket index status

Signed-off-by: Alan Protasio <alanprot@gmail.com>
---
 pkg/storage/tsdb/bucketindex/loader.go      | 16 ++++++++--------
 pkg/storage/tsdb/bucketindex/loader_test.go | 20 ++++++++++++++++++--
 2 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/pkg/storage/tsdb/bucketindex/loader.go b/pkg/storage/tsdb/bucketindex/loader.go
index 2390044d208..1718686d1b7 100644
--- a/pkg/storage/tsdb/bucketindex/loader.go
+++ b/pkg/storage/tsdb/bucketindex/loader.go
@@ -105,13 +105,19 @@ func (l *Loader) GetIndex(ctx context.Context, userID string) (*Index, Status, e
 	}
 	l.indexesMx.RUnlock()
 
+	ss, err := ReadSyncStatus(ctx, l.bkt, userID, l.logger)
+
+	if err != nil {
+		level.Warn(l.logger).Log("msg", "unable to read bucket index status", "user", userID, "err", err)
+	}
+
 	startTime := time.Now()
 	l.loadAttempts.Inc()
 	idx, err := ReadIndex(ctx, l.bkt, userID, l.cfgProvider, l.logger)
 	if err != nil {
 		// Cache the error, to avoid hammering the object store in case of persistent issues
 		// (eg. corrupted bucket index or not existing).
-		l.cacheIndex(userID, nil, UnknownStatus, err)
+		l.cacheIndex(userID, nil, ss, err)
 
 		if errors.Is(err, ErrIndexNotFound) {
 			level.Warn(l.logger).Log("msg", "bucket index not found", "user", userID)
@@ -124,13 +130,7 @@ func (l *Loader) GetIndex(ctx context.Context, userID string) (*Index, Status, e
 			level.Error(l.logger).Log("msg", "unable to load bucket index", "user", userID, "err", err)
 		}
 
-		return nil, UnknownStatus, err
-	}
-
-	ss, err := ReadSyncStatus(ctx, l.bkt, userID, l.logger)
-
-	if err != nil {
-		level.Warn(l.logger).Log("msg", "unable to read bucket index status", "user", userID, "err", err)
+		return nil, ss, err
 	}
 
 	// Cache the index.
diff --git a/pkg/storage/tsdb/bucketindex/loader_test.go b/pkg/storage/tsdb/bucketindex/loader_test.go
index 1cd9af0022a..cc4b95763ce 100644
--- a/pkg/storage/tsdb/bucketindex/loader_test.go
+++ b/pkg/storage/tsdb/bucketindex/loader_test.go
@@ -575,7 +575,7 @@ func TestLoader_ShouldOffloadIndexIfIdleTimeoutIsReachedDuringBackgroundUpdates(
 	))
 }
 
-func TestLoader_ShouldUpdateIndexInBackgroundOnPreviousKeyAcessDenied(t *testing.T) {
+func TestLoader_ShouldUpdateIndexInBackgroundOnPreviousKeyAccessDenied(t *testing.T) {
 	user := "user-1"
 	ctx := context.Background()
 	reg := prometheus.NewPedanticRegistry()
@@ -602,12 +602,28 @@ func TestLoader_ShouldUpdateIndexInBackgroundOnPreviousKeyAcessDenied(t *testing
 		require.NoError(t, services.StopAndAwaitTerminated(ctx, loader))
 	})
 
-	_, _, err := loader.GetIndex(ctx, user)
+	_, ss, err := loader.GetIndex(ctx, user)
 	require.True(t, errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied))
+	require.Equal(t, Unknown, ss.Status)
+
+	// Verify is the index sync status is being returned
+	ss.Status = CustomerManagedKeyError
+	ss.NonQueryableReason = CustomerManagedKeyError
+	WriteSyncStatus(ctx, bkt, user, ss, log.NewNopLogger())
+
+	// Check not cached
+	loader.deleteCachedIndex(user)
+	_, ss, err = loader.GetIndex(ctx, user)
+	require.True(t, errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied))
+	require.Equal(t, CustomerManagedKeyError, ss.Status)
 
 	// Check cached
 	require.NoError(t, loader.checkCachedIndexes(ctx))
 
+	_, ss, err = loader.GetIndex(ctx, user)
+	require.True(t, errors.Is(err, bucket.ErrCustomerManagedKeyAccessDenied))
+	require.Equal(t, CustomerManagedKeyError, ss.Status)
+
 	loader.bkt = bkt
 
 	// Upload the bucket index.