Skip to content

incusd/instance_exec: Relax connection timeout #2421

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

Jump to bottom
Merged
merged 1 commit into from
Aug 22, 2025
Merged

incusd/instance_exec: Relax connection timeout #2421

merged 1 commit into from
Aug 22, 2025

Conversation

stgraber
Copy link
Member

Non-interactive exec sessions need all 4 websockets connected within the timeout. Those are typically connected sequentially, so the 5s timeout may be tight on very high latency links.

@stgraber
incusd/instance_exec: Relax connection timeout
1108009 
Non-interactive exec sessions need all 4 websockets connected within the
timeout. Those are typically connected sequentially, so the 5s timeout
may be tight on very high latency links.

Signed-off-by: Stéphane Graber <stgraber@stgraber.org>
@stgraber stgraber mentioned this pull request Aug 22, 2025
Closed
@Gurpartap
Copy link

Is this timeout in incus-agent related?

incus/cmd/incus-agent/exec.go

Lines 200 to 208 in 5e171b4

// As this function only gets called when the exec request has WaitForWS enabled, we expect the client to
// connect to all of the required websockets within a short period of time and we won't proceed until then.
logger.Debug("Waiting for exec websockets to connect")
select {
case <-s.requiredConnectedCtx.Done():
break
case <-time.After(time.Second * 5):
return errors.New("Timed out waiting for websockets to connect")
}

@hallyn hallyn merged commit ae0e7ec into lxc:main Aug 22, 2025
34 of 38 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hallyn hallyn hallyn approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@stgraber @Gurpartap @hallyn