filer: stop remote.unmount from deleting the remote objects (#10811)

* filer: add filer.options.disable_remote_storage_deletion for cache-only deletes

Deleting a filer entry under a remote.mount path also deletes the backing
object from the remote store (maybeDeleteFromRemote). Deployments that use a
remote mount as a read-through cache in front of an authoritative,
externally-managed object store cannot allow this: the filer typically holds
read-only credentials, so the remote delete fails and the entire delete
errors out; and even where it would succeed, it destroys data the filer does
not own.

Add filer.options.disable_remote_storage_deletion (default false, so existing
behaviour is unchanged). When enabled, maybeDeleteFromRemote is skipped for
both single-entry and recursive folder deletes: local metadata and cached
chunks are still removed, but the remote object is left intact.

* filer: assert local removal in cache-only recursive delete test

The recursive cache-only delete test only checked that no remote delete
happened; it did not verify the local child and directory entries were
removed. Add FindEntry assertions so a regression that skips local
recursive deletion is caught.

* filer: reload the remote mount mapping when /etc/remote changes

The mapping was only read at startup, so remote.unmount left the mount live
in the filer: the purge that follows the mapping delete then went to the
remote store and wiped every object under the mount.

Rebuild the rules trie and the conf map from scratch on each load, since
ptrie cannot drop a key, and swap them under a lock.

* filer: drop the filer-wide remote deletion switch

With the mapping reloaded on unmount, the purge no longer reaches the remote
store, so there is nothing left for the switch to protect against.

---------

Co-authored-by: Chris Lu <chris.lu@gmail.com>

weed/filer/filer_lazy_remote_test.go

@@ -300,6 +300,19 @@ func registerStubMaker(t *testing.T, storageType string, client remote_storage.R
 	}
 }
 
+// putConfigEntry stores a filer entry whose content is a serialized configuration.
+func putConfigEntry(store *stubFilerStore, path string, content []byte) {
+	store.entries[path] = &Entry{
+		FullPath: util.FullPath(path),
+		Attr: Attr{
+			Mtime:  time.Unix(1700000000, 0),
+			Crtime: time.Unix(1700000000, 0),
+			Mode:   0644,
+		},
+		Content: content,
+	}
+}
+
 // --- tests ---
 
 func TestMaybeLazyFetchFromRemote_HitsRemoteAndPersists(t *testing.T) {
@@ -581,6 +594,57 @@ func TestDeleteEntryMetaAndData_IsFromOtherClusterSkipsRemoteDelete(t *testing.T
 	require.Len(t, stub.removeCalls, 0)
 }
 
+func TestDeleteEntryMetaAndData_UnmountedDirectorySkipsRemoteDelete(t *testing.T) {
+	const storageType = "stub_lazy_unmounted"
+	stub := &stubRemoteClient{}
+	defer registerStubMaker(t, storageType, stub)()
+
+	store := newStubFilerStore()
+	confContent, err := proto.Marshal(&remote_pb.RemoteConf{Name: "cloud1", Type: storageType})
+	require.NoError(t, err)
+	putConfigEntry(store, DirectoryEtcRemote+"/cloud1"+REMOTE_STORAGE_CONF_SUFFIX, confContent)
+	mountedContent, err := proto.Marshal(&remote_pb.RemoteStorageMapping{
+		Mappings: map[string]*remote_pb.RemoteStorageLocation{
+			"/buckets/mybucket": {Name: "cloud1", Bucket: "mybucket", Path: "/"},
+		},
+	})
+	require.NoError(t, err)
+	putConfigEntry(store, DirectoryEtcRemote+"/"+REMOTE_STORAGE_MOUNT_FILE, mountedContent)
+
+	filePath := util.FullPath("/buckets/mybucket/cached.txt")
+	store.entries[string(filePath)] = &Entry{
+		FullPath: filePath,
+		Attr: Attr{
+			Mtime:    time.Unix(1700000000, 0),
+			Crtime:   time.Unix(1700000000, 0),
+			Mode:     0644,
+			FileSize: 64,
+		},
+		Remote: &filer_pb.RemoteEntry{RemoteMtime: 1700000000, RemoteSize: 64},
+	}
+
+	f := newTestFiler(t, store, NewFilerRemoteStorage())
+	f.LoadRemoteStorageConfAndMapping()
+	_, remoteLoc := f.RemoteStorage.FindMountDirectory(filePath)
+	require.NotNil(t, remoteLoc)
+
+	unmountedContent, err := proto.Marshal(&remote_pb.RemoteStorageMapping{})
+	require.NoError(t, err)
+	putConfigEntry(store, DirectoryEtcRemote+"/"+REMOTE_STORAGE_MOUNT_FILE, unmountedContent)
+	f.onMetadataChangeEvent(&filer_pb.SubscribeMetadataResponse{
+		Directory: DirectoryEtcRemote,
+		EventNotification: &filer_pb.EventNotification{
+			NewEntry: &filer_pb.Entry{Name: REMOTE_STORAGE_MOUNT_FILE, Content: unmountedContent},
+		},
+	})
+	_, remoteLoc = f.RemoteStorage.FindMountDirectory(filePath)
+	require.Nil(t, remoteLoc)
+
+	require.NoError(t, f.DeleteEntryMetaAndData(context.Background(), filePath, false, false, true, false, nil, 0))
+	require.Len(t, stub.deleteCalls, 0)
+	require.Len(t, stub.removeCalls, 0)
+}
+
 func TestDeleteEntryMetaAndData_RemoteOnlyFileNotUnderMountSkipsRemoteDelete(t *testing.T) {
 	const storageType = "stub_lazy_delete_not_under_mount"
 	stub := &stubRemoteClient{}

weed/filer/filer_on_meta_event.go

@@ -136,5 +136,11 @@ func (f *Filer) LoadRemoteStorageConfAndMapping() {
 	}
 }
 func (f *Filer) maybeReloadRemoteStorageConfigurationAndMapping(event *filer_pb.SubscribeMetadataResponse) {
-	// FIXME add reloading
+	if !filer_pb.MetadataEventTouchesDirectory(event, DirectoryEtcRemote) {
+		return
+	}
+
+	// a mount left behind after remote.unmount would still send deletes to a
+	// remote the filer no longer owns
+	f.LoadRemoteStorageConfAndMapping()
 }

weed/filer/remote_storage.go

@@ -5,6 +5,7 @@ import (
 	"fmt"
 	"math"
 	"strings"
+	"sync"
 
 	"github.com/seaweedfs/seaweedfs/weed/pb"
 	"github.com/seaweedfs/seaweedfs/weed/pb/remote_pb"
@@ -22,6 +23,9 @@ const REMOTE_STORAGE_CONF_SUFFIX = ".conf"
 const REMOTE_STORAGE_MOUNT_FILE = "mount.mapping"
 
 type FilerRemoteStorage struct {
+	// guards rules and storageNameToConf, which are replaced wholesale
+	// whenever /etc/remote changes
+	mu                sync.RWMutex
 	rules             ptrie.Trie[*remote_pb.RemoteStorageLocation]
 	storageNameToConf map[string]*remote_pb.RemoteConf
 }
@@ -48,44 +52,62 @@ func (rs *FilerRemoteStorage) LoadRemoteStorageConfigurationsAndMapping(filer *F
 		return
 	}
 
+	// build into fresh containers so an unmounted directory disappears instead
+	// of lingering in the trie, which has no way to drop a key
+	rules := ptrie.New[*remote_pb.RemoteStorageLocation]()
+	storageNameToConf := make(map[string]*remote_pb.RemoteConf)
+
 	for _, entry := range entries {
 		if entry.Name() == REMOTE_STORAGE_MOUNT_FILE {
-			if err := rs.loadRemoteStorageMountMapping(entry.Content); err != nil {
+			if err := loadRemoteStorageMountMapping(rules, entry.Content); err != nil {
 				return err
 			}
 			continue
 		}
 		if !strings.HasSuffix(entry.Name(), REMOTE_STORAGE_CONF_SUFFIX) {
-			return nil
+			continue
 		}
 		conf := &remote_pb.RemoteConf{}
 		if err := proto.Unmarshal(entry.Content, conf); err != nil {
 			return fmt.Errorf("unmarshal %s/%s: %v", DirectoryEtcRemote, entry.Name(), err)
 		}
-		rs.storageNameToConf[conf.Name] = conf
+		storageNameToConf[conf.Name] = conf
 	}
+
+	rs.mu.Lock()
+	rs.rules, rs.storageNameToConf = rules, storageNameToConf
+	rs.mu.Unlock()
+
 	return nil
 }
 
-func (rs *FilerRemoteStorage) loadRemoteStorageMountMapping(data []byte) (err error) {
+func loadRemoteStorageMountMapping(rules ptrie.Trie[*remote_pb.RemoteStorageLocation], data []byte) (err error) {
 	mappings := &remote_pb.RemoteStorageMapping{}
 	if err := proto.Unmarshal(data, mappings); err != nil {
 		return fmt.Errorf("unmarshal %s/%s: %v", DirectoryEtcRemote, REMOTE_STORAGE_MOUNT_FILE, err)
 	}
 	for dir, storageLocation := range mappings.Mappings {
-		rs.mapDirectoryToRemoteStorage(util.FullPath(dir), storageLocation)
+		putDirectoryToRemoteStorage(rules, util.FullPath(dir), storageLocation)
 	}
 	return nil
 }
 
 func (rs *FilerRemoteStorage) mapDirectoryToRemoteStorage(dir util.FullPath, loc *remote_pb.RemoteStorageLocation) {
-	rs.rules.Put([]byte(dir+"/"), loc)
+	rs.mu.Lock()
+	defer rs.mu.Unlock()
+	putDirectoryToRemoteStorage(rs.rules, dir, loc)
+}
+
+func putDirectoryToRemoteStorage(rules ptrie.Trie[*remote_pb.RemoteStorageLocation], dir util.FullPath, loc *remote_pb.RemoteStorageLocation) {
+	rules.Put([]byte(dir+"/"), loc)
 }
 
 // FindMountDirectory returns the mount directory and location for p. When multiple
 // mounts match (e.g. /buckets/b and /buckets/b/prefix), ptrie MatchPrefix visits
 // shorter prefixes first, so the last match is the longest prefix.
 func (rs *FilerRemoteStorage) FindMountDirectory(p util.FullPath) (mountDir util.FullPath, remoteLocation *remote_pb.RemoteStorageLocation) {
+	rs.mu.RLock()
+	defer rs.mu.RUnlock()
 	rs.rules.MatchPrefix([]byte(p), func(key []byte, value *remote_pb.RemoteStorageLocation) bool {
 		mountDir = util.FullPath(string(key[:len(key)-1]))
 		remoteLocation = value
@@ -95,22 +117,18 @@ func (rs *FilerRemoteStorage) FindMountDirectory(p util.FullPath) (mountDir util
 }
 
 func (rs *FilerRemoteStorage) FindRemoteStorageClient(p util.FullPath) (client remote_storage.RemoteStorageClient, remoteConf *remote_pb.RemoteConf, found bool) {
-	var storageLocation *remote_pb.RemoteStorageLocation
-	rs.rules.MatchPrefix([]byte(p), func(key []byte, value *remote_pb.RemoteStorageLocation) bool {
-		storageLocation = value
-		return true
-	})
-
+	_, storageLocation := rs.FindMountDirectory(p)
 	if storageLocation == nil {
-		found = false
-		return
+		return nil, nil, false
 	}
 
 	return rs.GetRemoteStorageClient(storageLocation.Name)
 }
 
 func (rs *FilerRemoteStorage) GetRemoteStorageClient(storageName string) (client remote_storage.RemoteStorageClient, remoteConf *remote_pb.RemoteConf, found bool) {
+	rs.mu.RLock()
 	remoteConf, found = rs.storageNameToConf[storageName]
+	rs.mu.RUnlock()
 	if !found {
 		return
 	}

weed/shell/command_remote_unmount.go

@@ -65,7 +65,8 @@ func (c *commandRemoteUnmount) Do(args []string, commandEnv *CommandEnv, writer
 		return fmt.Errorf("directory %s is not mounted", *dir)
 	}
 
-	// store a mount configuration in filer
+	// delete the mount mapping first: the filer reloads it on the spot, so the
+	// purge below stays local instead of deleting the remote objects
 	fmt.Fprintf(writer, "deleting mount for %s ...\n", *dir)
 	if err = filer.DeleteMountMapping(commandEnv, *dir); err != nil {
 		return fmt.Errorf("delete mount mapping: %w", err)