Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitb29e8fa

Browse files
committed
chore: minor touch-ups
Signed-off-by: Danny Kopping <dannykopping@gmail.com>
1 parent41e5e0c commitb29e8fa

File tree

3 files changed

+44
-28
lines changed

3 files changed

+44
-28
lines changed

‎coderd/provisionerdserver/provisionerdserver.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1841,7 +1841,7 @@ func (s *server) notifyWorkspaceDeleted(ctx context.Context, workspace database.
18411841
}
18421842

18431843
func (s*server)notifyPrebuiltWorkspaceResourceReplacement(ctx context.Context,workspace database.Workspace,build database.WorkspaceBuild,claimantID uuid.UUID,replacements []*sdkproto.ResourceReplacement) {
1844-
ifclaimantID==uuid.Nil {
1844+
ifclaimantID==uuid.Nil||len(replacements)==0{
18451845
// This is not a prebuild claim.
18461846
return
18471847
}
@@ -1878,8 +1878,9 @@ func (s *server) notifyPrebuiltWorkspaceResourceReplacement(ctx context.Context,
18781878
// Associate this notification with all the related entities.
18791879
workspace.ID,workspace.OwnerID,workspace.TemplateID,workspace.OrganizationID,
18801880
);err!=nil {
1881-
s.Logger.Warn(ctx,"failed to notify of prebuilt workspace resource replacement",slog.Error(err))
1882-
break
1881+
s.Logger.Warn(ctx,"failed to notify of prebuilt workspace resource replacement",slog.Error(err),
1882+
slog.F("user_id",templateAdmin.ID.String()),slog.F("user_email",templateAdmin.Email))
1883+
continue
18831884
}
18841885
}
18851886
}

‎provisioner/terraform/executor.go

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -300,10 +300,14 @@ func (e *executor) plan(ctx, killCtx context.Context, env, vars []string, logr l
300300
graphTimings:=newTimingAggregator(database.ProvisionerJobTimingStageGraph)
301301
graphTimings.ingest(createGraphTimingsEvent(timingGraphStart))
302302

303-
state,plan,replacements,err:=e.planResources(ctx,killCtx,planfilePath)
303+
state,plan,err:=e.planResources(ctx,killCtx,planfilePath)
304304
iferr!=nil {
305305
graphTimings.ingest(createGraphTimingsEvent(timingGraphErrored))
306-
returnnil,err
306+
returnnil,xerrors.Errorf("plan resources: %w",err)
307+
}
308+
planJSON,err:=json.Marshal(plan)
309+
iferr!=nil {
310+
returnnil,xerrors.Errorf("marshal plan: %w",err)
307311
}
308312

309313
graphTimings.ingest(createGraphTimingsEvent(timingGraphComplete))
@@ -312,17 +316,22 @@ func (e *executor) plan(ctx, killCtx context.Context, env, vars []string, logr l
312316
// the point of prebuilding if the expensive resource is replaced once claimed!
313317
var (
314318
isPrebuildClaimAttempt=!destroy&&metadata.GetPrebuildClaimForUserId()!=""
315-
reps []*proto.ResourceReplacement
319+
resReps []*proto.ResourceReplacement
316320
)
317-
ifcount:=len(replacements);count>0&&isPrebuildClaimAttempt {
318-
// TODO(dannyk): we should log drift always (not just during prebuild claim attempts); we're validating that this output
319-
// will not be overwhelming for end-users, but it'll certainly be super valuable for template admins
320-
// to diagnose this resource replacement issue, at least.
321-
e.logDrift(ctx,killCtx,planfilePath,logr)
322-
323-
reps=make([]*proto.ResourceReplacement,0,len(replacements))
324-
forn,p:=rangereplacements {
325-
reps=append(reps,&proto.ResourceReplacement{
321+
ifrepsFromPlan:=findResourceReplacements(plan);len(repsFromPlan)>0 {
322+
ifisPrebuildClaimAttempt {
323+
// TODO(dannyk): we should log drift always (not just during prebuild claim attempts); we're validating that this output
324+
// will not be overwhelming for end-users, but it'll certainly be super valuable for template admins
325+
// to diagnose this resource replacement issue, at least.
326+
// Once prebuilds moves out of beta, consider deleting this condition.
327+
328+
// Lock held before calling (see top of method).
329+
e.logDrift(ctx,killCtx,planfilePath,logr)
330+
}
331+
332+
resReps=make([]*proto.ResourceReplacement,0,len(repsFromPlan))
333+
forn,p:=rangerepsFromPlan {
334+
resReps=append(resReps,&proto.ResourceReplacement{
326335
Resource:n,
327336
Paths:p,
328337
})
@@ -335,8 +344,8 @@ func (e *executor) plan(ctx, killCtx context.Context, env, vars []string, logr l
335344
ExternalAuthProviders:state.ExternalAuthProviders,
336345
Timings:append(e.timings.aggregate(),graphTimings.aggregate()...),
337346
Presets:state.Presets,
338-
Plan:plan,
339-
ResourceReplacements:reps,
347+
Plan:planJSON,
348+
ResourceReplacements:resReps,
340349
},nil
341350
}
342351

@@ -358,18 +367,18 @@ func onlyDataResources(sm tfjson.StateModule) tfjson.StateModule {
358367
}
359368

360369
// planResources must only be called while the lock is held.
361-
func (e*executor)planResources(ctx,killCtx context.Context,planfilePathstring) (*State,json.RawMessage,resourceReplacements,error) {
370+
func (e*executor)planResources(ctx,killCtx context.Context,planfilePathstring) (*State,*tfjson.Plan,error) {
362371
ctx,span:=e.server.startTrace(ctx,tracing.FuncName())
363372
deferspan.End()
364373

365374
plan,err:=e.parsePlan(ctx,killCtx,planfilePath)
366375
iferr!=nil {
367-
returnnil,nil,nil,xerrors.Errorf("show terraform plan file: %w",err)
376+
returnnil,nil,xerrors.Errorf("show terraform plan file: %w",err)
368377
}
369378

370379
rawGraph,err:=e.graph(ctx,killCtx)
371380
iferr!=nil {
372-
returnnil,nil,nil,xerrors.Errorf("graph: %w",err)
381+
returnnil,nil,xerrors.Errorf("graph: %w",err)
373382
}
374383
modules:= []*tfjson.StateModule{}
375384
ifplan.PriorState!=nil {
@@ -387,15 +396,10 @@ func (e *executor) planResources(ctx, killCtx context.Context, planfilePath stri
387396

388397
state,err:=ConvertState(ctx,modules,rawGraph,e.server.logger)
389398
iferr!=nil {
390-
returnnil,nil,nil,err
391-
}
392-
393-
planJSON,err:=json.Marshal(plan)
394-
iferr!=nil {
395-
returnnil,nil,nil,err
399+
returnnil,nil,err
396400
}
397401

398-
returnstate,planJSON,findResourceReplacements(plan),nil
402+
returnstate,plan,nil
399403
}
400404

401405
// parsePlan must only be called while the lock is held.

‎provisioner/terraform/resource_replacements.go

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,18 @@ func findResourceReplacements(plan *tfjson.Plan) resourceReplacements {
4242
continue
4343
}
4444

45-
// Replacing our resources, no problem!
45+
// Replacing our resources: could be a problem - but we ignore since they're "virtual" resources. If any of these
46+
// resources' attributes are referenced by non-coder resources, those will show up as transitive changes there.
47+
// i.e. if the coder_agent.id attribute is used in docker_container.env
48+
//
49+
// Replacing our resources is not strictly a problem in and of itself.
50+
//
51+
// NOTE:
52+
// We may need to special-case coder_agent in the future. Currently, coder_agent is replaced on every build
53+
// because it only supports Create but not Update: https://github.com/coder/terraform-provider-coder/blob/5648efb/provider/agent.go#L28
54+
// When we can modify an agent's attributes, some of which may be immutable (like "arch") and some may not (like "env"),
55+
// then we'll have to handle this specifically.
56+
// This will only become relevant once we support multiple agents: https://github.com/coder/coder/issues/17388
4657
ifstrings.Index(ch.Type,"coder_")==0 {
4758
continue
4859
}

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp