Skip to content

feat: Add connection_timeout and troubleshooting_url to agent #4937

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 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
Nov 9, 2022
70 changes: 48 additions & 22 deletions cli/cliui/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,11 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error {
if err != nil {
return xerrors.Errorf("fetch: %w", err)
}

if agent.Status == codersdk.WorkspaceAgentConnected {
return nil
}
if agent.Status == codersdk.WorkspaceAgentDisconnected {
opts.WarnInterval = 0
}

spin := spinner.New(spinner.CharSets[78], 100*time.Millisecond, spinner.WithColor("fgHiGreen"))
spin.Writer = writer
spin.ForceOutput = true
Expand All @@ -65,43 +64,70 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error {
os.Exit(1)
}()

ticker := time.NewTicker(opts.FetchInterval)
defer ticker.Stop()
timer := time.NewTimer(opts.WarnInterval)
defer timer.Stop()
go func() {
select {
case <-ctx.Done():
return
case <-timer.C:
}
warningShown := false
warnAfter := time.NewTimer(opts.WarnInterval)
defer warnAfter.Stop()
showWarning := func() {
warnAfter.Stop()

resourceMutex.Lock()
defer resourceMutex.Unlock()
message := "Don't panic, your workspace is booting up!"
if agent.Status == codersdk.WorkspaceAgentDisconnected {
message = "The workspace agent lost connection! Wait for it to reconnect or restart your workspace."
if warningShown {
return
}
warningShown = true

message := waitingMessage(agent)
// This saves the cursor position, then defers clearing from the cursor
// position to the end of the screen.
_, _ = fmt.Fprintf(writer, "\033[s\r\033[2K%s\n\n", Styles.Paragraph.Render(Styles.Prompt.String()+message))
defer fmt.Fprintf(writer, "\033[u\033[J")
}
go func() {
select {
case <-ctx.Done():
case <-warnAfter.C:
showWarning()
}
}()

fetchInterval := time.NewTicker(opts.FetchInterval)
defer fetchInterval.Stop()
for {
select {
case <-ctx.Done():
return ctx.Err()
case <-ticker.C:
case <-fetchInterval.C:
}
resourceMutex.Lock()
agent, err = opts.Fetch(ctx)
if err != nil {
return xerrors.Errorf("fetch: %w", err)
}
if agent.Status != codersdk.WorkspaceAgentConnected {
resourceMutex.Unlock()
continue
return xerrors.Errorf("fetch: %w", err)
}
resourceMutex.Unlock()
return nil
switch agent.Status {
case codersdk.WorkspaceAgentConnected:
return nil
case codersdk.WorkspaceAgentTimeout, codersdk.WorkspaceAgentDisconnected:
showWarning()
}
}
}

func waitingMessage(agent codersdk.WorkspaceAgent) string {
var m string
switch agent.Status {
case codersdk.WorkspaceAgentTimeout:
m = "The workspace agent is having trouble connecting."
case codersdk.WorkspaceAgentDisconnected:
m = "The workspace agent lost connection!"
default:
// Not a failure state, no troubleshooting necessary.
return "Don't panic, your workspace is booting up!"
}
if agent.TroubleshootingURL != "" {
return fmt.Sprintf("%s See troubleshooting instructions at: %s", m, agent.TroubleshootingURL)
}
return fmt.Sprintf("%s Wait for it to (re)connect or restart your workspace.", m)
}
7 changes: 7 additions & 0 deletions cli/cliui/resources.go
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,13 @@ func renderAgentStatus(agent codersdk.WorkspaceAgent) string {
since := database.Now().Sub(*agent.DisconnectedAt)
return Styles.Error.Render("⦾ disconnected") + " " +
Styles.Placeholder.Render("["+strconv.Itoa(int(since.Seconds()))+"s]")
case codersdk.WorkspaceAgentTimeout:
since := database.Now().Sub(agent.CreatedAt)
return fmt.Sprintf(
"%s %s",
Styles.Warn.Render("⦾ timeout"),
Styles.Placeholder.Render("["+strconv.Itoa(int(since.Seconds()))+"s]"),
)
case codersdk.WorkspaceAgentConnected:
return Styles.Keyword.Render("⦿ connected")
default:
Expand Down
2 changes: 1 addition & 1 deletion cli/speedtest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func TestSpeedtest(t *testing.T) {
if testing.Short() {
t.Skip("This test takes a minimum of 5ms per a hardcoded value in Tailscale!")
}
client, workspace, agentToken := setupWorkspaceForAgent(t)
client, workspace, agentToken := setupWorkspaceForAgent(t, nil)
agentClient := codersdk.New(client.URL)
agentClient.SessionToken = agentToken
agentCloser := agent.New(agent.Options{
Expand Down
46 changes: 40 additions & 6 deletions cli/ssh_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,13 @@ import (
"github.com/coder/coder/testutil"
)

func setupWorkspaceForAgent(t *testing.T) (*codersdk.Client, codersdk.Workspace, string) {
func setupWorkspaceForAgent(t *testing.T, mutate func([]*proto.Agent) []*proto.Agent) (*codersdk.Client, codersdk.Workspace, string) {
Copy link
Member

Choose a reason for hiding this comment

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

Maybe mutate ...func? You would get rid of the nil in setupWorkspaceForAgent(t, nil).

Copy link
Member Author

Choose a reason for hiding this comment

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

Clean suggestion, but I think we can keep this explicit for now, makes it slightly more convenient to add new arguments, if needed and I don't see a use-case for multiple mutation functions.

Copy link
Member

Choose a reason for hiding this comment

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

In general, the nil is considered to be an anti-pattern as it has an ambiguous meaning. This is the main reason why I raised this comment, so feel free to leave it as is.

t.Helper()
if mutate == nil {
mutate = func(a []*proto.Agent) []*proto.Agent {
return a
}
}
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerDaemon: true})
user := coderdtest.CreateFirstUser(t, client)
agentToken := uuid.NewString()
Expand All @@ -45,12 +50,12 @@ func setupWorkspaceForAgent(t *testing.T) (*codersdk.Client, codersdk.Workspace,
Resources: []*proto.Resource{{
Name: "dev",
Type: "google_compute_instance",
Agents: []*proto.Agent{{
Agents: mutate([]*proto.Agent{{
Id: uuid.NewString(),
Auth: &proto.Agent_Token{
Token: agentToken,
},
}},
}}),
}},
},
},
Expand All @@ -69,7 +74,7 @@ func TestSSH(t *testing.T) {
t.Run("ImmediateExit", func(t *testing.T) {
t.Parallel()

client, workspace, agentToken := setupWorkspaceForAgent(t)
client, workspace, agentToken := setupWorkspaceForAgent(t, nil)
cmd, root := clitest.New(t, "ssh", workspace.Name)
clitest.SetupConfig(t, client, root)
pty := ptytest.New(t)
Expand Down Expand Up @@ -100,9 +105,38 @@ func TestSSH(t *testing.T) {
pty.WriteLine("exit")
<-cmdDone
})
t.Run("ShowTroubleshootingURLAfterTimeout", func(t *testing.T) {
t.Parallel()

wantURL := "https://example.com/troubleshoot"
client, workspace, _ := setupWorkspaceForAgent(t, func(a []*proto.Agent) []*proto.Agent {
// Unfortunately, one second is the lowest
// we can go because 0 disables the feature.
a[0].ConnectionTimeoutSeconds = 1
a[0].TroubleshootingUrl = wantURL
return a
})
cmd, root := clitest.New(t, "ssh", workspace.Name)
clitest.SetupConfig(t, client, root)
pty := ptytest.New(t)
cmd.SetIn(pty.Input())
cmd.SetErr(pty.Output())
cmd.SetOut(pty.Output())

ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong)
defer cancel()

cmdDone := tGo(t, func() {
err := cmd.ExecuteContext(ctx)
assert.ErrorIs(t, err, context.Canceled)
})
pty.ExpectMatch(wantURL)
cancel()
<-cmdDone
})
t.Run("Stdio", func(t *testing.T) {
t.Parallel()
client, workspace, agentToken := setupWorkspaceForAgent(t)
client, workspace, agentToken := setupWorkspaceForAgent(t, nil)
_, _ = tGoContext(t, func(ctx context.Context) {
// Run this async so the SSH command has to wait for
// the build and agent to connect!
Expand Down Expand Up @@ -171,7 +205,7 @@ func TestSSH(t *testing.T) {

t.Parallel()

client, workspace, agentToken := setupWorkspaceForAgent(t)
client, workspace, agentToken := setupWorkspaceForAgent(t, nil)

agentClient := codersdk.New(client.URL)
agentClient.SessionToken = agentToken
Expand Down
31 changes: 17 additions & 14 deletions coderd/database/databasefake/databasefake.go
Original file line number Diff line number Diff line change
Expand Up @@ -154,6 +154,7 @@ func (q *fakeQuerier) AcquireProvisionerJob(_ context.Context, arg database.Acqu
}
return database.ProvisionerJob{}, sql.ErrNoRows
}

func (*fakeQuerier) DeleteOldAgentStats(_ context.Context) error {
// no-op
return nil
Expand Down Expand Up @@ -2362,20 +2363,22 @@ func (q *fakeQuerier) InsertWorkspaceAgent(_ context.Context, arg database.Inser
defer q.mutex.Unlock()

agent := database.WorkspaceAgent{
ID: arg.ID,
CreatedAt: arg.CreatedAt,
UpdatedAt: arg.UpdatedAt,
ResourceID: arg.ResourceID,
AuthToken: arg.AuthToken,
AuthInstanceID: arg.AuthInstanceID,
EnvironmentVariables: arg.EnvironmentVariables,
Name: arg.Name,
Architecture: arg.Architecture,
OperatingSystem: arg.OperatingSystem,
Directory: arg.Directory,
StartupScript: arg.StartupScript,
InstanceMetadata: arg.InstanceMetadata,
ResourceMetadata: arg.ResourceMetadata,
ID: arg.ID,
CreatedAt: arg.CreatedAt,
UpdatedAt: arg.UpdatedAt,
ResourceID: arg.ResourceID,
AuthToken: arg.AuthToken,
AuthInstanceID: arg.AuthInstanceID,
EnvironmentVariables: arg.EnvironmentVariables,
Name: arg.Name,
Architecture: arg.Architecture,
OperatingSystem: arg.OperatingSystem,
Directory: arg.Directory,
StartupScript: arg.StartupScript,
InstanceMetadata: arg.InstanceMetadata,
ResourceMetadata: arg.ResourceMetadata,
ConnectionTimeoutSeconds: arg.ConnectionTimeoutSeconds,
TroubleshootingURL: arg.TroubleshootingURL,
}

q.provisionerJobAgents = append(q.provisionerJobAgents, agent)
Expand Down
8 changes: 7 additions & 1 deletion coderd/database/dump.sql

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
BEGIN;

ALTER TABLE workspace_agents
DROP COLUMN connection_timeout_seconds;

ALTER TABLE workspace_agents
DROP COLUMN troubleshooting_url;

COMMIT;
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
BEGIN;

ALTER TABLE workspace_agents
ADD COLUMN connection_timeout_seconds integer NOT NULL DEFAULT 0;

COMMENT ON COLUMN workspace_agents.connection_timeout_seconds IS 'Connection timeout in seconds, 0 means disabled.';

ALTER TABLE workspace_agents
ADD COLUMN troubleshooting_url text NOT NULL DEFAULT '';

COMMENT ON COLUMN workspace_agents.troubleshooting_url IS 'URL for troubleshooting the agent.';

COMMIT;
4 changes: 4 additions & 0 deletions coderd/database/models.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading