Skip to content

Commit 31d8e62

Browse files
EmyrkEmyrk
committed
fixups
1 parent e880894 commit 31d8e62

File tree

5 files changed

+40
-20
lines changed

5 files changed

+40
-20
lines changed

‎coderd/provisionerdserver/provisionerdserver.go

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1323,7 +1323,8 @@ func (s *server) prepareForNotifyWorkspaceManualBuildFailed(ctx context.Context,
13231323

13241324
func (s *server) UploadFile(stream proto.DRPCProvisionerDaemon_UploadFileStream) error {
13251325
var file *sdkproto.DataBuilder
1326-
defer stream.Close()
1326+
// Always terminate the stream with an empty response.
1327+
defer stream.SendAndClose(&proto.Empty{})
13271328

13281329
UploadFileStream:
13291330
for {

‎provisioner/terraform/executor.go

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -258,13 +258,15 @@ func getStateFilePath(workdir string) string {
258258
}
259259

260260
// revive:disable-next-line:flag-parameter
261-
func (e *executor) plan(ctx, killCtx context.Context, env, vars []string, logr logSink, metadata *proto.Metadata) (*proto.PlanComplete, error) {
261+
func (e *executor) plan(ctx, killCtx context.Context, env, vars []string, logr logSink, req *proto.PlanRequest) (*proto.PlanComplete, error) {
262262
ctx, span := e.server.startTrace(ctx, tracing.FuncName())
263263
defer span.End()
264264

265265
e.mut.Lock()
266266
defer e.mut.Unlock()
267267

268+
metadata := req.Metadata
269+
268270
planfilePath := getPlanFilePath(e.workdir)
269271
args := []string{
270272
"plan",
@@ -312,10 +314,14 @@ func (e *executor) plan(ctx, killCtx context.Context, env, vars []string, logr l
312314

313315
graphTimings.ingest(createGraphTimingsEvent(timingGraphComplete))
314316

315-
moduleFiles, err := GetModulesArchive(os.DirFS(e.workdir))
316-
if err != nil {
317-
// TODO: we probably want to persist this error or make it louder eventually
318-
e.logger.Warn(ctx, "failed to archive terraform modules", slog.Error(err))
317+
var moduleFiles []byte
318+
// Skipping modules archiving is useful if the caller does not need it, eg during a workspace build.
319+
if !req.OmitModuleFiles {
320+
moduleFiles, err = GetModulesArchive(os.DirFS(e.workdir))
321+
if err != nil {
322+
// TODO: we probably want to persist this error or make it louder eventually
323+
e.logger.Warn(ctx, "failed to archive terraform modules", slog.Error(err))
324+
}
319325
}
320326

321327
// When a prebuild claim attempt is made, log a warning if a resource is due to be replaced, since this will obviate
@@ -355,11 +361,6 @@ func (e *executor) plan(ctx, killCtx context.Context, env, vars []string, logr l
355361
ModuleFiles: moduleFiles,
356362
}
357363

358-
//if protobuf.Size(msg) > drpcsdk.MaxMessageSize {
359-
// e.logger.Warn(ctx, "cannot persist terraform modules, message payload too big", slog.F("archive_size", len(msg.ModuleFiles)))
360-
// msg.ModuleFiles = nil
361-
//}
362-
363364
return msg, nil
364365
}
365366

‎provisioner/terraform/provision.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -163,7 +163,7 @@ func (s *server) Plan(
163163
return provisionersdk.PlanErrorf("plan vars: %s", err)
164164
}
165165

166-
resp, err := e.plan(ctx, killCtx, env, vars, sess, request.Metadata)
166+
resp, err := e.plan(ctx, killCtx, env, vars, sess, request)
167167
if err != nil {
168168
return provisionersdk.PlanErrorf("%s", err.Error())
169169
}

‎provisionerd/provisionerd.go

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -556,7 +556,14 @@ func (p *Server) UploadModuleFiles(ctx context.Context, moduleFiles []byte) erro
556556
}
557557
}
558558

559-
return &proto.Empty{}, nil
559+
resp, err := stream.CloseAndRecv()
560+
if err != nil {
561+
if retryable(err) { // Do not retry
562+
return nil, xerrors.Errorf("close stream: %s", err.Error())
563+
}
564+
return nil, xerrors.Errorf("close stream: %w", err)
565+
}
566+
return resp, nil
560567
})
561568
if err != nil {
562569
return xerrors.Errorf("upload module files: %w", err)
@@ -566,13 +573,14 @@ func (p *Server) UploadModuleFiles(ctx context.Context, moduleFiles []byte) erro
566573
}
567574

568575
func (p *Server) CompleteJob(ctx context.Context, in *proto.CompletedJob) error {
576+
// If the moduleFiles exceed the max message size, we need to upload them separately.
569577
if ti, ok := in.Type.(*proto.CompletedJob_TemplateImport_); ok {
570578
messageSize := protobuf.Size(in)
571579
if messageSize > drpcsdk.MaxMessageSize &&
572580
messageSize-len(ti.TemplateImport.ModuleFiles) < drpcsdk.MaxMessageSize {
573-
581+
// Hashing the module files to reference them in the CompletedJob message.
574582
moduleFilesHash := sha256.Sum256(ti.TemplateImport.ModuleFiles)
575-
// Split the module files from the message if it exceeds the max size.
583+
576584
moduleFiles := ti.TemplateImport.ModuleFiles
577585
ti.TemplateImport.ModuleFiles = nil // Clear the files in the final message
578586
ti.TemplateImport.ModuleFilesHash = moduleFilesHash[:]

‎provisionerd/runner/runner.go

Lines changed: 15 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -555,7 +555,7 @@ func (r *Runner) runTemplateImport(ctx context.Context) (*proto.CompletedJob, *p
555555
CoderUrl: r.job.GetTemplateImport().Metadata.CoderUrl,
556556
WorkspaceOwnerGroups: r.job.GetTemplateImport().Metadata.WorkspaceOwnerGroups,
557557
WorkspaceTransition: sdkproto.WorkspaceTransition_START,
558-
})
558+
}, false)
559559
if err != nil {
560560
return nil, r.failedJobf("template import provision for start: %s", err)
561561
}
@@ -571,7 +571,8 @@ func (r *Runner) runTemplateImport(ctx context.Context) (*proto.CompletedJob, *p
571571
CoderUrl: r.job.GetTemplateImport().Metadata.CoderUrl,
572572
WorkspaceOwnerGroups: r.job.GetTemplateImport().Metadata.WorkspaceOwnerGroups,
573573
WorkspaceTransition: sdkproto.WorkspaceTransition_STOP,
574-
})
574+
}, true, // Modules downloded on the start provision
575+
)
575576
if err != nil {
576577
return nil, r.failedJobf("template import provision for stop: %s", err)
577578
}
@@ -597,7 +598,8 @@ func (r *Runner) runTemplateImport(ctx context.Context) (*proto.CompletedJob, *p
597598
StopModules: stopProvision.Modules,
598599
Presets: startProvision.Presets,
599600
Plan: startProvision.Plan,
600-
ModuleFiles: startProvision.ModuleFiles,
601+
// ModuleFiles are not on the stopProvision. So grab from the startProvision.
602+
ModuleFiles: startProvision.ModuleFiles,
601603
},
602604
},
603605
}, nil
@@ -666,8 +668,8 @@ type templateImportProvision struct {
666668
// Performs a dry-run provision when importing a template.
667669
// This is used to detect resources that would be provisioned for a workspace in various states.
668670
// It doesn't define values for rich parameters as they're unknown during template import.
669-
func (r *Runner) runTemplateImportProvision(ctx context.Context, variableValues []*sdkproto.VariableValue, metadata *sdkproto.Metadata) (*templateImportProvision, error) {
670-
return r.runTemplateImportProvisionWithRichParameters(ctx, variableValues, nil, metadata)
671+
func (r *Runner) runTemplateImportProvision(ctx context.Context, variableValues []*sdkproto.VariableValue, metadata *sdkproto.Metadata, omitModules bool) (*templateImportProvision, error) {
672+
return r.runTemplateImportProvisionWithRichParameters(ctx, variableValues, nil, metadata, omitModules)
671673
}
672674

673675
// Performs a dry-run provision with provided rich parameters.
@@ -677,6 +679,7 @@ func (r *Runner) runTemplateImportProvisionWithRichParameters(
677679
variableValues []*sdkproto.VariableValue,
678680
richParameterValues []*sdkproto.RichParameterValue,
679681
metadata *sdkproto.Metadata,
682+
omitModules bool,
680683
) (*templateImportProvision, error) {
681684
ctx, span := r.startTrace(ctx, tracing.FuncName())
682685
defer span.End()
@@ -696,6 +699,7 @@ func (r *Runner) runTemplateImportProvisionWithRichParameters(
696699
// Template import has no previous values
697700
PreviousParameterValues: make([]*sdkproto.RichParameterValue, 0),
698701
VariableValues: variableValues,
702+
OmitModuleFiles: omitModules,
699703
}}})
700704
if err != nil {
701705
return nil, xerrors.Errorf("start provision: %w", err)
@@ -849,6 +853,7 @@ func (r *Runner) runTemplateDryRun(ctx context.Context) (*proto.CompletedJob, *p
849853
r.job.GetTemplateDryRun().GetVariableValues(),
850854
r.job.GetTemplateDryRun().GetRichParameterValues(),
851855
metadata,
856+
false,
852857
)
853858
if err != nil {
854859
return nil, r.failedJobf("run dry-run provision job: %s", err)
@@ -911,6 +916,10 @@ func (r *Runner) buildWorkspace(ctx context.Context, stage string, req *sdkproto
911916
Output: msgType.Log.Output,
912917
Stage: stage,
913918
})
919+
case *sdkproto.Response_DataUpload:
920+
continue // Only for template imports
921+
case *sdkproto.Response_ChunkPiece:
922+
continue // Only for template imports
914923
default:
915924
// Stop looping!
916925
return msg, nil
@@ -1003,6 +1012,7 @@ func (r *Runner) runWorkspaceBuild(ctx context.Context) (*proto.CompletedJob, *p
10031012
resp, failed := r.buildWorkspace(ctx, "Planning infrastructure", &sdkproto.Request{
10041013
Type: &sdkproto.Request_Plan{
10051014
Plan: &sdkproto.PlanRequest{
1015+
OmitModuleFiles: true, // Only useful for template imports
10061016
Metadata: r.job.GetWorkspaceBuild().Metadata,
10071017
RichParameterValues: r.job.GetWorkspaceBuild().RichParameterValues,
10081018
PreviousParameterValues: r.job.GetWorkspaceBuild().PreviousParameterValues,

0 commit comments

Comments
 (0)