chore: distinct operations for provisioner's 'parse', 'init', 'plan', 'apply', 'graph' #21064
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Member
Author
|
Warning This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
This stack of pull requests is managed by Graphite. Learn more about stacking. |
03de57d to
4a270f4
Compare
4a270f4 to
24d4837
Compare
b1f75f3 to
eed2f12
Compare
24d4837 to
98ad5d4
Compare
eed2f12 to
56dffc9
Compare
834b599 to
ec54ef3
Compare
56dffc9 to
cf94b5d
Compare
ec54ef3 to
4efa20c
Compare
cf94b5d to
6b8d466
Compare
4efa20c to
3c9ea20
Compare
3b5d651 to
04bc86a
Compare
spikecurtis
reviewed
Dec 10, 2025
| // TODO: These are defined as different, but can they be? | ||
| // Terraform downloads modules regardless of `count`, so this should be the same | ||
| StartModules: initResp.Modules, | ||
| StopModules: initResp.Modules, |
Contributor
There was a problem hiding this comment.
These were added for telemetry --- I think likely just a mistaken belief that they could be different, since we returned it on Plan and run 2 different plans for the different transitions.
Member
Author
There was a problem hiding this comment.
Ah. This lives in the provisionerd proto. So I won't touch this in this PR
Issue made to track: #21206
Emyrk
commented
Dec 10, 2025
Comment on lines
-347
to
-360
| 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)) | ||
| } | ||
| } | ||
|
|
Member
Author
There was a problem hiding this comment.
Moved to the init step, where modules are downloaded.
Comment on lines
-421
to
-455
| // 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
-605
to
-609
| // `terraform show` & `terraform graph` | ||
| state, err := e.stateResources(ctx, killCtx) | ||
| if err != nil { | ||
| return nil, err | ||
| } |
| // TODO: These are defined as different, but can they be? | ||
| // Terraform downloads modules regardless of `count`, so this should be the same | ||
| StartModules: initResp.Modules, | ||
| StopModules: initResp.Modules, |
Member
Author
There was a problem hiding this comment.
Ah. This lives in the provisionerd proto. So I won't touch this in this PR
Issue made to track: #21206
spikecurtis
approved these changes
Dec 11, 2025
6b8d466 to
f06c861
Compare
4ba2b34 to
9f2ddb8
Compare
'parse', 'init', 'plan', 'apply', 'graph' More granular provisioner options.
Quotas are not used in template imports, only builds
f06c861 to
c905ed3
Compare
9f2ddb8 to
077c0a7
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Provisioner steps broken into smaller granular actions.
message Request { oneof type { Config config = 1; ParseRequest parse = 2; + InitRequest init = 3; PlanRequest plan = 4; ApplyRequest apply = 5; + GraphRequest graph = 6; CancelRequest cancel = 7; } }Changes
ExtractArchivemoved toinitrequest (was inconfigure)tfstatemoved toplan(was inconfigure)GraphCompleteWhy do this?
To remove the need to run
terraform graphtwice. This allows the provisionersdk to run aplanorapplywithout agraph. Runninggraphas it's own step at the end of a build/import.