move around some functions

Author: Emyrk

coderd/provisionerdserver/provisionerdserver.go

@@ -699,16 +699,19 @@ func (s *server) acquireProtoJob(ctx context.Context, job database.ProvisionerJo
 			}
 		}
 
+		activeVersion := template.ActiveVersionID == templateVersion.ID
 		protoJob.Type = &proto.AcquiredJob_WorkspaceBuild_{
 			WorkspaceBuild: &proto.AcquiredJob_WorkspaceBuild{
-				WorkspaceBuildId:           workspaceBuild.ID.String(),
-				WorkspaceName:              workspace.Name,
-				State:                      workspaceBuild.ProvisionerState,
-				RichParameterValues:        convertRichParameterValues(workspaceBuildParameters),
-				PreviousParameterValues:    convertRichParameterValues(lastWorkspaceBuildParameters),
-				VariableValues:             asVariableValues(templateVariables),
-				ExternalAuthProviders:      externalAuthProviders,
-				ExpReuseTerraformWorkspace: ptr.Ref(false), // TODO: Toggle based on experiment
+				WorkspaceBuildId:        workspaceBuild.ID.String(),
+				WorkspaceName:           workspace.Name,
+				State:                   workspaceBuild.ProvisionerState,
+				RichParameterValues:     convertRichParameterValues(workspaceBuildParameters),
+				PreviousParameterValues: convertRichParameterValues(lastWorkspaceBuildParameters),
+				VariableValues:          asVariableValues(templateVariables),
+				ExternalAuthProviders:   externalAuthProviders,
+				// If active and experiment is enabled, allow workspace reuse existing TF
+				// workspaces (directories) for a faster startup.
+				ExpReuseTerraformWorkspace: ptr.Ref(activeVersion && s.Experiments.Enabled(codersdk.ExperimentTerraformWorkspace)),
 				Metadata: &sdkproto.Metadata{
 					CoderUrl:                      s.AccessURL.String(),
 					WorkspaceTransition:           transition,
@@ -722,6 +725,7 @@ func (s *server) acquireProtoJob(ctx context.Context, job database.ProvisionerJo
 					WorkspaceOwnerId:              owner.ID.String(),
 					TemplateId:                    template.ID.String(),
 					TemplateName:                  template.Name,
+					TemplateVersionId:             templateVersion.ID.String(),
 					TemplateVersion:               templateVersion.Name,
 					WorkspaceOwnerSessionToken:    sessionToken,
 					WorkspaceOwnerSshPublicKey:    ownerSSHPublicKey,

coderd/provisionerdserver/provisionerdserver_test.go

@@ -452,6 +452,7 @@ func TestAcquireJob(t *testing.T) {
 					TemplateId:                    template.ID.String(),
 					TemplateName:                  template.Name,
 					TemplateVersion:               version.Name,
+					TemplateVersionId:             version.ID.String(),
 					WorkspaceOwnerSessionToken:    sessionToken,
 					WorkspaceOwnerSshPublicKey:    sshKey.PublicKey,
 					WorkspaceOwnerSshPrivateKey:   sshKey.PrivateKey,

provisioner/terraform/executor.go

@@ -536,7 +536,11 @@ func (e *executor) graph(ctx, killCtx context.Context) (string, error) {
 	if err != nil {
 		return "", err
 	}
-	args := []string{"graph"}
+	args := []string{
+		"graph",
+		// TODO: When the plan is present, we should probably use it?
+		// "-plan=" + e.files.PlanFilePath(),
+	}
 	if ver.GreaterThanOrEqual(version170) {
 		args = append(args, "-type=plan")
 	}

provisionerd/provisionerd_test.go

@@ -26,6 +26,7 @@ import (
 	"github.com/coder/coder/v2/provisionerd/proto"
 	"github.com/coder/coder/v2/provisionersdk"
 	sdkproto "github.com/coder/coder/v2/provisionersdk/proto"
+	"github.com/coder/coder/v2/provisionersdk/tfpath"
 	"github.com/coder/coder/v2/testutil"
 )
 
@@ -318,8 +319,8 @@ func TestProvisionerd(t *testing.T) {
 				JobId:       "test",
 				Provisioner: "someprovisioner",
 				TemplateSourceArchive: testutil.CreateTar(t, map[string]string{
-					"test.txt":                "content",
-					provisionersdk.ReadmeFile: "# A cool template 😎\n",
+					"test.txt":        "content",
+					tfpath.ReadmeFile: "# A cool template 😎\n",
 				}),
 				Type: &proto.AcquiredJob_TemplateImport_{
 					TemplateImport: &proto.AcquiredJob_TemplateImport{

provisionersdk/cleanup.go

@@ -11,7 +11,6 @@ import (
 	"github.com/stretchr/testify/require"
 
 	"cdr.dev/slog"
-	"github.com/coder/coder/v2/provisionersdk"
 	"github.com/coder/coder/v2/provisionersdk/tfpath"
 	"github.com/coder/coder/v2/testutil"
 )
@@ -47,9 +46,12 @@ func TestStaleSessions(t *testing.T) {
 		addSessionFolder(t, fs, second, now.Add(-8*24*time.Hour))
 		third := tfpath.Session(workDirectory, uuid.NewString())
 		addSessionFolder(t, fs, third, now.Add(-9*24*time.Hour))
+		// tfDir is a fake session that will clean up the others
+		tfDir := tfpath.Session(workDirectory, uuid.NewString())
 
 		// when
-		provisionersdk.CleanStaleSessions(ctx, workDirectory, fs, now, logger)
+		err := tfDir.CleanStaleSessions(ctx, logger, fs, now)
+		require.NoError(t, err)
 
 		// then
 		entries, err := afero.ReadDir(fs, workDirectory)
@@ -70,9 +72,11 @@ func TestStaleSessions(t *testing.T) {
 		addSessionFolder(t, fs, first, now.Add(-7*24*time.Hour))
 		second := tfpath.Session(workDirectory, uuid.NewString())
 		addSessionFolder(t, fs, second, now.Add(-6*24*time.Hour))
+		tfDir := tfpath.Session(workDirectory, uuid.NewString())
 
 		// when
-		provisionersdk.CleanStaleSessions(ctx, workDirectory, fs, now, logger)
+		err := tfDir.CleanStaleSessions(ctx, logger, fs, now)
+		require.NoError(t, err)
 
 		// then
 		entries, err := afero.ReadDir(fs, workDirectory)
@@ -94,9 +98,11 @@ func TestStaleSessions(t *testing.T) {
 		addSessionFolder(t, fs, first, now.Add(-6*24*time.Hour))
 		second := tfpath.Session(workDirectory, uuid.NewString())
 		addSessionFolder(t, fs, second, now.Add(-5*24*time.Hour))
+		tfDir := tfpath.Session(workDirectory, uuid.NewString())
 
 		// when
-		provisionersdk.CleanStaleSessions(ctx, workDirectory, fs, now, logger)
+		err := tfDir.CleanStaleSessions(ctx, logger, fs, now)
+		require.NoError(t, err)
 
 		// then
 		entries, err := afero.ReadDir(fs, workDirectory)

provisionersdk/cleanup_test.go

@@ -16,21 +16,13 @@ import (
 	"github.com/coder/coder/v2/codersdk"
 	"github.com/coder/coder/v2/codersdk/drpcsdk"
 	"github.com/coder/coder/v2/provisionersdk/tfpath"
-	"github.com/coder/coder/v2/provisionersdk/x"
+	"github.com/coder/coder/v2/provisionersdk/tfpath/x"
 
 	protobuf "google.golang.org/protobuf/proto"
 
 	"github.com/coder/coder/v2/provisionersdk/proto"
 )
 
-const (
-	// ReadmeFile is the location we look for to extract documentation from template versions.
-	ReadmeFile = "README.md"
-
-	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.
 type protoServer struct {
 	server Server
@@ -45,11 +37,6 @@ func (p *protoServer) Session(stream proto.DRPCProvisioner_SessionStream) error
 		server: p.server,
 	}
 
-	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.Files, err)
-	}
-
 	s.Files = tfpath.Session(p.opts.WorkDirectory, sessID)
 
 	defer func() {
@@ -70,13 +57,13 @@ func (p *protoServer) Session(stream proto.DRPCProvisioner_SessionStream) error
 	}
 
 	if p.opts.Experiments.Enabled(codersdk.ExperimentTerraformWorkspace) {
-		// TODO: Also indicate if opted into the feature via config
 		s.Files = x.SessionDir(p.opts.WorkDirectory, sessID, config)
+	}
 
-		err = s.Files.CleanInactiveTemplateVersions(s.Context(), s.Logger, afero.NewOsFs())
-		if err != nil {
-			return xerrors.Errorf("unable to clean inactive versions %q: %w", s.Files.WorkDirectory(), err)
-		}
+	// Cleanup any previously left stale sessions.
+	err = s.Files.CleanStaleSessions(s.Context(), s.Logger, afero.NewOsFs(), time.Now())
+	if err != nil {
+		return xerrors.Errorf("unable to clean stale sessions %q: %w", s.Files, err)
 	}
 
 	err = s.Files.ExtractArchive(s.Context(), s.Logger, afero.NewOsFs(), s.Config)

provisionersdk/session.go

@@ -30,13 +30,17 @@ type LayoutInterface interface {
 	ModulesFilePath() string
 	ExtractArchive(ctx context.Context, logger slog.Logger, fs afero.Fs, cfg *proto.Config) error
 	Cleanup(ctx context.Context, logger slog.Logger, fs afero.Fs)
+	CleanStaleSessions(ctx context.Context, logger slog.Logger, fs afero.Fs, now time.Time) error
 }
 
+var _ LayoutInterface = (*Layout)(nil)
+
 const (
 	// ReadmeFile is the location we look for to extract documentation from template versions.
 	ReadmeFile = "README.md"
 
-	sessionDirPrefix = "Session"
+	sessionDirPrefix      = "Session"
+	staleSessionRetention = 7 * 24 * time.Hour
 )
 
 // Session creates a directory structure layout for terraform execution. The
@@ -47,6 +51,10 @@ func Session(parentDirPath, sessionID string) Layout {
 	return Layout(filepath.Join(parentDirPath, sessionDirPrefix+sessionID))
 }
 
+func FromWorkingDirectory(workDir string) Layout {
+	return Layout(workDir)
+}
+
 // Layout is the terraform execution working directory structure.
 // It also contains some methods for common file operations within that layout.
 // Such as "Cleanup" and "ExtractArchive".
@@ -203,3 +211,40 @@ func (l Layout) Cleanup(ctx context.Context, logger slog.Logger, fs afero.Fs) {
 	logger.Error(ctx, "failed to clean up work directory after multiple attempts",
 		slog.F("path", path), slog.Error(err))
 }
+
+// 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 (l Layout) CleanStaleSessions(ctx context.Context, logger slog.Logger, fs afero.Fs, now time.Time) error {
+	parent := filepath.Dir(l.WorkDirectory())
+	entries, err := afero.ReadDir(fs, filepath.Dir(l.WorkDirectory()))
+	if err != nil {
+		return xerrors.Errorf("can't read %q directory", parent)
+	}
+
+	for _, fi := range entries {
+		dirName := fi.Name()
+
+		if fi.IsDir() && isValidSessionDir(dirName) {
+			sessionDirPath := filepath.Join(parent, dirName)
+
+			modTime := fi.ModTime() // fallback to modTime if modTime is not available (afero)
+
+			if modTime.Add(staleSessionRetention).After(now) {
+				continue
+			}
+
+			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)
+			}
+		}
+	}
+	return nil
+}
+
+func isValidSessionDir(dirName string) bool {
+	match, err := filepath.Match(sessionDirPrefix+"*", dirName)
+	return err == nil && match
+}