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

docker compose rework #256

Draft
wants to merge 3 commits into
base: develop
Choose a base branch
from
Draft

docker compose rework #256

wants to merge 3 commits into from

Conversation

jimmcgaw
Copy link
Collaborator

@jimmcgaw jimmcgaw commented Feb 12, 2023

What's in this PR:

  1. A Dockerfile was created for building the leapchat server as a go binary docker image.
  2. A Dockerfile was created for building a frontend image.
  3. Updated the docker compose file to build the frontend image, go server image, as well as the two database images.
  4. Updated the docker-compose file database image steps to include passing the PostgREST config and running the init script in the postgres container.

WIP: have the sql init shell script run in the postgres container when it boots up.

@jimmcgaw jimmcgaw requested a review from elimisteve February 12, 2023 02:05
.gitignore Outdated Show resolved Hide resolved
Makefile Outdated Show resolved Hide resolved
@elimisteve
Copy link
Member

Nice PR! Thank you.

I think I have a permanent solution to both the miniware/ issue and a temporary fix for the React dependency hell issue that's good enough for the foreseeable future.

If I solve the former, shall I just push it to this branch? Or shall I branch off of it so you can take a look?

@jimmcgaw
Copy link
Collaborator Author

Nice PR! Thank you.

I think I have a permanent solution to both the miniware/ issue and a temporary fix for the React dependency hell issue that's good enough for the foreseeable future.

If I solve the former, shall I just push it to this branch? Or shall I branch off of it so you can take a look?

Feel free to add whatever to this branch. ⚡

@elimisteve
Copy link
Member

@jimmcgaw Just pushed! Got the Go build working. docker compose up now almost works 😄

@elimisteve
Copy link
Member

@jimmcgaw Just pushed! Got the Go build working. docker compose up now almost works smile

@jimmcgaw Want to take it from here with the dockerization? While I work on dependency hell sheeit.

frontend/package.json Outdated Show resolved Hide resolved
@elimisteve
Copy link
Member

What do you think is the best way to make it so we can modify LeapChat source files on our host but have the dev server that's running in Docker update when something changes? Shall we attach a volume (that is the folder containing the source code on the host) to the frontend container from the outside?

@elimisteve
Copy link
Member

The database steps might need tweaking. This is just a starting point for doing local development with docker.

Excellent.

I believe the main reason why the Docker config didn't work in the past is because db/init_sql.sh is never executed inside the postgres container.

Just created an issue for this: #257 ...and added it to this new milestone: https://github.com/cryptag/leapchat/milestone/7

@jimmcgaw jimmcgaw changed the title docker rework first steps; update frontend packages in separate directory docker compose rework Feb 12, 2023
Dockerfile.dev Outdated Show resolved Hide resolved
@elimisteve
Copy link
Member

I think we need to create Dockerfile.postgrest.dev and have it copy db/postgrest.conf into the postgrest container when building it.

@elimisteve
Copy link
Member

Created an issue for that: #259

@elimisteve
Copy link
Member

elimisteve commented Feb 12, 2023

Alright, this PR and branch are all yours 😅 .

In my mind the last remaining things needed to do here in the milestone I made for dockerization: https://github.com/cryptag/leapchat/milestone/7

@jimmcgaw
Copy link
Collaborator Author

I think we need to create Dockerfile.postgrest.dev and have it copy db/postgrest.conf into the postgrest container when building it.

I don't think we need to do that. I create a volume that mounts the /db directory into the container, and a command that runs the postgrest command with the conf file, where it's mounted in the container, when it starts up. If that doesn't work I'll tweak it.

@jimmcgaw
Copy link
Collaborator Author

The database steps might need tweaking. This is just a starting point for doing local development with docker.

Excellent.

I believe the main reason why the Docker config didn't work in the past is because db/init_sql.sh is never executed inside the postgres container.

Just created an issue for this: #257 ...and added it to this new milestone: https://github.com/cryptag/leapchat/milestone/7

Yep. I started a volume mount in postgres container that mounts to /docker-entrypoint-initdb.d/, which is the directory that a postgres container will run scripts out of, if they are present, when initialized. I haven't confirmed that this works yet.

@jimmcgaw
Copy link
Collaborator Author

What do you think is the best way to make it so we can modify LeapChat source files on our host but have the dev server that's running in Docker update when something changes? Shall we attach a volume (that is the folder containing the source code on the host) to the frontend container from the outside?

For the frontend stuff, yes.

For the go, the ideal workflow isn't clear to me yet. I don't think I've ever worked with Docker with a compiled (ie not interpreted or JIT compiled) language. Any ideas there?

docker-compose.yml Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
postgres:
image: postgres:latest
ports:
- 127.0.0.1:5432:5432
- 5432:5433
Copy link
Member

Choose a reason for hiding this comment

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

@jimmcgaw I think these should be reversed, yes? To avoid port collisions on the server.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Yep, got these backwards. 🙅

Copy link
Member

Choose a reason for hiding this comment

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

👍

Dockerfile.dev Outdated

EXPOSE 8080

CMD ["/leapchat"]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@elimisteve So you're right, npm run dev just does a build to the /build directory. The leapchat binary just serves content directly from there.

I removed the Docker frontend container / file. I don't think it makes sense right here, given the current workflow.

Instead, in the immediate-term, I think we can create a volume in this container that binds build to a directory next to the binary in the container? npm start would then run the watch-build loop and update the build dir in the go container.

Copy link
Member

@elimisteve elimisteve Feb 13, 2023

Choose a reason for hiding this comment

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

@jimmcgaw Removing that other container and mounting build into the Go container sounds perfect.

The only detail I'd add is: instead of putting the leapchat Go binary at /leapchat, let's put it next to the other *.go files just so that leapchat is running in the same environment everywhere: in Docker, on my laptop, and on the production server. I mention that here because this implies putting the build directory right next to leapchat and thus inside the /app/github.com/cryptag/leapchat directory.

Sound good?

EDIT: I expanded on and improved on this idea below.

Copy link
Member

Choose a reason for hiding this comment

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

In that case I think we just need this:

FROM golang:1.19-alpine

RUN mkdir -p /app/github.com/cryptag/leapchat/miniware

WORKDIR /app/github.com/cryptag/leapchat/

COPY miniware/* miniware/
COPY *.go .
COPY go.* .
COPY LICENSE.md .

RUN go build

EXPOSE 8080

CMD ["./leapchat"]

And then to mount ./build as /app/github.com/cryptag/leapchat/build inside the container. (I believe /app/github.com/cryptag/leapchat/build doesn't need to and indeed probably shouldn't exist inside the container, since mounting a volume to that directory would overwrite it or maybe not work?)

Copy link
Member

Choose a reason for hiding this comment

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

Actually, if this works we can make it more like you suggested above, with a simpler directory structure in the container:

FROM golang:1.19-alpine

RUN mkdir -p /leapchat/miniware

WORKDIR /leapchat

COPY miniware/* miniware/
COPY *.go .
COPY go.* .
COPY LICENSE.md .

RUN go build

EXPOSE 8080

CMD ["./leapchat"]

Copy link
Member

@elimisteve elimisteve left a comment

Choose a reason for hiding this comment

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

Looking pretty great! See comments.

postgres:
image: postgres:latest
ports:
- 127.0.0.1:5432:5432
- 5432:5433
Copy link
Member

Choose a reason for hiding this comment

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

👍

Dockerfile.dev Outdated

EXPOSE 8080

CMD ["/leapchat"]
Copy link
Member

@elimisteve elimisteve Feb 13, 2023

Choose a reason for hiding this comment

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

@jimmcgaw Removing that other container and mounting build into the Go container sounds perfect.

The only detail I'd add is: instead of putting the leapchat Go binary at /leapchat, let's put it next to the other *.go files just so that leapchat is running in the same environment everywhere: in Docker, on my laptop, and on the production server. I mention that here because this implies putting the build directory right next to leapchat and thus inside the /app/github.com/cryptag/leapchat directory.

Sound good?

EDIT: I expanded on and improved on this idea below.

Dockerfile.dev Outdated

EXPOSE 8080

CMD ["/leapchat"]
Copy link
Member

Choose a reason for hiding this comment

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

In that case I think we just need this:

FROM golang:1.19-alpine

RUN mkdir -p /app/github.com/cryptag/leapchat/miniware

WORKDIR /app/github.com/cryptag/leapchat/

COPY miniware/* miniware/
COPY *.go .
COPY go.* .
COPY LICENSE.md .

RUN go build

EXPOSE 8080

CMD ["./leapchat"]

And then to mount ./build as /app/github.com/cryptag/leapchat/build inside the container. (I believe /app/github.com/cryptag/leapchat/build doesn't need to and indeed probably shouldn't exist inside the container, since mounting a volume to that directory would overwrite it or maybe not work?)

Makefile Outdated
docker compose up -d --build

cleanv:
docker compose down -v
Copy link
Member

Choose a reason for hiding this comment

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

(Copied from the email I just sent since this is better context for it --)

How about if I [some time soon, but not in this branch right now] add make release to cut a new (tagged, versioned) release, and make deploy to create a production tarball then scp it to the server?

Copy link
Member

@elimisteve elimisteve left a comment

Choose a reason for hiding this comment

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

Oops, I meant for these to be one-off comments, not part of a new review

docker-compose.yml Outdated Show resolved Hide resolved
Dockerfile.dev Outdated

EXPOSE 8080

CMD ["/leapchat"]
Copy link
Member

Choose a reason for hiding this comment

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

Actually, if this works we can make it more like you suggested above, with a simpler directory structure in the container:

FROM golang:1.19-alpine

RUN mkdir -p /leapchat/miniware

WORKDIR /leapchat

COPY miniware/* miniware/
COPY *.go .
COPY go.* .
COPY LICENSE.md .

RUN go build

EXPOSE 8080

CMD ["./leapchat"]

leapchat.go Show resolved Hide resolved
Makefile Show resolved Hide resolved

WORKDIR /app/github.com/cryptag/leapchat/miniware

WORKDIR /app/github.com/cryptag/leapchat
Copy link
Member

Choose a reason for hiding this comment

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

@jimmcgaw I believe we want the 3 lines above into these 2, so that our working directory exists and so we don't have the additional WORKDIR:

RUN mkdir -p /app/github.com/cryptag/leapchat/miniware
WORKDIR /app/github.com/cryptag/leapchat

Copy link
Member

Choose a reason for hiding this comment

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

Then I think this PR is ready to merge! Is it working well for you locally?

Copy link
Member

Choose a reason for hiding this comment

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

Oh yeah there needs to be a bit more done: https://github.com/cryptag/leapchat/milestone/7

@elimisteve
Copy link
Member

elimisteve commented Feb 13, 2023

For the go, the ideal workflow isn't clear to me yet. I don't think I've ever worked with Docker with a compiled (ie not interpreted or JIT compiled) language. Any ideas there?

The Go binary will need to change so infrequently, and it compiles so quickly, that I say we don't worry about having anything watch the file system for changes to .go files; let's just manually rebuild the Go container when we need to.

Sound good?

@bhagwatis
Copy link

New to docker and postgrest. I have installed new version of docker. Containers installed posgresql, pgadmin4 and postgrest. Postgrest does not start in the container nor am i able to ping that specific port. Can someone please help me here?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants