-
Notifications
You must be signed in to change notification settings - Fork 22
Replace login_before_ready
with startup_script_behavior
#125
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 2 commits
609d6e0
386efce
b9521bf
b6f0451
f763469
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,23 +16,31 @@ import ( | |
func agentResource() *schema.Resource { | ||
return &schema.Resource{ | ||
Description: "Use this resource to associate an agent.", | ||
CreateContext: func(c context.Context, resourceData *schema.ResourceData, i interface{}) diag.Diagnostics { | ||
CreateContext: func(ctx context.Context, resourceData *schema.ResourceData, i interface{}) diag.Diagnostics { | ||
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. nit: I'm curious if there is a way to check if BTW It would be nice to write a comment on why we need to call it in both contexts. I think that we usually stick with 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. Both are called, but I never got this method working because the value reverts at the end resulting in This function was actually supposed to be removed in favor of empty default for
mafredri marked this conversation as resolved.
Show resolved
Hide resolved
|
||
// This should be a real authentication token! | ||
resourceData.SetId(uuid.NewString()) | ||
err := resourceData.Set("token", uuid.NewString()) | ||
if err != nil { | ||
return diag.FromErr(err) | ||
} | ||
err = updateStartupScriptBehaviorIfLoginBeforeReady(resourceData) | ||
if err != nil { | ||
return diag.FromErr(err) | ||
} | ||
return updateInitScript(resourceData, i) | ||
}, | ||
ReadWithoutTimeout: func(c context.Context, resourceData *schema.ResourceData, i interface{}) diag.Diagnostics { | ||
ReadWithoutTimeout: func(ctx context.Context, resourceData *schema.ResourceData, i interface{}) diag.Diagnostics { | ||
err := resourceData.Set("token", uuid.NewString()) | ||
if err != nil { | ||
return diag.FromErr(err) | ||
} | ||
err = updateStartupScriptBehaviorIfLoginBeforeReady(resourceData) | ||
if err != nil { | ||
return diag.FromErr(err) | ||
} | ||
return updateInitScript(resourceData, i) | ||
}, | ||
DeleteContext: func(c context.Context, rd *schema.ResourceData, i interface{}) diag.Diagnostics { | ||
DeleteContext: func(ctx context.Context, resourceData *schema.ResourceData, i interface{}) diag.Diagnostics { | ||
return nil | ||
}, | ||
Schema: map[string]*schema.Schema{ | ||
|
@@ -131,11 +139,29 @@ func agentResource() *schema.Resource { | |
Description: "The path to a file within the workspace containing a message to display to users when they login via SSH. A typical value would be /etc/motd.", | ||
}, | ||
"login_before_ready": { | ||
Type: schema.TypeBool, | ||
Default: true, // Change default value to false in a future release. | ||
ForceNew: true, | ||
Optional: true, | ||
Description: "This option defines whether or not the user can (by default) login to the workspace before it is ready. Ready means that e.g. the startup_script is done and has exited. When enabled, users may see an incomplete workspace when logging in.", | ||
// Note: When this is removed, "startup_script_behavior" should | ||
// be set to "non-blocking" by default (instead of empty string). | ||
Type: schema.TypeBool, | ||
Default: true, | ||
ForceNew: true, | ||
Optional: true, | ||
Description: "This option defines whether or not the user can (by default) login to the workspace before it is ready. Ready means that e.g. the startup_script is done and has exited. When enabled, users may see an incomplete workspace when logging in.", | ||
Deprecated: "Configure startup_script_behavior instead. This attribute will be removed in a future version of the provider.", | ||
ConflictsWith: []string{"startup_script_behavior"}, | ||
}, | ||
"startup_script_behavior": { | ||
// Note: Our default value is "non-blocking" but we do not set | ||
// it here because we want to be able to differentiate between | ||
// the user setting this or "login_before_ready". For all | ||
// intents and purposes, until deprecation, setting | ||
// "login_before_ready = false" is equivalent to setting | ||
// "startup_script_behavior = blocking". | ||
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. We will handle this case in coder/coder, if |
||
Type: schema.TypeString, | ||
ForceNew: true, | ||
Optional: true, | ||
Description: "This option sets the behavior of the startup_script. When set to \"blocking\", the startup_script must exit before the workspace is ready. When set to \"non-blocking\", the startup_script may run in the background and the workspace will be ready immediately. Default is \"non-blocking\", although \"blocking\" is recommended.", | ||
ValidateFunc: validation.StringInSlice([]string{"blocking", "non-blocking"}, false), | ||
ConflictsWith: []string{"login_before_ready"}, | ||
}, | ||
"metadata": { | ||
Type: schema.TypeList, | ||
|
@@ -251,3 +277,19 @@ func updateInitScript(resourceData *schema.ResourceData, i interface{}) diag.Dia | |
} | ||
return nil | ||
} | ||
|
||
func updateStartupScriptBehaviorIfLoginBeforeReady(resourceData *schema.ResourceData) error { | ||
if rc := resourceData.GetRawConfig(); !rc.IsNull() { | ||
if attr := rc.GetAttr("login_before_ready"); !attr.IsNull() { | ||
behavior := "non-blocking" | ||
if attr.False() { | ||
behavior = "blocking" | ||
} | ||
err := resourceData.Set("startup_script_behavior", behavior) | ||
if err != nil { | ||
return err | ||
} | ||
} | ||
} | ||
return nil | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,7 +36,7 @@ func TestAgent(t *testing.T) { | |
motd_file = "/etc/motd" | ||
shutdown_script = "echo bye bye" | ||
shutdown_script_timeout = 120 | ||
login_before_ready = false | ||
startup_script_behavior = "blocking" | ||
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. nit: can we add some test cases when 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. Added a bunch of test cases, although since there's no logic now it's a bit silly. 😄 |
||
} | ||
`, | ||
Check: func(state *terraform.State) error { | ||
|
@@ -58,7 +58,7 @@ func TestAgent(t *testing.T) { | |
"motd_file", | ||
"shutdown_script", | ||
"shutdown_script_timeout", | ||
"login_before_ready", | ||
"startup_script_behavior", | ||
} { | ||
value := resource.Primary.Attributes[key] | ||
t.Logf("%q = %q", key, value) | ||
|
Uh oh!
There was an error while loading. Please reload this page.