Skip to content

Commit 9ca5b44

Browse files
EmyrkEmyrk
authored
chore: implement persistent terraform directories (experimental) (#20563)
Prior to this, every workspace build ran `terraform init` in a fresh directory. This would mean the `modules` are downloaded fresh. If the module is not pinned, subsequent workspace builds would have different modules.
1 parent 14f0844 commit 9ca5b44

File tree

11 files changed

+428
-81
lines changed

11 files changed

+428
-81
lines changed

‎coderd/provisionerdserver/provisionerdserver.go‎

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -699,16 +699,19 @@ func (s *server) acquireProtoJob(ctx context.Context, job database.ProvisionerJo
699699
}
700700
}
701701

702+
activeVersion := template.ActiveVersionID == templateVersion.ID
702703
protoJob.Type = &proto.AcquiredJob_WorkspaceBuild_{
703704
WorkspaceBuild: &proto.AcquiredJob_WorkspaceBuild{
704-
WorkspaceBuildId: workspaceBuild.ID.String(),
705-
WorkspaceName: workspace.Name,
706-
State: workspaceBuild.ProvisionerState,
707-
RichParameterValues: convertRichParameterValues(workspaceBuildParameters),
708-
PreviousParameterValues: convertRichParameterValues(lastWorkspaceBuildParameters),
709-
VariableValues: asVariableValues(templateVariables),
710-
ExternalAuthProviders: externalAuthProviders,
711-
ExpReuseTerraformWorkspace: ptr.Ref(false), // TODO: Toggle based on experiment
705+
WorkspaceBuildId: workspaceBuild.ID.String(),
706+
WorkspaceName: workspace.Name,
707+
State: workspaceBuild.ProvisionerState,
708+
RichParameterValues: convertRichParameterValues(workspaceBuildParameters),
709+
PreviousParameterValues: convertRichParameterValues(lastWorkspaceBuildParameters),
710+
VariableValues: asVariableValues(templateVariables),
711+
ExternalAuthProviders: externalAuthProviders,
712+
// If active and experiment is enabled, allow workspace reuse existing TF
713+
// workspaces (directories) for a faster startup.
714+
ExpReuseTerraformWorkspace: ptr.Ref(activeVersion && s.Experiments.Enabled(codersdk.ExperimentTerraformWorkspace)),
712715
Metadata: &sdkproto.Metadata{
713716
CoderUrl: s.AccessURL.String(),
714717
WorkspaceTransition: transition,
@@ -722,6 +725,7 @@ func (s *server) acquireProtoJob(ctx context.Context, job database.ProvisionerJo
722725
WorkspaceOwnerId: owner.ID.String(),
723726
TemplateId: template.ID.String(),
724727
TemplateName: template.Name,
728+
TemplateVersionId: templateVersion.ID.String(),
725729
TemplateVersion: templateVersion.Name,
726730
WorkspaceOwnerSessionToken: sessionToken,
727731
WorkspaceOwnerSshPublicKey: ownerSSHPublicKey,

‎coderd/provisionerdserver/provisionerdserver_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -452,6 +452,7 @@ func TestAcquireJob(t *testing.T) {
452452
TemplateId: template.ID.String(),
453453
TemplateName: template.Name,
454454
TemplateVersion: version.Name,
455+
TemplateVersionId: version.ID.String(),
455456
WorkspaceOwnerSessionToken: sessionToken,
456457
WorkspaceOwnerSshPublicKey: sshKey.PublicKey,
457458
WorkspaceOwnerSshPrivateKey: sshKey.PrivateKey,

‎provisioner/terraform/executor.go‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ type executor struct {
4141
// cachePath and files must not be used by multiple processes at once.
4242
cachePath string
4343
cliConfigPath string
44-
files tfpath.Layout
44+
files tfpath.Layouter
4545
// used to capture execution times at various stages
4646
timings *timingAggregator
4747
}
@@ -536,7 +536,11 @@ func (e *executor) graph(ctx, killCtx context.Context) (string, error) {
536536
if err != nil {
537537
return "", err
538538
}
539-
args := []string{"graph"}
539+
args := []string{
540+
"graph",
541+
// TODO: When the plan is present, we should probably use it?
542+
// "-plan=" + e.files.PlanFilePath(),
543+
}
540544
if ver.GreaterThanOrEqual(version170) {
541545
args = append(args, "-type=plan")
542546
}

‎provisioner/terraform/modules.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ func parseModulesFile(filePath string) ([]*proto.Module, error) {
5858
// getModules returns the modules from the modules file if it exists.
5959
// It returns nil if the file does not exist.
6060
// Modules become available after terraform init.
61-
func getModules(files tfpath.Layout) ([]*proto.Module, error) {
61+
func getModules(files tfpath.Layouter) ([]*proto.Module, error) {
6262
filePath := files.ModulesFilePath()
6363
if _, err := os.Stat(filePath); os.IsNotExist(err) {
6464
return nil, nil

‎provisioner/terraform/serve.go‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -161,7 +161,7 @@ func (s *server) startTrace(ctx context.Context, name string, opts ...trace.Span
161161
))...)
162162
}
163163

164-
func (s *server) executor(files tfpath.Layout, stage database.ProvisionerJobTimingStage) *executor {
164+
func (s *server) executor(files tfpath.Layouter, stage database.ProvisionerJobTimingStage) *executor {
165165
return &executor{
166166
server: s,
167167
mut: s.execMut,

‎provisionerd/provisionerd_test.go‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import (
2626
"github.com/coder/coder/v2/provisionerd/proto"
2727
"github.com/coder/coder/v2/provisionersdk"
2828
sdkproto "github.com/coder/coder/v2/provisionersdk/proto"
29+
"github.com/coder/coder/v2/provisionersdk/tfpath"
2930
"github.com/coder/coder/v2/testutil"
3031
)
3132

@@ -318,8 +319,8 @@ func TestProvisionerd(t *testing.T) {
318319
JobId: "test",
319320
Provisioner: "someprovisioner",
320321
TemplateSourceArchive: testutil.CreateTar(t, map[string]string{
321-
"test.txt": "content",
322-
provisionersdk.ReadmeFile: "# A cool template 😎\n",
322+
"test.txt": "content",
323+
tfpath.ReadmeFile: "# A cool template 😎\n",
323324
}),
324325
Type: &proto.AcquiredJob_TemplateImport_{
325326
TemplateImport: &proto.AcquiredJob_TemplateImport{

‎provisionersdk/cleanup.go‎

Lines changed: 0 additions & 48 deletions
This file was deleted.

‎provisionersdk/cleanup_test.go‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"github.com/stretchr/testify/require"
1212

1313
"cdr.dev/slog"
14-
"github.com/coder/coder/v2/provisionersdk"
1514
"github.com/coder/coder/v2/provisionersdk/tfpath"
1615
"github.com/coder/coder/v2/testutil"
1716
)
@@ -47,9 +46,12 @@ func TestStaleSessions(t *testing.T) {
4746
addSessionFolder(t, fs, second, now.Add(-8*24*time.Hour))
4847
third := tfpath.Session(workDirectory, uuid.NewString())
4948
addSessionFolder(t, fs, third, now.Add(-9*24*time.Hour))
49+
// tfDir is a fake session that will clean up the others
50+
tfDir := tfpath.Session(workDirectory, uuid.NewString())
5051

5152
// when
52-
provisionersdk.CleanStaleSessions(ctx, workDirectory, fs, now, logger)
53+
err := tfDir.CleanStaleSessions(ctx, logger, fs, now)
54+
require.NoError(t, err)
5355

5456
// then
5557
entries, err := afero.ReadDir(fs, workDirectory)
@@ -70,9 +72,11 @@ func TestStaleSessions(t *testing.T) {
7072
addSessionFolder(t, fs, first, now.Add(-7*24*time.Hour))
7173
second := tfpath.Session(workDirectory, uuid.NewString())
7274
addSessionFolder(t, fs, second, now.Add(-6*24*time.Hour))
75+
tfDir := tfpath.Session(workDirectory, uuid.NewString())
7376

7477
// when
75-
provisionersdk.CleanStaleSessions(ctx, workDirectory, fs, now, logger)
78+
err := tfDir.CleanStaleSessions(ctx, logger, fs, now)
79+
require.NoError(t, err)
7680

7781
// then
7882
entries, err := afero.ReadDir(fs, workDirectory)
@@ -94,9 +98,11 @@ func TestStaleSessions(t *testing.T) {
9498
addSessionFolder(t, fs, first, now.Add(-6*24*time.Hour))
9599
second := tfpath.Session(workDirectory, uuid.NewString())
96100
addSessionFolder(t, fs, second, now.Add(-5*24*time.Hour))
101+
tfDir := tfpath.Session(workDirectory, uuid.NewString())
97102

98103
// when
99-
provisionersdk.CleanStaleSessions(ctx, workDirectory, fs, now, logger)
104+
err := tfDir.CleanStaleSessions(ctx, logger, fs, now)
105+
require.NoError(t, err)
100106

101107
// then
102108
entries, err := afero.ReadDir(fs, workDirectory)

‎provisionersdk/session.go‎

Lines changed: 13 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,16 @@ import (
1313
"golang.org/x/xerrors"
1414

1515
"cdr.dev/slog"
16+
"github.com/coder/coder/v2/codersdk"
1617
"github.com/coder/coder/v2/codersdk/drpcsdk"
1718
"github.com/coder/coder/v2/provisionersdk/tfpath"
19+
"github.com/coder/coder/v2/provisionersdk/tfpath/x"
1820

1921
protobuf "google.golang.org/protobuf/proto"
2022

2123
"github.com/coder/coder/v2/provisionersdk/proto"
2224
)
2325

24-
const (
25-
// ReadmeFile is the location we look for to extract documentation from template versions.
26-
ReadmeFile = "README.md"
27-
28-
sessionDirPrefix = "Session"
29-
staleSessionRetention = 7 * 24 * time.Hour
30-
)
31-
3226
// protoServer is a wrapper that translates the dRPC protocol into a Session with method calls into the Server.
3327
type protoServer struct {
3428
server Server
@@ -43,11 +37,6 @@ func (p *protoServer) Session(stream proto.DRPCProvisioner_SessionStream) error
4337
server: p.server,
4438
}
4539

46-
err := CleanStaleSessions(s.Context(), p.opts.WorkDirectory, afero.NewOsFs(), time.Now(), s.Logger)
47-
if err != nil {
48-
return xerrors.Errorf("unable to clean stale sessions %q: %w", s.Files, err)
49-
}
50-
5140
s.Files = tfpath.Session(p.opts.WorkDirectory, sessID)
5241

5342
defer func() {
@@ -67,6 +56,16 @@ func (p *protoServer) Session(stream proto.DRPCProvisioner_SessionStream) error
6756
s.logLevel = proto.LogLevel_value[strings.ToUpper(s.Config.ProvisionerLogLevel)]
6857
}
6958

59+
if p.opts.Experiments.Enabled(codersdk.ExperimentTerraformWorkspace) {
60+
s.Files = x.SessionDir(p.opts.WorkDirectory, sessID, config)
61+
}
62+
63+
// Cleanup any previously left stale sessions.
64+
err = s.Files.CleanStaleSessions(s.Context(), s.Logger, afero.NewOsFs(), time.Now())
65+
if err != nil {
66+
return xerrors.Errorf("unable to clean stale sessions %q: %w", s.Files, err)
67+
}
68+
7069
err = s.Files.ExtractArchive(s.Context(), s.Logger, afero.NewOsFs(), s.Config)
7170
if err != nil {
7271
return xerrors.Errorf("extract archive: %w", err)
@@ -199,7 +198,7 @@ func (s *Session) handleRequests() error {
199198

200199
type Session struct {
201200
Logger slog.Logger
202-
Files tfpath.Layout
201+
Files tfpath.Layouter
203202
Config *proto.Config
204203

205204
server Server

‎provisionersdk/tfpath/tfpath.go‎

Lines changed: 61 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,28 @@ import (
1919
"github.com/coder/coder/v2/provisionersdk/proto"
2020
)
2121

22+
type Layouter interface {
23+
WorkDirectory() string
24+
StateFilePath() string
25+
PlanFilePath() string
26+
TerraformLockFile() string
27+
ReadmeFilePath() string
28+
TerraformMetadataDir() string
29+
ModulesDirectory() string
30+
ModulesFilePath() string
31+
ExtractArchive(ctx context.Context, logger slog.Logger, fs afero.Fs, cfg *proto.Config) error
32+
Cleanup(ctx context.Context, logger slog.Logger, fs afero.Fs)
33+
CleanStaleSessions(ctx context.Context, logger slog.Logger, fs afero.Fs, now time.Time) error
34+
}
35+
36+
var _ Layouter = (*Layout)(nil)
37+
2238
const (
2339
// ReadmeFile is the location we look for to extract documentation from template versions.
2440
ReadmeFile = "README.md"
2541

26-
sessionDirPrefix = "Session"
42+
sessionDirPrefix = "Session"
43+
staleSessionRetention = 7 * 24 * time.Hour
2744
)
2845

2946
// Session creates a directory structure layout for terraform execution. The
@@ -34,6 +51,10 @@ func Session(parentDirPath, sessionID string) Layout {
3451
return Layout(filepath.Join(parentDirPath, sessionDirPrefix+sessionID))
3552
}
3653

54+
func FromWorkingDirectory(workDir string) Layout {
55+
return Layout(workDir)
56+
}
57+
3758
// Layout is the terraform execution working directory structure.
3859
// It also contains some methods for common file operations within that layout.
3960
// Such as "Cleanup" and "ExtractArchive".
@@ -82,6 +103,8 @@ func (l Layout) ExtractArchive(ctx context.Context, logger slog.Logger, fs afero
82103
return xerrors.Errorf("create work directory %q: %w", l.WorkDirectory(), err)
83104
}
84105

106+
// TODO: Pass in cfg.TemplateSourceArchive, not the full config.
107+
// niling out the config field is a bit hacky.
85108
reader := tar.NewReader(bytes.NewBuffer(cfg.TemplateSourceArchive))
86109
// for safety, nil out the reference on Config, since the reader now owns it.
87110
cfg.TemplateSourceArchive = nil
@@ -190,3 +213,40 @@ func (l Layout) Cleanup(ctx context.Context, logger slog.Logger, fs afero.Fs) {
190213
logger.Error(ctx, "failed to clean up work directory after multiple attempts",
191214
slog.F("path", path), slog.Error(err))
192215
}
216+
217+
// CleanStaleSessions browses the work directory searching for stale session
218+
// directories. Coder provisioner is supposed to remove them once after finishing the provisioning,
219+
// but there is a risk of keeping them in case of a failure.
220+
func (l Layout) CleanStaleSessions(ctx context.Context, logger slog.Logger, fs afero.Fs, now time.Time) error {
221+
parent := filepath.Dir(l.WorkDirectory())
222+
entries, err := afero.ReadDir(fs, filepath.Dir(l.WorkDirectory()))
223+
if err != nil {
224+
return xerrors.Errorf("can't read %q directory", parent)
225+
}
226+
227+
for _, fi := range entries {
228+
dirName := fi.Name()
229+
230+
if fi.IsDir() && isValidSessionDir(dirName) {
231+
sessionDirPath := filepath.Join(parent, dirName)
232+
233+
modTime := fi.ModTime() // fallback to modTime if modTime is not available (afero)
234+
235+
if modTime.Add(staleSessionRetention).After(now) {
236+
continue
237+
}
238+
239+
logger.Info(ctx, "remove stale session directory", slog.F("session_path", sessionDirPath))
240+
err = fs.RemoveAll(sessionDirPath)
241+
if err != nil {
242+
return xerrors.Errorf("can't remove %q directory: %w", sessionDirPath, err)
243+
}
244+
}
245+
}
246+
return nil
247+
}
248+
249+
func isValidSessionDir(dirName string) bool {
250+
match, err := filepath.Match(sessionDirPrefix+"*", dirName)
251+
return err == nil && match
252+
}

0 commit comments

Comments
 (0)