Skip to content

Conversation

finestructure
Copy link
Member

@finestructure finestructure commented Jan 2, 2025

Adds a redis service to app.yml.

@cla-bot cla-bot bot added the cla-signed label Jan 2, 2025
@finestructure
Copy link
Member Author

I'll ad hoc deploy this from the branch to see how it goes in dev.

REDIS_ARGS: '--maxmemory 4GB --maxmemory-policy allkeys-lru'
ports:
- '6379:6379'
- '8001:8001'
Copy link
Member Author

Choose a reason for hiding this comment

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

We'll need to clarify the situation around the ports before deploying this in prod. I don't think they're open due to the firewall sitting in front of it but it needs double checking.

We should be able to connect to the 8001 port for the insights GUI via an ssh tunnel/port forward. I did that i the past but not in a while - need to dig up the commands again.

Copy link
Collaborator

Choose a reason for hiding this comment

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

If it’s a swarm, there may need to be firewalls opened between the hosts - I don’t think it’s available in prod based on the current network security groups.

Copy link
Member Author

Choose a reason for hiding this comment

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

All the swarm hosts are in the same NSG and I'm pretty sure we only need to open 8001 (to Bastion) in case we want to use the Redis GUI.

I've verified that this is working as intended on dev (which is technically also a swarm albeit with just one node).

@finestructure
Copy link
Member Author

Having some trouble getting this to work with service order. I suspect depends_on is looking for a health check and the redis container doesn't have one.

I think I know how to fix this but I'll leave it until tomorrow.

@heckj
Copy link
Collaborator

heckj commented Jan 3, 2025

did some reading when I got settled, and depends_on without a health check only verified start order, as you presumed. I found this example of using a CLI heathcheck for redis:

redis:
image: redis:latest
ports: - "6379:6379"
healthcheck:
test: ["CMD", "redis-cli", "ping"]
interval: 10s
timeout: 5s
retries: 3
start_period: 10s

@finestructure finestructure force-pushed the add-redis-cache branch 3 times, most recently from 4d8466c to 8371d00 Compare January 3, 2025 08:27
@finestructure
Copy link
Member Author

Ok, this is now working:

[ INFO ] Redis set/get result: 2025-01-03 08:43:49 +0000 [component: server]

The issue was that docker swarm deploy is stuck on the v3 docker compose file format and didn't support the new keys I was trying.

And using just redis as a depends_on entry is problematic, because it seems that the default redis container doesn't implement a health check which this relies on. So what happens is that the server doesn't launch if that dependency is set because it doesn't see redis being healthy.

That'd be fixable but I've simply worked around it by putting the temporary test write in a Task { } with a delay.

We won't really have to worry about launch order for the real deployment so this is a non-issue.

@finestructure
Copy link
Member Author

Ok, connecting to the redis GUI also works via

ssh -N -i .ssh/bastion_id_rsa -L 8001:d1:8001 docker-user@bastion

The port is only open to connections from bastion.

Screenshot 2025-01-03 at 10 10 49

networks:
- backend
networks:
- backend
Copy link
Member Author

Choose a reason for hiding this comment

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

The alerting: section was indented with 4 spaces rather than 2 spaces like the rest.

@finestructure finestructure marked this pull request as ready for review January 3, 2025 09:28
@finestructure finestructure merged commit ff67a00 into main Jan 3, 2025
6 checks passed
@finestructure finestructure deleted the add-redis-cache branch January 3, 2025 10:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants