-
Notifications
You must be signed in to change notification settings - Fork 901
feat: Add UI for awaiting agent connections #578
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
Changes from 1 commit
7ed283c
048f29a
2152758
e774dd7
172d4fe
82f60db
6bff973
3654022
aef12e1
15cae15
fb9fbb6
c91b3cc
9ad326b
342b03d
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ package cliui | |
import ( | ||
"context" | ||
"fmt" | ||
"sync" | ||
"time" | ||
|
||
"github.com/briandowns/spinner" | ||
|
@@ -19,13 +20,15 @@ type AgentOptions struct { | |
WarnInterval time.Duration | ||
} | ||
|
||
// Agent displays a spinning indicator that waits for a workspace agent to connect. | ||
func Agent(cmd *cobra.Command, opts AgentOptions) error { | ||
if opts.FetchInterval == 0 { | ||
opts.FetchInterval = 500 * time.Millisecond | ||
} | ||
if opts.WarnInterval == 0 { | ||
opts.WarnInterval = 30 * time.Second | ||
} | ||
var resourceMutex sync.Mutex | ||
resource, err := opts.Fetch(cmd.Context()) | ||
if err != nil { | ||
return xerrors.Errorf("fetch: %w", err) | ||
|
@@ -52,6 +55,8 @@ func Agent(cmd *cobra.Command, opts AgentOptions) error { | |
return | ||
case <-timer.C: | ||
} | ||
resourceMutex.Lock() | ||
defer resourceMutex.Unlock() | ||
message := "Don't panic, your workspace is booting up!" | ||
if resource.Agent.Status == codersdk.WorkspaceAgentDisconnected { | ||
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. This will fail We should use atomic or just a mutex to protect that 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. Good point. I didn't even have a test for this, and now I shall add one. |
||
message = "The workspace agent lost connection! Wait for it to reconnect or run: " + Styles.Code.Render("coder workspaces rebuild "+opts.WorkspaceName) | ||
|
@@ -67,13 +72,16 @@ func Agent(cmd *cobra.Command, opts AgentOptions) error { | |
return cmd.Context().Err() | ||
case <-ticker.C: | ||
} | ||
resourceMutex.Lock() | ||
resource, err = opts.Fetch(cmd.Context()) | ||
if err != nil { | ||
return xerrors.Errorf("fetch: %w", err) | ||
} | ||
if resource.Agent.Status != codersdk.WorkspaceAgentConnected { | ||
resourceMutex.Unlock() | ||
continue | ||
} | ||
resourceMutex.Unlock() | ||
return nil | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,53 @@ | ||
package cliui_test | ||
|
||
import ( | ||
"context" | ||
"testing" | ||
"time" | ||
|
||
"github.com/spf13/cobra" | ||
"github.com/stretchr/testify/require" | ||
"go.uber.org/atomic" | ||
|
||
"github.com/coder/coder/cli/cliui" | ||
"github.com/coder/coder/codersdk" | ||
"github.com/coder/coder/pty/ptytest" | ||
) | ||
|
||
func TestAgent(t *testing.T) { | ||
t.Parallel() | ||
var disconnected atomic.Bool | ||
ptty := ptytest.New(t) | ||
cmd := &cobra.Command{ | ||
RunE: func(cmd *cobra.Command, args []string) error { | ||
err := cliui.Agent(cmd, cliui.AgentOptions{ | ||
WorkspaceName: "example", | ||
Fetch: func(ctx context.Context) (codersdk.WorkspaceResource, error) { | ||
resource := codersdk.WorkspaceResource{ | ||
Agent: &codersdk.WorkspaceAgent{ | ||
Status: codersdk.WorkspaceAgentDisconnected, | ||
}, | ||
} | ||
if disconnected.Load() { | ||
resource.Agent.Status = codersdk.WorkspaceAgentConnected | ||
} | ||
return resource, nil | ||
}, | ||
FetchInterval: time.Millisecond, | ||
WarnInterval: 10 * time.Millisecond, | ||
}) | ||
return err | ||
}, | ||
} | ||
cmd.SetOutput(ptty.Output()) | ||
cmd.SetIn(ptty.Input()) | ||
done := make(chan struct{}) | ||
go func() { | ||
defer close(done) | ||
err := cmd.Execute() | ||
require.NoError(t, err) | ||
}() | ||
ptty.ExpectMatch("lost connection") | ||
disconnected.Store(true) | ||
<-done | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we comment what this function is doing? With lack of returns except
error
, it's a bit unclear.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, certainly should.