From 7004c5d4c86f291e7837e259fb7e31e7f092b4a0 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 25 Jan 2023 14:11:25 +0000 Subject: [PATCH 1/9] feat(cli): Add support for `delay_login_until_ready` Ref: #5749 --- cli/cliui/agent.go | 126 +++++++++++--- cli/cliui/agent_test.go | 237 +++++++++++++++++++++++++-- cli/ssh.go | 15 +- cli/testdata/coder_ssh_--help.golden | 4 + 4 files changed, 339 insertions(+), 43 deletions(-) diff --git a/cli/cliui/agent.go b/cli/cliui/agent.go index 023891f678fc3..ce1d3da3ffb98 100644 --- a/cli/cliui/agent.go +++ b/cli/cliui/agent.go @@ -10,16 +10,19 @@ import ( "time" "github.com/briandowns/spinner" + "github.com/muesli/reflow/indent" + "github.com/muesli/reflow/wordwrap" "golang.org/x/xerrors" "github.com/coder/coder/codersdk" ) type AgentOptions struct { - WorkspaceName string - Fetch func(context.Context) (codersdk.WorkspaceAgent, error) - FetchInterval time.Duration - WarnInterval time.Duration + WorkspaceName string + Fetch func(context.Context) (codersdk.WorkspaceAgent, error) + FetchInterval time.Duration + WarnInterval time.Duration + SkipDelayLoginUntilReady bool } // Agent displays a spinning indicator that waits for a workspace agent to connect. @@ -36,14 +39,16 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { return xerrors.Errorf("fetch: %w", err) } - if agent.Status == codersdk.WorkspaceAgentConnected { + // Fast path if the agent is ready (avoid showing connecting prompt). + if agent.Status == codersdk.WorkspaceAgentConnected && + (!agent.DelayLoginUntilReady || opts.SkipDelayLoginUntilReady || agent.LifecycleState == codersdk.WorkspaceAgentLifecycleReady) { return nil } spin := spinner.New(spinner.CharSets[78], 100*time.Millisecond, spinner.WithColor("fgHiGreen")) spin.Writer = writer spin.ForceOutput = true - spin.Suffix = " Waiting for connection from " + Styles.Field.Render(agent.Name) + "..." + spin.Suffix = waitingMessage(agent).Spin spin.Start() defer spin.Stop() @@ -65,7 +70,7 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { os.Exit(1) }() - var waitMessage string + waitMessage := &message{} messageAfter := time.NewTimer(opts.WarnInterval) defer messageAfter.Stop() showMessage := func() { @@ -73,11 +78,11 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { defer resourceMutex.Unlock() m := waitingMessage(agent) - if m == waitMessage { + if m.Prompt == waitMessage.Prompt { return } moveUp := "" - if waitMessage != "" { + if waitMessage.Prompt != "" { // If this is an update, move a line up // to keep it tidy and aligned. moveUp = "\033[1A" @@ -86,20 +91,26 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { // Stop the spinner while we write our message. spin.Stop() + spin.Suffix = waitMessage.Spin // Clear the line and (if necessary) move up a line to write our message. - _, _ = fmt.Fprintf(writer, "\033[2K%s%s\n\n", moveUp, Styles.Paragraph.Render(Styles.Prompt.String()+waitMessage)) + _, _ = fmt.Fprintf(writer, "\033[2K%s\n%s\n", moveUp, waitMessage.Prompt) select { case <-ctx.Done(): default: // Safe to resume operation. - spin.Start() + if spin.Suffix != "" { + spin.Start() + } } } + messageAfterDone := make(chan struct{}) go func() { select { case <-ctx.Done(): + close(messageAfterDone) case <-messageAfter.C: messageAfter.Stop() + close(messageAfterDone) showMessage() } }() @@ -121,6 +132,26 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { resourceMutex.Unlock() switch agent.Status { case codersdk.WorkspaceAgentConnected: + // NOTE(mafredri): Once we have access to the workspace agent's + // startup script logs, we can show them here. + // https://github.com/coder/coder/issues/2957 + if agent.DelayLoginUntilReady && !opts.SkipDelayLoginUntilReady { + switch agent.LifecycleState { + case codersdk.WorkspaceAgentLifecycleCreated, codersdk.WorkspaceAgentLifecycleStarting: + select { + case <-messageAfterDone: + showMessage() + default: + // This state is normal, we don't want + // to show a message prematurely. + } + case codersdk.WorkspaceAgentLifecycleReady: + return nil + default: + showMessage() + } + continue + } return nil case codersdk.WorkspaceAgentTimeout, codersdk.WorkspaceAgentDisconnected: showMessage() @@ -128,19 +159,74 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { } } -func waitingMessage(agent codersdk.WorkspaceAgent) string { - var m string +type message struct { + Spin string + Prompt string + Troubleshoot bool + Error bool +} + +func waitingMessage(agent codersdk.WorkspaceAgent) (m *message) { + m = &message{ + Prompt: "Don't panic, your workspace is booting up!", + Spin: fmt.Sprintf(" Waiting for connection from %s...", Styles.Field.Render(agent.Name)), + } + defer func() { + // We don't want to wrap the troubleshooting URL, so we'll handle word + // wrapping ourselves (vs using lipgloss). + w := wordwrap.NewWriter(Styles.Paragraph.GetWidth() - Styles.Paragraph.GetMarginLeft()*2) + w.Breakpoints = []rune{' ', '\n'} + + _, _ = fmt.Fprint(w, m.Prompt) + if m.Troubleshoot { + if agent.TroubleshootingURL != "" { + _, _ = fmt.Fprintf(w, " See troubleshooting instructions at:\n%s", agent.TroubleshootingURL) + } else { + _, _ = fmt.Fprint(w, " Wait for it to (re)connect or restart your workspace.") + } + } + _, _ = fmt.Fprint(w, "\n") + + if m.Error { + _, _ = fmt.Fprint(w, "\nPress Ctrl+C to exit.\n") + } + + // We want to prefix the prompt with a caret, but we want text on the + // following lines to align with the text on the first line (i.e. added + // spacing). + ind := " " + Styles.Prompt.String() + iw := indent.NewWriter(1, func(w io.Writer) { + _, _ = w.Write([]byte(ind)) + ind = " " // Set indentation to space after initial prompt. + }) + _, _ = fmt.Fprint(iw, w.String()) + m.Prompt = iw.String() + }() + switch agent.Status { case codersdk.WorkspaceAgentTimeout: - m = "The workspace agent is having trouble connecting." + m.Prompt = "The workspace agent is having trouble connecting." case codersdk.WorkspaceAgentDisconnected: - m = "The workspace agent lost connection!" + m.Prompt = "The workspace agent lost connection!" + case codersdk.WorkspaceAgentConnected: + m.Prompt = "Don't panic, your workspace is starting up!" + m.Spin = fmt.Sprintf(" Waiting for %s to finish starting up...", Styles.Field.Render(agent.Name)) + + switch agent.LifecycleState { + case codersdk.WorkspaceAgentLifecycleStartTimeout: + m.Prompt = "The workspace agent is taking longer than expected to start." + case codersdk.WorkspaceAgentLifecycleStartError: + m.Spin = "" + m.Prompt = "The workspace agent ran into a problem during startup." + m.Error = true + default: + // Not a failure state, no troubleshooting necessary. + return m + } 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 m } - return fmt.Sprintf("%s Wait for it to (re)connect or restart your workspace.", m) + m.Troubleshoot = true + return m } diff --git a/cli/cliui/agent_test.go b/cli/cliui/agent_test.go index 4f549f39f055e..e33b9b2630d36 100644 --- a/cli/cliui/agent_test.go +++ b/cli/cliui/agent_test.go @@ -7,6 +7,7 @@ import ( "github.com/spf13/cobra" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "go.uber.org/atomic" "github.com/coder/coder/cli/cliui" @@ -17,13 +18,17 @@ import ( func TestAgent(t *testing.T) { t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + var disconnected atomic.Bool ptty := ptytest.New(t) cmd := &cobra.Command{ - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { err := cliui.Agent(cmd.Context(), cmd.OutOrStdout(), cliui.AgentOptions{ WorkspaceName: "example", - Fetch: func(ctx context.Context) (codersdk.WorkspaceAgent, error) { + Fetch: func(_ context.Context) (codersdk.WorkspaceAgent, error) { agent := codersdk.WorkspaceAgent{ Status: codersdk.WorkspaceAgentDisconnected, } @@ -46,33 +51,34 @@ func TestAgent(t *testing.T) { err := cmd.Execute() assert.NoError(t, err) }() - ptty.ExpectMatch("lost connection") + ptty.ExpectMatchContext(ctx, "lost connection") disconnected.Store(true) <-done } -func TestAgentTimeoutWithTroubleshootingURL(t *testing.T) { +func TestAgent_TimeoutWithTroubleshootingURL(t *testing.T) { t.Parallel() - ctx, _ := testutil.Context(t) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() wantURL := "https://coder.com/troubleshoot" var connected, timeout atomic.Bool cmd := &cobra.Command{ - RunE: func(cmd *cobra.Command, args []string) error { + RunE: func(cmd *cobra.Command, _ []string) error { err := cliui.Agent(cmd.Context(), cmd.OutOrStdout(), cliui.AgentOptions{ WorkspaceName: "example", - Fetch: func(ctx context.Context) (codersdk.WorkspaceAgent, error) { + Fetch: func(_ context.Context) (codersdk.WorkspaceAgent, error) { agent := codersdk.WorkspaceAgent{ Status: codersdk.WorkspaceAgentConnecting, - TroubleshootingURL: "https://coder.com/troubleshoot", + TroubleshootingURL: wantURL, } switch { + case !connected.Load() && timeout.Load(): + agent.Status = codersdk.WorkspaceAgentTimeout case connected.Load(): agent.Status = codersdk.WorkspaceAgentConnected - case timeout.Load(): - agent.Status = codersdk.WorkspaceAgentTimeout } return agent, nil }, @@ -85,15 +91,212 @@ func TestAgentTimeoutWithTroubleshootingURL(t *testing.T) { ptty := ptytest.New(t) cmd.SetOutput(ptty.Output()) cmd.SetIn(ptty.Input()) - done := make(chan struct{}) + done := make(chan error, 1) go func() { - defer close(done) - err := cmd.ExecuteContext(ctx) - assert.NoError(t, err) + done <- cmd.ExecuteContext(ctx) }() - ptty.ExpectMatch("Don't panic") + ptty.ExpectMatchContext(ctx, "Don't panic, your workspace is booting") timeout.Store(true) - ptty.ExpectMatch(wantURL) + ptty.ExpectMatchContext(ctx, wantURL) connected.Store(true) - <-done + require.NoError(t, <-done) +} + +func TestAgent_StartupTimeout(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + wantURL := "https://coder.com/this-is-a-really-long-troubleshooting-url-that-should-not-wrap" + + var status, state atomic.String + setStatus := func(s codersdk.WorkspaceAgentStatus) { status.Store(string(s)) } + setState := func(s codersdk.WorkspaceAgentLifecycle) { state.Store(string(s)) } + cmd := &cobra.Command{ + RunE: func(cmd *cobra.Command, _ []string) error { + err := cliui.Agent(cmd.Context(), cmd.OutOrStdout(), cliui.AgentOptions{ + WorkspaceName: "example", + Fetch: func(_ context.Context) (codersdk.WorkspaceAgent, error) { + agent := codersdk.WorkspaceAgent{ + Status: codersdk.WorkspaceAgentConnecting, + DelayLoginUntilReady: true, + LifecycleState: codersdk.WorkspaceAgentLifecycleCreated, + TroubleshootingURL: wantURL, + } + + if s := status.Load(); s != "" { + agent.Status = codersdk.WorkspaceAgentStatus(s) + } + if s := state.Load(); s != "" { + agent.LifecycleState = codersdk.WorkspaceAgentLifecycle(s) + } + return agent, nil + }, + FetchInterval: time.Millisecond, + WarnInterval: time.Second, + SkipDelayLoginUntilReady: false, + }) + return err + }, + } + + ptty := ptytest.New(t) + cmd.SetOutput(ptty.Output()) + cmd.SetIn(ptty.Input()) + done := make(chan error, 1) + go func() { + done <- cmd.ExecuteContext(ctx) + }() + setStatus(codersdk.WorkspaceAgentConnecting) + ptty.ExpectMatchContext(ctx, "Don't panic, your workspace is booting") + setStatus(codersdk.WorkspaceAgentConnected) + setState(codersdk.WorkspaceAgentLifecycleStarting) + ptty.ExpectMatchContext(ctx, "Don't panic, your workspace is starting") + setState(codersdk.WorkspaceAgentLifecycleStartTimeout) + ptty.ExpectMatchContext(ctx, "is taking longer") + ptty.ExpectMatchContext(ctx, wantURL) + setState(codersdk.WorkspaceAgentLifecycleStartError) + ptty.ExpectMatchContext(ctx, "ran into a problem") + // Error should exit?? For now we make it ready. + setState(codersdk.WorkspaceAgentLifecycleReady) + require.NoError(t, <-done) +} + +func TestAgent_SkipDelayLoginUntilReady(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + wantURL := "https://coder.com/this-is-a-really-long-troubleshooting-url-that-should-not-wrap" + + var status, state atomic.String + setStatus := func(s codersdk.WorkspaceAgentStatus) { status.Store(string(s)) } + setState := func(s codersdk.WorkspaceAgentLifecycle) { state.Store(string(s)) } + cmd := &cobra.Command{ + RunE: func(cmd *cobra.Command, _ []string) error { + err := cliui.Agent(cmd.Context(), cmd.OutOrStdout(), cliui.AgentOptions{ + WorkspaceName: "example", + Fetch: func(_ context.Context) (codersdk.WorkspaceAgent, error) { + agent := codersdk.WorkspaceAgent{ + Status: codersdk.WorkspaceAgentConnecting, + DelayLoginUntilReady: true, + LifecycleState: codersdk.WorkspaceAgentLifecycleCreated, + TroubleshootingURL: wantURL, + } + + if s := status.Load(); s != "" { + agent.Status = codersdk.WorkspaceAgentStatus(s) + } + if s := state.Load(); s != "" { + agent.LifecycleState = codersdk.WorkspaceAgentLifecycle(s) + } + return agent, nil + }, + FetchInterval: time.Millisecond, + WarnInterval: time.Second, + SkipDelayLoginUntilReady: true, + }) + return err + }, + } + + ptty := ptytest.New(t) + cmd.SetOutput(ptty.Output()) + cmd.SetIn(ptty.Input()) + done := make(chan error, 1) + go func() { + done <- cmd.ExecuteContext(ctx) + }() + setStatus(codersdk.WorkspaceAgentConnecting) + ptty.ExpectMatchContext(ctx, "Don't panic, your workspace is booting") + + setStatus(codersdk.WorkspaceAgentConnected) + require.NoError(t, <-done, "created - should exit early") + + setState(codersdk.WorkspaceAgentLifecycleStarting) + go func() { done <- cmd.ExecuteContext(ctx) }() + require.NoError(t, <-done, "starting - should exit early") + + setState(codersdk.WorkspaceAgentLifecycleStartTimeout) + go func() { done <- cmd.ExecuteContext(ctx) }() + require.NoError(t, <-done, "start timeout - should exit early") + + setState(codersdk.WorkspaceAgentLifecycleStartError) + go func() { done <- cmd.ExecuteContext(ctx) }() + require.NoError(t, <-done, "start error - should exit early") + + setState(codersdk.WorkspaceAgentLifecycleReady) + go func() { done <- cmd.ExecuteContext(ctx) }() + require.NoError(t, <-done, "ready - should exit early") +} + +func TestAgent_DelayLoginUntilReadyDisabled(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + wantURL := "https://coder.com/this-is-a-really-long-troubleshooting-url-that-should-not-wrap" + + var status, state atomic.String + setStatus := func(s codersdk.WorkspaceAgentStatus) { status.Store(string(s)) } + setState := func(s codersdk.WorkspaceAgentLifecycle) { state.Store(string(s)) } + cmd := &cobra.Command{ + RunE: func(cmd *cobra.Command, _ []string) error { + err := cliui.Agent(cmd.Context(), cmd.OutOrStdout(), cliui.AgentOptions{ + WorkspaceName: "example", + Fetch: func(_ context.Context) (codersdk.WorkspaceAgent, error) { + agent := codersdk.WorkspaceAgent{ + Status: codersdk.WorkspaceAgentConnecting, + DelayLoginUntilReady: false, + LifecycleState: codersdk.WorkspaceAgentLifecycleCreated, + TroubleshootingURL: wantURL, + } + + if s := status.Load(); s != "" { + agent.Status = codersdk.WorkspaceAgentStatus(s) + } + if s := state.Load(); s != "" { + agent.LifecycleState = codersdk.WorkspaceAgentLifecycle(s) + } + return agent, nil + }, + FetchInterval: time.Millisecond, + WarnInterval: time.Second, + SkipDelayLoginUntilReady: false, + }) + return err + }, + } + + ptty := ptytest.New(t) + cmd.SetOutput(ptty.Output()) + cmd.SetIn(ptty.Input()) + done := make(chan error, 1) + go func() { + done <- cmd.ExecuteContext(ctx) + }() + setStatus(codersdk.WorkspaceAgentConnecting) + ptty.ExpectMatchContext(ctx, "Don't panic, your workspace is booting") + + setStatus(codersdk.WorkspaceAgentConnected) + require.NoError(t, <-done, "created - should exit early") + + setState(codersdk.WorkspaceAgentLifecycleStarting) + go func() { done <- cmd.ExecuteContext(ctx) }() + require.NoError(t, <-done, "starting - should exit early") + + setState(codersdk.WorkspaceAgentLifecycleStartTimeout) + go func() { done <- cmd.ExecuteContext(ctx) }() + require.NoError(t, <-done, "start timeout - should exit early") + + setState(codersdk.WorkspaceAgentLifecycleStartError) + go func() { done <- cmd.ExecuteContext(ctx) }() + require.NoError(t, <-done, "start error - should exit early") + + setState(codersdk.WorkspaceAgentLifecycleReady) + go func() { done <- cmd.ExecuteContext(ctx) }() + require.NoError(t, <-done, "ready - should exit early") } diff --git a/cli/ssh.go b/cli/ssh.go index 0d642754aed59..fe8bd7e23cd72 100644 --- a/cli/ssh.go +++ b/cli/ssh.go @@ -40,12 +40,13 @@ var ( func ssh() *cobra.Command { var ( - stdio bool - shuffle bool - forwardAgent bool - forwardGPG bool - identityAgent string - wsPollInterval time.Duration + stdio bool + shuffle bool + forwardAgent bool + forwardGPG bool + identityAgent string + wsPollInterval time.Duration + skipDelayLoginUntilReady bool ) cmd := &cobra.Command{ Annotations: workspaceCommand, @@ -90,6 +91,7 @@ func ssh() *cobra.Command { Fetch: func(ctx context.Context) (codersdk.WorkspaceAgent, error) { return client.WorkspaceAgent(ctx, workspaceAgent.ID) }, + SkipDelayLoginUntilReady: skipDelayLoginUntilReady, }) if err != nil { return xerrors.Errorf("await agent: %w", err) @@ -242,6 +244,7 @@ func ssh() *cobra.Command { cliflag.BoolVarP(cmd.Flags(), &forwardGPG, "forward-gpg", "G", "CODER_SSH_FORWARD_GPG", false, "Specifies whether to forward the GPG agent. Unsupported on Windows workspaces, but supports all clients. Requires gnupg (gpg, gpgconf) on both the client and workspace. The GPG agent must already be running locally and will not be started for you. If a GPG agent is already running in the workspace, it will be attempted to be killed.") cliflag.StringVarP(cmd.Flags(), &identityAgent, "identity-agent", "", "CODER_SSH_IDENTITY_AGENT", "", "Specifies which identity agent to use (overrides $SSH_AUTH_SOCK), forward agent must also be enabled") cliflag.DurationVarP(cmd.Flags(), &wsPollInterval, "workspace-poll-interval", "", "CODER_WORKSPACE_POLL_INTERVAL", workspacePollInterval, "Specifies how often to poll for workspace automated shutdown.") + cliflag.BoolVarP(cmd.Flags(), &skipDelayLoginUntilReady, "skip-delay-login-until-ready", "", "CODER_SSH_SKIP_DELAY_LOGIN_UNTIL_READY", false, "Specifies whether to login to a workspace that has not finished starting up (only applicable when the delay login until ready option is enabled).") return cmd } diff --git a/cli/testdata/coder_ssh_--help.golden b/cli/testdata/coder_ssh_--help.golden index 86010356f49a6..edad782e96c27 100644 --- a/cli/testdata/coder_ssh_--help.golden +++ b/cli/testdata/coder_ssh_--help.golden @@ -20,6 +20,10 @@ Flags: $SSH_AUTH_SOCK), forward agent must also be enabled. Consumes $CODER_SSH_IDENTITY_AGENT + --skip-delay-login-until-ready Specifies whether to login to a workspace that has + not finished starting up (only applicable when the + delay login until ready option is enabled). + Consumes $CODER_SSH_SKIP_DELAY_LOGIN_UNTIL_READY --stdio Specifies whether to emit SSH output over stdin/stdout. Consumes $CODER_SSH_STDIO From fd4a5968c17291ca418189cf24de814c5ebdc4bc Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Wed, 25 Jan 2023 14:36:26 +0000 Subject: [PATCH 2/9] Run go mod tidy --- go.mod | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/go.mod b/go.mod index 48a2c7bcfd568..19c6e8c5956af 100644 --- a/go.mod +++ b/go.mod @@ -273,7 +273,7 @@ require ( github.com/mitchellh/go-ps v1.0.0 // indirect github.com/moby/term v0.0.0-20210619224110-3f7ff695adc6 // indirect github.com/muesli/ansi v0.0.0-20211031195517-c9f0611b6c70 // indirect - github.com/muesli/reflow v0.3.0 // indirect + github.com/muesli/reflow v0.3.0 github.com/muesli/termenv v0.11.1-0.20220212125758-44cd13922739 // indirect github.com/niklasfasching/go-org v1.6.5 // indirect github.com/nu7hatch/gouuid v0.0.0-20131221200532-179d4d0c4d8d // indirect From b7a69a6cd57cd33ae12f001912c3d601d77faa61 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 26 Jan 2023 10:33:36 +0000 Subject: [PATCH 3/9] Rename flag to --no-wait --- cli/cliui/agent.go | 14 +++++++------- cli/cliui/agent_test.go | 18 +++++++++--------- cli/ssh.go | 18 +++++++++--------- cli/testdata/coder_ssh_--help.golden | 8 ++++---- 4 files changed, 29 insertions(+), 29 deletions(-) diff --git a/cli/cliui/agent.go b/cli/cliui/agent.go index ce1d3da3ffb98..1934a1e7cdab1 100644 --- a/cli/cliui/agent.go +++ b/cli/cliui/agent.go @@ -18,11 +18,11 @@ import ( ) type AgentOptions struct { - WorkspaceName string - Fetch func(context.Context) (codersdk.WorkspaceAgent, error) - FetchInterval time.Duration - WarnInterval time.Duration - SkipDelayLoginUntilReady bool + WorkspaceName string + Fetch func(context.Context) (codersdk.WorkspaceAgent, error) + FetchInterval time.Duration + WarnInterval time.Duration + NoWait bool // If true, don't wait for the agent to be ready. } // Agent displays a spinning indicator that waits for a workspace agent to connect. @@ -41,7 +41,7 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { // Fast path if the agent is ready (avoid showing connecting prompt). if agent.Status == codersdk.WorkspaceAgentConnected && - (!agent.DelayLoginUntilReady || opts.SkipDelayLoginUntilReady || agent.LifecycleState == codersdk.WorkspaceAgentLifecycleReady) { + (!agent.DelayLoginUntilReady || opts.NoWait || agent.LifecycleState == codersdk.WorkspaceAgentLifecycleReady) { return nil } @@ -135,7 +135,7 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { // NOTE(mafredri): Once we have access to the workspace agent's // startup script logs, we can show them here. // https://github.com/coder/coder/issues/2957 - if agent.DelayLoginUntilReady && !opts.SkipDelayLoginUntilReady { + if agent.DelayLoginUntilReady && !opts.NoWait { switch agent.LifecycleState { case codersdk.WorkspaceAgentLifecycleCreated, codersdk.WorkspaceAgentLifecycleStarting: select { diff --git a/cli/cliui/agent_test.go b/cli/cliui/agent_test.go index e33b9b2630d36..70e107f2adf50 100644 --- a/cli/cliui/agent_test.go +++ b/cli/cliui/agent_test.go @@ -133,9 +133,9 @@ func TestAgent_StartupTimeout(t *testing.T) { } return agent, nil }, - FetchInterval: time.Millisecond, - WarnInterval: time.Second, - SkipDelayLoginUntilReady: false, + FetchInterval: time.Millisecond, + WarnInterval: time.Second, + NoWait: false, }) return err }, @@ -194,9 +194,9 @@ func TestAgent_SkipDelayLoginUntilReady(t *testing.T) { } return agent, nil }, - FetchInterval: time.Millisecond, - WarnInterval: time.Second, - SkipDelayLoginUntilReady: true, + FetchInterval: time.Millisecond, + WarnInterval: time.Second, + NoWait: true, }) return err }, @@ -263,9 +263,9 @@ func TestAgent_DelayLoginUntilReadyDisabled(t *testing.T) { } return agent, nil }, - FetchInterval: time.Millisecond, - WarnInterval: time.Second, - SkipDelayLoginUntilReady: false, + FetchInterval: time.Millisecond, + WarnInterval: time.Second, + NoWait: false, }) return err }, diff --git a/cli/ssh.go b/cli/ssh.go index fe8bd7e23cd72..3b0316a29a0b0 100644 --- a/cli/ssh.go +++ b/cli/ssh.go @@ -40,13 +40,13 @@ var ( func ssh() *cobra.Command { var ( - stdio bool - shuffle bool - forwardAgent bool - forwardGPG bool - identityAgent string - wsPollInterval time.Duration - skipDelayLoginUntilReady bool + stdio bool + shuffle bool + forwardAgent bool + forwardGPG bool + identityAgent string + wsPollInterval time.Duration + noWait bool ) cmd := &cobra.Command{ Annotations: workspaceCommand, @@ -91,7 +91,7 @@ func ssh() *cobra.Command { Fetch: func(ctx context.Context) (codersdk.WorkspaceAgent, error) { return client.WorkspaceAgent(ctx, workspaceAgent.ID) }, - SkipDelayLoginUntilReady: skipDelayLoginUntilReady, + NoWait: noWait, }) if err != nil { return xerrors.Errorf("await agent: %w", err) @@ -244,7 +244,7 @@ func ssh() *cobra.Command { cliflag.BoolVarP(cmd.Flags(), &forwardGPG, "forward-gpg", "G", "CODER_SSH_FORWARD_GPG", false, "Specifies whether to forward the GPG agent. Unsupported on Windows workspaces, but supports all clients. Requires gnupg (gpg, gpgconf) on both the client and workspace. The GPG agent must already be running locally and will not be started for you. If a GPG agent is already running in the workspace, it will be attempted to be killed.") cliflag.StringVarP(cmd.Flags(), &identityAgent, "identity-agent", "", "CODER_SSH_IDENTITY_AGENT", "", "Specifies which identity agent to use (overrides $SSH_AUTH_SOCK), forward agent must also be enabled") cliflag.DurationVarP(cmd.Flags(), &wsPollInterval, "workspace-poll-interval", "", "CODER_WORKSPACE_POLL_INTERVAL", workspacePollInterval, "Specifies how often to poll for workspace automated shutdown.") - cliflag.BoolVarP(cmd.Flags(), &skipDelayLoginUntilReady, "skip-delay-login-until-ready", "", "CODER_SSH_SKIP_DELAY_LOGIN_UNTIL_READY", false, "Specifies whether to login to a workspace that has not finished starting up (only applicable when the delay login until ready option is enabled).") + cliflag.BoolVarP(cmd.Flags(), &noWait, "no-wait", "", "CODER_SSH_NO_WAIT", false, "Specifies whether to wait for a workspace to finish starting up before logging in (only applicable when the delay login until ready option is enabled).") return cmd } diff --git a/cli/testdata/coder_ssh_--help.golden b/cli/testdata/coder_ssh_--help.golden index edad782e96c27..242167540e53d 100644 --- a/cli/testdata/coder_ssh_--help.golden +++ b/cli/testdata/coder_ssh_--help.golden @@ -20,10 +20,10 @@ Flags: $SSH_AUTH_SOCK), forward agent must also be enabled. Consumes $CODER_SSH_IDENTITY_AGENT - --skip-delay-login-until-ready Specifies whether to login to a workspace that has - not finished starting up (only applicable when the - delay login until ready option is enabled). - Consumes $CODER_SSH_SKIP_DELAY_LOGIN_UNTIL_READY + --no-wait Specifies whether to wait for a workspace to finish + starting up before logging in (only applicable when + the delay login until ready option is enabled). + Consumes $CODER_SSH_NO_WAIT --stdio Specifies whether to emit SSH output over stdin/stdout. Consumes $CODER_SSH_STDIO From 4f461811bfec020373fd588ec2a4fdbbf22167df Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 26 Jan 2023 11:20:59 +0000 Subject: [PATCH 4/9] Exit on startup error --- cli/cliui/agent.go | 11 ++++---- cli/cliui/agent_test.go | 59 ++++++++++++++++++++++++++++++++++++++--- cli/ssh.go | 3 +++ 3 files changed, 63 insertions(+), 10 deletions(-) diff --git a/cli/cliui/agent.go b/cli/cliui/agent.go index 1934a1e7cdab1..640c6495e860d 100644 --- a/cli/cliui/agent.go +++ b/cli/cliui/agent.go @@ -17,6 +17,8 @@ import ( "github.com/coder/coder/codersdk" ) +var AgentStartError = xerrors.New("agent startup exited with non-zero exit status") + type AgentOptions struct { WorkspaceName string Fetch func(context.Context) (codersdk.WorkspaceAgent, error) @@ -147,6 +149,9 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { } case codersdk.WorkspaceAgentLifecycleReady: return nil + case codersdk.WorkspaceAgentLifecycleStartError: + showMessage() + return AgentStartError default: showMessage() } @@ -163,7 +168,6 @@ type message struct { Spin string Prompt string Troubleshoot bool - Error bool } func waitingMessage(agent codersdk.WorkspaceAgent) (m *message) { @@ -187,10 +191,6 @@ func waitingMessage(agent codersdk.WorkspaceAgent) (m *message) { } _, _ = fmt.Fprint(w, "\n") - if m.Error { - _, _ = fmt.Fprint(w, "\nPress Ctrl+C to exit.\n") - } - // We want to prefix the prompt with a caret, but we want text on the // following lines to align with the text on the first line (i.e. added // spacing). @@ -218,7 +218,6 @@ func waitingMessage(agent codersdk.WorkspaceAgent) (m *message) { case codersdk.WorkspaceAgentLifecycleStartError: m.Spin = "" m.Prompt = "The workspace agent ran into a problem during startup." - m.Error = true default: // Not a failure state, no troubleshooting necessary. return m diff --git a/cli/cliui/agent_test.go b/cli/cliui/agent_test.go index 70e107f2adf50..8d90d79e2aff1 100644 --- a/cli/cliui/agent_test.go +++ b/cli/cliui/agent_test.go @@ -156,14 +156,65 @@ func TestAgent_StartupTimeout(t *testing.T) { setState(codersdk.WorkspaceAgentLifecycleStartTimeout) ptty.ExpectMatchContext(ctx, "is taking longer") ptty.ExpectMatchContext(ctx, wantURL) - setState(codersdk.WorkspaceAgentLifecycleStartError) - ptty.ExpectMatchContext(ctx, "ran into a problem") - // Error should exit?? For now we make it ready. setState(codersdk.WorkspaceAgentLifecycleReady) require.NoError(t, <-done) } -func TestAgent_SkipDelayLoginUntilReady(t *testing.T) { +func TestAgent_StartErrorExit(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) + defer cancel() + + wantURL := "https://coder.com/this-is-a-really-long-troubleshooting-url-that-should-not-wrap" + + var status, state atomic.String + setStatus := func(s codersdk.WorkspaceAgentStatus) { status.Store(string(s)) } + setState := func(s codersdk.WorkspaceAgentLifecycle) { state.Store(string(s)) } + cmd := &cobra.Command{ + RunE: func(cmd *cobra.Command, _ []string) error { + err := cliui.Agent(cmd.Context(), cmd.OutOrStdout(), cliui.AgentOptions{ + WorkspaceName: "example", + Fetch: func(_ context.Context) (codersdk.WorkspaceAgent, error) { + agent := codersdk.WorkspaceAgent{ + Status: codersdk.WorkspaceAgentConnecting, + DelayLoginUntilReady: true, + LifecycleState: codersdk.WorkspaceAgentLifecycleCreated, + TroubleshootingURL: wantURL, + } + + if s := status.Load(); s != "" { + agent.Status = codersdk.WorkspaceAgentStatus(s) + } + if s := state.Load(); s != "" { + agent.LifecycleState = codersdk.WorkspaceAgentLifecycle(s) + } + return agent, nil + }, + FetchInterval: time.Millisecond, + WarnInterval: time.Second, + NoWait: false, + }) + return err + }, + } + + ptty := ptytest.New(t) + cmd.SetOutput(ptty.Output()) + cmd.SetIn(ptty.Input()) + done := make(chan error, 1) + go func() { + done <- cmd.ExecuteContext(ctx) + }() + setStatus(codersdk.WorkspaceAgentConnected) + setState(codersdk.WorkspaceAgentLifecycleStarting) + ptty.ExpectMatchContext(ctx, "Don't panic, your workspace is starting") + setState(codersdk.WorkspaceAgentLifecycleStartError) + ptty.ExpectMatchContext(ctx, "ran into a problem") + require.Error(t, <-done, "lifecycle start_error should exit with error") +} + +func TestAgent_NoWait(t *testing.T) { t.Parallel() ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) diff --git a/cli/ssh.go b/cli/ssh.go index 3b0316a29a0b0..4a7ce49294461 100644 --- a/cli/ssh.go +++ b/cli/ssh.go @@ -94,6 +94,9 @@ func ssh() *cobra.Command { NoWait: noWait, }) if err != nil { + if xerrors.Is(err, cliui.AgentStartError) { + return xerrors.New("Agent startup script exited with non-zero status, use --no-wait to login anyway.") + } return xerrors.Errorf("await agent: %w", err) } From 489ed6a507a6b41fd7674cc52342ca2cc0587ba9 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Thu, 26 Jan 2023 11:32:21 +0000 Subject: [PATCH 5/9] Show error message when using --no-wait --- cli/cliui/agent.go | 20 +++++++++++++++++--- 1 file changed, 17 insertions(+), 3 deletions(-) diff --git a/cli/cliui/agent.go b/cli/cliui/agent.go index 640c6495e860d..2a113462d5376 100644 --- a/cli/cliui/agent.go +++ b/cli/cliui/agent.go @@ -42,8 +42,10 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { } // Fast path if the agent is ready (avoid showing connecting prompt). + // We don't take the fast path for opts.NoWait yet because we want to + // show the message. if agent.Status == codersdk.WorkspaceAgentConnected && - (!agent.DelayLoginUntilReady || opts.NoWait || agent.LifecycleState == codersdk.WorkspaceAgentLifecycleReady) { + (!agent.DelayLoginUntilReady || agent.LifecycleState == codersdk.WorkspaceAgentLifecycleReady) { return nil } @@ -51,8 +53,6 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { spin.Writer = writer spin.ForceOutput = true spin.Suffix = waitingMessage(agent).Spin - spin.Start() - defer spin.Stop() ctx, cancelFunc := context.WithCancel(ctx) defer cancelFunc() @@ -105,6 +105,20 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { } } } + + // Fast path for showing the error message even when using no wait, + // we do this just before starting the spinner to avoid needless + // spinning. + if agent.Status == codersdk.WorkspaceAgentConnected && + agent.DelayLoginUntilReady && opts.NoWait { + showMessage() + return nil + } + + // Start spinning after fast paths are handled. + spin.Start() + defer spin.Stop() + messageAfterDone := make(chan struct{}) go func() { select { From 8ded3c80193f3881630d66d1549e8899f397db40 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 27 Jan 2023 11:06:45 +0000 Subject: [PATCH 6/9] Cleanup and reword messeages --- cli/cliui/agent.go | 94 ++++++++++++++++++++--------------------- cli/cliui/agent_test.go | 11 ++--- cli/ssh.go | 3 ++ 3 files changed, 54 insertions(+), 54 deletions(-) diff --git a/cli/cliui/agent.go b/cli/cliui/agent.go index 2a113462d5376..4be234fb5b399 100644 --- a/cli/cliui/agent.go +++ b/cli/cliui/agent.go @@ -49,37 +49,20 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { return nil } + ctx, cancel := signal.NotifyContext(ctx, os.Interrupt) + defer cancel() + spin := spinner.New(spinner.CharSets[78], 100*time.Millisecond, spinner.WithColor("fgHiGreen")) spin.Writer = writer spin.ForceOutput = true - spin.Suffix = waitingMessage(agent).Spin - - ctx, cancelFunc := context.WithCancel(ctx) - defer cancelFunc() - stopSpin := make(chan os.Signal, 1) - signal.Notify(stopSpin, os.Interrupt) - defer signal.Stop(stopSpin) - go func() { - select { - case <-ctx.Done(): - return - case <-stopSpin: - } - cancelFunc() - signal.Stop(stopSpin) - spin.Stop() - // nolint:revive - os.Exit(1) - }() + spin.Suffix = waitingMessage(agent, opts).Spin waitMessage := &message{} - messageAfter := time.NewTimer(opts.WarnInterval) - defer messageAfter.Stop() - showMessage := func() { + showMessage := func(keepSpinning bool) { resourceMutex.Lock() defer resourceMutex.Unlock() - m := waitingMessage(agent) + m := waitingMessage(agent, opts) if m.Prompt == waitMessage.Prompt { return } @@ -100,7 +83,7 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { case <-ctx.Done(): default: // Safe to resume operation. - if spin.Suffix != "" { + if keepSpinning && spin.Suffix != "" { spin.Start() } } @@ -111,23 +94,26 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { // spinning. if agent.Status == codersdk.WorkspaceAgentConnected && agent.DelayLoginUntilReady && opts.NoWait { - showMessage() + showMessage(false) return nil } // Start spinning after fast paths are handled. - spin.Start() + if spin.Suffix != "" { + spin.Start() + } defer spin.Stop() - messageAfterDone := make(chan struct{}) + warnAfter := time.NewTimer(opts.WarnInterval) + defer warnAfter.Stop() + warningShown := make(chan struct{}) go func() { select { case <-ctx.Done(): - close(messageAfterDone) - case <-messageAfter.C: - messageAfter.Stop() - close(messageAfterDone) - showMessage() + close(warningShown) + case <-warnAfter.C: + close(warningShown) + showMessage(true) } }() @@ -153,27 +139,27 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { // https://github.com/coder/coder/issues/2957 if agent.DelayLoginUntilReady && !opts.NoWait { switch agent.LifecycleState { - case codersdk.WorkspaceAgentLifecycleCreated, codersdk.WorkspaceAgentLifecycleStarting: - select { - case <-messageAfterDone: - showMessage() - default: - // This state is normal, we don't want - // to show a message prematurely. - } case codersdk.WorkspaceAgentLifecycleReady: return nil + case codersdk.WorkspaceAgentLifecycleStartTimeout: + showMessage(false) case codersdk.WorkspaceAgentLifecycleStartError: - showMessage() + showMessage(false) return AgentStartError default: - showMessage() + select { + case <-warningShown: + showMessage(true) + default: + // This state is normal, we don't want + // to show a message prematurely. + } } continue } return nil case codersdk.WorkspaceAgentTimeout, codersdk.WorkspaceAgentDisconnected: - showMessage() + showMessage(true) } } } @@ -184,12 +170,19 @@ type message struct { Troubleshoot bool } -func waitingMessage(agent codersdk.WorkspaceAgent) (m *message) { +func waitingMessage(agent codersdk.WorkspaceAgent, opts AgentOptions) (m *message) { m = &message{ + Spin: fmt.Sprintf("Waiting for connection from %s...", Styles.Field.Render(agent.Name)), Prompt: "Don't panic, your workspace is booting up!", - Spin: fmt.Sprintf(" Waiting for connection from %s...", Styles.Field.Render(agent.Name)), } defer func() { + if opts.NoWait { + m.Spin = "" + } + if m.Spin != "" { + m.Spin = " " + m.Spin + } + // We don't want to wrap the troubleshooting URL, so we'll handle word // wrapping ourselves (vs using lipgloss). w := wordwrap.NewWriter(Styles.Paragraph.GetWidth() - Styles.Paragraph.GetMarginLeft()*2) @@ -223,15 +216,18 @@ func waitingMessage(agent codersdk.WorkspaceAgent) (m *message) { case codersdk.WorkspaceAgentDisconnected: m.Prompt = "The workspace agent lost connection!" case codersdk.WorkspaceAgentConnected: - m.Prompt = "Don't panic, your workspace is starting up!" - m.Spin = fmt.Sprintf(" Waiting for %s to finish starting up...", Styles.Field.Render(agent.Name)) + m.Spin = fmt.Sprintf("Waiting for %s to become ready...", Styles.Field.Render(agent.Name)) + m.Prompt = "Don't panic, your workspace agent has connected and the workspace is getting ready!" + if opts.NoWait { + m.Prompt = "Your workspace is still getting ready, it may be in an incomplete state." + } switch agent.LifecycleState { case codersdk.WorkspaceAgentLifecycleStartTimeout: - m.Prompt = "The workspace agent is taking longer than expected to start." + m.Prompt = "The workspace is taking longer than expected to get ready, the agent startup script is still executing." case codersdk.WorkspaceAgentLifecycleStartError: m.Spin = "" - m.Prompt = "The workspace agent ran into a problem during startup." + m.Prompt = "The workspace ran into a problem while getting ready, the agent startup script exited with non-zero status." default: // Not a failure state, no troubleshooting necessary. return m diff --git a/cli/cliui/agent_test.go b/cli/cliui/agent_test.go index 8d90d79e2aff1..4521e443ae822 100644 --- a/cli/cliui/agent_test.go +++ b/cli/cliui/agent_test.go @@ -134,7 +134,7 @@ func TestAgent_StartupTimeout(t *testing.T) { return agent, nil }, FetchInterval: time.Millisecond, - WarnInterval: time.Second, + WarnInterval: time.Millisecond, NoWait: false, }) return err @@ -152,7 +152,7 @@ func TestAgent_StartupTimeout(t *testing.T) { ptty.ExpectMatchContext(ctx, "Don't panic, your workspace is booting") setStatus(codersdk.WorkspaceAgentConnected) setState(codersdk.WorkspaceAgentLifecycleStarting) - ptty.ExpectMatchContext(ctx, "Don't panic, your workspace is starting") + ptty.ExpectMatchContext(ctx, "workspace is getting ready") setState(codersdk.WorkspaceAgentLifecycleStartTimeout) ptty.ExpectMatchContext(ctx, "is taking longer") ptty.ExpectMatchContext(ctx, wantURL) @@ -192,7 +192,7 @@ func TestAgent_StartErrorExit(t *testing.T) { return agent, nil }, FetchInterval: time.Millisecond, - WarnInterval: time.Second, + WarnInterval: 60 * time.Second, NoWait: false, }) return err @@ -208,10 +208,11 @@ func TestAgent_StartErrorExit(t *testing.T) { }() setStatus(codersdk.WorkspaceAgentConnected) setState(codersdk.WorkspaceAgentLifecycleStarting) - ptty.ExpectMatchContext(ctx, "Don't panic, your workspace is starting") + ptty.ExpectMatchContext(ctx, "to become ready...") setState(codersdk.WorkspaceAgentLifecycleStartError) ptty.ExpectMatchContext(ctx, "ran into a problem") - require.Error(t, <-done, "lifecycle start_error should exit with error") + err := <-done + require.ErrorIs(t, err, cliui.AgentStartError, "lifecycle start_error should exit with error") } func TestAgent_NoWait(t *testing.T) { diff --git a/cli/ssh.go b/cli/ssh.go index 4a7ce49294461..2076c88541d3f 100644 --- a/cli/ssh.go +++ b/cli/ssh.go @@ -94,6 +94,9 @@ func ssh() *cobra.Command { NoWait: noWait, }) if err != nil { + if xerrors.Is(err, context.Canceled) { + return cliui.Canceled + } if xerrors.Is(err, cliui.AgentStartError) { return xerrors.New("Agent startup script exited with non-zero status, use --no-wait to login anyway.") } From cdbf70c582603b2207f51d41c447ced5fc4e5710 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 27 Jan 2023 11:19:31 +0000 Subject: [PATCH 7/9] fixup! Cleanup and reword messeages --- cli/cliui/agent.go | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/cli/cliui/agent.go b/cli/cliui/agent.go index 4be234fb5b399..940d4188739a4 100644 --- a/cli/cliui/agent.go +++ b/cli/cliui/agent.go @@ -58,7 +58,7 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { spin.Suffix = waitingMessage(agent, opts).Spin waitMessage := &message{} - showMessage := func(keepSpinning bool) { + showMessage := func() { resourceMutex.Lock() defer resourceMutex.Unlock() @@ -83,7 +83,7 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { case <-ctx.Done(): default: // Safe to resume operation. - if keepSpinning && spin.Suffix != "" { + if spin.Suffix != "" { spin.Start() } } @@ -94,7 +94,7 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { // spinning. if agent.Status == codersdk.WorkspaceAgentConnected && agent.DelayLoginUntilReady && opts.NoWait { - showMessage(false) + showMessage() return nil } @@ -113,7 +113,7 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { close(warningShown) case <-warnAfter.C: close(warningShown) - showMessage(true) + showMessage() } }() @@ -142,14 +142,14 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { case codersdk.WorkspaceAgentLifecycleReady: return nil case codersdk.WorkspaceAgentLifecycleStartTimeout: - showMessage(false) + showMessage() case codersdk.WorkspaceAgentLifecycleStartError: - showMessage(false) + showMessage() return AgentStartError default: select { case <-warningShown: - showMessage(true) + showMessage() default: // This state is normal, we don't want // to show a message prematurely. @@ -159,7 +159,7 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error { } return nil case codersdk.WorkspaceAgentTimeout, codersdk.WorkspaceAgentDisconnected: - showMessage(true) + showMessage() } } } From 00ab3baacf7c2f6d902c744d580a34388b64e458 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 27 Jan 2023 11:27:53 +0000 Subject: [PATCH 8/9] test: Fix ShowTroubleshootingURLAfterTimeout --- cli/ssh_test.go | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cli/ssh_test.go b/cli/ssh_test.go index d74a5fd2c1b63..bf0aab1417fb4 100644 --- a/cli/ssh_test.go +++ b/cli/ssh_test.go @@ -28,6 +28,7 @@ import ( "github.com/coder/coder/agent" "github.com/coder/coder/cli/clitest" + "github.com/coder/coder/cli/cliui" "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/codersdk" "github.com/coder/coder/provisioner/echo" @@ -136,7 +137,7 @@ func TestSSH(t *testing.T) { cmdDone := tGo(t, func() { err := cmd.ExecuteContext(ctx) - assert.ErrorIs(t, err, context.Canceled) + assert.ErrorIs(t, err, cliui.Canceled) }) pty.ExpectMatch(wantURL) cancel() From 68d4c4a3bfcf86e47c84b6b709140f401f905e50 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Fri, 27 Jan 2023 14:23:15 +0000 Subject: [PATCH 9/9] Improve --no-wait flag description --- cli/ssh.go | 2 +- cli/testdata/coder_ssh_--help.golden | 9 ++++++--- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/cli/ssh.go b/cli/ssh.go index 2076c88541d3f..6f8aaf7854832 100644 --- a/cli/ssh.go +++ b/cli/ssh.go @@ -250,7 +250,7 @@ func ssh() *cobra.Command { cliflag.BoolVarP(cmd.Flags(), &forwardGPG, "forward-gpg", "G", "CODER_SSH_FORWARD_GPG", false, "Specifies whether to forward the GPG agent. Unsupported on Windows workspaces, but supports all clients. Requires gnupg (gpg, gpgconf) on both the client and workspace. The GPG agent must already be running locally and will not be started for you. If a GPG agent is already running in the workspace, it will be attempted to be killed.") cliflag.StringVarP(cmd.Flags(), &identityAgent, "identity-agent", "", "CODER_SSH_IDENTITY_AGENT", "", "Specifies which identity agent to use (overrides $SSH_AUTH_SOCK), forward agent must also be enabled") cliflag.DurationVarP(cmd.Flags(), &wsPollInterval, "workspace-poll-interval", "", "CODER_WORKSPACE_POLL_INTERVAL", workspacePollInterval, "Specifies how often to poll for workspace automated shutdown.") - cliflag.BoolVarP(cmd.Flags(), &noWait, "no-wait", "", "CODER_SSH_NO_WAIT", false, "Specifies whether to wait for a workspace to finish starting up before logging in (only applicable when the delay login until ready option is enabled).") + cliflag.BoolVarP(cmd.Flags(), &noWait, "no-wait", "", "CODER_SSH_NO_WAIT", false, "Specifies whether to wait for a workspace to become ready before logging in (only applicable when the delay login until ready option is enabled). Note that the workspace agent may still be in the process of executing the startup script and the workspace may be in an incomplete state.") return cmd } diff --git a/cli/testdata/coder_ssh_--help.golden b/cli/testdata/coder_ssh_--help.golden index 242167540e53d..b539c1a76b34a 100644 --- a/cli/testdata/coder_ssh_--help.golden +++ b/cli/testdata/coder_ssh_--help.golden @@ -20,9 +20,12 @@ Flags: $SSH_AUTH_SOCK), forward agent must also be enabled. Consumes $CODER_SSH_IDENTITY_AGENT - --no-wait Specifies whether to wait for a workspace to finish - starting up before logging in (only applicable when - the delay login until ready option is enabled). + --no-wait Specifies whether to wait for a workspace to become + ready before logging in (only applicable when the + delay login until ready option is enabled). Note + that the workspace agent may still be in the + process of executing the startup script and the + workspace may be in an incomplete state. Consumes $CODER_SSH_NO_WAIT --stdio Specifies whether to emit SSH output over stdin/stdout.