- Notifications
You must be signed in to change notification settings - Fork1.1k
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.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Changes from1 commit
41f2de08a1f50f38a0251157d13bf5484ec82bbc2a0fc48c4420c352e3be0473aea6661d2f797File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading.Please reload this page.
Jump to
Uh oh!
There was an error while loading.Please reload this page.
Diff view
Diff view
- Loading branch information
Uh oh!
There was an error while loading.Please reload this page.
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 !dependsOnAgent(graph, agent, attrs.AgentID, resource) { | ||
| continue | ||
| } | ||
| agent.Apps = append(agent.Apps, &proto.App{ | ||
| Slug: attrs.Slug, | ||
| DisplayName: attrs.DisplayName, | ||
| @@ -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 | ||
| } | ||
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. Member There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| 1.8.5 |