Skip to content

feat: restart stopped workspaces on ssh command #11050

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 10 commits into from
Dec 8, 2023
Merged

Conversation

Emyrk
Copy link
Member

@Emyrk Emyrk commented Dec 5, 2023

What this does

On coder ssh or ssh if the target workspace is off, the workspace will be started.

ssh coder.local-docker                                                                                                                           02:08:08 PM
Workspace was stopped, starting workspace to allow connection "local-docker"...
==> ⧗ Queued
=== ✔ Queued [0ms]
==> ⧗ Running
=== ✔ Running [4ms]
==> ⧗ Setting up
=== ✔ Setting up [29ms]
==> ⧗ Planning infrastructure
... rest of logs ...

Feature can be opt out, so by default this is on. To opt out:

$ coder ssh --disable-autostart local-docker
# Or
$ coder config-ssh --disable-autostart

Closes: #4973

VScode extension?

Vscode uses coder vscodessh and is unaffected:

func (r *RootCmd) vscodeSSH() *clibase.Cmd {

Emyrk added 2 commits December 5, 2023 12:26
This is opt out by default. VScode ssh does not have this behavior
@Emyrk Emyrk changed the title feat: autostart stopped workspaces on ssh feat: start stopped workspaces on ssh Dec 6, 2023
@Emyrk Emyrk requested a review from mafredri December 6, 2023 14:46
@Emyrk Emyrk changed the title feat: start stopped workspaces on ssh feat: restart stopped workspaces on ssh command Dec 6, 2023
Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

Nice clean addition!

}
// The workspace needs to be stopped before we can start it.
// It cannot be in any pending or failed state.
if workspace.LatestBuild.Status != codersdk.WorkspaceStatusStopped {
Copy link
Member

Choose a reason for hiding this comment

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

A case that I would personally like for to work is that I can ssh into a stopping workspace and have it wait until the stop is complete and then either issue the start or wait for a start to complete.

No action necessary for this PR, though. Just writing this down as a food for thought.

Copy link
Member Author

Choose a reason for hiding this comment

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

Things could be improved to handle more cases, but that felt a bit overkill atm.

cli/ssh_test.go Outdated
err := inv.WithContext(ctx).Run()
assert.NoError(t, err)
})
pty.ExpectMatch("⧗ Running")
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if this has the potential to be racy, i.e. workspace started before we end up streaming logs. 🤔

Copy link
Member Author

Choose a reason for hiding this comment

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

That is a good point. I am unsure how to prevent that 🤔.

We really should have a controllable provisioner in our unit tests to trigger this stuff 😢

Copy link
Member Author

Choose a reason for hiding this comment

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

Actually, we have coderdtest.AwaitWorkspaceAgents(t, client, workspace.ID) before I run exit which is sufficient to wait for the workspace to be ready.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah, I think workspace being ready shouldn't be an issue. But I'm wondering if the provisioner log missing could be an issue depending on at what stage of the workspace lifecycle we connect to it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yup, I removed it because I check if the agent is ready right below it. So That agent ready will always work, even if we miss the logs.

@Emyrk Emyrk requested a review from mafredri December 7, 2023 22:25
Copy link
Member

@mafredri mafredri left a comment

Choose a reason for hiding this comment

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

:shipit:

@Emyrk Emyrk merged commit cb89bc1 into main Dec 8, 2023
@Emyrk Emyrk deleted the stevenmasley/ssh_autostart branch December 8, 2023 16:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Auto start workspace when I connect via SSH or visit an app
2 participants