blobfixture: tolerate non-found errors

pkg/roachprod/blobfixture/registry.go

@@ -184,23 +184,45 @@ func (r *Registry) URI(path string) url.URL {
 	return copy
 }
 
+// maybeReadFile attempts to read a file and returns its contents. Returns nil bytes
+// if the file does not exist. This is useful for handling cases where files may have
+// been concurrently deleted by GC.
+func (r *Registry) maybeReadFile(ctx context.Context, filename string) ([]byte, error) {
+	bytes, err := func() ([]byte, error) {
+		file, _, err := r.storage.ReadFile(ctx, filename, cloud.ReadOptions{})
+		if err != nil {
+			return nil, err
+		}
+		defer func() { _ = file.Close(ctx) }()
+
+		bytes, err := ioctx.ReadAll(ctx, file)
+		if err != nil {
+			return nil, err
+		}
+		return bytes, nil
+	}()
+	if errors.Is(err, cloud.ErrFileDoesNotExist) {
+		return nil, nil
+	}
+	return bytes, err
+}
+
+// listFixtures lists all fixtures of the given kind.
 func (r *Registry) listFixtures(
 	ctx context.Context, kindPrefix string, l *logger.Logger,
 ) ([]FixtureMetadata, error) {
 	if l != nil {
 		l.Printf("listing fixtures: %s", kindPrefix)
 	}
-	result := []FixtureMetadata{}
+	var result []FixtureMetadata
+
 	err := r.storage.List(ctx, kindPrefix /*delimiter*/, "", func(found string) error {
-		file, _, err := r.storage.ReadFile(ctx, path.Join(kindPrefix, found), cloud.ReadOptions{})
+		json, err := r.maybeReadFile(ctx, path.Join(kindPrefix, found))
 		if err != nil {
 			return err
 		}
-		defer func() { _ = file.Close(ctx) }()
-
-		json, err := ioctx.ReadAll(ctx, file)
-		if err != nil {
-			return err
+		if json == nil {
+			return nil // Skip files that don't exist (may have been GC'd)
 		}
 
 		metadata := FixtureMetadata{}
@@ -209,7 +231,6 @@ func (r *Registry) listFixtures(
 		}
 
 		result = append(result, metadata)
-
 		return nil
 	})
 	if err != nil {
@@ -239,13 +260,14 @@ func (r *Registry) upsertMetadata(metadata FixtureMetadata) error {
 }
 
 func (r *Registry) deleteMetadata(metadata FixtureMetadata) error {
-	return errors.Wrap(r.storage.Delete(context.Background(), metadata.MetadataPath), "unable to delete fixture metadata")
+	return errors.Wrap(r.storage.Delete(context.Background(), metadata.MetadataPath), "failed to delete metadata")
 }
 
 func (r *Registry) deleteBlobsMatchingPrefix(prefix string) error {
-	return r.storage.List(context.Background(), prefix, "", func(path string) error {
+	err := r.storage.List(context.Background(), prefix, "", func(path string) error {
 		return r.storage.Delete(context.Background(), prefix+path)
 	})
+	return errors.Wrapf(err, "failed to delete blobs matching prefix %q", prefix)
 }
 
 // ScratchHandle is returned by Registry.Create and is used to mark a fixture

pkg/roachprod/blobfixture/registry_test.go

@@ -226,3 +226,46 @@ func TestFixtureRegistryURI(t *testing.T) {
 		"nodelocal://1/roachprod/v25.1/metadata/test-kind/20240601-1223",
 		metaUri.String())
 }
+
+// setupTestFile creates a test file with given content in the registry
+func setupTestFile(t *testing.T, registry *Registry, path string, content []byte) {
+	t.Helper()
+	ctx := context.Background()
+	writer, err := registry.storage.Writer(ctx, path)
+	require.NoError(t, err)
+	_, err = writer.Write(content)
+	require.NoError(t, err)
+	require.NoError(t, writer.Close())
+}
+
+// checkFileContent verifies file content matches expected or doesn't exist
+func checkFileContent(t *testing.T, registry *Registry, path string, expectedContent []byte) {
+	t.Helper()
+	ctx := context.Background()
+	content, err := registry.maybeReadFile(ctx, path)
+	if expectedContent == nil {
+		require.Nil(t, content)
+		return
+	}
+	require.NoError(t, err)
+	require.Equal(t, expectedContent, content)
+}
+
+func TestRegistryHelpers(t *testing.T) {
+	defer leaktest.AfterTest(t)()
+
+	registry := newTestRegistry(t, "nodelocal://1/roachtest/v25.1")
+	ctx := context.Background()
+
+	testData := []byte("test content")
+	testPath := "test/file.txt"
+
+	// Test reading non-existent file
+	content, err := registry.maybeReadFile(ctx, "nonexistent")
+	require.NoError(t, err)
+	require.Nil(t, content)
+
+	// Test reading existing file
+	setupTestFile(t, registry, testPath, testData)
+	checkFileContent(t, registry, testPath, testData)
+}