-
Notifications
You must be signed in to change notification settings - Fork 889
chore(examples): Add login_before_ready
and startup_script_timeout
#5880
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
Conversation
# install and start code-server | ||
curl -fsSL https://code-server.dev/install.sh | sh -s -- --version 4.8.3 | tee code-server-install.log |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed tee
here because install log is already available in agent startup script log (and before this refactor, this file was overwritten by the following tee
.
curl -fsSL https://code-server.dev/install.sh | sh -s -- --version 4.8.3 | tee code-server-install.log | ||
code-server --auth none --port 13337 | tee code-server-install.log & | ||
curl -fsSL https://code-server.dev/install.sh | sh -s -- --version 4.8.3 | ||
code-server --auth none --port 13337 >/tmp/code-server.log 2>&1 & |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redirecting both stdout and stderr for log. Could also use 2>&1 | tee /tmp/code-server.log
if preferable.
os = "linux" | ||
dir = "/home/coder" | ||
startup_script = <<EOT | ||
#!/bin/bash |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed all shebangs because I didn't want users to get the wrong idea that we obey the shebang. In unix, we should run script via /bin/bash
if it has this header, but we don't. Likewise for #!/bin/sh
we should use /bin/sh
, but we don't.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to test this against dev.coder.com before we merge? I was under the impression that background jobs would be killed when the parent process is, but I don't understand the inner workings of how Coder runs the startup script so I might be missing something.
@bpmct I could make the dogfood changes a separate PR, but we've already been using this pattern for a long time in the dev.coder.com default dogfood template. It launches code-server via & and the startup script will end. I imagine some version of /bin/sh may behave such that it will not exit until the child process exits, but most of the time this is not the case. A simple test:
This works fine and the sleep command keeps running. I've also tested all the docker templates in my home deployment and they seem to behave correctly. |
delay_login_until_ready
and startup_script_timeout
login_before_ready
and startup_script_timeout
Refs: #5749
Depends on: #5749
delay_login_until_ready
andstartup_script_timeout