Skip to content

chore: distinct operations for provisioner's 'parse', 'init', 'plan', 'apply', 'graph' #21064

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
Emyrk wants to merge 4 commits into
base: stevenmasley/provisioner_script_race
Choose a base branch
from
+2,154 −1,120
Open

chore: distinct operations for provisioner's 'parse', 'init', 'plan', 'apply', 'graph' #21064

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
04bc86a
chore: distinct operations for provisioner's tf operations
Emyrk Dec 2, 2025
83326c9
remove daily cost from template import data
Emyrk Dec 10, 2025
9cc7ea7
move buildworkspace -> apply
Emyrk Dec 10, 2025
4ba2b34
chore: update provisioner requests comment
Emyrk Dec 10, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
305 changes: 263 additions & 42 deletions provisioner/echo/serve.go
View file
Open in desktop

Large diffs are not rendered by default.

123 changes: 16 additions & 107 deletions provisioner/terraform/executor.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import (
"cdr.dev/slog"
"github.com/coder/coder/v2/provisionersdk/tfpath"

"github.com/coder/coder/v2/coderd/database"
"github.com/coder/coder/v2/coderd/tracing"
"github.com/coder/coder/v2/provisionersdk/proto"
)
Expand Down Expand Up @@ -283,7 +282,7 @@ func (e *executor) init(ctx, killCtx context.Context, logr logSink) error {
func checksumFileCRC32(ctx context.Context, logger slog.Logger, path string) uint32 {
content, err := os.ReadFile(path)
if err != nil {
logger.Debug(ctx, "file %s does not exist or can't be read, skip checksum calculation")
logger.Debug(ctx, "file does not exist or can't be read, skip checksum calculation", slog.F("path", path))
return 0
}
return crc32.ChecksumIEEE(content)
Expand Down Expand Up @@ -330,34 +329,16 @@ func (e *executor) plan(ctx, killCtx context.Context, env, vars []string, logr l
return nil, xerrors.Errorf("terraform plan: %w", err)
}

// Capture the duration of the call to `terraform graph`.
graphTimings := newTimingAggregator(database.ProvisionerJobTimingStageGraph)
graphTimings.ingest(createGraphTimingsEvent(timingGraphStart))

state, plan, err := e.planResources(ctx, killCtx, planfilePath)
plan, err := e.parsePlan(ctx, killCtx, planfilePath)
if err != nil {
graphTimings.ingest(createGraphTimingsEvent(timingGraphErrored))
return nil, xerrors.Errorf("plan resources: %w", err)
return nil, xerrors.Errorf("show terraform plan file: %w", err)
}

planJSON, err := json.Marshal(plan)
if err != nil {
return nil, xerrors.Errorf("marshal plan: %w", err)
}

graphTimings.ingest(createGraphTimingsEvent(timingGraphComplete))

var moduleFiles []byte
// Skipping modules archiving is useful if the caller does not need it, eg during
// a workspace build. This removes some added costs of sending the modules
// payload back to coderd if coderd is just going to ignore it.
if !req.OmitModuleFiles {
moduleFiles, err = GetModulesArchive(os.DirFS(e.files.WorkDirectory()))
if err != nil {
// TODO: we probably want to persist this error or make it louder eventually
e.logger.Warn(ctx, "failed to archive terraform modules", slog.Error(err))
}
}

Comment on lines -347 to -360
Copy link
Member Author

@Emyrk Emyrk Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to the init step, where modules are downloaded.

// When a prebuild claim attempt is made, log a warning if a resource is due to be replaced, since this will obviate
// the point of prebuilding if the expensive resource is replaced once claimed!
var (
Expand All @@ -384,18 +365,16 @@ func (e *executor) plan(ctx, killCtx context.Context, env, vars []string, logr l
}
}

state, err := ConvertPlanState(plan)
if err != nil {
return nil, xerrors.Errorf("convert plan state: %w", err)
}

msg := &proto.PlanComplete{
Parameters: state.Parameters,
Resources: state.Resources,
ExternalAuthProviders: state.ExternalAuthProviders,
Timings: graphTimings.aggregate(),
Presets: state.Presets,
Plan: planJSON,
ResourceReplacements: resReps,
ModuleFiles: moduleFiles,
HasAiTasks: state.HasAITasks,
AiTasks: state.AITasks,
HasExternalAgents: state.HasExternalAgents,
Plan: planJSON,
DailyCost: state.DailyCost,
ResourceReplacements: resReps,
AiTaskCount: state.AITaskCount,
}

return msg, nil
Expand All @@ -418,42 +397,6 @@ func onlyDataResources(sm tfjson.StateModule) tfjson.StateModule {
return filtered
}

// planResources must only be called while the lock is held.
func (e *executor) planResources(ctx, killCtx context.Context, planfilePath string) (*State, *tfjson.Plan, error) {
ctx, span := e.server.startTrace(ctx, tracing.FuncName())
defer span.End()

plan, err := e.parsePlan(ctx, killCtx, planfilePath)
if err != nil {
return nil, nil, xerrors.Errorf("show terraform plan file: %w", err)
}

rawGraph, err := e.graph(ctx, killCtx)
if err != nil {
return nil, nil, xerrors.Errorf("graph: %w", err)
}
modules := []*tfjson.StateModule{}
if plan.PriorState != nil {
// We need the data resources for rich parameters. For some reason, they
// only show up in the PriorState.
//
// We don't want all prior resources, because Quotas (and
// future features) would never know which resources are getting
// deleted by a stop.

filtered := onlyDataResources(*plan.PriorState.Values.RootModule)
modules = append(modules, &filtered)
}
modules = append(modules, plan.PlannedValues.RootModule)

state, err := ConvertState(ctx, modules, rawGraph, e.server.logger)
if err != nil {
return nil, nil, err
}

return state, plan, nil
}
Comment on lines -421 to -455
Copy link
Member Author

@Emyrk Emyrk Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to graph


// parsePlan must only be called while the lock is held.
func (e *executor) parsePlan(ctx, killCtx context.Context, planfilePath string) (*tfjson.Plan, error) {
ctx, span := e.server.startTrace(ctx, tracing.FuncName())
Expand Down Expand Up @@ -541,9 +484,11 @@ func (e *executor) graph(ctx, killCtx context.Context) (string, error) {
// TODO: When the plan is present, we should probably use it?
// "-plan=" + e.files.PlanFilePath(),
}

if ver.GreaterThanOrEqual(version170) {
args = append(args, "-type=plan")
}

var out strings.Builder
cmd := exec.CommandContext(killCtx, e.binaryPath, args...) // #nosec
cmd.Stdout = &out
Expand Down Expand Up @@ -602,53 +547,17 @@ func (e *executor) apply(
return nil, xerrors.Errorf("terraform apply: %w", err)
}

// `terraform show` & `terraform graph`
state, err := e.stateResources(ctx, killCtx)
if err != nil {
return nil, err
}
Comment on lines -605 to -609
Copy link
Member Author

@Emyrk Emyrk Dec 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved to graph

statefilePath := e.files.StateFilePath()
stateContent, err := os.ReadFile(statefilePath)
if err != nil {
return nil, xerrors.Errorf("read statefile %q: %w", statefilePath, err)
}

return &proto.ApplyComplete{
Parameters: state.Parameters,
Resources: state.Resources,
ExternalAuthProviders: state.ExternalAuthProviders,
State: stateContent,
AiTasks: state.AITasks,
State: stateContent,
}, nil
}

// stateResources must only be called while the lock is held.
func (e *executor) stateResources(ctx, killCtx context.Context) (*State, error) {
ctx, span := e.server.startTrace(ctx, tracing.FuncName())
defer span.End()

state, err := e.state(ctx, killCtx)
if err != nil {
return nil, err
}
rawGraph, err := e.graph(ctx, killCtx)
if err != nil {
return nil, xerrors.Errorf("get terraform graph: %w", err)
}
converted := &State{}
if state.Values == nil {
return converted, nil
}

converted, err = ConvertState(ctx, []*tfjson.StateModule{
state.Values.RootModule,
}, rawGraph, e.server.logger)
if err != nil {
return nil, err
}
return converted, nil
}

// state must only be called while the lock is held.
func (e *executor) state(ctx, killCtx context.Context) (*tfjson.State, error) {
ctx, span := e.server.startTrace(ctx, tracing.FuncName())
Expand Down
80 changes: 80 additions & 0 deletions provisioner/terraform/planresources.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
package terraform

import (
tfjson "github.com/hashicorp/terraform-json"
"github.com/mitchellh/mapstructure"
"golang.org/x/xerrors"
)

type PlanState struct {
DailyCost int32
AITaskCount int32
}

func planModules(plan *tfjson.Plan) []*tfjson.StateModule {
modules := []*tfjson.StateModule{}
if plan.PriorState != nil {
// We need the data resources for rich parameters. For some reason, they
// only show up in the PriorState.
//
// We don't want all prior resources, because Quotas (and
// future features) would never know which resources are getting
// deleted by a stop.

filtered := onlyDataResources(*plan.PriorState.Values.RootModule)
modules = append(modules, &filtered)
}
modules = append(modules, plan.PlannedValues.RootModule)
return modules
}

// ConvertPlanState consumes a terraform plan json output and produces a thinner
// version of `State` to be used before `terraform apply`. `ConvertState`
// requires `terraform graph`, this does not.
func ConvertPlanState(plan *tfjson.Plan) (*PlanState, error) {
modules := planModules(plan)

var dailyCost int32
var aiTaskCount int32
for _, mod := range modules {
err := forEachResource(mod, func(res *tfjson.StateResource) error {
switch res.Type {
case "coder_metadata":
var attrs resourceMetadataAttributes
err := mapstructure.Decode(res.AttributeValues, &attrs)
if err != nil {
return xerrors.Errorf("decode metadata attributes: %w", err)
}
dailyCost += attrs.DailyCost
case "coder_ai_task":
aiTaskCount++
}
return nil
})
if err != nil {
return nil, xerrors.Errorf("parse plan: %w", err)
}
}

return &PlanState{
DailyCost: dailyCost,
AITaskCount: aiTaskCount,
}, nil
}

func forEachResource(input *tfjson.StateModule, do func(res *tfjson.StateResource) error) error {
for _, res := range input.Resources {
err := do(res)
if err != nil {
return xerrors.Errorf("in module %s: %w", input.Address, err)
}
}

for _, mod := range input.ChildModules {
err := forEachResource(mod, do)
if err != nil {
return xerrors.Errorf("in module %s: %w", mod.Address, err)
}
}
return nil
}
Loading
Loading