Skip to content

fix(provisioner): handle multiple agents, apps, scripts and envs #13741

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

Merged
merged 11 commits into from
Jul 3, 2024
Merged

fix(provisioner): handle multiple agents, apps, scripts and envs #13741

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
41f2de0
fix: multiple agents, independent apps
mtojek Jul 1, 2024
8a1f50f
WIP
mtojek Jul 2, 2024
38a0251
scripts support
mtojek Jul 2, 2024
157d13b
WIP
mtojek Jul 2, 2024
f5484ec
Fix: script tests
mtojek Jul 3, 2024
82bbc2a
generate 1.8.4
mtojek Jul 3, 2024
0fc48c4
fmt
mtojek Jul 3, 2024
420c352
fix
mtojek Jul 3, 2024
e3be047
scripts
mtojek Jul 3, 2024
3aea666
Envs and scripts
mtojek Jul 3, 2024
1d2f797
fix: naming
mtojek Jul 3, 2024
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
Prev Previous commit
Next Next commit
WIP
  • Loading branch information
@mtojek
mtojek committed Jul 2, 2024
commit 8a1f50f7df81c4f9de29f7cea94b84bb0c34a62e
28 changes: 27 additions & 1 deletion provisioner/terraform/resources.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -427,9 +427,11 @@ func ConvertState(modules []*tfjson.StateModule, rawGraph string) (*State, error
for _, agents := range resourceAgents {
for _, agent := range agents {
// Find agents with the matching ID and associate them!
if agent.Id != attrs.AgentID {

if !dependsOnAgent(graph, agent, attrs.AgentID, resource) {
continue
}

agent.Apps = append(agent.Apps, &proto.App{
Slug: attrs.Slug,
DisplayName: attrs.DisplayName,
Expand Down Expand Up @@ -748,6 +750,30 @@ func convertAddressToLabel(address string) string {
return cut
}

func dependsOnAgent(graph *gographviz.Graph, agent *proto.Agent, appAgentID string, resource *tfjson.StateResource) bool {
// Plan: we need to find if there is edge between the agent and the app.
if agent.Id == "" && appAgentID == "" {
appNodeSuffix := fmt.Sprintf(`] coder_app.%s (expand)"`, resource.Name)
agentNodeSuffix := fmt.Sprintf(`] coder_agent.%s (expand)"`, agent.Name)

// Traverse the graph to check if the coder_app depends on coder_agent.
for _, dst := range graph.Edges.SrcToDsts {
for _, edges := range dst {
for _, edge := range edges {
if strings.HasSuffix(edge.Src, appNodeSuffix) &&
strings.HasSuffix(edge.Dst, agentNodeSuffix) {
return true
}
}
}
}
return false
}
Copy link
Member

Choose a reason for hiding this comment

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

My main concern is that we are traversing this graph multiple times in a tight loop.
Could we instead traverse the graph once, build a shallow map of agent -> resource, and use that to determine the agent-resource dependency?

Copy link
Member Author

Choose a reason for hiding this comment

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

The main reason I'm hesitant to do this, it would be another cache/state used only to prevent this problem. Considering we have 1 agent on average, 2 in total, it may not introduce a heavy performance impact.

Anyway, if you think this is a must-do, I will improve the loop 👍 I admit, I focused mostly on fixing the problem.

Copy link
Member

Choose a reason for hiding this comment

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

That's fair. Let's make it work correctly first, and then see about benchmarking / speeding it up.


// Provision: agent ID and app ID are present
return agent.Id == appAgentID
}

type graphResource struct {
Label string
Depth uint
Expand Down
2 changes: 1 addition & 1 deletion provisioner/terraform/testdata/version.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1 +1 @@
1.8.4
1.8.5