Skip to content

Commit 0babc3c

Browse files
johnstcnjohnstcn
authored
fix(provisioner/terraform/cleanup): use mtime instead of atime (#10892)
- Updates plugin staleness check to check mtime instead of atime, as atime has been shown to be unreliable - Updates existing unit test to use a real filesystem as Afero's in-memory FS doesn't support atimes at all
1 parent 707d0e9 commit 0babc3c

File tree

2 files changed

+38
-31
lines changed

2 files changed

+38
-31
lines changed

‎provisioner/terraform/cleanup.go

Lines changed: 12 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,6 @@ import (
77
"strings"
88
"time"
99

10-
"github.com/djherbis/times"
1110
"github.com/spf13/afero"
1211
"golang.org/x/xerrors"
1312

@@ -76,16 +75,16 @@ func CleanStaleTerraformPlugins(ctx context.Context, cachePath string, fs afero.
7675
// Identify stale plugins
7776
var stalePlugins []string
7877
for _, pluginPath := range pluginPaths {
79-
accessTime, err := latestAccessTime(fs, pluginPath)
78+
modTime, err := latestModTime(fs, pluginPath)
8079
if err != nil {
81-
return xerrors.Errorf("unable to evaluate latest access time for directory %q: %w", pluginPath, err)
80+
return xerrors.Errorf("unable to evaluate latest mtime for directory %q: %w", pluginPath, err)
8281
}
8382

84-
if accessTime.Add(staleTerraformPluginRetention).Before(now) {
85-
logger.Info(ctx, "plugin directory is stale and will be removed", slog.F("plugin_path", pluginPath))
83+
if modTime.Add(staleTerraformPluginRetention).Before(now) {
84+
logger.Info(ctx, "plugin directory is stale and will be removed", slog.F("plugin_path", pluginPath), slog.F("mtime", modTime))
8685
stalePlugins = append(stalePlugins, pluginPath)
8786
} else {
88-
logger.Debug(ctx, "plugin directory is not stale", slog.F("plugin_path", pluginPath))
87+
logger.Debug(ctx, "plugin directory is not stale", slog.F("plugin_path", pluginPath), slog.F("mtime", modTime))
8988
}
9089
}
9190

@@ -127,22 +126,19 @@ func CleanStaleTerraformPlugins(ctx context.Context, cachePath string, fs afero.
127126
return nil
128127
}
129128

130-
// latestAccessTime walks recursively through the directory content, and locates
131-
// the last accessed file.
132-
func latestAccessTime(fs afero.Fs, pluginPath string) (time.Time, error) {
129+
// latestModTime walks recursively through the directory content, and locates
130+
// the last created/modified file.
131+
func latestModTime(fs afero.Fs, pluginPath string) (time.Time, error) {
133132
var latest time.Time
134133
err := afero.Walk(fs, pluginPath, func(path string, info os.FileInfo, err error) error {
135134
if err != nil {
136135
return err
137136
}
138137

139-
accessTime := info.ModTime() // fallback to modTime if accessTime is not available (afero)
140-
if info.Sys() != nil {
141-
timeSpec := times.Get(info)
142-
accessTime = timeSpec.AccessTime()
143-
}
144-
if latest.Before(accessTime) {
145-
latest = accessTime
138+
// atime is not reliable, so always use mtime.
139+
modTime := info.ModTime()
140+
if modTime.After(latest) {
141+
latest = modTime
146142
}
147143
return nil
148144
})

‎provisioner/terraform/cleanup_test.go

Lines changed: 26 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -29,20 +29,22 @@ const cachePath = "/tmp/coder/provisioner-0/tf"
2929
var updateGoldenFiles = flag.Bool("update", false, "Update golden files")
3030

3131
var (
32+
now = time.Date(2023, 6, 3, 4, 5, 6, 0, time.UTC)
3233
coderPluginPath = filepath.Join("registry.terraform.io", "coder", "coder", "0.11.1", "darwin_arm64")
3334
dockerPluginPath = filepath.Join("registry.terraform.io", "kreuzwerker", "docker", "2.25.0", "darwin_arm64")
3435
)
3536

3637
func TestPluginCache_Golden(t *testing.T) {
3738
t.Parallel()
3839

39-
prepare := func() (afero.Fs, time.Time, slog.Logger) {
40-
fs := afero.NewMemMapFs()
41-
now := time.Date(2023, time.June, 3, 4, 5, 6, 0, time.UTC)
40+
prepare := func() (afero.Fs, slog.Logger) {
41+
// afero.MemMapFs does not modify atimes, so use a real FS instead.
42+
tmpDir := t.TempDir()
43+
fs := afero.NewBasePathFs(afero.NewOsFs(), tmpDir)
4244
logger := slogtest.Make(t, nil).
4345
Leveled(slog.LevelDebug).
4446
Named("cleanup-test")
45-
return fs, now, logger
47+
return fs, logger
4648
}
4749

4850
t.Run("all plugins are stale", func(t *testing.T) {
@@ -51,7 +53,7 @@ func TestPluginCache_Golden(t *testing.T) {
5153
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
5254
defer cancel()
5355

54-
fs, now, logger := prepare()
56+
fs, logger := prepare()
5557

5658
// given
5759
// This plugin is older than 30 days.
@@ -79,7 +81,7 @@ func TestPluginCache_Golden(t *testing.T) {
7981
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
8082
defer cancel()
8183

82-
fs, now, logger := prepare()
84+
fs, logger := prepare()
8385

8486
// given
8587
addPluginFile(t, fs, coderPluginPath, "terraform-provider-coder_v0.11.1", now.Add(-2*time.Hour))
@@ -106,17 +108,17 @@ func TestPluginCache_Golden(t *testing.T) {
106108
ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
107109
defer cancel()
108110

109-
fs, now, logger := prepare()
111+
fs, logger := prepare()
110112

111113
// given
112114
addPluginFile(t, fs, coderPluginPath, "terraform-provider-coder_v0.11.1", now.Add(-63*24*time.Hour))
113115
addPluginFile(t, fs, coderPluginPath, "LICENSE", now.Add(-33*24*time.Hour))
114116
addPluginFile(t, fs, coderPluginPath, "README.md", now.Add(-31*24*time.Hour))
115-
addPluginFolder(t, fs, coderPluginPath, "new_folder", now.Add(-4*time.Hour)) // touched
116-
addPluginFile(t, fs, coderPluginPath, filepath.Join("new_folder", "foobar.tf"), now.Add(-43*24*time.Hour))
117+
addPluginFolder(t, fs, coderPluginPath, "new_folder", now.Add(-43*24*time.Hour))
118+
addPluginFile(t, fs, coderPluginPath, filepath.Join("new_folder", "foobar.tf"), now.Add(-4*time.Hour)) // touched
117119

118120
addPluginFile(t, fs, dockerPluginPath, "terraform-provider-docker_v2.25.0", now.Add(-31*24*time.Hour))
119-
addPluginFile(t, fs, dockerPluginPath, "LICENSE", now.Add(-2*time.Hour))
121+
addPluginFile(t, fs, dockerPluginPath, "LICENSE", now.Add(-2*time.Hour)) // also touched
120122
addPluginFile(t, fs, dockerPluginPath, "README.md", now.Add(-33*24*time.Hour))
121123

122124
// when
@@ -127,25 +129,34 @@ func TestPluginCache_Golden(t *testing.T) {
127129
})
128130
}
129131

130-
func addPluginFile(t *testing.T, fs afero.Fs, pluginPath string, resourcePath string, accessTime time.Time) {
132+
func addPluginFile(t *testing.T, fs afero.Fs, pluginPath string, resourcePath string, mtime time.Time) {
131133
err := fs.MkdirAll(filepath.Join(cachePath, pluginPath), 0o755)
132134
require.NoError(t, err, "can't create test folder for plugin file")
133135

134-
err = fs.Chtimes(filepath.Join(cachePath, pluginPath), accessTime, accessTime)
136+
err = fs.Chtimes(filepath.Join(cachePath, pluginPath), now, mtime)
135137
require.NoError(t, err, "can't set times")
136138

137139
err = afero.WriteFile(fs, filepath.Join(cachePath, pluginPath, resourcePath), []byte("foo"), 0o644)
138140
require.NoError(t, err, "can't create test file")
139141

140-
err = fs.Chtimes(filepath.Join(cachePath, pluginPath, resourcePath), accessTime, accessTime)
142+
err = fs.Chtimes(filepath.Join(cachePath, pluginPath, resourcePath), now, mtime)
141143
require.NoError(t, err, "can't set times")
144+
145+
// as creating a file will update mtime of parent, we also want to
146+
// set the mtime of parent to match that of the new child.
147+
parent, _ := filepath.Split(filepath.Join(cachePath, pluginPath, resourcePath))
148+
parentInfo, err := fs.Stat(parent)
149+
require.NoError(t, err, "can't stat parent")
150+
if parentInfo.ModTime().After(mtime) {
151+
require.NoError(t, fs.Chtimes(parent, now, mtime), "can't set mtime of parent to match child")
152+
}
142153
}
143154

144-
func addPluginFolder(t *testing.T, fs afero.Fs, pluginPath string, folderPath string, accessTime time.Time) {
155+
func addPluginFolder(t *testing.T, fs afero.Fs, pluginPath string, folderPath string, mtime time.Time) {
145156
err := fs.MkdirAll(filepath.Join(cachePath, pluginPath, folderPath), 0o755)
146157
require.NoError(t, err, "can't create plugin folder")
147158

148-
err = fs.Chtimes(filepath.Join(cachePath, pluginPath, folderPath), accessTime, accessTime)
159+
err = fs.Chtimes(filepath.Join(cachePath, pluginPath, folderPath), now, mtime)
149160
require.NoError(t, err, "can't set times")
150161
}
151162

0 commit comments

Comments
 (0)