refactor: have services return task specifications

internal/app/module_test.go

@@ -72,7 +72,7 @@ func TestModule_MultipleFormat(t *testing.T) {
 	tm.Send(tea.KeyMsg{Type: tea.KeyCtrlA})
 	tm.Type("f")
 	waitFor(t, tm, func(s string) bool {
-		return matchPattern(t, "TaskGroup.*format", s) &&
+		return matchPattern(t, "TaskGroup.*fmt", s) &&
 			matchPattern(t, `modules/a.*exited`, s) &&
 			matchPattern(t, `modules/b.*exited`, s) &&
 			matchPattern(t, `modules/c.*exited`, s)
@@ -139,7 +139,7 @@ func TestModuleList_ReloadWorkspacesMultipleModules(t *testing.T) {
 	tm.Send(tea.KeyMsg{Type: tea.KeyCtrlW})
 
 	waitFor(t, tm, func(s string) bool {
-		return matchPattern(t, "TaskGroup.*reload-workspace", s) &&
+		return matchPattern(t, "TaskGroup.*workspace list", s) &&
 			matchPattern(t, "modules/a.*exited", s) &&
 			matchPattern(t, "modules/b.*exited", s) &&
 			matchPattern(t, "modules/c.*exited", s)
@@ -320,7 +320,6 @@ func TestModule_MultipleDestroy(t *testing.T) {
 	tm.Send(tea.KeyMsg{Type: tea.KeyCtrlA})
 	tm.Type("i")
 	waitFor(t, tm, func(s string) bool {
-		t.Log(s)
 		return matchPattern(t, "TaskGroup.*init", s) &&
 			matchPattern(t, `modules/a.*exited`, s) &&
 			matchPattern(t, `modules/b.*exited`, s) &&
@@ -332,7 +331,6 @@ func TestModule_MultipleDestroy(t *testing.T) {
 
 	// Expect three modules to be listed, along with their default workspace.
 	waitFor(t, tm, func(s string) bool {
-		t.Log(s)
 		return matchPattern(t, "modules/a.*default", s) &&
 			matchPattern(t, "modules/b.*default", s) &&
 			matchPattern(t, "modules/c.*default", s)

internal/app/terragrunt_test.go

@@ -106,7 +106,6 @@ func TestTerragrunt_Dependencies(t *testing.T) {
 
 	// Expect 6 apply tasks.
 	waitFor(t, tm, func(s string) bool {
-		t.Log(s)
 		return matchPattern(t, "TaskGroup.*apply.*6/6", s) &&
 			matchPattern(t, `modules/vpc.*default.*\+0~0-0`, s) &&
 			matchPattern(t, `modules/redis.*default.*\+0~0-0`, s) &&

internal/app/workspace_test.go

@@ -210,7 +210,6 @@ func TestWorkspace_MultipleDestroy(t *testing.T) {
 	tm.Send(tea.KeyMsg{Type: tea.KeyCtrlA})
 	tm.Type("i")
 	waitFor(t, tm, func(s string) bool {
-		t.Log(s)
 		return matchPattern(t, "TaskGroup.*init", s) &&
 			matchPattern(t, `modules/a.*exited`, s) &&
 			matchPattern(t, `modules/b.*exited`, s) &&
@@ -286,8 +285,8 @@ func TestWorkspace_Delete(t *testing.T) {
 
 	tm := setupAndInitModuleWithTwoWorkspaces(t)
 
-	// Filter workspaces to only show dev workspace. This is the only to ensure
-	// the dev workspace is currently highlighted.
+	// Filter workspaces to only show dev workspace. This is the only way to
+	// ensure the dev workspace is currently highlighted.
 
 	// Focus filter widget
 	tm.Type("/")
@@ -320,7 +319,7 @@ func TestWorkspace_Delete(t *testing.T) {
 	tm.Type("y")
 
 	waitFor(t, tm, func(s string) bool {
-		return matchPattern(t, `Task.*workspace delete.*dev.*modules/a.*exited`, s) &&
+		return matchPattern(t, `Task.*workspace delete.*modules/a.*exited`, s) &&
 			strings.Contains(s, `Deleted workspace "dev"!`)
 	})
 }

internal/module/service.go

@@ -33,7 +33,7 @@ type ServiceOptions struct {
 }
 
 type taskCreator interface {
-	Create(spec task.CreateOptions) (*task.Task, error)
+	Create(spec task.Spec) (*task.Task, error)
 }
 
 type moduleTable interface {
@@ -111,7 +111,7 @@ func (s *Service) Reload() (added []string, removed []string, err error) {
 }
 
 func (s *Service) loadTerragruntDependencies() error {
-	task, err := s.tasks.Create(task.CreateOptions{
+	task, err := s.tasks.Create(task.Spec{
 		Parent:  resource.GlobalResource,
 		Command: []string{"graph-dependencies"},
 		Wait:    true,
@@ -194,32 +194,35 @@ func (s *Service) stripWorkdirFromPath(path string) (string, error) {
 }
 
 // Init invokes terraform init on the module.
-func (s *Service) Init(moduleID resource.ID) (*task.Task, error) {
-	mod, err := s.Get(moduleID)
-	if err != nil {
-		return nil, fmt.Errorf("initializing module: %w", err)
-	}
-
-	// create asynchronous task that runs terraform init
-	tsk, err := s.CreateTask(mod, task.CreateOptions{
+func (s *Service) Init(moduleID resource.ID) (task.Spec, error) {
+	return s.updateSpec(moduleID, task.Spec{
 		Command:  []string{"init"},
 		Args:     []string{"-input=false"},
 		Blocking: true,
 		// The terraform plugin cache is not concurrency-safe, so only allow one
 		// init task to run at any given time.
 		Exclusive: s.pluginCache,
-		AfterCreate: func(*task.Task) {
+		AfterCreate: func(task *task.Task) {
 			// Trigger a workspace reload if the module doesn't yet have a
 			// current workspace
+			mod := task.Module().(*Module)
 			if mod.CurrentWorkspaceID == nil {
 				s.Publish(resource.UpdatedEvent, mod)
 			}
 		},
 	})
-	if err != nil {
-		return nil, err
-	}
-	return tsk, nil
+}
+
+func (s *Service) Format(moduleID resource.ID) (task.Spec, error) {
+	return s.updateSpec(moduleID, task.Spec{
+		Command: []string{"fmt"},
+	})
+}
+
+func (s *Service) Validate(moduleID resource.ID) (task.Spec, error) {
+	return s.updateSpec(moduleID, task.Spec{
+		Command: []string{"validate"},
+	})
 }
 
 func (s *Service) List() []*Module {
@@ -252,31 +255,13 @@ func (s *Service) SetCurrent(moduleID, workspaceID resource.ID) error {
 	return nil
 }
 
-func (s *Service) Format(moduleID resource.ID) (*task.Task, error) {
+// updateSpec updates the task spec with common module settings.
+func (s *Service) updateSpec(moduleID resource.ID, spec task.Spec) (task.Spec, error) {
 	mod, err := s.table.Get(moduleID)
 	if err != nil {
-		return nil, fmt.Errorf("formatting module: %w", err)
+		return task.Spec{}, err
 	}
-
-	return s.CreateTask(mod, task.CreateOptions{
-		Command: []string{"fmt"},
-	})
-}
-
-func (s *Service) Validate(moduleID resource.ID) (*task.Task, error) {
-	mod, err := s.table.Get(moduleID)
-	if err != nil {
-		return nil, fmt.Errorf("validating module: %w", err)
-	}
-
-	return s.CreateTask(mod, task.CreateOptions{
-		Command: []string{"validate"},
-	})
-}
-
-// TODO: move this logic into task.Create
-func (s *Service) CreateTask(mod *Module, opts task.CreateOptions) (*task.Task, error) {
-	opts.Parent = mod
-	opts.Path = mod.Path
-	return s.tasks.Create(opts)
+	spec.Parent = mod
+	spec.Path = mod.Path
+	return spec, nil
 }

internal/run/service.go

@@ -1,7 +1,6 @@
 package run
 
 import (
-	"errors"
 	"fmt"
 	"slices"
 
@@ -70,21 +69,26 @@ func NewService(opts ServiceOptions) *Service {
 }
 
 // Plan creates a plan task.
-func (s *Service) Plan(workspaceID resource.ID, opts CreateOptions) (*task.Task, error) {
-	task, err := s.plan(workspaceID, opts)
+func (s *Service) Plan(workspaceID resource.ID, opts CreateOptions) (task.Spec, error) {
+	spec, err := s.plan(workspaceID, opts)
 	if err != nil {
-		s.logger.Error("creating plan task", "error", err)
-		return nil, err
+		s.logger.Error("creating plan spec", "error", err)
+		return task.Spec{}, err
 	}
-	return task, nil
+	return spec, nil
 }
 
-func (s *Service) plan(workspaceID resource.ID, opts CreateOptions) (*task.Task, error) {
+func (s *Service) plan(workspaceID resource.ID, opts CreateOptions) (task.Spec, error) {
 	run, err := s.newRun(workspaceID, opts)
 	if err != nil {
-		return nil, err
+		return task.Spec{}, err
 	}
-	task, err := s.createTask(run, task.CreateOptions{
+	s.table.Add(run.ID, run)
+
+	return task.Spec{
+		Parent:  run,
+		Path:    run.ModulePath(),
+		Env:     []string{workspace.TerraformEnv(run.WorkspaceName())},
 		Command: []string{"plan"},
 		Args:    run.planArgs(),
 		// TODO: explain why plan is blocking (?)
@@ -107,12 +111,7 @@ func (s *Service) plan(workspaceID resource.ID, opts CreateOptions) (*task.Task,
 				s.logger.Error("finishing plan", "error", err, "run", run)
 			}
 		},
-	})
-	if err != nil {
-		return nil, err
-	}
-	s.table.Add(run.ID, run)
-	return task, nil
+	}, nil
 }
 
 // Apply creates a task for a terraform apply.
@@ -122,45 +121,7 @@ func (s *Service) plan(workspaceID resource.ID, opts CreateOptions) (*task.Task,
 //
 // If opts is nil, then it will apply an existing plan. The ID must specify an
 // existing run that has successfully created a plan.
-func (s *Service) Apply(id resource.ID, opts *CreateOptions) (*task.Task, error) {
-	spec, _, err := s.createApplySpec(id, opts)
-	if err != nil {
-		return nil, err
-	}
-	return s.tasks.Create(spec)
-}
-
-// MultiApply creates a task group of one or more apply tasks. See Apply() for
-// info on parameters.
-//
-// You cannot apply a combination of destory and non-destroy plans, because that
-// is incompatible with the dependency graph that is created to order the tasks.
-func (s *Service) MultiApply(opts *CreateOptions, ids ...resource.ID) (*task.Group, error) {
-	if len(ids) == 0 {
-		return nil, errors.New("no IDs specified")
-	}
-	var destroy *bool
-	specs := make([]task.CreateOptions, 0, len(ids))
-	for _, id := range ids {
-		spec, run, err := s.createApplySpec(id, opts)
-		if err != nil {
-			return nil, err
-		}
-		if destroy == nil {
-			destroy = &run.Destroy
-		} else if *destroy != run.Destroy {
-			return nil, errors.New("cannot apply a combination of destroy and non-destroy plans")
-		}
-
-		specs = append(specs, spec)
-	}
-	// All tasks should have the same description, so use the first one.
-	desc := specs[0].Description
-	return s.tasks.CreateDependencyGroup(desc, *destroy, specs...)
-}
-
-func (s *Service) createApplySpec(id resource.ID, opts *CreateOptions) (task.CreateOptions, *Run, error) {
-	// Create or retrieve existing run.
+func (s *Service) Apply(id resource.ID, opts *CreateOptions) (task.Spec, error) {
 	var (
 		run *Run
 		err error
@@ -177,15 +138,22 @@ func (s *Service) createApplySpec(id resource.ID, opts *CreateOptions) (task.Cre
 		}
 	}
 	if err != nil {
-		return task.CreateOptions{}, nil, err
+		return task.Spec{}, err
 	}
 	s.table.Add(run.ID, run)
 
-	spec := task.CreateOptions{
+	spec := task.Spec{
+		Parent:      run,
+		Path:        run.ModulePath(),
 		Command:     []string{"apply"},
 		Args:        run.applyArgs(),
+		Env:         []string{workspace.TerraformEnv(run.WorkspaceName())},
 		Blocking:    true,
 		Description: ApplyTaskDescription(run.Destroy),
+		// If terragrunt is in use then respect module dependencies.
+		RespectModuleDependencies: s.terragrunt,
+		// Module dependencies are reversed for a destroy.
+		InverseDependencyOrder: run.Destroy,
 		AfterQueued: func(*task.Task) {
 			run.updateStatus(ApplyQueued)
 		},
@@ -205,14 +173,19 @@ func (s *Service) createApplySpec(id resource.ID, opts *CreateOptions) (task.Cre
 			}
 
 			if !s.disableReloadAfterApply {
-				s.states.Reload(run.WorkspaceID())
+				spec, err := s.states.Reload(run.WorkspaceID())
+				if err != nil {
+					s.logger.Error("reloading state following apply", "error", err, "run", run)
+					return
+				}
+				if _, err := s.tasks.Create(spec); err != nil {
+					s.logger.Error("reloading state following apply", "error", err, "run", run)
+					return
+				}
 			}
 		},
 	}
-	if err := s.addWorkspaceAndPathToTaskSpec(run, &spec); err != nil {
-		return task.CreateOptions{}, nil, err
-	}
-	return spec, run, nil
+	return spec, nil
 }
 
 func (s *Service) Get(runID resource.ID) (*Run, error) {
@@ -281,28 +254,3 @@ func (s *Service) delete(id resource.ID) error {
 	s.table.Delete(id)
 	return nil
 }
-
-func (s *Service) createTask(run *Run, opts task.CreateOptions) (*task.Task, error) {
-	if err := s.addWorkspaceAndPathToTaskSpec(run, &opts); err != nil {
-		return nil, err
-	}
-	return s.tasks.Create(opts)
-}
-
-func (s *Service) addWorkspaceAndPathToTaskSpec(run *Run, opts *task.CreateOptions) error {
-	opts.Parent = run
-
-	ws, err := s.workspaces.Get(run.WorkspaceID())
-	if err != nil {
-		return err
-	}
-	opts.Env = []string{ws.TerraformEnv()}
-
-	mod, err := s.modules.Get(ws.ModuleID())
-	if err != nil {
-		return err
-	}
-	opts.Path = mod.Path
-
-	return nil
-}