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

Update Docker-compose file #1071

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
[![CircleCI](https://circleci.com/gh/stringer-rss/stringer/tree/main.svg?style=svg)](https://circleci.com/gh/stringer-rss/stringer/tree/main)
[![Code Climate](https://api.codeclimate.com/v1/badges/899c5407c870e541af4e/maintainability)](https://codeclimate.com/github/stringer-rss/stringer/maintainability)
[![Coverage Status](https://coveralls.io/repos/github/stringer-rss/stringer/badge.svg?branch=main)](https://coveralls.io/github/stringer-rss/stringer?branch=main)
![Docker Pulls](https://img.shields.io/docker/pulls/mockdeep/stringer?label=Docker%20Pulls)
[![GitHub Sponsors](https://img.shields.io/github/sponsors/mockdeep?logo=github)](https://github.com/sponsors/mockdeep)

### A self-hosted, anti-social RSS reader.
Expand All @@ -24,7 +25,7 @@ Stringer is a Ruby app based on Rails, PostgreSQL, Backbone.js and GoodJob.
Stringer will run just fine on the Eco/Basic Heroku plans.

Instructions are provided for deploying to [Heroku manually](/docs/Heroku.md), to any Ruby
compatible [Linux-based VPS](/docs/VPS.md), to [Docker](docs/docker.md) and to [OpenShift](/docs/OpenShift.md).
compatible [Linux-based VPS](/docs/VPS.md), to [Docker](docs/Docker.md) and to [OpenShift](/docs/OpenShift.md).

## Niceties

Expand Down
35 changes: 24 additions & 11 deletions docker-compose.yml
Original file line number Diff line number Diff line change
@@ -1,27 +1,40 @@
version: '2'
version: '3.8'

services:
postgres:
image: postgres:9.5-alpine
image: postgres:12-alpine
hostname: stringer-postgres
restart: always
networks:
- stringer
volumes:
- ~/stringer:/var/lib/postgresql/data
- ./postgres:/var/lib/postgresql/data
environment:
- POSTGRES_PASSWORD=super_secret_password
- POSTGRES_PASSWORD=<PLEASE_ENTER_YOUR_PASSWORD>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Personally I don't care too much for changes like these since the password will need to be changed anyway. If we want to make it so that the container won't launch without a password change then we should probably just not set this value

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm fine with cosmetic changes, and this seems like maybe an improvement. If there's a way to force the user to input it, though, so that they can't miss it, that seems even better.

- POSTGRES_USER=db_user
- POSTGRES_DB=stringer

web:
image: mockdeep/stringer
image: mockdeep/stringer:latest
Copy link
Collaborator

Choose a reason for hiding this comment

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

these images should actually move to stringerrss/stringer at some point. Not sure if now is the time, since we're currently unable to publish the image just yet.

build: .
depends_on:
- postgres
restart: always
ports:
- 80:8080
- '127.0.0.1:8080:8080'
Copy link
Collaborator

Choose a reason for hiding this comment

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

This will only work for users who have a reverse proxy on their machine; it's safer this way (and imo should be the way to go) but it's something to be aware of when merging this in. The docker compose file doesn't actually have a reverse proxy listed in it.

networks:
- stringer-network
environment:
- SECRET_KEY_BASE=<your configuration>
- ENCRYPTION_PRIMARY_KEY<your configuration>
- ENCRYPTION_DETERMINISTIC_KEY=<your configuration>
- ENCRYPTION_KEY_DERIVATION_SALT=<your configuration>
- SECRET_KEY_BASE=$(openssl rand -hex 64)
- ENCRYPTION_PRIMARY_KEY=$(openssl rand -hex 64)
- ENCRYPTION_DETERMINISTIC_KEY=$(openssl rand -hex 64)
- ENCRYPTION_KEY_DERIVATION_SALT=$(openssl rand -hex 64)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not a huge fan of this - this will generate new keys every time. So if someone accidentally re-ups the container, I think it ends up generating new keys and so their data is now locked behind (lost) encryption keys.

- FETCH_FEEDS_CRON="*/5 * * * *" # optional
- CLEANUP_CRON="0 0 * * *" # optional
- PORT=8080
- DATABASE_URL=postgres://db_user:super_secret_password@postgres:5432/stringer
- DATABASE_URL=postgres://db_user:<PLEASE_ENTER_YOUR_PASSWORD>@stringer-postgres:5432/stringer

networks:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you think you could separate out some of these changes into different PRs? I think it would be helpful to discuss them independently so that I can get a better understanding of what each of them is doing. I'm sure some of them would be easy to merge, but others seem like they might cause more problems for our users and I'd like to think about them more.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey no problem, I can separate into :

  • Pg upgrade and location
  • Cosmetic Improve (README, etc...)
  • Docker Image source

If you validate this I will do it during the week !

Copy link
Collaborator

Choose a reason for hiding this comment

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

That sounds reasonable. I think the network updates might be a good 4th PR, too.

stringer:
external: false
name: stringer
13 changes: 7 additions & 6 deletions docs/docker.md → docs/Docker.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

## Production ready setup using docker-compose

Download [docker-compose.yml](../docker-compose.yml) and in the corresponding folder, run `docker-compose up -d`, give it a second and visit `localhost`
Download [docker-compose.yml](../docker-compose.yml) and in the corresponding folder, run `docker-compose up -d`, give it a second and visit `localhost:8080`

## Production ready manual setup

Expand All @@ -20,11 +20,12 @@ The following steps can be used to setup Stringer on Docker, using a Postgres da
docker run --detach \
--name stringer-postgres \
--restart always \
--volume /srv/stringer/data:/var/lib/postgresql/data \
--volume ./postgres:/var/lib/postgresql/data \
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you explain this change to me? I'm not at all familiar with docker, so not sure why this would change.

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 thought it would be better to gives to people a command which isn't install things all around your disks. Like that a postgres folder will be created to handle the docker volume at the current working directory.

I can revert this part if you want !

Copy link
Collaborator

Choose a reason for hiding this comment

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

How will this impact users who have existing installations? Will they need to move their data directory? I think ideally we wouldn't make upgrades more trouble, at least until other upgrade issues are resolved.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Existing users will need to move their directory from /srv/stringer/data to ./postgres, yeah. I think this is a better default, though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I noticed in the docker-compose that the default dir there is ~/stringer which imo is an even better default.

--net stringer \
-e POSTGRES_PASSWORD=myPassword \
-e POSTGRES_PASSWORD=<PLEASE_ENTER_YOUR_PASSWORD> \
-e POSTGRES_USER=db_user \
-e POSTGRES_DB=stringer \
postgres:9.5-alpine
postgres:12-alpine
Copy link
Collaborator

Choose a reason for hiding this comment

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

How come you chose version 12 instead of the most recent 15?

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 had an issues with postgres 9 which update all my folder permissions. Apparently it was fixed in 12 version, and it's the first version with the fix. May be you can use 15 version, but I haven't try it.

I could do some test if you want !

Copy link
Collaborator

Choose a reason for hiding this comment

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

@Ni-g-3l sure, that would be helpful! I'm running 15.3 with my instance. It's not a huge deal, though, we can always upgrade more later.

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 just upgraded my instance, let's pass this week and see what will happen.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

First issues incomming, you can't point to same folder postgres12 and postgres15
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

I see. I wonder if upgrading to 12 will cause issues for upgrading users, too. I guess we can leave it at 12 for now and see how things go.

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is more because of an upgrade of postgres using the same data directory, rather than a problem with postgres 15 itself. So updating from 9 to 12 is already going to give issues, I think we may as well do 15.

I think this is probably where a stringer export comes in - we could export the data, upgrade to a newer version of postgres, and then re-import.

imo, we should set this to 15. If a new user comes to stringer, they'll want the latest version of postgres; if a previous user is updating, they should know that they need to back up their system and that changing postgres versions incurs problems. We may want to announce this in some way in the next release though, as a PSA.

Copy link
Collaborator

Choose a reason for hiding this comment

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

(also, fwiw, someone else I know from the FOSS community created this repo that auto-upgrades postgres without issues like this, and then runs the container as postgres normally would. But it's an additional layer of complexity, and I imagine postgres will eventually coalesce things into their docker image)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Hmm, I'm not sure what the best approach here is. I'm imagining the upgrade process would be smoother if it was more gradual, but that seems like a pretty lengthy process. That repo you linked could be really handy to smooth that process.

We may want to announce this in some way in the next release though, as a PSA.

I unfortunately haven't figured out a good release process yet. Maybe we need to set up a changelog and tagging somehow. It's a little weird, though, since we aren't releasing a gem or the like.

```

3. Run the Stringer Docker image:
Expand All @@ -35,15 +36,15 @@ docker run --detach \
--net stringer \
--restart always \
-e PORT=8080 \
-e DATABASE_URL=postgres://postgres:myPassword@stringer-postgres/stringer \
-e DATABASE_URL=postgres://db_user:<PLEASE_ENTER_YOUR_PASSWORD>@stringer-postgres/stringer \
-e SECRET_KEY_BASE=$(openssl rand -hex 64) \
-e ENCRYPTION_PRIMARY_KEY=$(openssl rand -hex 64) \
-e ENCRYPTION_DETERMINISTIC_KEY=$(openssl rand -hex 64) \
-e ENCRYPTION_KEY_DERIVATION_SALT=$(openssl rand -hex 64) \
-e FETCH_FEEDS_CRON="*/5 * * * *" \ # optional
-e CLEANUP_CRON="0 0 * * *" \ # optional
-p 127.0.0.1:8080:8080 \
stringer-rss/stringer
mockdeep/stringer:latest
```

That's it! You now have a fully working Stringer instance up and running!
Expand Down