Skip to content

feat(cli/ssh): implement wait options and deprecate no-wait #7894

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 5 commits into from
Jun 8, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 5 additions & 5 deletions cli/cliui/agent.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ type AgentOptions struct {
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.
Wait bool // If true, wait for the agent to be ready (startup script).
}

// Agent displays a spinning indicator that waits for a workspace agent to connect.
Expand Down Expand Up @@ -96,7 +96,7 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error {
// we do this just before starting the spinner to avoid needless
// spinning.
if agent.Status == codersdk.WorkspaceAgentConnected &&
agent.StartupScriptBehavior == codersdk.WorkspaceAgentStartupScriptBehaviorBlocking && opts.NoWait {
agent.StartupScriptBehavior == codersdk.WorkspaceAgentStartupScriptBehaviorBlocking && !opts.Wait {
showMessage()
return nil
}
Expand Down Expand Up @@ -140,7 +140,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.StartupScriptBehavior == codersdk.WorkspaceAgentStartupScriptBehaviorBlocking && !opts.NoWait {
if agent.StartupScriptBehavior == codersdk.WorkspaceAgentStartupScriptBehaviorBlocking && opts.Wait {
switch agent.LifecycleState {
case codersdk.WorkspaceAgentLifecycleReady:
return nil
Expand Down Expand Up @@ -183,7 +183,7 @@ func waitingMessage(agent codersdk.WorkspaceAgent, opts AgentOptions) (m *messag
Prompt: "Don't panic, your workspace is booting up!",
}
defer func() {
if agent.Status == codersdk.WorkspaceAgentConnected && opts.NoWait {
if agent.Status == codersdk.WorkspaceAgentConnected && !opts.Wait {
m.Spin = ""
}
if m.Spin != "" {
Expand Down Expand Up @@ -225,7 +225,7 @@ func waitingMessage(agent codersdk.WorkspaceAgent, opts AgentOptions) (m *messag
case codersdk.WorkspaceAgentConnected:
m.Spin = fmt.Sprintf("Waiting for %s to become ready...", DefaultStyles.Field.Render(agent.Name))
m.Prompt = "Don't panic, your workspace agent has connected and the workspace is getting ready!"
if opts.NoWait {
if !opts.Wait {
m.Prompt = "Your workspace is still getting ready, it may be in an incomplete state."
}

Expand Down
8 changes: 4 additions & 4 deletions cli/cliui/agent_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ func TestAgent_StartupTimeout(t *testing.T) {
},
FetchInterval: time.Millisecond,
WarnInterval: time.Millisecond,
NoWait: false,
Wait: true,
})
return err
},
Expand Down Expand Up @@ -199,7 +199,7 @@ func TestAgent_StartErrorExit(t *testing.T) {
},
FetchInterval: time.Millisecond,
WarnInterval: 60 * time.Second,
NoWait: false,
Wait: true,
})
return err
},
Expand Down Expand Up @@ -255,7 +255,7 @@ func TestAgent_NoWait(t *testing.T) {
},
FetchInterval: time.Millisecond,
WarnInterval: time.Second,
NoWait: true,
Wait: false,
})
return err
},
Expand Down Expand Up @@ -325,7 +325,7 @@ func TestAgent_StartupScriptBehaviorNonBlocking(t *testing.T) {
},
FetchInterval: time.Millisecond,
WarnInterval: time.Second,
NoWait: false,
Wait: true,
})
return err
},
Expand Down
1 change: 1 addition & 0 deletions cli/portforward.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ func (r *RootCmd) portForward() *clibase.Cmd {
Fetch: func(ctx context.Context) (codersdk.WorkspaceAgent, error) {
return client.WorkspaceAgent(ctx, workspaceAgent.ID)
},
Wait: false,
Copy link
Member Author

Choose a reason for hiding this comment

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

Review: Before change from NoWait -> Wait, these were bugs, added the explicit false here to signal intent.

})
if err != nil {
return xerrors.Errorf("await agent: %w", err)
Expand Down
1 change: 1 addition & 0 deletions cli/speedtest.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ func (r *RootCmd) speedtest() *clibase.Cmd {
Fetch: func(ctx context.Context) (codersdk.WorkspaceAgent, error) {
return client.WorkspaceAgent(ctx, workspaceAgent.ID)
},
Wait: false,
})
if err != nil && !xerrors.Is(err, cliui.AgentStartError) {
return xerrors.Errorf("await agent: %w", err)
Expand Down
39 changes: 37 additions & 2 deletions cli/ssh.go
Original file line number Diff line number Diff line change
Expand Up @@ -42,13 +42,15 @@ var (
autostopNotifyCountdown = []time.Duration{30 * time.Minute}
)

//nolint:gocyclo
func (r *RootCmd) ssh() *clibase.Cmd {
var (
stdio bool
forwardAgent bool
forwardGPG bool
identityAgent string
wsPollInterval time.Duration
waitEnum string
noWait bool
logDir string
logToFile bool
Expand Down Expand Up @@ -105,6 +107,30 @@ func (r *RootCmd) ssh() *clibase.Cmd {
return err
}

// Select the startup script behavior based on template configuration or flags.
var wait bool
switch waitEnum {
case "yes":
wait = true
case "no":
wait = false
case "auto":
switch workspaceAgent.StartupScriptBehavior {
case codersdk.WorkspaceAgentStartupScriptBehaviorBlocking:
wait = true
case codersdk.WorkspaceAgentStartupScriptBehaviorNonBlocking:
wait = false
default:
return xerrors.Errorf("unknown startup script behavior %q", workspaceAgent.StartupScriptBehavior)
}
default:
return xerrors.Errorf("unknown wait value %q", waitEnum)
}
// The `--no-wait` flag is deprecated, but for now, check it.
if noWait {
wait = false
}

templateVersion, err := client.TemplateVersion(ctx, workspace.LatestBuild.TemplateVersionID)
if err != nil {
return err
Expand Down Expand Up @@ -134,7 +160,7 @@ func (r *RootCmd) ssh() *clibase.Cmd {
Fetch: func(ctx context.Context) (codersdk.WorkspaceAgent, error) {
return client.WorkspaceAgent(ctx, workspaceAgent.ID)
},
NoWait: noWait,
Wait: wait,
})
if err != nil {
if xerrors.Is(err, context.Canceled) {
Expand Down Expand Up @@ -313,6 +339,13 @@ func (r *RootCmd) ssh() *clibase.Cmd {
return nil
},
}
waitOption := clibase.Option{
Flag: "wait",
Env: "CODER_SSH_WAIT",
Description: "Specifies whether or not to wait for the startup script to finish executing. Auto means that the agent startup script behavior configured in the workspace template is used.",
Default: "auto",
Value: clibase.EnumOf(&waitEnum, "yes", "no", "auto"),
}
cmd.Options = clibase.OptionSet{
{
Flag: "stdio",
Expand Down Expand Up @@ -347,11 +380,13 @@ func (r *RootCmd) ssh() *clibase.Cmd {
Default: "1m",
Value: clibase.DurationOf(&wsPollInterval),
},
waitOption,
{
Flag: "no-wait",
Env: "CODER_SSH_NO_WAIT",
Description: "Specifies whether to wait for a workspace to become ready before logging in (only applicable when the startup script behavior is blocking). 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.",
Description: "Enter workspace immediately after the agent has connected. This is the default if the template has configured the agent startup script behavior as non-blocking.",
Value: clibase.BoolOf(&noWait),
UseInstead: []clibase.Option{waitOption},
Copy link
Member

Choose a reason for hiding this comment

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

It took me a hot second to see this as I was looking for Deprecated 🙃

},
{
Flag: "log-dir",
Expand Down
15 changes: 9 additions & 6 deletions cli/testdata/coder_ssh_--help.golden
Original file line number Diff line number Diff line change
Expand Up @@ -25,15 +25,18 @@ Start a shell into a workspace
Enable diagnostic logging to file.

--no-wait bool, $CODER_SSH_NO_WAIT
Specifies whether to wait for a workspace to become ready before
logging in (only applicable when the startup script behavior is
blocking). 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.

Enter workspace immediately after the agent has connected. This is the
default if the template has configured the agent startup script
behavior as non-blocking.
DEPRECATED
--stdio bool, $CODER_SSH_STDIO
Specifies whether to emit SSH output over stdin/stdout.

--wait yes|no|auto, $CODER_SSH_WAIT (default: auto)
Specifies whether or not to wait for the startup script to finish
executing. Auto means that the agent startup script behavior
configured in the workspace template is used.

--workspace-poll-interval duration, $CODER_WORKSPACE_POLL_INTERVAL (default: 1m)
Specifies how often to poll for workspace automated shutdown.

Expand Down
1 change: 1 addition & 0 deletions coderd/database/dbfake/databasefake.go
Original file line number Diff line number Diff line change
Expand Up @@ -3060,6 +3060,7 @@ func (q *fakeQuerier) InsertWorkspaceAgent(_ context.Context, arg database.Inser
Architecture: arg.Architecture,
OperatingSystem: arg.OperatingSystem,
Directory: arg.Directory,
StartupScriptBehavior: arg.StartupScriptBehavior,
StartupScript: arg.StartupScript,
InstanceMetadata: arg.InstanceMetadata,
ResourceMetadata: arg.ResourceMetadata,
Expand Down
12 changes: 11 additions & 1 deletion docs/cli/ssh.md
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ Enable diagnostic logging to file.
| Type | <code>bool</code> |
| Environment | <code>$CODER_SSH_NO_WAIT</code> |

Specifies whether to wait for a workspace to become ready before logging in (only applicable when the startup script behavior is blocking). 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.
Enter workspace immediately after the agent has connected. This is the default if the template has configured the agent startup script behavior as non-blocking.

### --stdio

Expand All @@ -76,6 +76,16 @@ Specifies whether to wait for a workspace to become ready before logging in (onl

Specifies whether to emit SSH output over stdin/stdout.

### --wait

| | |
| ----------- | ---------------------------- | --- | ------------ |
| Type | <code>enum[yes | no | auto]</code> |
| Environment | <code>$CODER_SSH_WAIT</code> |
| Default | <code>auto</code> |

Specifies whether or not to wait for the startup script to finish executing. Auto means that the agent startup script behavior configured in the workspace template is used.

### --workspace-poll-interval

| | |
Expand Down