From 277d7c25a8bc846c29f68f43ce78187f67f031b5 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Tue, 5 Sep 2023 16:57:45 +0200 Subject: [PATCH 01/25] TODOs --- provisioner/terraform/provision.go | 2 ++ provisionersdk/session.go | 3 +++ 2 files changed, 5 insertions(+) diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index ab832e4408683..f1963b5569ac2 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -89,6 +89,8 @@ func (s *server) Plan( } } + // TODO Clean stale TF files + s.logger.Debug(ctx, "running initialization") err := e.init(ctx, killCtx, sess) if err != nil { diff --git a/provisionersdk/session.go b/provisionersdk/session.go index dfcd981ce77f5..7e72f3882030d 100644 --- a/provisionersdk/session.go +++ b/provisionersdk/session.go @@ -35,6 +35,9 @@ func (p *protoServer) Session(stream proto.DRPCProvisioner_SessionStream) error stream: stream, server: p.server, } + + // TODO Clean stale sessions in WorkDir + sessDir := fmt.Sprintf("Session%s", sessID) s.WorkDirectory = filepath.Join(p.opts.WorkDirectory, sessDir) err := os.MkdirAll(s.WorkDirectory, 0o700) From afb0982d2ebca2bcf066b2defd0c27a014e65a7c Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 6 Sep 2023 10:50:35 +0200 Subject: [PATCH 02/25] Remove stale session directories --- go.mod | 5 +++- go.sum | 2 ++ provisionersdk/session.go | 61 ++++++++++++++++++++++++++++++++++----- 3 files changed, 60 insertions(+), 8 deletions(-) diff --git a/go.mod b/go.mod index 91fee4537be8d..9b23702528ece 100644 --- a/go.mod +++ b/go.mod @@ -196,7 +196,10 @@ require ( tailscale.com v1.46.1 ) -require gopkg.in/DataDog/dd-trace-go.v1 v1.54.0 +require ( + github.com/djherbis/atime v1.1.0 + gopkg.in/DataDog/dd-trace-go.v1 v1.54.0 +) require ( cloud.google.com/go/compute v1.23.0 // indirect diff --git a/go.sum b/go.sum index 13525ca7e7816..e7705f8235aea 100644 --- a/go.sum +++ b/go.sum @@ -270,6 +270,8 @@ github.com/dgryski/go-farm v0.0.0-20200201041132-a6ae2369ad13 h1:fAjc9m62+UWV/WA github.com/dgryski/trifles v0.0.0-20200323201526-dd97f9abfb48 h1:fRzb/w+pyskVMQ+UbP35JkH8yB7MYb4q/qhBarqZE6g= github.com/dgryski/trifles v0.0.0-20200323201526-dd97f9abfb48/go.mod h1:if7Fbed8SFyPtHLHbg49SI7NAdJiC5WIA09pe59rfAA= github.com/dhui/dktest v0.3.16 h1:i6gq2YQEtcrjKbeJpBkWjE8MmLZPYllcjOFbTZuPDnw= +github.com/djherbis/atime v1.1.0 h1:rgwVbP/5by8BvvjBNrbh64Qz33idKT3pSnMSJsxhi0g= +github.com/djherbis/atime v1.1.0/go.mod h1:28OF6Y8s3NQWwacXc5eZTsEsiMzp7LF8MbXE+XJPdBE= github.com/dlclark/regexp2 v1.7.0 h1:7lJfhqlPssTb1WQx4yvTHN0uElPEv52sbaECrAQxjAo= github.com/dlclark/regexp2 v1.7.0/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8= github.com/docker/cli v23.0.5+incompatible h1:ufWmAOuD3Vmr7JP2G5K3cyuNC4YZWiAsuDEvFVVDafE= diff --git a/provisionersdk/session.go b/provisionersdk/session.go index 7e72f3882030d..26413b31aac52 100644 --- a/provisionersdk/session.go +++ b/provisionersdk/session.go @@ -11,6 +11,7 @@ import ( "strings" "time" + "github.com/djherbis/atime" "github.com/google/uuid" "golang.org/x/xerrors" @@ -18,9 +19,13 @@ import ( "github.com/coder/coder/v2/provisionersdk/proto" ) -// ReadmeFile is the location we look for to extract documentation from template -// versions. -const ReadmeFile = "README.md" +const ( + // ReadmeFile is the location we look for to extract documentation from template versions. + ReadmeFile = "README.md" + + sessionDirPrefix = "Session" + staleSessionDaysThreshold = 7 * 24 * time.Hour +) // protoServer is a wrapper that translates the dRPC protocol into a Session with method calls into the Server. type protoServer struct { @@ -36,11 +41,13 @@ func (p *protoServer) Session(stream proto.DRPCProvisioner_SessionStream) error server: p.server, } - // TODO Clean stale sessions in WorkDir + err := cleanStaleSessions(s.Context(), p.opts.WorkDirectory, time.Now(), s.Logger) + if err != nil { + return xerrors.Errorf("clean state sessions %q: %w", s.WorkDirectory, err) + } - sessDir := fmt.Sprintf("Session%s", sessID) - s.WorkDirectory = filepath.Join(p.opts.WorkDirectory, sessDir) - err := os.MkdirAll(s.WorkDirectory, 0o700) + s.WorkDirectory = filepath.Join(p.opts.WorkDirectory, sessionDir(sessID)) + err = os.MkdirAll(s.WorkDirectory, 0o700) if err != nil { return xerrors.Errorf("create work directory %q: %w", s.WorkDirectory, err) } @@ -319,3 +326,43 @@ func (r *request[R, C]) do() (C, error) { return c, nil } } + +func cleanStaleSessions(ctx context.Context, workDirectory string, now time.Time, logger slog.Logger) error { + entries, err := os.ReadDir(workDirectory) + if err != nil { + return xerrors.Errorf("can't read %q directory", workDirectory) + } + + for _, entry := range entries { + dirName := entry.Name() + + if entry.IsDir() && isValidSessionDir(dirName) { + sessionDirPath := filepath.Join(workDirectory, dirName) + fi, err := entry.Info() + if err != nil { + return xerrors.Errorf("can't read %q directory info", sessionDirPath) + } + + lastAccessTime := atime.Get(fi) + if lastAccessTime.Add(staleSessionDaysThreshold).After(now) { + continue + } + + logger.Info(ctx, "Remove stale session directory: %s", sessionDirPath) + err = os.RemoveAll(sessionDirPath) + if err != nil { + return xerrors.Errorf("can't remove %q directory", sessionDirPath) + } + } + } + return nil +} + +func sessionDir(sessID string) string { + return sessionDirPrefix + sessID +} + +func isValidSessionDir(dirName string) bool { + match, err := filepath.Match(sessionDirPrefix+"*", dirName) + return err == nil && match +} From 16244533418678013a0496ee39286320f734c9b4 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 6 Sep 2023 11:04:22 +0200 Subject: [PATCH 03/25] Fix lint --- provisionersdk/session.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/provisionersdk/session.go b/provisionersdk/session.go index 26413b31aac52..50a8480a1a4da 100644 --- a/provisionersdk/session.go +++ b/provisionersdk/session.go @@ -348,7 +348,7 @@ func cleanStaleSessions(ctx context.Context, workDirectory string, now time.Time continue } - logger.Info(ctx, "Remove stale session directory: %s", sessionDirPath) + logger.Info(ctx, "remove stale session directory: %s", sessionDirPath) err = os.RemoveAll(sessionDirPath) if err != nil { return xerrors.Errorf("can't remove %q directory", sessionDirPath) From 4bd9a56ab20456165135d143efc2c2ccc195403a Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 6 Sep 2023 11:47:06 +0200 Subject: [PATCH 04/25] log errors --- provisionersdk/session.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/provisionersdk/session.go b/provisionersdk/session.go index 50a8480a1a4da..8fc1921849a57 100644 --- a/provisionersdk/session.go +++ b/provisionersdk/session.go @@ -43,7 +43,7 @@ func (p *protoServer) Session(stream proto.DRPCProvisioner_SessionStream) error err := cleanStaleSessions(s.Context(), p.opts.WorkDirectory, time.Now(), s.Logger) if err != nil { - return xerrors.Errorf("clean state sessions %q: %w", s.WorkDirectory, err) + return xerrors.Errorf("clean stale sessions %q: %w", s.WorkDirectory, err) } s.WorkDirectory = filepath.Join(p.opts.WorkDirectory, sessionDir(sessID)) @@ -340,7 +340,7 @@ func cleanStaleSessions(ctx context.Context, workDirectory string, now time.Time sessionDirPath := filepath.Join(workDirectory, dirName) fi, err := entry.Info() if err != nil { - return xerrors.Errorf("can't read %q directory info", sessionDirPath) + return xerrors.Errorf("can't read %q directory info: %w", sessionDirPath, err) } lastAccessTime := atime.Get(fi) @@ -351,7 +351,7 @@ func cleanStaleSessions(ctx context.Context, workDirectory string, now time.Time logger.Info(ctx, "remove stale session directory: %s", sessionDirPath) err = os.RemoveAll(sessionDirPath) if err != nil { - return xerrors.Errorf("can't remove %q directory", sessionDirPath) + return xerrors.Errorf("can't remove %q directory: %w", sessionDirPath, err) } } } From d395d02bec456903e92b917bc6309c778ad53590 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 6 Sep 2023 12:23:34 +0200 Subject: [PATCH 05/25] use djherbis/times --- go.mod | 2 +- go.sum | 4 ++-- provisionersdk/session.go | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/go.mod b/go.mod index 9b23702528ece..888f303a08dda 100644 --- a/go.mod +++ b/go.mod @@ -197,7 +197,7 @@ require ( ) require ( - github.com/djherbis/atime v1.1.0 + github.com/djherbis/times v1.5.0 gopkg.in/DataDog/dd-trace-go.v1 v1.54.0 ) diff --git a/go.sum b/go.sum index e7705f8235aea..17cc96f37994c 100644 --- a/go.sum +++ b/go.sum @@ -270,8 +270,8 @@ github.com/dgryski/go-farm v0.0.0-20200201041132-a6ae2369ad13 h1:fAjc9m62+UWV/WA github.com/dgryski/trifles v0.0.0-20200323201526-dd97f9abfb48 h1:fRzb/w+pyskVMQ+UbP35JkH8yB7MYb4q/qhBarqZE6g= github.com/dgryski/trifles v0.0.0-20200323201526-dd97f9abfb48/go.mod h1:if7Fbed8SFyPtHLHbg49SI7NAdJiC5WIA09pe59rfAA= github.com/dhui/dktest v0.3.16 h1:i6gq2YQEtcrjKbeJpBkWjE8MmLZPYllcjOFbTZuPDnw= -github.com/djherbis/atime v1.1.0 h1:rgwVbP/5by8BvvjBNrbh64Qz33idKT3pSnMSJsxhi0g= -github.com/djherbis/atime v1.1.0/go.mod h1:28OF6Y8s3NQWwacXc5eZTsEsiMzp7LF8MbXE+XJPdBE= +github.com/djherbis/times v1.5.0 h1:79myA211VwPhFTqUk8xehWrsEO+zcIZj0zT8mXPVARU= +github.com/djherbis/times v1.5.0/go.mod h1:5q7FDLvbNg1L/KaBmPcWlVR9NmoKo3+ucqUA3ijQhA0= github.com/dlclark/regexp2 v1.7.0 h1:7lJfhqlPssTb1WQx4yvTHN0uElPEv52sbaECrAQxjAo= github.com/dlclark/regexp2 v1.7.0/go.mod h1:DHkYz0B9wPfa6wondMfaivmHpzrQ3v9q8cnmRbL6yW8= github.com/docker/cli v23.0.5+incompatible h1:ufWmAOuD3Vmr7JP2G5K3cyuNC4YZWiAsuDEvFVVDafE= diff --git a/provisionersdk/session.go b/provisionersdk/session.go index 8fc1921849a57..f6031c26ea251 100644 --- a/provisionersdk/session.go +++ b/provisionersdk/session.go @@ -11,7 +11,7 @@ import ( "strings" "time" - "github.com/djherbis/atime" + "github.com/djherbis/times" "github.com/google/uuid" "golang.org/x/xerrors" @@ -343,8 +343,8 @@ func cleanStaleSessions(ctx context.Context, workDirectory string, now time.Time return xerrors.Errorf("can't read %q directory info: %w", sessionDirPath, err) } - lastAccessTime := atime.Get(fi) - if lastAccessTime.Add(staleSessionDaysThreshold).After(now) { + timeSpec := times.Get(fi) + if timeSpec.AccessTime().Add(staleSessionDaysThreshold).After(now) { continue } From e2c71f87a7589fa4e3d4f381d40adc254a3753d7 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 6 Sep 2023 14:23:39 +0200 Subject: [PATCH 06/25] Stub for cleanStaleTerraformPlugins --- provisioner/terraform/provision.go | 29 +++++++++++++++++++++++++++-- provisionersdk/session.go | 5 ++++- 2 files changed, 31 insertions(+), 3 deletions(-) diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index f1963b5569ac2..8db44de77aa0f 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -89,10 +89,13 @@ func (s *server) Plan( } } - // TODO Clean stale TF files + err := cleanStaleTerraformPlugins(sess.Context(), s.cachePath, time.Now(), s.logger) + if err != nil { + return provisionersdk.PlanErrorf("unable to clean stale Terraform plugins: %s", err) + } s.logger.Debug(ctx, "running initialization") - err := e.init(ctx, killCtx, sess) + err = e.init(ctx, killCtx, sess) if err != nil { s.logger.Debug(ctx, "init failed", slog.Error(err)) return provisionersdk.PlanErrorf("initialize terraform: %s", err) @@ -241,3 +244,25 @@ func logTerraformEnvVars(sink logSink) { } } } + +// cleanStaleTerraformPlugins browses the Terraform cache directory +// and remove stale plugins that haven't been used for a while. +// +// Additionally, it sweeps empty, old directory trees. +// +// Sample cachePath: /Users//Library/Caches/coder/provisioner-/tf +func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time.Time, logger slog.Logger) error { + // Review cached Terraform plugins + // TODO + + // Identify stale plugins + // TODO + + // Remove stale plugins + // TODO + + // Maintain the directory tree + // TODO + + return nil +} diff --git a/provisionersdk/session.go b/provisionersdk/session.go index f6031c26ea251..975bc0773d39a 100644 --- a/provisionersdk/session.go +++ b/provisionersdk/session.go @@ -43,7 +43,7 @@ func (p *protoServer) Session(stream proto.DRPCProvisioner_SessionStream) error err := cleanStaleSessions(s.Context(), p.opts.WorkDirectory, time.Now(), s.Logger) if err != nil { - return xerrors.Errorf("clean stale sessions %q: %w", s.WorkDirectory, err) + return xerrors.Errorf("unable to clean stale sessions %q: %w", s.WorkDirectory, err) } s.WorkDirectory = filepath.Join(p.opts.WorkDirectory, sessionDir(sessID)) @@ -327,6 +327,9 @@ func (r *request[R, C]) do() (C, error) { } } +// cleanStaleSessions browses the work directory searching for stale session +// directories. Coder provisioner is supposed to remove them once after finishing the provisioning, +// but there is a risk of keeping them in case of a failure. func cleanStaleSessions(ctx context.Context, workDirectory string, now time.Time, logger slog.Logger) error { entries, err := os.ReadDir(workDirectory) if err != nil { From 19741fc8f78ea409fb5b4d1354ff610bccbc048a Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 6 Sep 2023 15:15:47 +0200 Subject: [PATCH 07/25] Review cached Terraform plugins --- provisioner/terraform/provision.go | 45 +++++++++++++++++++++++++++++- 1 file changed, 44 insertions(+), 1 deletion(-) diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index 8db44de77aa0f..d587ee46b373f 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -3,11 +3,14 @@ package terraform import ( "context" "fmt" + "io/fs" "os" + "path/filepath" "strings" "time" "cdr.dev/slog" + "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/tracing" "github.com/coder/coder/v2/provisionersdk" @@ -253,9 +256,49 @@ func logTerraformEnvVars(sink logSink) { // Sample cachePath: /Users//Library/Caches/coder/provisioner-/tf func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time.Time, logger slog.Logger) error { // Review cached Terraform plugins - // TODO + + // Filter directory trees matching pattern: //// + filterFunc := func(path string, info os.FileInfo) bool { + if !info.IsDir() { + return false + } + + relativePath, err := filepath.Rel(cachePath, path) + if err != nil { + logger.Error(ctx, "unable to evaluate a relative path (base: %s, target: %s): %w", cachePath, path, err) + return false + } + + parts := strings.Split(relativePath, string(filepath.Separator)) + if len(parts) >= 5 { + return false + } + return true + } + + var pluginPaths []string + err := filepath.Walk(cachePath, func(path string, info fs.FileInfo, err error) error { + if err != nil { + return err + } + + if !filterFunc(path, info) { + return nil + } + + logger.Debug(ctx, "Plugin directory discovered: %s", path) + pluginPaths = append(pluginPaths, path) + return nil + }) + if err != nil { + return xerrors.Errorf("unable to walk through cache directory %q: %w", cachePath, err) + } // Identify stale plugins + var stalePlugins []string + for _, pluginPath := range pluginPaths { + + } // TODO // Remove stale plugins From 2af24c2d40cbee6090ee25d998c41f36ba3b56f5 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 6 Sep 2023 16:09:10 +0200 Subject: [PATCH 08/25] WIP access time --- provisioner/terraform/provision.go | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index d587ee46b373f..98ac6cb7e469c 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -297,7 +297,10 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. // Identify stale plugins var stalePlugins []string for _, pluginPath := range pluginPaths { - + accessTime, err := latestAccessTime(pluginPath) + if err != nil { + return xerrors.Errorf("unable to evaluate latest access time for directory %q: %w", pluginPath, err) + } } // TODO @@ -309,3 +312,7 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. return nil } + +func latestAccessTime(path string) (time.Time, error) { + +} From 85a74cdc2f8f140920ccf96192e64f7dfee2298b Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Wed, 6 Sep 2023 16:16:38 +0200 Subject: [PATCH 09/25] WIP --- provisioner/terraform/provision.go | 9 +++++++-- provisionersdk/session.go | 6 +++--- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index 98ac6cb7e469c..eccadef9c1279 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -18,6 +18,8 @@ import ( "github.com/coder/terraform-provider-coder/provider" ) +const staleTerraformPluginRetention = 30 * 24 * time.Hour + func (s *server) setupContexts(parent context.Context, canceledOrComplete <-chan struct{}) ( ctx context.Context, cancel func(), killCtx context.Context, kill func(), ) { @@ -301,8 +303,11 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. if err != nil { return xerrors.Errorf("unable to evaluate latest access time for directory %q: %w", pluginPath, err) } + + if accessTime.Add(staleTerraformPluginRetention).Before(now) { + stalePlugins = append(stalePlugins, pluginPath) + } } - // TODO // Remove stale plugins // TODO @@ -314,5 +319,5 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. } func latestAccessTime(path string) (time.Time, error) { - + // TODO } diff --git a/provisionersdk/session.go b/provisionersdk/session.go index 975bc0773d39a..eebf013b7d0de 100644 --- a/provisionersdk/session.go +++ b/provisionersdk/session.go @@ -23,8 +23,8 @@ const ( // ReadmeFile is the location we look for to extract documentation from template versions. ReadmeFile = "README.md" - sessionDirPrefix = "Session" - staleSessionDaysThreshold = 7 * 24 * time.Hour + sessionDirPrefix = "Session" + staleSessionRetention = 7 * 24 * time.Hour ) // protoServer is a wrapper that translates the dRPC protocol into a Session with method calls into the Server. @@ -347,7 +347,7 @@ func cleanStaleSessions(ctx context.Context, workDirectory string, now time.Time } timeSpec := times.Get(fi) - if timeSpec.AccessTime().Add(staleSessionDaysThreshold).After(now) { + if timeSpec.AccessTime().Add(staleSessionRetention).After(now) { continue } From 20bd67c0d1e17c1a25da9ac8a4df0bb0594b08ff Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 7 Sep 2023 11:31:38 +0200 Subject: [PATCH 10/25] Implementation --- provisioner/terraform/provision.go | 70 ++++++++++++++++++++++++++---- 1 file changed, 62 insertions(+), 8 deletions(-) diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index eccadef9c1279..457e6890536a3 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -9,9 +9,12 @@ import ( "strings" "time" - "cdr.dev/slog" "golang.org/x/xerrors" + "cdr.dev/slog" + + "github.com/djherbis/times" + "github.com/coder/coder/v2/coderd/tracing" "github.com/coder/coder/v2/provisionersdk" "github.com/coder/coder/v2/provisionersdk/proto" @@ -257,7 +260,10 @@ func logTerraformEnvVars(sink logSink) { // // Sample cachePath: /Users//Library/Caches/coder/provisioner-/tf func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time.Time, logger slog.Logger) error { - // Review cached Terraform plugins + cachePath, err := filepath.Abs(cachePath) // sanity check in case the path is e.g. ../../../cache + if err != nil { + return xerrors.Errorf("unable to determine absolute path %q: %w", cachePath, err) + } // Filter directory trees matching pattern: //// filterFunc := func(path string, info os.FileInfo) bool { @@ -278,8 +284,9 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. return true } + // Review cached Terraform plugins var pluginPaths []string - err := filepath.Walk(cachePath, func(path string, info fs.FileInfo, err error) error { + err = filepath.Walk(cachePath, func(path string, info fs.FileInfo, err error) error { if err != nil { return err } @@ -310,14 +317,61 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. } // Remove stale plugins - // TODO + for _, stalePluginPath := range stalePlugins { + // Remove the plugin directory + err = os.RemoveAll(stalePluginPath) + if err != nil { + return xerrors.Errorf("unable to remove stale plugin %q: %w", stalePluginPath, err) + } + + // Compact the plugin structure by removing empty directories. + wd := stalePluginPath + level := 4 // //// + for { + level-- + if level == 0 { + break // do not compact further + } + + wd = filepath.Dir(wd) + + files, err := os.ReadDir(wd) + if err != nil { + return xerrors.Errorf("unable to read directory content %q: %w", wd, err) + } - // Maintain the directory tree - // TODO + if len(files) > 0 { + break // there are still other plugins + } + logger.Debug(ctx, "remove empty directory: %s", wd) + err = os.Remove(wd) + if err != nil { + return xerrors.Errorf("unable to remove directory %q: %w", wd, err) + } + } + } return nil } -func latestAccessTime(path string) (time.Time, error) { - // TODO +// latestAccessTime walks recursively through the directory content, and locates +// the last accessed file. +func latestAccessTime(pluginPath string) (time.Time, error) { + var latest time.Time + err := filepath.Walk(pluginPath, func(path string, info fs.FileInfo, err error) error { + if err != nil { + return err + } + + timeSpec := times.Get(info) + accessTime := timeSpec.AccessTime() + if latest.Before(accessTime) { + latest = accessTime + } + return nil + }) + if err != nil { + return time.Time{}, xerrors.Errorf("unable to walk the plugin path %q: %w", pluginPath, err) + } + return latest, nil } From 905388b639d8d14bcdf4f596627a445c90f0d7d8 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 7 Sep 2023 11:32:16 +0200 Subject: [PATCH 11/25] lint --- provisioner/terraform/provision.go | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index 457e6890536a3..ccaf0b94f2e4d 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -9,11 +9,9 @@ import ( "strings" "time" - "golang.org/x/xerrors" - "cdr.dev/slog" - "github.com/djherbis/times" + "golang.org/x/xerrors" "github.com/coder/coder/v2/coderd/tracing" "github.com/coder/coder/v2/provisionersdk" From cd0c134ce1ed65ada0aa74e5eaa75dabf2746e1f Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 7 Sep 2023 11:59:28 +0200 Subject: [PATCH 12/25] more fixes --- provisioner/terraform/provision.go | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index ccaf0b94f2e4d..0f945e1264b73 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -9,10 +9,11 @@ import ( "strings" "time" - "cdr.dev/slog" "github.com/djherbis/times" "golang.org/x/xerrors" + "cdr.dev/slog" + "github.com/coder/coder/v2/coderd/tracing" "github.com/coder/coder/v2/provisionersdk" "github.com/coder/coder/v2/provisionersdk/proto" @@ -276,10 +277,7 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. } parts := strings.Split(relativePath, string(filepath.Separator)) - if len(parts) >= 5 { - return false - } - return true + return len(parts) == 5 } // Review cached Terraform plugins @@ -293,7 +291,7 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. return nil } - logger.Debug(ctx, "Plugin directory discovered: %s", path) + logger.Debug(ctx, "plugin directory discovered: %s", path) pluginPaths = append(pluginPaths, path) return nil }) From 7c8ca9ff208df41fd72465980b5afc66738fac93 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 7 Sep 2023 12:18:38 +0200 Subject: [PATCH 13/25] polishing --- provisioner/terraform/provision.go | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index 0f945e1264b73..3ee260422dd0b 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -96,7 +96,7 @@ func (s *server) Plan( } } - err := cleanStaleTerraformPlugins(sess.Context(), s.cachePath, time.Now(), s.logger) + err := cleanStaleTerraformPlugins(sess.Context(), "/tmp/coder/provisioner-0/tf", time.Now(), s.logger) if err != nil { return provisionersdk.PlanErrorf("unable to clean stale Terraform plugins: %s", err) } @@ -264,6 +264,8 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. return xerrors.Errorf("unable to determine absolute path %q: %w", cachePath, err) } + logger.Info(ctx, "clean stale Terraform plugins", slog.F("cache_path", cachePath)) + // Filter directory trees matching pattern: //// filterFunc := func(path string, info os.FileInfo) bool { if !info.IsDir() { @@ -272,7 +274,7 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. relativePath, err := filepath.Rel(cachePath, path) if err != nil { - logger.Error(ctx, "unable to evaluate a relative path (base: %s, target: %s): %w", cachePath, path, err) + logger.Error(ctx, "unable to evaluate a relative path", slog.F("base", cachePath), slog.F("target", path), slog.Error(err)) return false } @@ -291,7 +293,7 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. return nil } - logger.Debug(ctx, "plugin directory discovered: %s", path) + logger.Debug(ctx, "plugin directory discovered", slog.F("path", path)) pluginPaths = append(pluginPaths, path) return nil }) @@ -308,7 +310,10 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. } if accessTime.Add(staleTerraformPluginRetention).Before(now) { + logger.Info(ctx, "plugin directory is stale and will be removed", slog.F("plugin_path", pluginPath)) stalePlugins = append(stalePlugins, pluginPath) + } else { + logger.Debug(ctx, "plugin directory is not stale", slog.F("plugin_path", pluginPath)) } } @@ -340,7 +345,7 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. break // there are still other plugins } - logger.Debug(ctx, "remove empty directory: %s", wd) + logger.Debug(ctx, "remove empty directory", slog.F("path", wd)) err = os.Remove(wd) if err != nil { return xerrors.Errorf("unable to remove directory %q: %w", wd, err) From 574f0843f0b0a2ebea9ceb4171e1aceb60c6aa92 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 7 Sep 2023 12:21:29 +0200 Subject: [PATCH 14/25] move to cleanups --- provisioner/terraform/cleanup.go | 139 +++++++++++++++++++++++++++++ provisioner/terraform/provision.go | 132 +-------------------------- provisionersdk/cleanup.go | 52 +++++++++++ provisionersdk/session.go | 41 +-------- 4 files changed, 193 insertions(+), 171 deletions(-) create mode 100644 provisioner/terraform/cleanup.go create mode 100644 provisionersdk/cleanup.go diff --git a/provisioner/terraform/cleanup.go b/provisioner/terraform/cleanup.go new file mode 100644 index 0000000000000..434185ecab66e --- /dev/null +++ b/provisioner/terraform/cleanup.go @@ -0,0 +1,139 @@ +package terraform + +import ( + "context" + "io/fs" + "os" + "path/filepath" + "strings" + "time" + + "cdr.dev/slog" + "github.com/djherbis/times" + "golang.org/x/xerrors" +) + +// cleanStaleTerraformPlugins browses the Terraform cache directory +// and remove stale plugins that haven't been used for a while. +// +// Additionally, it sweeps empty, old directory trees. +// +// Sample cachePath: /Users//Library/Caches/coder/provisioner-/tf +func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time.Time, logger slog.Logger) error { + cachePath, err := filepath.Abs(cachePath) // sanity check in case the path is e.g. ../../../cache + if err != nil { + return xerrors.Errorf("unable to determine absolute path %q: %w", cachePath, err) + } + + logger.Info(ctx, "clean stale Terraform plugins", slog.F("cache_path", cachePath)) + + // Filter directory trees matching pattern: //// + filterFunc := func(path string, info os.FileInfo) bool { + if !info.IsDir() { + return false + } + + relativePath, err := filepath.Rel(cachePath, path) + if err != nil { + logger.Error(ctx, "unable to evaluate a relative path", slog.F("base", cachePath), slog.F("target", path), slog.Error(err)) + return false + } + + parts := strings.Split(relativePath, string(filepath.Separator)) + return len(parts) == 5 + } + + // Review cached Terraform plugins + var pluginPaths []string + err = filepath.Walk(cachePath, func(path string, info fs.FileInfo, err error) error { + if err != nil { + return err + } + + if !filterFunc(path, info) { + return nil + } + + logger.Debug(ctx, "plugin directory discovered", slog.F("path", path)) + pluginPaths = append(pluginPaths, path) + return nil + }) + if err != nil { + return xerrors.Errorf("unable to walk through cache directory %q: %w", cachePath, err) + } + + // Identify stale plugins + var stalePlugins []string + for _, pluginPath := range pluginPaths { + accessTime, err := latestAccessTime(pluginPath) + if err != nil { + return xerrors.Errorf("unable to evaluate latest access time 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)) + stalePlugins = append(stalePlugins, pluginPath) + } else { + logger.Debug(ctx, "plugin directory is not stale", slog.F("plugin_path", pluginPath)) + } + } + + // Remove stale plugins + for _, stalePluginPath := range stalePlugins { + // Remove the plugin directory + err = os.RemoveAll(stalePluginPath) + if err != nil { + return xerrors.Errorf("unable to remove stale plugin %q: %w", stalePluginPath, err) + } + + // Compact the plugin structure by removing empty directories. + wd := stalePluginPath + level := 4 // //// + for { + level-- + if level == 0 { + break // do not compact further + } + + wd = filepath.Dir(wd) + + files, err := os.ReadDir(wd) + if err != nil { + return xerrors.Errorf("unable to read directory content %q: %w", wd, err) + } + + if len(files) > 0 { + break // there are still other plugins + } + + logger.Debug(ctx, "remove empty directory", slog.F("path", wd)) + err = os.Remove(wd) + if err != nil { + return xerrors.Errorf("unable to remove directory %q: %w", wd, err) + } + } + } + return nil +} + +// latestAccessTime walks recursively through the directory content, and locates +// the last accessed file. +func latestAccessTime(pluginPath string) (time.Time, error) { + var latest time.Time + err := filepath.Walk(pluginPath, func(path string, info fs.FileInfo, err error) error { + if err != nil { + return err + } + + timeSpec := times.Get(info) + accessTime := timeSpec.AccessTime() + if latest.Before(accessTime) { + latest = accessTime + } + return nil + }) + if err != nil { + return time.Time{}, xerrors.Errorf("unable to walk the plugin path %q: %w", pluginPath, err) + } + return latest, nil +} diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index 3ee260422dd0b..d75802e537051 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -3,21 +3,16 @@ package terraform import ( "context" "fmt" - "io/fs" "os" - "path/filepath" "strings" "time" - "github.com/djherbis/times" - "golang.org/x/xerrors" - "cdr.dev/slog" + "github.com/coder/terraform-provider-coder/provider" "github.com/coder/coder/v2/coderd/tracing" "github.com/coder/coder/v2/provisionersdk" "github.com/coder/coder/v2/provisionersdk/proto" - "github.com/coder/terraform-provider-coder/provider" ) const staleTerraformPluginRetention = 30 * 24 * time.Hour @@ -251,128 +246,3 @@ func logTerraformEnvVars(sink logSink) { } } } - -// cleanStaleTerraformPlugins browses the Terraform cache directory -// and remove stale plugins that haven't been used for a while. -// -// Additionally, it sweeps empty, old directory trees. -// -// Sample cachePath: /Users//Library/Caches/coder/provisioner-/tf -func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time.Time, logger slog.Logger) error { - cachePath, err := filepath.Abs(cachePath) // sanity check in case the path is e.g. ../../../cache - if err != nil { - return xerrors.Errorf("unable to determine absolute path %q: %w", cachePath, err) - } - - logger.Info(ctx, "clean stale Terraform plugins", slog.F("cache_path", cachePath)) - - // Filter directory trees matching pattern: //// - filterFunc := func(path string, info os.FileInfo) bool { - if !info.IsDir() { - return false - } - - relativePath, err := filepath.Rel(cachePath, path) - if err != nil { - logger.Error(ctx, "unable to evaluate a relative path", slog.F("base", cachePath), slog.F("target", path), slog.Error(err)) - return false - } - - parts := strings.Split(relativePath, string(filepath.Separator)) - return len(parts) == 5 - } - - // Review cached Terraform plugins - var pluginPaths []string - err = filepath.Walk(cachePath, func(path string, info fs.FileInfo, err error) error { - if err != nil { - return err - } - - if !filterFunc(path, info) { - return nil - } - - logger.Debug(ctx, "plugin directory discovered", slog.F("path", path)) - pluginPaths = append(pluginPaths, path) - return nil - }) - if err != nil { - return xerrors.Errorf("unable to walk through cache directory %q: %w", cachePath, err) - } - - // Identify stale plugins - var stalePlugins []string - for _, pluginPath := range pluginPaths { - accessTime, err := latestAccessTime(pluginPath) - if err != nil { - return xerrors.Errorf("unable to evaluate latest access time 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)) - stalePlugins = append(stalePlugins, pluginPath) - } else { - logger.Debug(ctx, "plugin directory is not stale", slog.F("plugin_path", pluginPath)) - } - } - - // Remove stale plugins - for _, stalePluginPath := range stalePlugins { - // Remove the plugin directory - err = os.RemoveAll(stalePluginPath) - if err != nil { - return xerrors.Errorf("unable to remove stale plugin %q: %w", stalePluginPath, err) - } - - // Compact the plugin structure by removing empty directories. - wd := stalePluginPath - level := 4 // //// - for { - level-- - if level == 0 { - break // do not compact further - } - - wd = filepath.Dir(wd) - - files, err := os.ReadDir(wd) - if err != nil { - return xerrors.Errorf("unable to read directory content %q: %w", wd, err) - } - - if len(files) > 0 { - break // there are still other plugins - } - - logger.Debug(ctx, "remove empty directory", slog.F("path", wd)) - err = os.Remove(wd) - if err != nil { - return xerrors.Errorf("unable to remove directory %q: %w", wd, err) - } - } - } - return nil -} - -// latestAccessTime walks recursively through the directory content, and locates -// the last accessed file. -func latestAccessTime(pluginPath string) (time.Time, error) { - var latest time.Time - err := filepath.Walk(pluginPath, func(path string, info fs.FileInfo, err error) error { - if err != nil { - return err - } - - timeSpec := times.Get(info) - accessTime := timeSpec.AccessTime() - if latest.Before(accessTime) { - latest = accessTime - } - return nil - }) - if err != nil { - return time.Time{}, xerrors.Errorf("unable to walk the plugin path %q: %w", pluginPath, err) - } - return latest, nil -} diff --git a/provisionersdk/cleanup.go b/provisionersdk/cleanup.go new file mode 100644 index 0000000000000..6aba8b2f48a25 --- /dev/null +++ b/provisionersdk/cleanup.go @@ -0,0 +1,52 @@ +package provisionersdk + +import ( + "context" + "os" + "path/filepath" + "time" + + "github.com/djherbis/times" + "golang.org/x/xerrors" + + "cdr.dev/slog" +) + +// cleanStaleSessions browses the work directory searching for stale session +// directories. Coder provisioner is supposed to remove them once after finishing the provisioning, +// but there is a risk of keeping them in case of a failure. +func cleanStaleSessions(ctx context.Context, workDirectory string, now time.Time, logger slog.Logger) error { + entries, err := os.ReadDir(workDirectory) + if err != nil { + return xerrors.Errorf("can't read %q directory", workDirectory) + } + + for _, entry := range entries { + dirName := entry.Name() + + if entry.IsDir() && isValidSessionDir(dirName) { + sessionDirPath := filepath.Join(workDirectory, dirName) + fi, err := entry.Info() + if err != nil { + return xerrors.Errorf("can't read %q directory info: %w", sessionDirPath, err) + } + + timeSpec := times.Get(fi) + if timeSpec.AccessTime().Add(staleSessionRetention).After(now) { + continue + } + + logger.Info(ctx, "remove stale session directory: %s", sessionDirPath) + err = os.RemoveAll(sessionDirPath) + if err != nil { + return xerrors.Errorf("can't remove %q directory: %w", sessionDirPath, err) + } + } + } + return nil +} + +func isValidSessionDir(dirName string) bool { + match, err := filepath.Match(sessionDirPrefix+"*", dirName) + return err == nil && match +} diff --git a/provisionersdk/session.go b/provisionersdk/session.go index eebf013b7d0de..0ccd1a7de74e7 100644 --- a/provisionersdk/session.go +++ b/provisionersdk/session.go @@ -11,11 +11,11 @@ import ( "strings" "time" - "github.com/djherbis/times" "github.com/google/uuid" "golang.org/x/xerrors" "cdr.dev/slog" + "github.com/coder/coder/v2/provisionersdk/proto" ) @@ -327,45 +327,6 @@ func (r *request[R, C]) do() (C, error) { } } -// cleanStaleSessions browses the work directory searching for stale session -// directories. Coder provisioner is supposed to remove them once after finishing the provisioning, -// but there is a risk of keeping them in case of a failure. -func cleanStaleSessions(ctx context.Context, workDirectory string, now time.Time, logger slog.Logger) error { - entries, err := os.ReadDir(workDirectory) - if err != nil { - return xerrors.Errorf("can't read %q directory", workDirectory) - } - - for _, entry := range entries { - dirName := entry.Name() - - if entry.IsDir() && isValidSessionDir(dirName) { - sessionDirPath := filepath.Join(workDirectory, dirName) - fi, err := entry.Info() - if err != nil { - return xerrors.Errorf("can't read %q directory info: %w", sessionDirPath, err) - } - - timeSpec := times.Get(fi) - if timeSpec.AccessTime().Add(staleSessionRetention).After(now) { - continue - } - - logger.Info(ctx, "remove stale session directory: %s", sessionDirPath) - err = os.RemoveAll(sessionDirPath) - if err != nil { - return xerrors.Errorf("can't remove %q directory: %w", sessionDirPath, err) - } - } - } - return nil -} - func sessionDir(sessID string) string { return sessionDirPrefix + sessID } - -func isValidSessionDir(dirName string) bool { - match, err := filepath.Match(sessionDirPrefix+"*", dirName) - return err == nil && match -} From 7aada74c21abc8ddb2cb1bb3ef94a746f3831494 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 7 Sep 2023 12:51:18 +0200 Subject: [PATCH 15/25] fix stat --- provisioner/terraform/cleanup.go | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/provisioner/terraform/cleanup.go b/provisioner/terraform/cleanup.go index 434185ecab66e..7cde56cb217ca 100644 --- a/provisioner/terraform/cleanup.go +++ b/provisioner/terraform/cleanup.go @@ -8,9 +8,10 @@ import ( "strings" "time" - "cdr.dev/slog" "github.com/djherbis/times" "golang.org/x/xerrors" + + "cdr.dev/slog" ) // cleanStaleTerraformPlugins browses the Terraform cache directory @@ -25,6 +26,14 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. return xerrors.Errorf("unable to determine absolute path %q: %w", cachePath, err) } + // Firstly, check if the cache path exists. + _, err = os.Stat(cachePath) + if os.IsNotExist(err) { + return nil + } else if err != nil { + return xerrors.Errorf("unable to stat cache path %q: %w", cachePath, err) + } + logger.Info(ctx, "clean stale Terraform plugins", slog.F("cache_path", cachePath)) // Filter directory trees matching pattern: //// From eabdacb3662766cc0cc1dcdef42747a0d2bb6168 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 7 Sep 2023 12:58:48 +0200 Subject: [PATCH 16/25] fix comment --- provisioner/terraform/cleanup.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/provisioner/terraform/cleanup.go b/provisioner/terraform/cleanup.go index 7cde56cb217ca..964e3761eb2d3 100644 --- a/provisioner/terraform/cleanup.go +++ b/provisioner/terraform/cleanup.go @@ -16,10 +16,12 @@ import ( // cleanStaleTerraformPlugins browses the Terraform cache directory // and remove stale plugins that haven't been used for a while. -// // Additionally, it sweeps empty, old directory trees. // -// Sample cachePath: /Users//Library/Caches/coder/provisioner-/tf +// Sample cachePath: +// +// /Users/john.doe/Library/Caches/coder/provisioner-1/tf +// /tmp/coder/provisioner-0/tf func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time.Time, logger slog.Logger) error { cachePath, err := filepath.Abs(cachePath) // sanity check in case the path is e.g. ../../../cache if err != nil { From 1b49d80dcdfb9835c7f65ad31b70845dd722eea4 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 7 Sep 2023 13:22:41 +0200 Subject: [PATCH 17/25] WIP use afero --- provisioner/terraform/cleanup.go | 22 ++++++++++---------- provisioner/terraform/cleanup_test.go | 29 +++++++++++++++++++++++++++ provisioner/terraform/provision.go | 4 +++- 3 files changed, 43 insertions(+), 12 deletions(-) create mode 100644 provisioner/terraform/cleanup_test.go diff --git a/provisioner/terraform/cleanup.go b/provisioner/terraform/cleanup.go index 964e3761eb2d3..a3f4ed0ec467d 100644 --- a/provisioner/terraform/cleanup.go +++ b/provisioner/terraform/cleanup.go @@ -2,19 +2,19 @@ package terraform import ( "context" - "io/fs" "os" "path/filepath" "strings" "time" "github.com/djherbis/times" + "github.com/spf13/afero" "golang.org/x/xerrors" "cdr.dev/slog" ) -// cleanStaleTerraformPlugins browses the Terraform cache directory +// CleanStaleTerraformPlugins browses the Terraform cache directory // and remove stale plugins that haven't been used for a while. // Additionally, it sweeps empty, old directory trees. // @@ -22,14 +22,14 @@ import ( // // /Users/john.doe/Library/Caches/coder/provisioner-1/tf // /tmp/coder/provisioner-0/tf -func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time.Time, logger slog.Logger) error { +func CleanStaleTerraformPlugins(ctx context.Context, cachePath string, fs afero.Fs, now time.Time, logger slog.Logger) error { cachePath, err := filepath.Abs(cachePath) // sanity check in case the path is e.g. ../../../cache if err != nil { return xerrors.Errorf("unable to determine absolute path %q: %w", cachePath, err) } // Firstly, check if the cache path exists. - _, err = os.Stat(cachePath) + _, err = fs.Stat(cachePath) if os.IsNotExist(err) { return nil } else if err != nil { @@ -56,7 +56,7 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. // Review cached Terraform plugins var pluginPaths []string - err = filepath.Walk(cachePath, func(path string, info fs.FileInfo, err error) error { + err = afero.Walk(fs, cachePath, func(path string, info os.FileInfo, err error) error { if err != nil { return err } @@ -76,7 +76,7 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. // Identify stale plugins var stalePlugins []string for _, pluginPath := range pluginPaths { - accessTime, err := latestAccessTime(pluginPath) + accessTime, err := latestAccessTime(fs, pluginPath) if err != nil { return xerrors.Errorf("unable to evaluate latest access time for directory %q: %w", pluginPath, err) } @@ -92,7 +92,7 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. // Remove stale plugins for _, stalePluginPath := range stalePlugins { // Remove the plugin directory - err = os.RemoveAll(stalePluginPath) + err = fs.RemoveAll(stalePluginPath) if err != nil { return xerrors.Errorf("unable to remove stale plugin %q: %w", stalePluginPath, err) } @@ -108,7 +108,7 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. wd = filepath.Dir(wd) - files, err := os.ReadDir(wd) + files, err := afero.ReadDir(fs, wd) if err != nil { return xerrors.Errorf("unable to read directory content %q: %w", wd, err) } @@ -118,7 +118,7 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. } logger.Debug(ctx, "remove empty directory", slog.F("path", wd)) - err = os.Remove(wd) + err = fs.Remove(wd) if err != nil { return xerrors.Errorf("unable to remove directory %q: %w", wd, err) } @@ -129,9 +129,9 @@ func cleanStaleTerraformPlugins(ctx context.Context, cachePath string, now time. // latestAccessTime walks recursively through the directory content, and locates // the last accessed file. -func latestAccessTime(pluginPath string) (time.Time, error) { +func latestAccessTime(fs afero.Fs, pluginPath string) (time.Time, error) { var latest time.Time - err := filepath.Walk(pluginPath, func(path string, info fs.FileInfo, err error) error { + err := afero.Walk(fs, pluginPath, func(path string, info os.FileInfo, err error) error { if err != nil { return err } diff --git a/provisioner/terraform/cleanup_test.go b/provisioner/terraform/cleanup_test.go new file mode 100644 index 0000000000000..3b8a73cf54724 --- /dev/null +++ b/provisioner/terraform/cleanup_test.go @@ -0,0 +1,29 @@ +package terraform_test + +import ( + "context" + "testing" + "time" + + "github.com/spf13/afero" + + "cdr.dev/slog/sloggers/slogtest" + "github.com/coder/coder/v2/provisioner/terraform" + "github.com/coder/coder/v2/testutil" +) + +const ( + cachePath = "/tmp/coder/provisioner-0/tf" +) + +func TestOnePluginIsStale(t *testing.T) { + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + fs := afero.NewMemMapFs() + now := time.Now() + logger := slogtest.Make(t, nil).Named("cleanup-test") + + terraform.CleanStaleTerraformPlugins(ctx, cachePath, fs, now, logger) + +} diff --git a/provisioner/terraform/provision.go b/provisioner/terraform/provision.go index d75802e537051..f1d5da3bbed9a 100644 --- a/provisioner/terraform/provision.go +++ b/provisioner/terraform/provision.go @@ -7,6 +7,8 @@ import ( "strings" "time" + "github.com/spf13/afero" + "cdr.dev/slog" "github.com/coder/terraform-provider-coder/provider" @@ -91,7 +93,7 @@ func (s *server) Plan( } } - err := cleanStaleTerraformPlugins(sess.Context(), "/tmp/coder/provisioner-0/tf", time.Now(), s.logger) + err := CleanStaleTerraformPlugins(sess.Context(), s.cachePath, afero.NewOsFs(), time.Now(), s.logger) if err != nil { return provisionersdk.PlanErrorf("unable to clean stale Terraform plugins: %s", err) } From 44e0517c48996c3a79c51e5979a96fafc757c20b Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 7 Sep 2023 17:31:48 +0200 Subject: [PATCH 18/25] Some golden files --- Makefile | 6 +- provisioner/terraform/cleanup.go | 10 +- provisioner/terraform/cleanup_test.go | 175 +++++++++++++++++- .../all_plugins_are_stale.txt.golden | 5 + .../one_plugin_file_is_touched.txt.golden | 22 +++ .../one_plugin_is_stale.txt.golden | 15 ++ provisioner/terraform/testdata/generate.sh | 6 + 7 files changed, 228 insertions(+), 11 deletions(-) create mode 100644 provisioner/terraform/testdata/cleanup-stale-plugins/all_plugins_are_stale.txt.golden create mode 100644 provisioner/terraform/testdata/cleanup-stale-plugins/one_plugin_file_is_touched.txt.golden create mode 100644 provisioner/terraform/testdata/cleanup-stale-plugins/one_plugin_is_stale.txt.golden diff --git a/Makefile b/Makefile index 65196d0a4bd1d..d800c05951114 100644 --- a/Makefile +++ b/Makefile @@ -570,7 +570,7 @@ coderd/apidoc/swagger.json: $(shell find ./scripts/apidocgen $(FIND_EXCLUSIONS) ./scripts/apidocgen/generate.sh pnpm run format:write:only ./docs/api ./docs/manifest.json ./coderd/apidoc/swagger.json -update-golden-files: cli/testdata/.gen-golden helm/coder/tests/testdata/.gen-golden helm/provisioner/tests/testdata/.gen-golden scripts/ci-report/testdata/.gen-golden enterprise/cli/testdata/.gen-golden coderd/.gen-golden +update-golden-files: cli/testdata/.gen-golden helm/coder/tests/testdata/.gen-golden helm/provisioner/tests/testdata/.gen-golden scripts/ci-report/testdata/.gen-golden enterprise/cli/testdata/.gen-golden coderd/.gen-golden provisioner/terraform/testdata/.gen-golden .PHONY: update-golden-files cli/testdata/.gen-golden: $(wildcard cli/testdata/*.golden) $(wildcard cli/*.tpl) $(GO_SRC_FILES) $(wildcard cli/*_test.go) @@ -593,6 +593,10 @@ coderd/.gen-golden: $(wildcard coderd/testdata/*/*.golden) $(GO_SRC_FILES) $(wil go test ./coderd -run="Test.*Golden$$" -update touch "$@" +provisioner/terraform/testdata/.gen-golden: $(wildcard provisioner/terraform/testdata/*/*.golden) $(GO_SRC_FILES) $(wildcard provisioner/terraform/*_test.go) + go test ./provisioner/terraform -run="Test.*Golden$$" -update + touch "$@" + scripts/ci-report/testdata/.gen-golden: $(wildcard scripts/ci-report/testdata/*) $(wildcard scripts/ci-report/*.go) go test ./scripts/ci-report -run=TestOutputMatchesGoldenFile -update touch "$@" diff --git a/provisioner/terraform/cleanup.go b/provisioner/terraform/cleanup.go index a3f4ed0ec467d..cdadbde3637cf 100644 --- a/provisioner/terraform/cleanup.go +++ b/provisioner/terraform/cleanup.go @@ -99,7 +99,7 @@ func CleanStaleTerraformPlugins(ctx context.Context, cachePath string, fs afero. // Compact the plugin structure by removing empty directories. wd := stalePluginPath - level := 4 // //// + level := 5 // //// for { level-- if level == 0 { @@ -136,8 +136,12 @@ func latestAccessTime(fs afero.Fs, pluginPath string) (time.Time, error) { return err } - timeSpec := times.Get(info) - accessTime := timeSpec.AccessTime() + var 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 } diff --git a/provisioner/terraform/cleanup_test.go b/provisioner/terraform/cleanup_test.go index 3b8a73cf54724..91425419379e4 100644 --- a/provisioner/terraform/cleanup_test.go +++ b/provisioner/terraform/cleanup_test.go @@ -1,12 +1,21 @@ package terraform_test import ( + "bytes" "context" + "flag" + "os" + "path/filepath" + "strings" "testing" "time" + "github.com/google/go-cmp/cmp" "github.com/spf13/afero" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "cdr.dev/slog" "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/v2/provisioner/terraform" "github.com/coder/coder/v2/testutil" @@ -16,14 +25,166 @@ const ( cachePath = "/tmp/coder/provisioner-0/tf" ) -func TestOnePluginIsStale(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) - defer cancel() +// updateGoldenFiles is a flag that can be set to update golden files. +var updateGoldenFiles = flag.Bool("update", false, "Update golden files") - fs := afero.NewMemMapFs() - now := time.Now() - logger := slogtest.Make(t, nil).Named("cleanup-test") +func TestPluginCache_Golden(t *testing.T) { + t.Parallel() - terraform.CleanStaleTerraformPlugins(ctx, cachePath, fs, now, logger) + t.Run("all plugins are stale", func(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + // prepare + fs := afero.NewMemMapFs() + now := time.Date(2023, time.June, 3, 4, 5, 6, 0, time.UTC) + + logger := slogtest.Make(t, nil). + Leveled(slog.LevelDebug). + Named("cleanup-test") + + // given + // This plugin is older than 30 days. + addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "terraform-provider-coder_v0.11.1", now.Add(-63*24*time.Hour)) + addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "LICENSE", now.Add(-33*24*time.Hour)) + addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "README.md", now.Add(-31*24*time.Hour)) + addPluginFolder(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "new_folder", now.Add(-31*24*time.Hour)) + addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "new_folder/foobar.tf", now.Add(-43*24*time.Hour)) + + // This plugin is older than 30 days. + addPluginFile(t, fs, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "terraform-provider-docker_v2.25.0", now.Add(-31*24*time.Hour)) + addPluginFile(t, fs, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "LICENSE", now.Add(-32*24*time.Hour)) + addPluginFile(t, fs, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "README.md", now.Add(-33*24*time.Hour)) + + // when + terraform.CleanStaleTerraformPlugins(ctx, cachePath, fs, now, logger) + + // then + diffFileSystem(t, fs) + }) + + t.Run("one plugin is stale", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + // prepare + fs := afero.NewMemMapFs() + now := time.Date(2023, time.June, 3, 4, 5, 6, 0, time.UTC) + + logger := slogtest.Make(t, nil). + Leveled(slog.LevelDebug). + Named("cleanup-test") + + // given + addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "terraform-provider-coder_v0.11.1", now.Add(-2*time.Hour)) + addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "LICENSE", now.Add(-3*time.Hour)) + addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "README.md", now.Add(-4*time.Hour)) + addPluginFolder(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "new_folder", now.Add(-5*time.Hour)) + addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "new_folder/foobar.tf", now.Add(-4*time.Hour)) + + // This plugin is older than 30 days. + addPluginFile(t, fs, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "terraform-provider-docker_v2.25.0", now.Add(-31*24*time.Hour)) + addPluginFile(t, fs, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "LICENSE", now.Add(-32*24*time.Hour)) + addPluginFile(t, fs, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "README.md", now.Add(-33*24*time.Hour)) + + // when + terraform.CleanStaleTerraformPlugins(ctx, cachePath, fs, now, logger) + + // then + diffFileSystem(t, fs) + }) + + t.Run("one plugin file is touched", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + // prepare + fs := afero.NewMemMapFs() + now := time.Date(2023, time.June, 3, 4, 5, 6, 0, time.UTC) + + logger := slogtest.Make(t, nil). + Leveled(slog.LevelDebug). + Named("cleanup-test") + + // given + addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "terraform-provider-coder_v0.11.1", now.Add(-63*24*time.Hour)) + addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "LICENSE", now.Add(-33*24*time.Hour)) + addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "README.md", now.Add(-31*24*time.Hour)) + addPluginFolder(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "new_folder", now.Add(-4*time.Hour)) + addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "new_folder/foobar.tf", now.Add(-43*24*time.Hour)) + + addPluginFile(t, fs, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "terraform-provider-docker_v2.25.0", now.Add(-31*24*time.Hour)) + addPluginFile(t, fs, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "LICENSE", now.Add(-2*time.Hour)) + addPluginFile(t, fs, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "README.md", now.Add(-33*24*time.Hour)) + + // when + terraform.CleanStaleTerraformPlugins(ctx, cachePath, fs, now, logger) + + // then + diffFileSystem(t, fs) + }) +} + +func addPluginFile(t *testing.T, fs afero.Fs, pluginPath string, resourcePath string, accessTime time.Time) { + err := fs.MkdirAll(filepath.Join(cachePath, pluginPath), 0755) + require.NoError(t, err, "can't create test folder for plugin file") + + err = fs.Chtimes(filepath.Join(cachePath, pluginPath), accessTime, accessTime) + require.NoError(t, err, "can't set times") + + err = afero.WriteFile(fs, filepath.Join(cachePath, pluginPath, resourcePath), []byte("foo"), 0644) + require.NoError(t, err, "can't create test file") + + err = fs.Chtimes(filepath.Join(cachePath, pluginPath, resourcePath), accessTime, accessTime) + require.NoError(t, err, "can't set times") +} + +func addPluginFolder(t *testing.T, fs afero.Fs, pluginPath string, folderPath string, accessTime time.Time) { + err := fs.MkdirAll(filepath.Join(cachePath, pluginPath, folderPath), 0755) + require.NoError(t, err, "can't create plugin folder") + + err = fs.Chtimes(filepath.Join(cachePath, pluginPath, folderPath), accessTime, accessTime) + require.NoError(t, err, "can't set times") +} + +func diffFileSystem(t *testing.T, fs afero.Fs) { + actual := dumpFileSystem(t, fs) + + partialName := strings.Join(strings.Split(t.Name(), "/")[1:], "_") + goldenFile := filepath.Join("testdata", "cleanup-stale-plugins", partialName+".txt.golden") + if *updateGoldenFiles { + err := os.MkdirAll(filepath.Dir(goldenFile), 0o755) + require.NoError(t, err, "want no error creating golden file directory") + + err = os.WriteFile(goldenFile, actual, 0600) + require.NoError(t, err, "want no error creating golden file") + return + } + + want, err := os.ReadFile(goldenFile) + require.NoError(t, err, "open golden file, run \"make update-golden-files\" and commit the changes") + assert.Empty(t, cmp.Diff(want, actual), "golden file mismatch (-want +got): %s, run \"make update-golden-files\", verify and commit the changes", goldenFile) +} + +func dumpFileSystem(t *testing.T, fs afero.Fs) []byte { + var buffer bytes.Buffer + err := afero.Walk(fs, "/", func(path string, info os.FileInfo, err error) error { + _, _ = buffer.WriteString(path) + _ = buffer.WriteByte(' ') + if info.IsDir() { + _ = buffer.WriteByte('d') + } else { + _ = buffer.WriteByte('f') + } + _ = buffer.WriteByte('\n') + return nil + }) + require.NoError(t, err, "can't dump the file system") + return buffer.Bytes() } diff --git a/provisioner/terraform/testdata/cleanup-stale-plugins/all_plugins_are_stale.txt.golden b/provisioner/terraform/testdata/cleanup-stale-plugins/all_plugins_are_stale.txt.golden new file mode 100644 index 0000000000000..385a8c0ae7835 --- /dev/null +++ b/provisioner/terraform/testdata/cleanup-stale-plugins/all_plugins_are_stale.txt.golden @@ -0,0 +1,5 @@ +/ d +/tmp d +/tmp/coder d +/tmp/coder/provisioner-0 d +/tmp/coder/provisioner-0/tf d diff --git a/provisioner/terraform/testdata/cleanup-stale-plugins/one_plugin_file_is_touched.txt.golden b/provisioner/terraform/testdata/cleanup-stale-plugins/one_plugin_file_is_touched.txt.golden new file mode 100644 index 0000000000000..399d94a8dfc2d --- /dev/null +++ b/provisioner/terraform/testdata/cleanup-stale-plugins/one_plugin_file_is_touched.txt.golden @@ -0,0 +1,22 @@ +/ d +/tmp d +/tmp/coder d +/tmp/coder/provisioner-0 d +/tmp/coder/provisioner-0/tf d +/tmp/coder/provisioner-0/tf/registry.terraform.io d +/tmp/coder/provisioner-0/tf/registry.terraform.io/coder d +/tmp/coder/provisioner-0/tf/registry.terraform.io/coder/coder d +/tmp/coder/provisioner-0/tf/registry.terraform.io/coder/coder/0.11.1 d +/tmp/coder/provisioner-0/tf/registry.terraform.io/coder/coder/0.11.1/darwin_arm64 d +/tmp/coder/provisioner-0/tf/registry.terraform.io/coder/coder/0.11.1/darwin_arm64/LICENSE f +/tmp/coder/provisioner-0/tf/registry.terraform.io/coder/coder/0.11.1/darwin_arm64/README.md f +/tmp/coder/provisioner-0/tf/registry.terraform.io/coder/coder/0.11.1/darwin_arm64/new_folder d +/tmp/coder/provisioner-0/tf/registry.terraform.io/coder/coder/0.11.1/darwin_arm64/new_folder/foobar.tf f +/tmp/coder/provisioner-0/tf/registry.terraform.io/coder/coder/0.11.1/darwin_arm64/terraform-provider-coder_v0.11.1 f +/tmp/coder/provisioner-0/tf/registry.terraform.io/kreuzwerker d +/tmp/coder/provisioner-0/tf/registry.terraform.io/kreuzwerker/docker d +/tmp/coder/provisioner-0/tf/registry.terraform.io/kreuzwerker/docker/2.25.0 d +/tmp/coder/provisioner-0/tf/registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64 d +/tmp/coder/provisioner-0/tf/registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64/LICENSE f +/tmp/coder/provisioner-0/tf/registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64/README.md f +/tmp/coder/provisioner-0/tf/registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64/terraform-provider-docker_v2.25.0 f diff --git a/provisioner/terraform/testdata/cleanup-stale-plugins/one_plugin_is_stale.txt.golden b/provisioner/terraform/testdata/cleanup-stale-plugins/one_plugin_is_stale.txt.golden new file mode 100644 index 0000000000000..7f6a2cf1b7f1a --- /dev/null +++ b/provisioner/terraform/testdata/cleanup-stale-plugins/one_plugin_is_stale.txt.golden @@ -0,0 +1,15 @@ +/ d +/tmp d +/tmp/coder d +/tmp/coder/provisioner-0 d +/tmp/coder/provisioner-0/tf d +/tmp/coder/provisioner-0/tf/registry.terraform.io d +/tmp/coder/provisioner-0/tf/registry.terraform.io/coder d +/tmp/coder/provisioner-0/tf/registry.terraform.io/coder/coder d +/tmp/coder/provisioner-0/tf/registry.terraform.io/coder/coder/0.11.1 d +/tmp/coder/provisioner-0/tf/registry.terraform.io/coder/coder/0.11.1/darwin_arm64 d +/tmp/coder/provisioner-0/tf/registry.terraform.io/coder/coder/0.11.1/darwin_arm64/LICENSE f +/tmp/coder/provisioner-0/tf/registry.terraform.io/coder/coder/0.11.1/darwin_arm64/README.md f +/tmp/coder/provisioner-0/tf/registry.terraform.io/coder/coder/0.11.1/darwin_arm64/new_folder d +/tmp/coder/provisioner-0/tf/registry.terraform.io/coder/coder/0.11.1/darwin_arm64/new_folder/foobar.tf f +/tmp/coder/provisioner-0/tf/registry.terraform.io/coder/coder/0.11.1/darwin_arm64/terraform-provider-coder_v0.11.1 f diff --git a/provisioner/terraform/testdata/generate.sh b/provisioner/terraform/testdata/generate.sh index 29e26a310c522..4ae1a87fb2504 100755 --- a/provisioner/terraform/testdata/generate.sh +++ b/provisioner/terraform/testdata/generate.sh @@ -13,6 +13,12 @@ for d in */; do continue fi + # This directory is used for a different purpose (quick workaround). + if [[ $name == "cleanup-stale-plugins" ]]; then + popd + continue + fi + terraform init -upgrade terraform plan -out terraform.tfplan terraform show -json ./terraform.tfplan | jq >"$name".tfplan.json From 18c5a1d20f712ac88b3d021d3f9999d95a01c95a Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 7 Sep 2023 18:14:19 +0200 Subject: [PATCH 19/25] Polishing --- provisioner/terraform/cleanup.go | 1 - provisioner/terraform/cleanup_test.go | 39 ++++----- provisionersdk/cleanup.go | 29 +++---- provisionersdk/cleanup_test.go | 112 ++++++++++++++++++++++++++ provisionersdk/session.go | 8 +- 5 files changed, 146 insertions(+), 43 deletions(-) create mode 100644 provisionersdk/cleanup_test.go diff --git a/provisioner/terraform/cleanup.go b/provisioner/terraform/cleanup.go index cdadbde3637cf..9b2ba651282b5 100644 --- a/provisioner/terraform/cleanup.go +++ b/provisioner/terraform/cleanup.go @@ -137,7 +137,6 @@ func latestAccessTime(fs afero.Fs, pluginPath string) (time.Time, error) { } var accessTime = info.ModTime() // fallback to modTime if accessTime is not available (afero) - if info.Sys() != nil { timeSpec := times.Get(info) accessTime = timeSpec.AccessTime() diff --git a/provisioner/terraform/cleanup_test.go b/provisioner/terraform/cleanup_test.go index 91425419379e4..5d8bd5be320b1 100644 --- a/provisioner/terraform/cleanup_test.go +++ b/provisioner/terraform/cleanup_test.go @@ -21,9 +21,7 @@ import ( "github.com/coder/coder/v2/testutil" ) -const ( - cachePath = "/tmp/coder/provisioner-0/tf" -) +const cachePath = "/tmp/coder/provisioner-0/tf" // updateGoldenFiles is a flag that can be set to update golden files. var updateGoldenFiles = flag.Bool("update", false, "Update golden files") @@ -31,19 +29,22 @@ var updateGoldenFiles = flag.Bool("update", false, "Update golden files") 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) + logger := slogtest.Make(t, nil). + Leveled(slog.LevelDebug). + Named("cleanup-test") + return fs, now, logger + } + t.Run("all plugins are stale", func(t *testing.T) { t.Parallel() ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) defer cancel() - // prepare - fs := afero.NewMemMapFs() - now := time.Date(2023, time.June, 3, 4, 5, 6, 0, time.UTC) - - logger := slogtest.Make(t, nil). - Leveled(slog.LevelDebug). - Named("cleanup-test") + fs, now, logger := prepare() // given // This plugin is older than 30 days. @@ -71,13 +72,7 @@ func TestPluginCache_Golden(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) defer cancel() - // prepare - fs := afero.NewMemMapFs() - now := time.Date(2023, time.June, 3, 4, 5, 6, 0, time.UTC) - - logger := slogtest.Make(t, nil). - Leveled(slog.LevelDebug). - Named("cleanup-test") + fs, now, logger := prepare() // given addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "terraform-provider-coder_v0.11.1", now.Add(-2*time.Hour)) @@ -104,19 +99,13 @@ func TestPluginCache_Golden(t *testing.T) { ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) defer cancel() - // prepare - fs := afero.NewMemMapFs() - now := time.Date(2023, time.June, 3, 4, 5, 6, 0, time.UTC) - - logger := slogtest.Make(t, nil). - Leveled(slog.LevelDebug). - Named("cleanup-test") + fs, now, logger := prepare() // given addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "terraform-provider-coder_v0.11.1", now.Add(-63*24*time.Hour)) addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "LICENSE", now.Add(-33*24*time.Hour)) addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "README.md", now.Add(-31*24*time.Hour)) - addPluginFolder(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "new_folder", now.Add(-4*time.Hour)) + addPluginFolder(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "new_folder", now.Add(-4*time.Hour)) // touched addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "new_folder/foobar.tf", now.Add(-43*24*time.Hour)) addPluginFile(t, fs, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "terraform-provider-docker_v2.25.0", now.Add(-31*24*time.Hour)) diff --git a/provisionersdk/cleanup.go b/provisionersdk/cleanup.go index 6aba8b2f48a25..da2f99dec9507 100644 --- a/provisionersdk/cleanup.go +++ b/provisionersdk/cleanup.go @@ -2,42 +2,43 @@ package provisionersdk import ( "context" - "os" "path/filepath" "time" "github.com/djherbis/times" + "github.com/spf13/afero" "golang.org/x/xerrors" "cdr.dev/slog" ) -// cleanStaleSessions browses the work directory searching for stale session +// CleanStaleSessions browses the work directory searching for stale session // directories. Coder provisioner is supposed to remove them once after finishing the provisioning, // but there is a risk of keeping them in case of a failure. -func cleanStaleSessions(ctx context.Context, workDirectory string, now time.Time, logger slog.Logger) error { - entries, err := os.ReadDir(workDirectory) +func CleanStaleSessions(ctx context.Context, workDirectory string, fs afero.Fs, now time.Time, logger slog.Logger) error { + entries, err := afero.ReadDir(fs, workDirectory) if err != nil { return xerrors.Errorf("can't read %q directory", workDirectory) } - for _, entry := range entries { - dirName := entry.Name() + for _, fi := range entries { + dirName := fi.Name() - if entry.IsDir() && isValidSessionDir(dirName) { + if fi.IsDir() && isValidSessionDir(dirName) { sessionDirPath := filepath.Join(workDirectory, dirName) - fi, err := entry.Info() - if err != nil { - return xerrors.Errorf("can't read %q directory info: %w", sessionDirPath, err) + + var accessTime = fi.ModTime() // fallback to modTime if accessTime is not available (afero) + if fi.Sys() != nil { + timeSpec := times.Get(fi) + accessTime = timeSpec.AccessTime() } - timeSpec := times.Get(fi) - if timeSpec.AccessTime().Add(staleSessionRetention).After(now) { + if accessTime.Add(staleSessionRetention).After(now) { continue } - logger.Info(ctx, "remove stale session directory: %s", sessionDirPath) - err = os.RemoveAll(sessionDirPath) + logger.Info(ctx, "remove stale session directory", slog.F("session_path", sessionDirPath)) + err = fs.RemoveAll(sessionDirPath) if err != nil { return xerrors.Errorf("can't remove %q directory: %w", sessionDirPath, err) } diff --git a/provisionersdk/cleanup_test.go b/provisionersdk/cleanup_test.go new file mode 100644 index 0000000000000..325862f9877ff --- /dev/null +++ b/provisionersdk/cleanup_test.go @@ -0,0 +1,112 @@ +package provisionersdk_test + +import ( + "context" + "path/filepath" + "testing" + "time" + + "github.com/google/uuid" + "github.com/spf13/afero" + "github.com/stretchr/testify/require" + + "cdr.dev/slog" + "cdr.dev/slog/sloggers/slogtest" + + "github.com/coder/coder/v2/provisionersdk" + "github.com/coder/coder/v2/testutil" +) + +const workDirectory = "/tmp/coder/provisioner-34/work" + +func TestStaleSessions(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) + logger := slogtest.Make(t, nil). + Leveled(slog.LevelDebug). + Named("cleanup-test") + return fs, now, logger + } + + t.Run("all sessions are stale", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + fs, now, logger := prepare() + + // given + first := provisionersdk.SessionDir(uuid.NewString()) + addSessionFolder(t, fs, first, now.Add(-7*24*time.Hour)) + second := provisionersdk.SessionDir(uuid.NewString()) + addSessionFolder(t, fs, second, now.Add(-8*24*time.Hour)) + third := provisionersdk.SessionDir(uuid.NewString()) + addSessionFolder(t, fs, third, now.Add(-9*24*time.Hour)) + + // when + provisionersdk.CleanStaleSessions(ctx, workDirectory, fs, now, logger) + + // then + entries, err := afero.ReadDir(fs, workDirectory) + require.NoError(t, err) + require.Empty(t, entries, "all session leftovers should be removed") + }) + + t.Run("one session is stale", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + fs, now, logger := prepare() + + // given + first := provisionersdk.SessionDir(uuid.NewString()) + addSessionFolder(t, fs, first, now.Add(-7*24*time.Hour)) + second := provisionersdk.SessionDir(uuid.NewString()) + addSessionFolder(t, fs, second, now.Add(-6*24*time.Hour)) + + // when + provisionersdk.CleanStaleSessions(ctx, workDirectory, fs, now, logger) + + // then + entries, err := afero.ReadDir(fs, workDirectory) + require.NoError(t, err) + require.Len(t, entries, 1, "one session should be present") + require.Equal(t, second, entries[0].Name(), 1) + }) + + t.Run("no stale sessions", func(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + fs, now, logger := prepare() + + // given + first := provisionersdk.SessionDir(uuid.NewString()) + addSessionFolder(t, fs, first, now.Add(-6*24*time.Hour)) + second := provisionersdk.SessionDir(uuid.NewString()) + addSessionFolder(t, fs, second, now.Add(-5*24*time.Hour)) + + // when + provisionersdk.CleanStaleSessions(ctx, workDirectory, fs, now, logger) + + // then + entries, err := afero.ReadDir(fs, workDirectory) + require.NoError(t, err) + require.Len(t, entries, 2, "both sessions should be present") + }) +} + +func addSessionFolder(t *testing.T, fs afero.Fs, sessionName string, accessTime time.Time) { + err := fs.MkdirAll(filepath.Join(workDirectory, sessionName), 0755) + require.NoError(t, err, "can't create session folder") + fs.Chtimes(filepath.Join(workDirectory, sessionName), accessTime, accessTime) + require.NoError(t, err, "can't set times") +} diff --git a/provisionersdk/session.go b/provisionersdk/session.go index 0ccd1a7de74e7..218807d97e884 100644 --- a/provisionersdk/session.go +++ b/provisionersdk/session.go @@ -12,6 +12,7 @@ import ( "time" "github.com/google/uuid" + "github.com/spf13/afero" "golang.org/x/xerrors" "cdr.dev/slog" @@ -41,12 +42,12 @@ func (p *protoServer) Session(stream proto.DRPCProvisioner_SessionStream) error server: p.server, } - err := cleanStaleSessions(s.Context(), p.opts.WorkDirectory, time.Now(), s.Logger) + err := CleanStaleSessions(s.Context(), p.opts.WorkDirectory, afero.NewOsFs(), time.Now(), s.Logger) if err != nil { return xerrors.Errorf("unable to clean stale sessions %q: %w", s.WorkDirectory, err) } - s.WorkDirectory = filepath.Join(p.opts.WorkDirectory, sessionDir(sessID)) + s.WorkDirectory = filepath.Join(p.opts.WorkDirectory, SessionDir(sessID)) err = os.MkdirAll(s.WorkDirectory, 0o700) if err != nil { return xerrors.Errorf("create work directory %q: %w", s.WorkDirectory, err) @@ -327,6 +328,7 @@ func (r *request[R, C]) do() (C, error) { } } -func sessionDir(sessID string) string { +// SessionDir returns the directory name with mandatory prefix. +func SessionDir(sessID string) string { return sessionDirPrefix + sessID } From b273cbf0505efe7aeff5175a9593c5bd91a90153 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 7 Sep 2023 18:18:40 +0200 Subject: [PATCH 20/25] make fmt --- provisioner/terraform/cleanup.go | 2 +- provisioner/terraform/cleanup_test.go | 8 ++++---- provisionersdk/cleanup.go | 2 +- provisionersdk/cleanup_test.go | 2 +- 4 files changed, 7 insertions(+), 7 deletions(-) diff --git a/provisioner/terraform/cleanup.go b/provisioner/terraform/cleanup.go index 9b2ba651282b5..65f876551b6d7 100644 --- a/provisioner/terraform/cleanup.go +++ b/provisioner/terraform/cleanup.go @@ -136,7 +136,7 @@ func latestAccessTime(fs afero.Fs, pluginPath string) (time.Time, error) { return err } - var accessTime = info.ModTime() // fallback to modTime if accessTime is not available (afero) + accessTime := info.ModTime() // fallback to modTime if accessTime is not available (afero) if info.Sys() != nil { timeSpec := times.Get(info) accessTime = timeSpec.AccessTime() diff --git a/provisioner/terraform/cleanup_test.go b/provisioner/terraform/cleanup_test.go index 5d8bd5be320b1..997d15c282e24 100644 --- a/provisioner/terraform/cleanup_test.go +++ b/provisioner/terraform/cleanup_test.go @@ -121,13 +121,13 @@ func TestPluginCache_Golden(t *testing.T) { } func addPluginFile(t *testing.T, fs afero.Fs, pluginPath string, resourcePath string, accessTime time.Time) { - err := fs.MkdirAll(filepath.Join(cachePath, pluginPath), 0755) + 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) require.NoError(t, err, "can't set times") - err = afero.WriteFile(fs, filepath.Join(cachePath, pluginPath, resourcePath), []byte("foo"), 0644) + 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) @@ -135,7 +135,7 @@ func addPluginFile(t *testing.T, fs afero.Fs, pluginPath string, resourcePath st } func addPluginFolder(t *testing.T, fs afero.Fs, pluginPath string, folderPath string, accessTime time.Time) { - err := fs.MkdirAll(filepath.Join(cachePath, pluginPath, folderPath), 0755) + 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) @@ -151,7 +151,7 @@ func diffFileSystem(t *testing.T, fs afero.Fs) { err := os.MkdirAll(filepath.Dir(goldenFile), 0o755) require.NoError(t, err, "want no error creating golden file directory") - err = os.WriteFile(goldenFile, actual, 0600) + err = os.WriteFile(goldenFile, actual, 0o600) require.NoError(t, err, "want no error creating golden file") return } diff --git a/provisionersdk/cleanup.go b/provisionersdk/cleanup.go index da2f99dec9507..8f940546cb05c 100644 --- a/provisionersdk/cleanup.go +++ b/provisionersdk/cleanup.go @@ -27,7 +27,7 @@ func CleanStaleSessions(ctx context.Context, workDirectory string, fs afero.Fs, if fi.IsDir() && isValidSessionDir(dirName) { sessionDirPath := filepath.Join(workDirectory, dirName) - var accessTime = fi.ModTime() // fallback to modTime if accessTime is not available (afero) + accessTime := fi.ModTime() // fallback to modTime if accessTime is not available (afero) if fi.Sys() != nil { timeSpec := times.Get(fi) accessTime = timeSpec.AccessTime() diff --git a/provisionersdk/cleanup_test.go b/provisionersdk/cleanup_test.go index 325862f9877ff..cf0296cb05927 100644 --- a/provisionersdk/cleanup_test.go +++ b/provisionersdk/cleanup_test.go @@ -105,7 +105,7 @@ func TestStaleSessions(t *testing.T) { } func addSessionFolder(t *testing.T, fs afero.Fs, sessionName string, accessTime time.Time) { - err := fs.MkdirAll(filepath.Join(workDirectory, sessionName), 0755) + err := fs.MkdirAll(filepath.Join(workDirectory, sessionName), 0o755) require.NoError(t, err, "can't create session folder") fs.Chtimes(filepath.Join(workDirectory, sessionName), accessTime, accessTime) require.NoError(t, err, "can't set times") From 6c45602ac5e7d8a35ed6afe5a16fdd239b22e1f4 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 7 Sep 2023 18:38:44 +0200 Subject: [PATCH 21/25] fix: windows --- provisioner/terraform/cleanup_test.go | 1 + 1 file changed, 1 insertion(+) diff --git a/provisioner/terraform/cleanup_test.go b/provisioner/terraform/cleanup_test.go index 997d15c282e24..586c58dca84c5 100644 --- a/provisioner/terraform/cleanup_test.go +++ b/provisioner/terraform/cleanup_test.go @@ -157,6 +157,7 @@ func diffFileSystem(t *testing.T, fs afero.Fs) { } want, err := os.ReadFile(goldenFile) + want = bytes.ReplaceAll(want, []byte{'\r', '\n'}, []byte{'\n'}) // fix for Windows require.NoError(t, err, "open golden file, run \"make update-golden-files\" and commit the changes") assert.Empty(t, cmp.Diff(want, actual), "golden file mismatch (-want +got): %s, run \"make update-golden-files\", verify and commit the changes", goldenFile) } From b35f7a7f9ae811ffa11896bdf6ce5e080df59659 Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 7 Sep 2023 19:14:37 +0200 Subject: [PATCH 22/25] Fix --- provisioner/terraform/cleanup_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/provisioner/terraform/cleanup_test.go b/provisioner/terraform/cleanup_test.go index 586c58dca84c5..f386c1b75ce2e 100644 --- a/provisioner/terraform/cleanup_test.go +++ b/provisioner/terraform/cleanup_test.go @@ -6,6 +6,7 @@ import ( "flag" "os" "path/filepath" + "runtime" "strings" "testing" "time" @@ -157,7 +158,9 @@ func diffFileSystem(t *testing.T, fs afero.Fs) { } want, err := os.ReadFile(goldenFile) - want = bytes.ReplaceAll(want, []byte{'\r', '\n'}, []byte{'\n'}) // fix for Windows + if runtime.GOOS == "windows" { + want = bytes.ReplaceAll(want, []byte("\r\n"), []byte("\n")) + } require.NoError(t, err, "open golden file, run \"make update-golden-files\" and commit the changes") assert.Empty(t, cmp.Diff(want, actual), "golden file mismatch (-want +got): %s, run \"make update-golden-files\", verify and commit the changes", goldenFile) } @@ -165,6 +168,7 @@ func diffFileSystem(t *testing.T, fs afero.Fs) { func dumpFileSystem(t *testing.T, fs afero.Fs) []byte { var buffer bytes.Buffer err := afero.Walk(fs, "/", func(path string, info os.FileInfo, err error) error { + path = strings.ReplaceAll(path, "\\", "/") // fix for Windows _, _ = buffer.WriteString(path) _ = buffer.WriteByte(' ') if info.IsDir() { From 1cd5fc2da47e1c2eb6c61b2818b95eac7b53f19e Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 7 Sep 2023 20:33:37 +0200 Subject: [PATCH 23/25] filepath.Join --- provisioner/terraform/cleanup_test.go | 55 +++++++++++++++------------ 1 file changed, 30 insertions(+), 25 deletions(-) diff --git a/provisioner/terraform/cleanup_test.go b/provisioner/terraform/cleanup_test.go index f386c1b75ce2e..efa8bf0eaa4de 100644 --- a/provisioner/terraform/cleanup_test.go +++ b/provisioner/terraform/cleanup_test.go @@ -27,6 +27,11 @@ const cachePath = "/tmp/coder/provisioner-0/tf" // updateGoldenFiles is a flag that can be set to update golden files. var updateGoldenFiles = flag.Bool("update", false, "Update golden files") +var ( + 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") +) + func TestPluginCache_Golden(t *testing.T) { t.Parallel() @@ -49,16 +54,16 @@ func TestPluginCache_Golden(t *testing.T) { // given // This plugin is older than 30 days. - addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "terraform-provider-coder_v0.11.1", now.Add(-63*24*time.Hour)) - addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "LICENSE", now.Add(-33*24*time.Hour)) - addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "README.md", now.Add(-31*24*time.Hour)) - addPluginFolder(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "new_folder", now.Add(-31*24*time.Hour)) - addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "new_folder/foobar.tf", 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, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "terraform-provider-docker_v2.25.0", now.Add(-31*24*time.Hour)) - addPluginFile(t, fs, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "LICENSE", now.Add(-32*24*time.Hour)) - addPluginFile(t, fs, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "README.md", 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) @@ -76,16 +81,16 @@ func TestPluginCache_Golden(t *testing.T) { fs, now, logger := prepare() // given - addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "terraform-provider-coder_v0.11.1", now.Add(-2*time.Hour)) - addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "LICENSE", now.Add(-3*time.Hour)) - addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "README.md", now.Add(-4*time.Hour)) - addPluginFolder(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "new_folder", now.Add(-5*time.Hour)) - addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "new_folder/foobar.tf", 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, "new_folder/foobar.tf", now.Add(-4*time.Hour)) // This plugin is older than 30 days. - addPluginFile(t, fs, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "terraform-provider-docker_v2.25.0", now.Add(-31*24*time.Hour)) - addPluginFile(t, fs, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "LICENSE", now.Add(-32*24*time.Hour)) - addPluginFile(t, fs, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "README.md", 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) @@ -103,15 +108,15 @@ func TestPluginCache_Golden(t *testing.T) { fs, now, logger := prepare() // given - addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "terraform-provider-coder_v0.11.1", now.Add(-63*24*time.Hour)) - addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "LICENSE", now.Add(-33*24*time.Hour)) - addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "README.md", now.Add(-31*24*time.Hour)) - addPluginFolder(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "new_folder", now.Add(-4*time.Hour)) // touched - addPluginFile(t, fs, "registry.terraform.io/coder/coder/0.11.1/darwin_arm64", "new_folder/foobar.tf", now.Add(-43*24*time.Hour)) - - addPluginFile(t, fs, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "terraform-provider-docker_v2.25.0", now.Add(-31*24*time.Hour)) - addPluginFile(t, fs, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "LICENSE", now.Add(-2*time.Hour)) - addPluginFile(t, fs, "registry.terraform.io/kreuzwerker/docker/2.25.0/darwin_arm64", "README.md", now.Add(-33*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(-4*time.Hour)) // touched + addPluginFile(t, fs, coderPluginPath, "new_folder/foobar.tf", now.Add(-43*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)) + addPluginFile(t, fs, dockerPluginPath, "README.md", now.Add(-33*24*time.Hour)) // when terraform.CleanStaleTerraformPlugins(ctx, cachePath, fs, now, logger) From 662c32ba5953475a51e4277c48c15c528bbb79ce Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 7 Sep 2023 20:37:58 +0200 Subject: [PATCH 24/25] filepath.Join --- provisioner/terraform/cleanup_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/provisioner/terraform/cleanup_test.go b/provisioner/terraform/cleanup_test.go index efa8bf0eaa4de..9bd750b5e572a 100644 --- a/provisioner/terraform/cleanup_test.go +++ b/provisioner/terraform/cleanup_test.go @@ -85,7 +85,7 @@ func TestPluginCache_Golden(t *testing.T) { 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, "new_folder/foobar.tf", now.Add(-4*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.Add(-31*24*time.Hour)) @@ -112,7 +112,7 @@ func TestPluginCache_Golden(t *testing.T) { 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, "new_folder/foobar.tf", now.Add(-43*24*time.Hour)) + addPluginFile(t, fs, coderPluginPath, filepath.Join("new_folder", "foobar.tf"), now.Add(-43*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)) From 4e6a09c176cd72e5b271ab8e438fd57c964817bb Mon Sep 17 00:00:00 2001 From: Marcin Tojek Date: Thu, 7 Sep 2023 20:44:19 +0200 Subject: [PATCH 25/25] oh do not run on windows --- provisioner/terraform/cleanup_test.go | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/provisioner/terraform/cleanup_test.go b/provisioner/terraform/cleanup_test.go index 9bd750b5e572a..42e97305df89b 100644 --- a/provisioner/terraform/cleanup_test.go +++ b/provisioner/terraform/cleanup_test.go @@ -1,3 +1,5 @@ +//go:build linux || darwin + package terraform_test import ( @@ -6,7 +8,6 @@ import ( "flag" "os" "path/filepath" - "runtime" "strings" "testing" "time" @@ -163,9 +164,6 @@ func diffFileSystem(t *testing.T, fs afero.Fs) { } want, err := os.ReadFile(goldenFile) - if runtime.GOOS == "windows" { - want = bytes.ReplaceAll(want, []byte("\r\n"), []byte("\n")) - } require.NoError(t, err, "open golden file, run \"make update-golden-files\" and commit the changes") assert.Empty(t, cmp.Diff(want, actual), "golden file mismatch (-want +got): %s, run \"make update-golden-files\", verify and commit the changes", goldenFile) } @@ -173,7 +171,6 @@ func diffFileSystem(t *testing.T, fs afero.Fs) { func dumpFileSystem(t *testing.T, fs afero.Fs) []byte { var buffer bytes.Buffer err := afero.Walk(fs, "/", func(path string, info os.FileInfo, err error) error { - path = strings.ReplaceAll(path, "\\", "/") // fix for Windows _, _ = buffer.WriteString(path) _ = buffer.WriteByte(' ') if info.IsDir() {