-
Notifications
You must be signed in to change notification settings - Fork 887
feat: Add built-in PostgreSQL for simple production setup #2345
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
6f71825
to
fd66daf
Compare
Wait, so it's just prod for dev envs? Maybe why not add a flag for wiping all user data, like on dev mode? |
That could help if a person was just testing Coder OSS. So, my final question is this: how do you actually get Coder into prod on machines? Running this is one thing, but how do you control Postgres to make it deployable? Last time I ran it, it gave an error of not giving passwords, when I just installed it a few minutes ago! I almost got it working, but it still failed. This could help, but a guide would be needed if we need to detail and make people understand how this has to work. Whoa got an idea(devilish grin in a positive way), this is a great PR btw! |
5211334
to
39fe970
Compare
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.
Wow, surprised at how little changes were needed, nice! (PS. I did not try running this, only reviewed the code.)
README.md
Outdated
# Automatically sets up PostgreSQL and an external access URL on *.try.coder.app | ||
coder server --postgres-builtin --tunnel |
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.
# Automatically sets up PostgreSQL and an external access URL on *.try.coder.app | |
coder server --postgres-builtin --tunnel | |
# Simple) Automatically sets up PostgreSQL and an external access URL on *.try.coder.app | |
coder server --postgres-builtin --tunnel |
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.
I think this helps indicate they are separate options and not commands to be run sequentially
# Requires a PostgreSQL instance and external access URL | ||
coder server --postgres-url <url> --access-url <url> |
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.
# Requires a PostgreSQL instance and external access URL | |
coder server --postgres-url <url> --access-url <url> | |
# Advanced) Requires a PostgreSQL instance and external access URL | |
coder server --postgres-url <url> --access-url <url> |
README.md
Outdated
# Automatically sets up PostgreSQL and an external access URL on *.try.coder.app | ||
coder server --postgres-builtin --tunnel |
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.
I think this helps indicate they are separate options and not commands to be run sequentially
}) | ||
|
||
t.Run("NoWarningWithRemoteAccessURL", func(t *testing.T) { | ||
t.Parallel() | ||
ctx, cancelFunc := context.WithCancel(context.Background()) | ||
defer cancelFunc() | ||
|
||
root, cfg := clitest.New(t, "server", "--dev", "--tunnel=false", "--address", ":0", "--access-url", "http://1.2.3.4:3000/") | ||
root, cfg := clitest.New(t, "server", "--in-memory", "--address", ":0", "--access-url", "http://1.2.3.4:3000/") |
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.
So --in-memory
still exists
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.
It does but is primarily used for testing.
@@ -47,10 +47,14 @@ To install, run: | |||
curl -fsSL https://coder.com/install.sh | sh | |||
``` | |||
|
|||
Once installed, you can run a temporary deployment in dev mode (all data is in-memory and destroyed on exit): | |||
Once installed, you can start a production deployment with a single command: |
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.
Sorry, but turning the DB off if the single instance of the app goes down is not what a lot of people would consider "production"
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.
Why not?
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.
We do essentially this same thing right now on our dogfood deployment, which I'd consider a production deployment of Coder.
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.
I agree with @f0ssel here -- I'd be more comfortable with a "traditionally-managed" database if I were setting up a prod deployment than just using the embedded one. Doesn't detract from the value of being able to instantly kick the tyres with a real database!
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.
Why though? I believe users should start real, production deployments with this setup. There isn't a technical reason why it's bad practice. Users can just backup the ~/.config/coder/postgres
directory on a CRON.
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.
Fair fair!
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.
Coupling the lifecycle of the database to the app lifecycle and running the DB on the same host are two pretty big red flags for any engineers that have to actually manage and troubleshoot the app in production.
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.
What are the specific red flags? You can still run the database independently.
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.
I'm saying that if a customer wanting to run 1000 devs and it be production ready I'd probably advise them:
- Run the DB on a dedicated host
- Run regular database backups and have a restore process
- Run multiple instances of the app server, optionally on multiple hosts behind a LB, optionally across different AZs.
- Have a way to run new webserver versions without causing downtime
Right now this command not only does none of those things, but also won't even allow you to run multiple instances of the web app since it will conflict on the DB setup.
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.
And similar to why people recommend not putting DB healthchecks in the webserver healthchecks on kubernetes - you don't want your DB taking your webapp down and vice versa, you want their lifecycles independent for obvious reasons.
Fixes #2321.
This removes dev-mode too! See the issue for rationale. I manually tested this on Linux and Windows too!
coder server
coder server --postgres-builtin
(first run)coder server --postgres-builtin
(subsequent run)coder server --postgres-builtin --tunnel
coder server postgres-builtin-url