Skip to content
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

Add prisma Docker target + CI to setup db in staging/prod #325

Merged
merged 1 commit into from
Mar 14, 2023

Conversation

humphd
Copy link
Contributor

@humphd humphd commented Mar 12, 2023

Part of #291

In order to ship to staging/production, we need to be able to set up the database. Doing so requires running Prisma with our schema from the staging/production VMs. This creates a Docker build stage that can be optionally run in a stand-alone way to accomplish this.

Every time we push to main, CI will create both startchart and starchart-prisma images. The latter is used to push the current schema to the database (there might be a better set of commands for us to run, let me know what you think).

We won't do this automatically, but I need this to exist so I can run it manually via Docker.

This change also does the following:

  1. Updates the SHA of the node 18 image we use (they shipped a security update recently)
  2. Adds a db:setup npm script that does everything except seeds the db
  3. Removes QEMU from the Docker jobs, since we aren't cross-compiling anymore

This doesn't need to get looked at until next week as part of 0.6.

@humphd humphd added category: deployment Related to building our local code into a working unit category: containers A tool that helps us make our solutions into separate microservice containers with docker labels Mar 12, 2023
@humphd humphd added this to the Milestone 0.6 milestone Mar 12, 2023
@humphd humphd requested review from shawnyu5, cychu42 and a user March 12, 2023 00:32
@humphd humphd self-assigned this Mar 12, 2023
@ghost
Copy link

ghost commented Mar 13, 2023

@humphd Interesting approach, but do we have to build a separate docker image just for this?

What if we used something like this lib:
https://www.npmjs.com/package/if-env
and decide to run the db:setup based on an external env?

@humphd
Copy link
Contributor Author

humphd commented Mar 13, 2023

@humphd Interesting approach, but do we have to build a separate docker image just for this?

What if we used something like this lib: https://www.npmjs.com/package/if-env and decide to run the db:setup based on an external env?

It's not a good idea to run npm in production (i.e., I don't want to add the extra process invocation when we run the app normally). For the case of the manual setup step, it's not a huge deal, because it is meant to exit on its own (i.e., it's not something we need to manage with signals).

I have to create a different image because I'm not using the usual stages to end at the production image.

Let me know if I'm misunderstanding your comment.

@humphd
Copy link
Contributor Author

humphd commented Mar 13, 2023

@dadolhay thinking about your comment more. What we could do is create a start.sh script, which does what you suggest, and then if you include DATABASE_SETUP=1 DATABASE_URL=mysql://... in the env, we could run the prisma commands for you before starting the app. That's cleaner than what I'm doing. I'll try an update to this.

@ghost
Copy link

ghost commented Mar 13, 2023

@humphd Thanks, that sounds like a much simpler solution

@humphd
Copy link
Contributor Author

humphd commented Mar 13, 2023

I've updated this PR and made some changes:

  1. I've removed the extra docker build step from CI. We'll continue to only have a single image
  2. I've added update-notifier=false to our .npmrc so that npm doesn't warn about newer versions when running in Docker. I've also copied .npmrc into the final Docker stage, so it uses it.
  3. I've copied the prisma/ directory to the final Docker stage, so we have access to the schema and migrations
  4. I've added a docker-entrypoint.sh bash script, which is called in our ENTRYPOINT. This script checks to see if DATABASE_SETUP=1 is set, and if it is, first runs npx prisma db push before starting the app.

Some questions I still have:

  1. is npx prisma db push the correct thing to do? Is there anything more I should do here, or different?
  2. does it make sense to start the container after we run the db setup? I'm not sure if it does. I feel like doing the database setup is a discrete step, separate from running the app, but I'm not sure.

To test this:

  1. Update dev-secrets/DATABASE_URL to be mysql://starchart:[email protected]:3306/starchart since you need to connect to the host from within a container (i.e., don't use localhost or 127.0.0.1)
  2. Build the image: docker build -t starchart .
  3. Run a container based on the image, simulating the secrets and setting the env var: docker run --rm -e DATABASE_SETUP=1 -v $PWD/dev-secrets:/run/secrets starchart. You can also run it without the env: docker run --rm starchart

NOTE: it currently crashes on startup because the env var for DATABASE_URL needs to be secrets instead. I've already fixed this, but need to rebase.

ghost
ghost previously approved these changes Mar 13, 2023
@ghost
Copy link

ghost commented Mar 13, 2023

  • is npx prisma db push the correct thing to do? Is there anything more I should do here, or different?

I think intil we reach the DB baseline, this should be OK. If we encounter errors, we can change it to a full reset, but that could be destruptive to manual testers

  • does it make sense to start the container after we run the db setup? I'm not sure if it does. I feel like doing the database setup is a discrete step, separate from running the app, but I'm not sure.

I think it does, spares an extra step, but this is not a strong opinion

Approved, I like this approach much more, it's a lot simpler

@humphd
Copy link
Contributor Author

humphd commented Mar 13, 2023

Rebased and cleaned up a bit. I like this approach better, too.

@humphd humphd requested a review from a user March 13, 2023 14:43
ghost
ghost previously approved these changes Mar 13, 2023
package.json Outdated
@@ -19,7 +19,8 @@
"db:reset": "prisma migrate reset",
"db:format": "prisma format",
"db:studio": "prisma studio",
"setup": "run-s db:generate db:push db:seed",
"db:setup": "run-s db:generate db:push",
"setup": "run-s db:setup db:seed",
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit. I'm wondering if there are better names to distinguish the two setups. They are both database related, and the only difference is whether we seed, not whether it's database related (db).
Maybe db:setup can be setup:seedless?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not actually using this in my code, so I think I'll revert it back to what it was.

@cychu42
Copy link
Contributor

cychu42 commented Mar 13, 2023

I think intil we reach the DB baseline, this should be OK. If we encounter errors, we can change it to a full reset, but that could be destruptive to manual testers

Agree. If we don't need to worry about migration files or checking for database drift (via shadow database), npx prisma db push should be fine.

does it make sense to start the container after we run the db setup? I'm not sure if it does. I feel like doing the database setup is a discrete step, separate from running the app, but I'm not sure.

I suppose you can look at container as part of the app and therefore make sense to start it as part of starting the app. Let me know if I'm misunderstanding you.

Copy link
Contributor

@cychu42 cychu42 left a comment

Choose a reason for hiding this comment

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

LGTM!

@humphd humphd merged commit d7f9694 into DevelopingSpace:main Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category: containers A tool that helps us make our solutions into separate microservice containers with docker category: deployment Related to building our local code into a working unit
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants