From 807f665587f50a6dfdc94a9efbb325c7f04baa9b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Sybren=20A=2E=20St=C3=BCvel?= Date: Wed, 25 Jun 2025 09:38:55 +0200 Subject: [PATCH] Remove the Shaman's "extra checkout paths" feature Remove the "extra checkout paths" feature in order to simplify the configuration file, and thus also the upcoming web interface to edit it. The "extra checkout paths" feature was added to aid in transition from the Flamenco v2 shaman system to the v3 system. It is very unlikely that there is still use of Flamenco v2 by people who will want to migrate to v3 in the future. I expect that if they wanted to, they'd have done so by now. Ref: #104403 --- CHANGELOG.md | 1 + internal/manager/config/defaults.go | 5 ++--- pkg/shaman/README.md | 3 --- pkg/shaman/cleanup.go | 3 +-- pkg/shaman/cleanup_test.go | 28 ++++------------------------ pkg/shaman/config/config.go | 2 -- pkg/shaman/config/testing.go | 5 ++--- 7 files changed, 10 insertions(+), 37 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 18ff1306..ad2d8646 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ bugs in actually-released versions. - Shift-click & ctrl-click can now be used to select multiple tasks ([#104386](https://projects.blender.org/studio/flamenco/pulls/104386)). The action buttons (requeue/pause/cancel) now work on all selected tasks. - While holding shift, the tasks table will not change the ordering of the tasks, to aid in shift-selecting ranges ([#104388](https://projects.blender.org/studio/flamenco/pulls/104388)). - Upgrade Blender Asset Tracer (BAT) to 1.19 to support Blender 5.0 ([BAT#92893](https://projects.blender.org/blender/blender-asset-tracer/pulls/92893)). +- Remove the "extra checkout paths" feature from the Shaman system and thus the configuration file ([#104403](https://projects.blender.org/studio/flamenco/issues/104403)). - Update Go & dependencies to fix security vulnerabilities: - https://pkg.go.dev/vuln/GO-2025-3751 - https://pkg.go.dev/vuln/GO-2025-3750 diff --git a/internal/manager/config/defaults.go b/internal/manager/config/defaults.go index 24031e25..d3cdc406 100644 --- a/internal/manager/config/defaults.go +++ b/internal/manager/config/defaults.go @@ -29,9 +29,8 @@ var defaultConfig = Conf{ // Enable Shaman by default, except on Windows where symlinks are still tricky. Enabled: runtime.GOOS != "windows", GarbageCollect: shaman_config.GarbageCollect{ - Period: 24 * time.Hour, - MaxAge: 31 * 24 * time.Hour, - ExtraCheckoutDirs: []string{}, + Period: 24 * time.Hour, + MaxAge: 31 * 24 * time.Hour, }, }, diff --git a/pkg/shaman/README.md b/pkg/shaman/README.md index 269c129e..a9f326fe 100644 --- a/pkg/shaman/README.md +++ b/pkg/shaman/README.md @@ -54,9 +54,6 @@ setting the following settings in `shaman.yaml`: sweeps. Default is `8h`. Set to `0` to disable garbage collection. - `garbageCollect.maxAge`: files that are newer than this age are not considered for garbage collection. Default is `744h` or 31 days. -- `garbageCollect.extraCheckoutPaths`: list of directories to include when - searching for symlinks. Shaman will never create a checkout here. - Default is empty. Every time a file is symlinked into a checkout directory, it is 'touched' (that is, its modification time is set to 'now'). diff --git a/pkg/shaman/cleanup.go b/pkg/shaman/cleanup.go index e9eaeb5c..0316a0a4 100644 --- a/pkg/shaman/cleanup.go +++ b/pkg/shaman/cleanup.go @@ -100,9 +100,8 @@ func (s *Server) GCStorage(doDryRun bool) (stats GCStats) { logger.Info().Int("numOldFiles", stats.numOldFiles). Msg("found old files, going to check for links") - // Scan the checkout area and extra checkout paths, and discard any old file that is linked. + // Scan the checkout area and discard any old file that is linked. dirsToCheck := []string{s.config.CheckoutPath()} - dirsToCheck = append(dirsToCheck, s.config.GarbageCollect.ExtraCheckoutDirs...) for _, checkDir := range dirsToCheck { if err := s.gcFilterLinkedFiles(checkDir, oldFiles, logger, &stats); err != nil { logger.Error(). diff --git a/pkg/shaman/cleanup_test.go b/pkg/shaman/cleanup_test.go index 6378bddf..cdb4bbe4 100644 --- a/pkg/shaman/cleanup_test.go +++ b/pkg/shaman/cleanup_test.go @@ -128,9 +128,6 @@ func TestGCComponents(t *testing.T) { server, cleanup := createTestShaman() defer cleanup() - extraCheckoutDir := filepath.Join(server.config.TestTempDir, "extra-checkout") - server.config.GarbageCollect.ExtraCheckoutDirs = []string{extraCheckoutDir} - filestore.LinkTestFileStore(server.config.FileStorePath()) copymap := func(somemap mtimeMap) mtimeMap { @@ -146,7 +143,6 @@ func TestGCComponents(t *testing.T) { makeOld(server, expectOld, "stored/30/928ffced04c7008f3324fded86d133effea50828f5ad896196f2a2e190ac7e/6001.blob") makeOld(server, expectOld, "stored/59/0c148428d5c35fab3ebad2f3365bb469ab9c531b60831f3e826c472027a0b9/3367.blob") makeOld(server, expectOld, "stored/80/b749c27b2fef7255e7e7b3c2029b03b31299c75ff1f1c72732081c70a713a3/7488.blob") - makeOld(server, expectOld, "stored/dc/89f15de821ad1df3e78f8ef455e653a2d1862f2eb3f5ee78aa4ca68eb6fb35/781.blob") // utility mapping to be able to find absolute paths more easily absPaths := map[string]string{} @@ -166,11 +162,8 @@ func TestGCComponents(t *testing.T) { err = server.checkoutMan.SymlinkToCheckout(absPaths["3367.blob"], server.config.CheckoutPath(), filepath.Join(checkoutInfo.RelativePath, "use-of-3367.blob")) require.NoError(t, err) - err = server.checkoutMan.SymlinkToCheckout(absPaths["781.blob"], extraCheckoutDir, - filepath.Join(checkoutInfo.RelativePath, "use-of-781.blob")) - require.NoError(t, err) - // There should only be two old file reported now. + // There should be two old file reported. expectRemovable := mtimeMap{ absPaths["6001.blob"]: expectOld[absPaths["6001.blob"]], absPaths["7488.blob"]: expectOld[absPaths["7488.blob"]], @@ -178,15 +171,11 @@ func TestGCComponents(t *testing.T) { oldFiles = copymap(expectOld) stats := GCStats{} err = server.gcFilterLinkedFiles(server.config.CheckoutPath(), oldFiles, log.With().Str("package", "shaman/test").Logger(), &stats) - assert.Equal(t, 1, stats.numSymlinksChecked) // 1 is in checkoutPath, the other in extraCheckoutDir + assert.Equal(t, 1, stats.numSymlinksChecked) require.NoError(t, err) - assert.Equal(t, len(expectRemovable)+1, len(oldFiles)) // one file is linked from the extra checkout dir - err = server.gcFilterLinkedFiles(extraCheckoutDir, oldFiles, log.With().Str("package", "shaman/test").Logger(), &stats) - assert.Equal(t, 2, stats.numSymlinksChecked) // 1 is in checkoutPath, the other in extraCheckoutDir - require.NoError(t, err) - assert.EqualValues(t, expectRemovable, oldFiles) + assert.Equal(t, len(expectRemovable), len(oldFiles)) - // Touching a file before requesting deletion should not delete it. + // Touching a file before requesting deletion should prevent its deletetion. now := time.Now() err = os.Chtimes(absPaths["6001.blob"], now, now) require.NoError(t, err) @@ -202,7 +191,6 @@ func TestGCComponents(t *testing.T) { assert.FileExists(t, absPaths["3367.blob"], "file should exist after GC") assert.FileExists(t, absPaths["6001.blob"], "file should exist after GC") - assert.FileExists(t, absPaths["781.blob"], "file should exist after GC") _, err = os.Stat(absPaths["7488.blob"]) assert.ErrorIs(t, err, fs.ErrNotExist, "file %s should NOT exist after GC", absPaths["7488.blob"]) } @@ -214,9 +202,6 @@ func TestGarbageCollect(t *testing.T) { server, cleanup := createTestShaman() defer cleanup() - extraCheckoutDir := filepath.Join(server.config.TestTempDir, "extra-checkout") - server.config.GarbageCollect.ExtraCheckoutDirs = []string{extraCheckoutDir} - filestore.LinkTestFileStore(server.config.FileStorePath()) // Make some files old. @@ -224,7 +209,6 @@ func TestGarbageCollect(t *testing.T) { makeOld(server, expectOld, "stored/30/928ffced04c7008f3324fded86d133effea50828f5ad896196f2a2e190ac7e/6001.blob") makeOld(server, expectOld, "stored/59/0c148428d5c35fab3ebad2f3365bb469ab9c531b60831f3e826c472027a0b9/3367.blob") makeOld(server, expectOld, "stored/80/b749c27b2fef7255e7e7b3c2029b03b31299c75ff1f1c72732081c70a713a3/7488.blob") - makeOld(server, expectOld, "stored/dc/89f15de821ad1df3e78f8ef455e653a2d1862f2eb3f5ee78aa4ca68eb6fb35/781.blob") // utility mapping to be able to find absolute paths more easily absPaths := map[string]string{} @@ -238,9 +222,6 @@ func TestGarbageCollect(t *testing.T) { err = server.checkoutMan.SymlinkToCheckout(absPaths["3367.blob"], server.config.CheckoutPath(), filepath.Join(checkoutInfo.RelativePath, "use-of-3367.blob")) require.NoError(t, err) - err = server.checkoutMan.SymlinkToCheckout(absPaths["781.blob"], extraCheckoutDir, - filepath.Join(checkoutInfo.RelativePath, "use-of-781.blob")) - require.NoError(t, err) // Running the garbage collector should only remove those two unused files. assert.FileExists(t, absPaths["6001.blob"], "file should exist before GC") @@ -255,6 +236,5 @@ func TestGarbageCollect(t *testing.T) { assert.ErrorIs(t, err, fs.ErrNotExist, "file %s should NOT exist after GC", absPaths["7488.blob"]) // Used files should still exist. - assert.FileExists(t, absPaths["781.blob"]) assert.FileExists(t, absPaths["3367.blob"]) } diff --git a/pkg/shaman/config/config.go b/pkg/shaman/config/config.go index 7c47bffa..71a5d908 100644 --- a/pkg/shaman/config/config.go +++ b/pkg/shaman/config/config.go @@ -55,8 +55,6 @@ type GarbageCollect struct { Period time.Duration `yaml:"period"` // How old files must be before they are GC'd: MaxAge time.Duration `yaml:"maxAge"` - // Paths to check for symlinks before GC'ing files. - ExtraCheckoutDirs []string `yaml:"extraCheckoutPaths"` // Used by the -gc CLI arg to silently disable the garbage collector // while we're performing a manual sweep. diff --git a/pkg/shaman/config/testing.go b/pkg/shaman/config/testing.go index c0d22967..95aeedd6 100644 --- a/pkg/shaman/config/testing.go +++ b/pkg/shaman/config/testing.go @@ -46,9 +46,8 @@ func CreateTestConfig() (conf Config, cleanup func()) { StoragePath: tempDir, GarbageCollect: GarbageCollect{ - Period: 8 * time.Hour, - MaxAge: 31 * 24 * time.Hour, - ExtraCheckoutDirs: []string{}, + Period: 8 * time.Hour, + MaxAge: 31 * 24 * time.Hour, }, }