docs(templates): document `startup_script_behavior` in-depth #7857

mafredri · 2023-06-05T16:35:11Z

BrunoQuaresma

Very well written!

matifali · 2023-06-06T14:09:04Z

docs/templates/index.md

+# Notice: var.repo and var.dotfiles_uri are specified elsewhere in the Terraform
+# code as input variables.
+REPO=${var.repo}
+DOTFILES_URI=${var.dotfiles_uri}


Suggested change

DOTFILES_URI=${var.dotfiles_uri}

DOTFILES_URI=${data.coder_parameter.dotfiles_uri}

Should we suggest using the coder_parameter resource here as we do not support terraform variables anymore for workspace-level variables?

I think that's a good idea, but I'd like to avoid making the change in this PR. Perhaps we should check if there's other places this correction needs to be done as well and do them all in one go?

matifali · 2023-06-06T14:10:42Z

docs/templates/index.md

+
+Use the Coder agent's `startup_script_behavior` to change the behavior between `blocking` and `non-blocking` (default). The blocking behavior is recommended for most use cases because it allows the startup script to complete before the user accesses the workspace. For example, let's say you want to check out a very large repo in the startup script. If the startup script is non-blocking, the user may log in via SSH or open the IDE before the repo is fully checked out. This can lead to a poor user experience.
+
+Whichever behavior is enabled, the user can still choose to override it by specifying the appropriate flags (or environment variables) in the CLI when connecting to the workspace. For example, `coder ssh --no-wait` will connect to the workspace without waiting for the startup script to complete.


Should we specify which are the appropriate environment variables here?

I wanted to keep this terse, so perhaps we should link to the troubleshooting section instead that already mentions them?

matifali

Except the 2 comments the document is very clear and easy to understand. ❤️

johnstcn

Nice work! Just a few comments.

docs/templates/index.md

johnstcn · 2023-06-06T13:50:41Z

docs/templates/index.md

+
+Here are a few guidelines for writing a good startup script (more on these below):
+
+1. Use `set -e` to exit the script if any command fails and `|| true` for commands that are allowed to fail


Would it make sense to recommend "unofficial Bash strict mode" (i.e. set -euo pipefail)?

Do you have a good link for it? Maybe we can link it as an apropos. Otherwise I think it's better to K.I.S.S. We don't want to end up writing a how-to-bash. 😅

As it's "unofficial", it's hard to find a good link. I like jevans' illustrated reasoning: https://wizardzines.com/comics/bash-errors/

But I agree, we don't want this to devolve into a full Bash scripting tutorial. At most, we link to https://tldp.org/LDP/abs/html/

Haha, I really want to link that comic now 😆

johnstcn · 2023-06-06T14:05:40Z

docs/templates/index.md

  EOT
 }
 ```

+The startup script can contain important steps that must be executed successfully so that the workspace is in a usable state, for this reason we recommend using `set -e` (exit on error) at the top and `|| true` (allow command to fail) to ensure the user is notified when something goes wrong. These are not shown in the example above because, while useful, they need to be used with care.


Should we just recommend full "unofficial strict mode" and recommend folks use shellcheck?

I expanded on this, LMKWYT.

PS. The downside with the expanded part, it technically could require a disclaimer, which I didn't put in.

Note that this only applies when using Bash or another advanced shell that supports this feature. If the users shell is /bin/sh then this will likely fail catastrophically.

Note^2 it can work with /bin/sh because often it's linked to bash

Again, we don't want to teach shell, but we also want to nudge in the right direction. Though problem.

johnstcn · 2023-06-06T14:11:14Z

docs/templates/index.md

  EOT
 }
 ```

+The startup script can contain important steps that must be executed successfully so that the workspace is in a usable state, for this reason we recommend using `set -e` (exit on error) at the top and `|| true` (allow command to fail) to ensure the user is notified when something goes wrong. These are not shown in the example above because, while useful, they need to be used with care.
+
+We also recommend that startup scripts always have an end, meaning that long running processes should be run in the background. This is usually achieved by adding `&` to the end of the command. For example, `sleep 10 &` will run the command in the background and allow the startup script to complete.


Suggested change

We also recommend that startup scripts always have an end, meaning that long running processes should be run in the background. This is usually achieved by adding `&` to the end of the command. For example, `sleep 10 &` will run the command in the background and allow the startup script to complete.

We also recommend that startup scripts exit as quickly as possible. Long-running processes should be run in the background. This is usually achieved by adding `&` to the end of the command. For example, `sleep 10 &` will run the command in the background and allow the startup script to complete.

I like the idea, but I think the fixup may give the wrong idea. We don't want people writing git clone [big repo] & so that the script completes ASAP. I'll noodle on it. 👍🏻

"Should exit as quickly as possible while leaving the workspace in a ready-to-use state"?

That's definitely better. Although I think it'd be simpler to invert it:

We also recommend that startup scripts do not run forever.

Thoughts? I think both your suggestion and this one are good a fit.

:D I like this one!

docs/templates/index.md

johnstcn · 2023-06-06T14:14:07Z

docs/templates/index.md

 ```

+This script tells us what command is being run and what the exit status is. If the exit status is non-zero, it means the command failed. Note that here we don't need `set -x` because we're manually echoing the commands which protects against sensitive information being shown in the log. We also don't need `set -e` because we're manually checking the exit status and exiting if it's non-zero.


Note that here we don't need set -x because we're manually echoing the commands which protects against sensitive information being shown in the log.

I think this is worth emphasising even more e.g. with a > **Note:**

Co-authored-by: Cian Johnston <cian@coder.com>

johnstcn

🌟

github-actions bot assigned mafredri Jun 5, 2023

mafredri changed the title ~~docs(templates): Document startup_script_behavior in-depth~~ docs(templates): document startup_script_behavior in-depth Jun 5, 2023

docs(templates): Document startup_script_behavior in-depth

50b4a64

Fixes #7759

mafredri force-pushed the mafredri/docs-document-startup-script-behavior branch from ec07da6 to 50b4a64 Compare June 6, 2023 13:32

Improve unable to access section

048a988

mafredri marked this pull request as ready for review June 6, 2023 13:41

mafredri requested review from BrunoQuaresma and bpmct June 6, 2023 13:42

BrunoQuaresma approved these changes Jun 6, 2023

View reviewed changes

Markdown link

9eb85f3

matifali reviewed Jun 6, 2023

View reviewed changes

matifali approved these changes Jun 6, 2023

View reviewed changes

johnstcn reviewed Jun 6, 2023

View reviewed changes

mafredri and others added 3 commits June 6, 2023 17:46

Update docs/templates/index.md

0eac1c3

Co-authored-by: Cian Johnston <cian@coder.com>

Update docs/templates/index.md

6560e08

Co-authored-by: Cian Johnston <cian@coder.com>

Improvements based on PR comments

d59549f

johnstcn approved these changes Jun 6, 2023

View reviewed changes

mafredri merged commit a77b48a into main Jun 7, 2023

mafredri deleted the mafredri/docs-document-startup-script-behavior branch June 7, 2023 09:29

github-actions bot locked and limited conversation to collaborators Jun 7, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

docs(templates): document `startup_script_behavior` in-depth #7857

docs(templates): document `startup_script_behavior` in-depth #7857

mafredri commented Jun 5, 2023 •

edited by ammario

Loading

BrunoQuaresma left a comment

matifali Jun 6, 2023

mafredri Jun 6, 2023

matifali Jun 6, 2023

mafredri Jun 6, 2023

matifali left a comment

johnstcn left a comment

johnstcn Jun 6, 2023

mafredri Jun 6, 2023

johnstcn Jun 6, 2023

mafredri Jun 6, 2023

johnstcn Jun 6, 2023

mafredri Jun 6, 2023

mafredri Jun 6, 2023 •

edited

Loading

johnstcn Jun 6, 2023

mafredri Jun 6, 2023 •

edited

Loading

johnstcn Jun 6, 2023

mafredri Jun 6, 2023

johnstcn Jun 6, 2023

johnstcn Jun 6, 2023

johnstcn left a comment

	DOTFILES_URI=${var.dotfiles_uri}
	DOTFILES_URI=${data.coder_parameter.dotfiles_uri}


		Use the Coder agent's `startup_script_behavior` to change the behavior between `blocking` and `non-blocking` (default). The blocking behavior is recommended for most use cases because it allows the startup script to complete before the user accesses the workspace. For example, let's say you want to check out a very large repo in the startup script. If the startup script is non-blocking, the user may log in via SSH or open the IDE before the repo is fully checked out. This can lead to a poor user experience.

		Whichever behavior is enabled, the user can still choose to override it by specifying the appropriate flags (or environment variables) in the CLI when connecting to the workspace. For example, `coder ssh --no-wait` will connect to the workspace without waiting for the startup script to complete.


		Here are a few guidelines for writing a good startup script (more on these below):

		1. Use `set -e` to exit the script if any command fails and `\|\| true` for commands that are allowed to fail

	We also recommend that startup scripts always have an end, meaning that long running processes should be run in the background. This is usually achieved by adding `&` to the end of the command. For example, `sleep 10 &` will run the command in the background and allow the startup script to complete.
	We also recommend that startup scripts exit as quickly as possible. Long-running processes should be run in the background. This is usually achieved by adding `&` to the end of the command. For example, `sleep 10 &` will run the command in the background and allow the startup script to complete.

		```

		This script tells us what command is being run and what the exit status is. If the exit status is non-zero, it means the command failed. Note that here we don't need `set -x` because we're manually echoing the commands which protects against sensitive information being shown in the log. We also don't need `set -e` because we're manually checking the exit status and exiting if it's non-zero.

docs(templates): document startup_script_behavior in-depth #7857

docs(templates): document startup_script_behavior in-depth #7857

Conversation

mafredri commented Jun 5, 2023 • edited by ammario Loading

BrunoQuaresma left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matifali left a comment

Choose a reason for hiding this comment

johnstcn left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mafredri Jun 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mafredri Jun 6, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

johnstcn left a comment

Choose a reason for hiding this comment

docs(templates): document `startup_script_behavior` in-depth #7857

docs(templates): document `startup_script_behavior` in-depth #7857

mafredri commented Jun 5, 2023 •

edited by ammario

Loading

mafredri Jun 6, 2023 •

edited

Loading

mafredri Jun 6, 2023 •

edited

Loading