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

Better dev docker containers, Simpler production docker file, caddy for ssl #136

Merged
merged 12 commits into from
Feb 5, 2025

Conversation

lil5
Copy link
Contributor

@lil5 lil5 commented Jan 2, 2025

  1. New docker files
  2. All use alpine instead of debian for smaller images.
  3. A make file is include for easier server restarts. make dev-restart-server
  4. The frontend container uses chokidar to rebuild after changes have been made automatically.

I tried to keep the changes scoped to just docker.


  • Tested make dev-start docker
  • Tested example raw linux
  • Tested example ssl caddy

Notes: example ssl caddy

Because this version of the docker image isn't on docker, it's a chicken or egg issue.
So to test it I run the old image, but need to change the Caddyfile to:

server40.vps.webdock.cloud {
        respond "hi"
}

server40.vps.webdock.cloud:7000 {
        reverse_proxy http://app:7000
}

And add - 7000:7000 to services -> caddy -> ports

I tested this on a vps with ports 80 443 7000 all available, caddy successfully automatically 🧙 sets up https

@mbaum0
Copy link
Contributor

mbaum0 commented Jan 10, 2025

I've merged this into my fork and have been using the new Makefile as part of my development flow for a week now. It's been a much smoother experience. Thanks for putting this together.

Any idea how to get automatic reloads working for the react frontend?

@balzack
Copy link
Owner

balzack commented Jan 11, 2025

I see this PR makes for a much better development experience. I also want to make sure it won't impact existing deployments.

This is probably an issue with my environment that I need to work through. When I re-deploy with the newly built container I get the following error:
/app/databag/net/server/entrypoint.sh: line 21: go: not found

I also noticed the switch to build for arm64 was removed. This was for multiarch containers allowing deployment to a higher powered RaspberryPi. Perhaps this is covered by the CGO_ENABLED switch, but I want to make sure this use case is still supported.

I appreciate these changes and will merge them in once I can properly test them. Thank you!

@lil5
Copy link
Contributor Author

lil5 commented Jan 11, 2025

@balzack

This is probably an issue with my environment that I need to work through. When I re-deploy with the newly built container I get the following error:
/app/databag/net/server/entrypoint.sh: line 21: go: not found

sounds like you don't have go/bin in your PATH env

@lil5
Copy link
Contributor Author

lil5 commented Jan 11, 2025

I also noticed the switch to build for arm64 was removed. This was for multiarch containers allowing deployment to a higher powered RaspberryPi. Perhaps this is covered by the CGO_ENABLED switch, but I want to make sure this use case is still supported.

I'll take a look to re-add that, I did try earlier but cross compilation wasn't working from my m2 MacBook, maybe my fedora pc will give better results.

@lil5
Copy link
Contributor Author

lil5 commented Jan 12, 2025

Any idea how to get automatic reloads working for the react frontend?

@mbaum0 Last time I attepted this I got very stuck as the api wasn't prefixed with /api/ making the proxy too complex to run a vite dev and proxy to the api.

I'll take another look after v3 is there

@lil5
Copy link
Contributor Author

lil5 commented Jan 12, 2025

@balzack

RUN if [ "$TARGETPLATFORM" = "linux/amd64" ]; then ARCHITECTURE=amd64; elif [ "$TARGETPLATFORM" = "linux/arm64" ]; then ARCHITECTURE=arm64; elif [ "$TARGETPLATFORM" = "linux/arm64" ]; then ARCHITECTURE=aarch64; else ARCHITECTURE=unsupported; fi \

If you're talking about the above command. I'll be adding something like this soon, I noticed building a go binary without explicitly saying which arch it should cross build to just doesn't work well.

CGO_ENABLED=1 is a requirement for sqlite to work, nothing to do with architecture.

@lil5
Copy link
Contributor Author

lil5 commented Jan 12, 2025

@balzack with 645f83a cross build should be a thing now

@balzack
Copy link
Owner

balzack commented Jan 18, 2025

Sorry I have been slow with the PR (mostly because docker is not a strength of mine).

In the docker file, the references to the host paths allows for development optimizations, but I hesitate on this for production. Reproducible builds become more difficult as it depends on the host state.

Does it seem okay to have a self contained production docker file with this version serving as the development container?

@lil5
Copy link
Contributor Author

lil5 commented Jan 21, 2025

I'll revert the yarn and go volumes, you're right it will only make the I make volume dependent

@mbaum0
Copy link
Contributor

mbaum0 commented Feb 5, 2025

@balzack is there anything else blocking this PR?

@balzack
Copy link
Owner

balzack commented Feb 5, 2025

I haven't merged it yet because I wanted to personally validate it with caddy, as I don't use it in my current environment. I was planning on doing that after this next SDK release, but it has grown in scope and taking longer than i planned.

That being said, I see you have already validated it, and I know it to be an improvement, so I will merge it. Thanks and sorry for the delay.

@balzack balzack merged commit 2b3052a into balzack:main Feb 5, 2025
1 check passed
@lil5
Copy link
Contributor Author

lil5 commented Feb 6, 2025

but it has grown in scope and taking longer than i planned.

Please consider archiving that branch and using it as a reference to implement each small change, bit, by, bit.

@balzack
Copy link
Owner

balzack commented Feb 6, 2025

Is the concern around stability of such a large change in the code base, or is it about the changes in your forks that will certainly conflict with the large change? There are no changes in the backend Go app that need to be merged, if that makes things any easier.

The goal of the branch (as you had suggested) was to have a separate SDK that implements the abstract data sharing logic of the network, while the App code implements the service specific logic making it a messaging application. Initially I tried for a bit-by-bit approach, but the changes cascaded into something really hard to manage.

Would it be of value, when the branch is ready, to copy the new client code to { app/sdk, app/client/web, app/client/mobile }? At that point, with a published SDK API, you could advise a manageable way of merging the two application directories ( net/web & app/client/web }

@lil5
Copy link
Contributor Author

lil5 commented Feb 6, 2025

Is the concern around stability of such a large change in the code base

It includes a lot of features at once.

  1. move to vite
  2. move to typescript
  3. create sdk
  4. use sdk for mobile & web
  5. add bot example

while using the sdk is cool I'd suggest getting 1 & 2 out first, then merge the sdk afterwards, then the bot

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