From 0a16268f5df68516fe780d01a60270a3e37bdf5e Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 27 Nov 2023 14:11:19 +0000 Subject: [PATCH 1/2] fix(provisioner): cleanup: use mtime instead of atime --- provisioner/terraform/cleanup.go | 28 ++++----- provisioner/terraform/cleanup_test.go | 83 +++++++++++++++------------ 2 files changed, 59 insertions(+), 52 deletions(-) diff --git a/provisioner/terraform/cleanup.go b/provisioner/terraform/cleanup.go index 65f876551b6d7..9480185ad24df 100644 --- a/provisioner/terraform/cleanup.go +++ b/provisioner/terraform/cleanup.go @@ -7,7 +7,6 @@ import ( "strings" "time" - "github.com/djherbis/times" "github.com/spf13/afero" "golang.org/x/xerrors" @@ -76,16 +75,16 @@ func CleanStaleTerraformPlugins(ctx context.Context, cachePath string, fs afero. // Identify stale plugins var stalePlugins []string for _, pluginPath := range pluginPaths { - accessTime, err := latestAccessTime(fs, pluginPath) + modTime, err := latestModTime(fs, pluginPath) if err != nil { - return xerrors.Errorf("unable to evaluate latest access time for directory %q: %w", pluginPath, err) + return xerrors.Errorf("unable to evaluate latest mtime for directory %q: %w", pluginPath, err) } - if accessTime.Add(staleTerraformPluginRetention).Before(now) { - logger.Info(ctx, "plugin directory is stale and will be removed", slog.F("plugin_path", pluginPath)) + if modTime.Add(staleTerraformPluginRetention).Before(now) { + logger.Info(ctx, "plugin directory is stale and will be removed", slog.F("plugin_path", pluginPath), slog.F("mtime", modTime)) stalePlugins = append(stalePlugins, pluginPath) } else { - logger.Debug(ctx, "plugin directory is not stale", slog.F("plugin_path", pluginPath)) + logger.Debug(ctx, "plugin directory is not stale", slog.F("plugin_path", pluginPath), slog.F("mtime", modTime)) } } @@ -127,22 +126,19 @@ func CleanStaleTerraformPlugins(ctx context.Context, cachePath string, fs afero. return nil } -// latestAccessTime walks recursively through the directory content, and locates -// the last accessed file. -func latestAccessTime(fs afero.Fs, pluginPath string) (time.Time, error) { +// latestModTime walks recursively through the directory content, and locates +// the last created/modified file. +func latestModTime(fs afero.Fs, pluginPath string) (time.Time, error) { var latest time.Time err := afero.Walk(fs, pluginPath, func(path string, info os.FileInfo, err error) error { if err != nil { return err } - accessTime := info.ModTime() // fallback to modTime if accessTime is not available (afero) - if info.Sys() != nil { - timeSpec := times.Get(info) - accessTime = timeSpec.AccessTime() - } - if latest.Before(accessTime) { - latest = accessTime + // atime is not reliable, so always use mtime. + modTime := info.ModTime() + if modTime.After(latest) { + latest = modTime } return nil }) diff --git a/provisioner/terraform/cleanup_test.go b/provisioner/terraform/cleanup_test.go index 42e97305df89b..aa2e1cddc5b1b 100644 --- a/provisioner/terraform/cleanup_test.go +++ b/provisioner/terraform/cleanup_test.go @@ -29,6 +29,7 @@ const cachePath = "/tmp/coder/provisioner-0/tf" var updateGoldenFiles = flag.Bool("update", false, "Update golden files") var ( + now = time.Date(2023, 6, 3, 4, 5, 6, 0, time.UTC) coderPluginPath = filepath.Join("registry.terraform.io", "coder", "coder", "0.11.1", "darwin_arm64") dockerPluginPath = filepath.Join("registry.terraform.io", "kreuzwerker", "docker", "2.25.0", "darwin_arm64") ) @@ -36,13 +37,14 @@ var ( func TestPluginCache_Golden(t *testing.T) { t.Parallel() - prepare := func() (afero.Fs, time.Time, slog.Logger) { - fs := afero.NewMemMapFs() - now := time.Date(2023, time.June, 3, 4, 5, 6, 0, time.UTC) + prepare := func() (afero.Fs, slog.Logger) { + // afero.MemMapFs does not modify atimes, so use a real FS instead. + tmpDir := t.TempDir() + fs := afero.NewBasePathFs(afero.NewOsFs(), tmpDir) logger := slogtest.Make(t, nil). Leveled(slog.LevelDebug). Named("cleanup-test") - return fs, now, logger + return fs, logger } t.Run("all plugins are stale", func(t *testing.T) { @@ -51,20 +53,20 @@ func TestPluginCache_Golden(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) defer cancel() - fs, now, logger := prepare() + fs, logger := prepare() // given // This plugin is older than 30 days. - addPluginFile(t, fs, coderPluginPath, "terraform-provider-coder_v0.11.1", now.Add(-63*24*time.Hour)) - addPluginFile(t, fs, coderPluginPath, "LICENSE", now.Add(-33*24*time.Hour)) - addPluginFile(t, fs, coderPluginPath, "README.md", now.Add(-31*24*time.Hour)) - addPluginFolder(t, fs, coderPluginPath, "new_folder", now.Add(-31*24*time.Hour)) - addPluginFile(t, fs, coderPluginPath, filepath.Join("new_folder", "foobar.tf"), now.Add(-43*24*time.Hour)) + addPluginFile(t, fs, coderPluginPath, "terraform-provider-coder_v0.11.1", now, now.Add(-63*24*time.Hour)) + addPluginFile(t, fs, coderPluginPath, "LICENSE", now, now.Add(-33*24*time.Hour)) + addPluginFile(t, fs, coderPluginPath, "README.md", now, now.Add(-31*24*time.Hour)) + addPluginFolder(t, fs, coderPluginPath, "new_folder", now, now.Add(-31*24*time.Hour)) + addPluginFile(t, fs, coderPluginPath, filepath.Join("new_folder", "foobar.tf"), now, now.Add(-43*24*time.Hour)) // This plugin is older than 30 days. - addPluginFile(t, fs, dockerPluginPath, "terraform-provider-docker_v2.25.0", now.Add(-31*24*time.Hour)) - addPluginFile(t, fs, dockerPluginPath, "LICENSE", now.Add(-32*24*time.Hour)) - addPluginFile(t, fs, dockerPluginPath, "README.md", now.Add(-33*24*time.Hour)) + addPluginFile(t, fs, dockerPluginPath, "terraform-provider-docker_v2.25.0", now, now.Add(-31*24*time.Hour)) + addPluginFile(t, fs, dockerPluginPath, "LICENSE", now, now.Add(-32*24*time.Hour)) + addPluginFile(t, fs, dockerPluginPath, "README.md", now, now.Add(-33*24*time.Hour)) // when terraform.CleanStaleTerraformPlugins(ctx, cachePath, fs, now, logger) @@ -79,19 +81,19 @@ func TestPluginCache_Golden(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) defer cancel() - fs, now, logger := prepare() + fs, logger := prepare() // given - addPluginFile(t, fs, coderPluginPath, "terraform-provider-coder_v0.11.1", now.Add(-2*time.Hour)) - addPluginFile(t, fs, coderPluginPath, "LICENSE", now.Add(-3*time.Hour)) - addPluginFile(t, fs, coderPluginPath, "README.md", now.Add(-4*time.Hour)) - addPluginFolder(t, fs, coderPluginPath, "new_folder", now.Add(-5*time.Hour)) - addPluginFile(t, fs, coderPluginPath, filepath.Join("new_folder", "foobar.tf"), now.Add(-4*time.Hour)) + addPluginFile(t, fs, coderPluginPath, "terraform-provider-coder_v0.11.1", now, now.Add(-2*time.Hour)) + addPluginFile(t, fs, coderPluginPath, "LICENSE", now, now.Add(-3*time.Hour)) + addPluginFile(t, fs, coderPluginPath, "README.md", now, now.Add(-4*time.Hour)) + addPluginFolder(t, fs, coderPluginPath, "new_folder", now, now.Add(-5*time.Hour)) + addPluginFile(t, fs, coderPluginPath, filepath.Join("new_folder", "foobar.tf"), now, now.Add(-4*time.Hour)) // This plugin is older than 30 days. - addPluginFile(t, fs, dockerPluginPath, "terraform-provider-docker_v2.25.0", now.Add(-31*24*time.Hour)) - addPluginFile(t, fs, dockerPluginPath, "LICENSE", now.Add(-32*24*time.Hour)) - addPluginFile(t, fs, dockerPluginPath, "README.md", now.Add(-33*24*time.Hour)) + addPluginFile(t, fs, dockerPluginPath, "terraform-provider-docker_v2.25.0", now, now.Add(-31*24*time.Hour)) + addPluginFile(t, fs, dockerPluginPath, "LICENSE", now, now.Add(-32*24*time.Hour)) + addPluginFile(t, fs, dockerPluginPath, "README.md", now, now.Add(-33*24*time.Hour)) // when terraform.CleanStaleTerraformPlugins(ctx, cachePath, fs, now, logger) @@ -106,18 +108,18 @@ func TestPluginCache_Golden(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) defer cancel() - fs, now, logger := prepare() + fs, logger := prepare() // given - addPluginFile(t, fs, coderPluginPath, "terraform-provider-coder_v0.11.1", now.Add(-63*24*time.Hour)) - addPluginFile(t, fs, coderPluginPath, "LICENSE", now.Add(-33*24*time.Hour)) - addPluginFile(t, fs, coderPluginPath, "README.md", now.Add(-31*24*time.Hour)) - addPluginFolder(t, fs, coderPluginPath, "new_folder", now.Add(-4*time.Hour)) // touched - addPluginFile(t, fs, coderPluginPath, filepath.Join("new_folder", "foobar.tf"), now.Add(-43*24*time.Hour)) + addPluginFile(t, fs, coderPluginPath, "terraform-provider-coder_v0.11.1", now, now.Add(-63*24*time.Hour)) + addPluginFile(t, fs, coderPluginPath, "LICENSE", now, now.Add(-33*24*time.Hour)) + addPluginFile(t, fs, coderPluginPath, "README.md", now, now.Add(-31*24*time.Hour)) + addPluginFolder(t, fs, coderPluginPath, "new_folder", now, now.Add(-43*24*time.Hour)) + addPluginFile(t, fs, coderPluginPath, filepath.Join("new_folder", "foobar.tf"), now, now.Add(-4*time.Hour)) // touched - addPluginFile(t, fs, dockerPluginPath, "terraform-provider-docker_v2.25.0", now.Add(-31*24*time.Hour)) - addPluginFile(t, fs, dockerPluginPath, "LICENSE", now.Add(-2*time.Hour)) - addPluginFile(t, fs, dockerPluginPath, "README.md", now.Add(-33*24*time.Hour)) + addPluginFile(t, fs, dockerPluginPath, "terraform-provider-docker_v2.25.0", now, now.Add(-31*24*time.Hour)) + addPluginFile(t, fs, dockerPluginPath, "LICENSE", now, now.Add(-2*time.Hour)) // also touched + addPluginFile(t, fs, dockerPluginPath, "README.md", now, now.Add(-33*24*time.Hour)) // when terraform.CleanStaleTerraformPlugins(ctx, cachePath, fs, now, logger) @@ -127,25 +129,34 @@ func TestPluginCache_Golden(t *testing.T) { }) } -func addPluginFile(t *testing.T, fs afero.Fs, pluginPath string, resourcePath string, accessTime time.Time) { +func addPluginFile(t *testing.T, fs afero.Fs, pluginPath string, resourcePath string, atime, mtime time.Time) { err := fs.MkdirAll(filepath.Join(cachePath, pluginPath), 0o755) require.NoError(t, err, "can't create test folder for plugin file") - err = fs.Chtimes(filepath.Join(cachePath, pluginPath), accessTime, accessTime) + err = fs.Chtimes(filepath.Join(cachePath, pluginPath), atime, mtime) require.NoError(t, err, "can't set times") err = afero.WriteFile(fs, filepath.Join(cachePath, pluginPath, resourcePath), []byte("foo"), 0o644) require.NoError(t, err, "can't create test file") - err = fs.Chtimes(filepath.Join(cachePath, pluginPath, resourcePath), accessTime, accessTime) + err = fs.Chtimes(filepath.Join(cachePath, pluginPath, resourcePath), atime, mtime) require.NoError(t, err, "can't set times") + + // as creating a file will update mtime of parent, we also want to + // set the mtime of parent to match that of the new child. + parent, _ := filepath.Split(filepath.Join(cachePath, pluginPath, resourcePath)) + parentInfo, err := fs.Stat(parent) + require.NoError(t, err, "can't stat parent") + if parentInfo.ModTime().After(mtime) { + require.NoError(t, fs.Chtimes(parent, atime, mtime), "can't set mtime of parent to match child") + } } -func addPluginFolder(t *testing.T, fs afero.Fs, pluginPath string, folderPath string, accessTime time.Time) { +func addPluginFolder(t *testing.T, fs afero.Fs, pluginPath string, folderPath string, atime, mtime time.Time) { err := fs.MkdirAll(filepath.Join(cachePath, pluginPath, folderPath), 0o755) require.NoError(t, err, "can't create plugin folder") - err = fs.Chtimes(filepath.Join(cachePath, pluginPath, folderPath), accessTime, accessTime) + err = fs.Chtimes(filepath.Join(cachePath, pluginPath, folderPath), atime, mtime) require.NoError(t, err, "can't set times") } From 97b92715bafd97813098f0a47ad3bf366473d8cf Mon Sep 17 00:00:00 2001 From: Cian Johnston Date: Mon, 27 Nov 2023 14:29:26 +0000 Subject: [PATCH 2/2] remove unnecessary atime argument given we just use atime=now all the time --- provisioner/terraform/cleanup_test.go | 60 +++++++++++++-------------- 1 file changed, 30 insertions(+), 30 deletions(-) diff --git a/provisioner/terraform/cleanup_test.go b/provisioner/terraform/cleanup_test.go index aa2e1cddc5b1b..e234dec0deb44 100644 --- a/provisioner/terraform/cleanup_test.go +++ b/provisioner/terraform/cleanup_test.go @@ -57,16 +57,16 @@ func TestPluginCache_Golden(t *testing.T) { // given // This plugin is older than 30 days. - addPluginFile(t, fs, coderPluginPath, "terraform-provider-coder_v0.11.1", now, now.Add(-63*24*time.Hour)) - addPluginFile(t, fs, coderPluginPath, "LICENSE", now, now.Add(-33*24*time.Hour)) - addPluginFile(t, fs, coderPluginPath, "README.md", now, now.Add(-31*24*time.Hour)) - addPluginFolder(t, fs, coderPluginPath, "new_folder", now, now.Add(-31*24*time.Hour)) - addPluginFile(t, fs, coderPluginPath, filepath.Join("new_folder", "foobar.tf"), now, now.Add(-43*24*time.Hour)) + addPluginFile(t, fs, coderPluginPath, "terraform-provider-coder_v0.11.1", now.Add(-63*24*time.Hour)) + addPluginFile(t, fs, coderPluginPath, "LICENSE", now.Add(-33*24*time.Hour)) + addPluginFile(t, fs, coderPluginPath, "README.md", now.Add(-31*24*time.Hour)) + addPluginFolder(t, fs, coderPluginPath, "new_folder", now.Add(-31*24*time.Hour)) + addPluginFile(t, fs, coderPluginPath, filepath.Join("new_folder", "foobar.tf"), now.Add(-43*24*time.Hour)) // This plugin is older than 30 days. - addPluginFile(t, fs, dockerPluginPath, "terraform-provider-docker_v2.25.0", now, now.Add(-31*24*time.Hour)) - addPluginFile(t, fs, dockerPluginPath, "LICENSE", now, now.Add(-32*24*time.Hour)) - addPluginFile(t, fs, dockerPluginPath, "README.md", now, now.Add(-33*24*time.Hour)) + addPluginFile(t, fs, dockerPluginPath, "terraform-provider-docker_v2.25.0", now.Add(-31*24*time.Hour)) + addPluginFile(t, fs, dockerPluginPath, "LICENSE", now.Add(-32*24*time.Hour)) + addPluginFile(t, fs, dockerPluginPath, "README.md", now.Add(-33*24*time.Hour)) // when terraform.CleanStaleTerraformPlugins(ctx, cachePath, fs, now, logger) @@ -84,16 +84,16 @@ func TestPluginCache_Golden(t *testing.T) { fs, logger := prepare() // given - addPluginFile(t, fs, coderPluginPath, "terraform-provider-coder_v0.11.1", now, now.Add(-2*time.Hour)) - addPluginFile(t, fs, coderPluginPath, "LICENSE", now, now.Add(-3*time.Hour)) - addPluginFile(t, fs, coderPluginPath, "README.md", now, now.Add(-4*time.Hour)) - addPluginFolder(t, fs, coderPluginPath, "new_folder", now, now.Add(-5*time.Hour)) - addPluginFile(t, fs, coderPluginPath, filepath.Join("new_folder", "foobar.tf"), now, now.Add(-4*time.Hour)) + addPluginFile(t, fs, coderPluginPath, "terraform-provider-coder_v0.11.1", now.Add(-2*time.Hour)) + addPluginFile(t, fs, coderPluginPath, "LICENSE", now.Add(-3*time.Hour)) + addPluginFile(t, fs, coderPluginPath, "README.md", now.Add(-4*time.Hour)) + addPluginFolder(t, fs, coderPluginPath, "new_folder", now.Add(-5*time.Hour)) + addPluginFile(t, fs, coderPluginPath, filepath.Join("new_folder", "foobar.tf"), now.Add(-4*time.Hour)) // This plugin is older than 30 days. - addPluginFile(t, fs, dockerPluginPath, "terraform-provider-docker_v2.25.0", now, now.Add(-31*24*time.Hour)) - addPluginFile(t, fs, dockerPluginPath, "LICENSE", now, now.Add(-32*24*time.Hour)) - addPluginFile(t, fs, dockerPluginPath, "README.md", now, now.Add(-33*24*time.Hour)) + addPluginFile(t, fs, dockerPluginPath, "terraform-provider-docker_v2.25.0", now.Add(-31*24*time.Hour)) + addPluginFile(t, fs, dockerPluginPath, "LICENSE", now.Add(-32*24*time.Hour)) + addPluginFile(t, fs, dockerPluginPath, "README.md", now.Add(-33*24*time.Hour)) // when terraform.CleanStaleTerraformPlugins(ctx, cachePath, fs, now, logger) @@ -111,15 +111,15 @@ func TestPluginCache_Golden(t *testing.T) { fs, logger := prepare() // given - addPluginFile(t, fs, coderPluginPath, "terraform-provider-coder_v0.11.1", now, now.Add(-63*24*time.Hour)) - addPluginFile(t, fs, coderPluginPath, "LICENSE", now, now.Add(-33*24*time.Hour)) - addPluginFile(t, fs, coderPluginPath, "README.md", now, now.Add(-31*24*time.Hour)) - addPluginFolder(t, fs, coderPluginPath, "new_folder", now, now.Add(-43*24*time.Hour)) - addPluginFile(t, fs, coderPluginPath, filepath.Join("new_folder", "foobar.tf"), now, now.Add(-4*time.Hour)) // touched + addPluginFile(t, fs, coderPluginPath, "terraform-provider-coder_v0.11.1", now.Add(-63*24*time.Hour)) + addPluginFile(t, fs, coderPluginPath, "LICENSE", now.Add(-33*24*time.Hour)) + addPluginFile(t, fs, coderPluginPath, "README.md", now.Add(-31*24*time.Hour)) + addPluginFolder(t, fs, coderPluginPath, "new_folder", now.Add(-43*24*time.Hour)) + addPluginFile(t, fs, coderPluginPath, filepath.Join("new_folder", "foobar.tf"), now.Add(-4*time.Hour)) // touched - addPluginFile(t, fs, dockerPluginPath, "terraform-provider-docker_v2.25.0", now, now.Add(-31*24*time.Hour)) - addPluginFile(t, fs, dockerPluginPath, "LICENSE", now, now.Add(-2*time.Hour)) // also touched - addPluginFile(t, fs, dockerPluginPath, "README.md", now, now.Add(-33*24*time.Hour)) + addPluginFile(t, fs, dockerPluginPath, "terraform-provider-docker_v2.25.0", now.Add(-31*24*time.Hour)) + addPluginFile(t, fs, dockerPluginPath, "LICENSE", now.Add(-2*time.Hour)) // also touched + addPluginFile(t, fs, dockerPluginPath, "README.md", now.Add(-33*24*time.Hour)) // when terraform.CleanStaleTerraformPlugins(ctx, cachePath, fs, now, logger) @@ -129,17 +129,17 @@ func TestPluginCache_Golden(t *testing.T) { }) } -func addPluginFile(t *testing.T, fs afero.Fs, pluginPath string, resourcePath string, atime, mtime time.Time) { +func addPluginFile(t *testing.T, fs afero.Fs, pluginPath string, resourcePath string, mtime time.Time) { err := fs.MkdirAll(filepath.Join(cachePath, pluginPath), 0o755) require.NoError(t, err, "can't create test folder for plugin file") - err = fs.Chtimes(filepath.Join(cachePath, pluginPath), atime, mtime) + err = fs.Chtimes(filepath.Join(cachePath, pluginPath), now, mtime) require.NoError(t, err, "can't set times") err = afero.WriteFile(fs, filepath.Join(cachePath, pluginPath, resourcePath), []byte("foo"), 0o644) require.NoError(t, err, "can't create test file") - err = fs.Chtimes(filepath.Join(cachePath, pluginPath, resourcePath), atime, mtime) + err = fs.Chtimes(filepath.Join(cachePath, pluginPath, resourcePath), now, mtime) require.NoError(t, err, "can't set times") // as creating a file will update mtime of parent, we also want to @@ -148,15 +148,15 @@ func addPluginFile(t *testing.T, fs afero.Fs, pluginPath string, resourcePath st parentInfo, err := fs.Stat(parent) require.NoError(t, err, "can't stat parent") if parentInfo.ModTime().After(mtime) { - require.NoError(t, fs.Chtimes(parent, atime, mtime), "can't set mtime of parent to match child") + require.NoError(t, fs.Chtimes(parent, now, mtime), "can't set mtime of parent to match child") } } -func addPluginFolder(t *testing.T, fs afero.Fs, pluginPath string, folderPath string, atime, mtime time.Time) { +func addPluginFolder(t *testing.T, fs afero.Fs, pluginPath string, folderPath string, mtime time.Time) { err := fs.MkdirAll(filepath.Join(cachePath, pluginPath, folderPath), 0o755) require.NoError(t, err, "can't create plugin folder") - err = fs.Chtimes(filepath.Join(cachePath, pluginPath, folderPath), atime, mtime) + err = fs.Chtimes(filepath.Join(cachePath, pluginPath, folderPath), now, mtime) require.NoError(t, err, "can't set times") }