Skip to content

refactor: deprecate login_before_ready in favor of startup_script_behavior #7837

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.

Sign up for GitHub

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 3 commits into from
Jun 6, 2023
Merged

refactor: deprecate login_before_ready in favor of startup_script_behavior #7837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7baef4d
refactor: Deprecate `login_before_ready` in favor of `startup_script_…
mafredri Jun 5, 2023
011f82b
PR fixes
mafredri Jun 5, 2023
91939df
Comma, comma, come again
mafredri Jun 5, 2023
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
6 changes: 3 additions & 3 deletions cli/cliui/agent.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ func Agent(ctx context.Context, writer io.Writer, opts AgentOptions) error {
// We don't take the fast path for opts.NoWait yet because we want to
// show the message.
if agent.Status == codersdk.WorkspaceAgentConnected &&
(agent.LoginBeforeReady || agent.LifecycleState == codersdk.WorkspaceAgentLifecycleReady) {
(agent.StartupScriptBehavior == codersdk.WorkspaceAgentStartupScriptBehaviorNonBlocking || agent.LifecycleState == codersdk.WorkspaceAgentLifecycleReady) {
return nil
}

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.LoginBeforeReady && opts.NoWait {
agent.StartupScriptBehavior == codersdk.WorkspaceAgentStartupScriptBehaviorBlocking && opts.NoWait {
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.LoginBeforeReady && !opts.NoWait {
if agent.StartupScriptBehavior == codersdk.WorkspaceAgentStartupScriptBehaviorBlocking && !opts.NoWait {
switch agent.LifecycleState {
case codersdk.WorkspaceAgentLifecycleReady:
return nil
Expand Down
44 changes: 22 additions & 22 deletions cli/cliui/agent_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ func TestAgent(t *testing.T) {
WorkspaceName: "example",
Fetch: func(_ context.Context) (codersdk.WorkspaceAgent, error) {
agent := codersdk.WorkspaceAgent{
Status: codersdk.WorkspaceAgentDisconnected,
LoginBeforeReady: true,
Status: codersdk.WorkspaceAgentDisconnected,
StartupScriptBehavior: codersdk.WorkspaceAgentStartupScriptBehaviorNonBlocking,
}
if disconnected.Load() {
agent.Status = codersdk.WorkspaceAgentConnected
Expand Down Expand Up @@ -73,9 +73,9 @@ func TestAgent_TimeoutWithTroubleshootingURL(t *testing.T) {
WorkspaceName: "example",
Fetch: func(_ context.Context) (codersdk.WorkspaceAgent, error) {
agent := codersdk.WorkspaceAgent{
Status: codersdk.WorkspaceAgentConnecting,
TroubleshootingURL: wantURL,
LoginBeforeReady: true,
Status: codersdk.WorkspaceAgentConnecting,
TroubleshootingURL: wantURL,
StartupScriptBehavior: codersdk.WorkspaceAgentStartupScriptBehaviorNonBlocking,
}
switch {
case !connected.Load() && timeout.Load():
Expand Down Expand Up @@ -124,10 +124,10 @@ func TestAgent_StartupTimeout(t *testing.T) {
WorkspaceName: "example",
Fetch: func(_ context.Context) (codersdk.WorkspaceAgent, error) {
agent := codersdk.WorkspaceAgent{
Status: codersdk.WorkspaceAgentConnecting,
LoginBeforeReady: false,
LifecycleState: codersdk.WorkspaceAgentLifecycleCreated,
TroubleshootingURL: wantURL,
Status: codersdk.WorkspaceAgentConnecting,
StartupScriptBehavior: codersdk.WorkspaceAgentStartupScriptBehaviorBlocking,
LifecycleState: codersdk.WorkspaceAgentLifecycleCreated,
TroubleshootingURL: wantURL,
}

if s := status.Load(); s != "" {
Expand Down Expand Up @@ -183,10 +183,10 @@ func TestAgent_StartErrorExit(t *testing.T) {
WorkspaceName: "example",
Fetch: func(_ context.Context) (codersdk.WorkspaceAgent, error) {
agent := codersdk.WorkspaceAgent{
Status: codersdk.WorkspaceAgentConnecting,
LoginBeforeReady: false,
LifecycleState: codersdk.WorkspaceAgentLifecycleCreated,
TroubleshootingURL: wantURL,
Status: codersdk.WorkspaceAgentConnecting,
StartupScriptBehavior: codersdk.WorkspaceAgentStartupScriptBehaviorBlocking,
LifecycleState: codersdk.WorkspaceAgentLifecycleCreated,
TroubleshootingURL: wantURL,
}

if s := status.Load(); s != "" {
Expand Down Expand Up @@ -239,10 +239,10 @@ func TestAgent_NoWait(t *testing.T) {
WorkspaceName: "example",
Fetch: func(_ context.Context) (codersdk.WorkspaceAgent, error) {
agent := codersdk.WorkspaceAgent{
Status: codersdk.WorkspaceAgentConnecting,
LoginBeforeReady: false,
LifecycleState: codersdk.WorkspaceAgentLifecycleCreated,
TroubleshootingURL: wantURL,
Status: codersdk.WorkspaceAgentConnecting,
StartupScriptBehavior: codersdk.WorkspaceAgentStartupScriptBehaviorBlocking,
LifecycleState: codersdk.WorkspaceAgentLifecycleCreated,
TroubleshootingURL: wantURL,
}

if s := status.Load(); s != "" {
Expand Down Expand Up @@ -292,7 +292,7 @@ func TestAgent_NoWait(t *testing.T) {
require.NoError(t, <-done, "ready - should exit early")
}

func TestAgent_LoginBeforeReadyEnabled(t *testing.T) {
func TestAgent_StartupScriptBehaviorNonBlocking(t *testing.T) {
t.Parallel()

ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort)
Expand All @@ -309,10 +309,10 @@ func TestAgent_LoginBeforeReadyEnabled(t *testing.T) {
WorkspaceName: "example",
Fetch: func(_ context.Context) (codersdk.WorkspaceAgent, error) {
agent := codersdk.WorkspaceAgent{
Status: codersdk.WorkspaceAgentConnecting,
LoginBeforeReady: true,
LifecycleState: codersdk.WorkspaceAgentLifecycleCreated,
TroubleshootingURL: wantURL,
Status: codersdk.WorkspaceAgentConnecting,
StartupScriptBehavior: codersdk.WorkspaceAgentStartupScriptBehaviorNonBlocking,
LifecycleState: codersdk.WorkspaceAgentLifecycleCreated,
TroubleshootingURL: wantURL,
}

if s := status.Load(); s != "" {
Expand Down
2 changes: 1 addition & 1 deletion cli/ssh.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,7 +350,7 @@ func (r *RootCmd) ssh() *clibase.Cmd {
{
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 login before ready option has not been 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.",
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.",
Value: clibase.BoolOf(&noWait),
},
{
Expand Down
6 changes: 3 additions & 3 deletions cli/testdata/coder_ssh_--help.golden
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,9 +26,9 @@ Start a shell into a workspace

--no-wait bool, $CODER_SSH_NO_WAIT
Specifies whether to wait for a workspace to become ready before
logging in (only applicable when the login before ready option has not
been enabled). Note that the workspace agent may still be in the
process of executing the startup script and the workspace may be in an
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.

--stdio bool, $CODER_SSH_STDIO
Expand Down
16 changes: 15 additions & 1 deletion coderd/apidoc/docs.go
View file
Open in desktop

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

13 changes: 12 additions & 1 deletion coderd/apidoc/swagger.json
View file
Open in desktop

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

3 changes: 2 additions & 1 deletion coderd/database/dbauthz/system_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -211,7 +211,8 @@ func (s *MethodTestSuite) TestSystemFunctions() {
}))
s.Run("InsertWorkspaceAgent", s.Subtest(func(db database.Store, check *expects) {
check.Args(database.InsertWorkspaceAgentParams{
ID: uuid.New(),
ID: uuid.New(),
StartupScriptBehavior: database.StartupScriptBehaviorNonBlocking,
}).Asserts(rbac.ResourceSystem, rbac.ActionCreate)
}))
s.Run("InsertWorkspaceApp", s.Subtest(func(db database.Store, check *expects) {
Expand Down
2 changes: 1 addition & 1 deletion coderd/database/dbgen/generator.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func WorkspaceAgent(t testing.TB, db database.Store, orig database.WorkspaceAgen
ConnectionTimeoutSeconds: takeFirst(orig.ConnectionTimeoutSeconds, 3600),
TroubleshootingURL: takeFirst(orig.TroubleshootingURL, "https://example.com"),
MOTDFile: takeFirst(orig.TroubleshootingURL, ""),
LoginBeforeReady: takeFirst(orig.LoginBeforeReady, false),
StartupScriptBehavior: takeFirst(orig.StartupScriptBehavior, "non-blocking"),
StartupScriptTimeoutSeconds: takeFirst(orig.StartupScriptTimeoutSeconds, 3600),
})
require.NoError(t, err, "insert workspace agent")
Expand Down
11 changes: 8 additions & 3 deletions coderd/database/dump.sql
View file
Open in desktop

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

12 changes: 12 additions & 0 deletions .../database/migrations/000125_rename_login_before_ready_to_startup_script_behavior.down.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
BEGIN;

ALTER TABLE workspace_agents ADD COLUMN login_before_ready boolean NOT NULL DEFAULT TRUE;
Copy link
Member

Choose a reason for hiding this comment

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

nit: Do we need begin & commit around this block?

Copy link
Member Author

Choose a reason for hiding this comment

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

I suppose yes, fixed. I'm sometimes on the fence since we're not consistent about always using them. We should probably always enforce it, can't think of a reason not to.


UPDATE workspace_agents SET login_before_ready = CASE WHEN startup_script_behavior = 'non-blocking' THEN TRUE ELSE FALSE END;

ALTER TABLE workspace_agents DROP COLUMN startup_script_behavior;
DROP TYPE startup_script_behavior;

COMMENT ON COLUMN workspace_agents.login_before_ready IS 'If true, the agent will delay logins until it is ready (e.g. executing startup script has ended).';

COMMIT;
12 changes: 12 additions & 0 deletions ...rd/database/migrations/000125_rename_login_before_ready_to_startup_script_behavior.up.sql
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
BEGIN;

CREATE TYPE startup_script_behavior AS ENUM ('blocking', 'non-blocking');
ALTER TABLE workspace_agents ADD COLUMN startup_script_behavior startup_script_behavior NOT NULL DEFAULT 'non-blocking';

UPDATE workspace_agents SET startup_script_behavior = (CASE WHEN login_before_ready THEN 'non-blocking' ELSE 'blocking' END)::startup_script_behavior;

ALTER TABLE workspace_agents DROP COLUMN login_before_ready;

COMMENT ON COLUMN workspace_agents.startup_script_behavior IS 'When startup script behavior is non-blocking, the workspace will be ready and accessible upon agent connection, when it is blocking, workspace will wait for the startup script to complete before becoming ready and accessible.';

COMMIT;
62 changes: 60 additions & 2 deletions coderd/database/models.go
View file
Open in desktop

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

Loading