Page MenuHomePhabricator

[services] Tunnelbroker - RabbitMQ Docker image for dev environment
ClosedPublic

Authored by max on May 9 2022, 2:47 AM.
Tags
None
Referenced Files
Unknown Object (File)
Fri, Dec 20, 2:02 AM
Unknown Object (File)
Fri, Dec 20, 1:00 AM
Unknown Object (File)
Sun, Dec 8, 7:38 PM
Unknown Object (File)
Wed, Dec 4, 2:40 PM
Unknown Object (File)
Wed, Dec 4, 2:40 PM
Unknown Object (File)
Wed, Dec 4, 2:38 PM
Unknown Object (File)
Wed, Dec 4, 2:38 PM
Unknown Object (File)
Wed, Dec 4, 2:38 PM

Details

Summary

For a local Tunnelbroker development environment, we need to use a local instance of the RabbitMQ server.
Add the RabbitMQ docker image after the localstack image to the docker-compose.
AMQP(s) ports 5672 and 5671 are forwarded to use in local or docker builds.
The rabbitMQ management console is listening on localhost:15672.
The local image user and password are set to comm:comm.

Test Plan

Successfully connected to the rabbitMQ by AMQP client and logged into the local management web-gui.

Diff Detail

Repository
rCOMM Comm
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

tomek requested changes to this revision.May 9 2022, 7:14 AM
tomek added inline comments.
services/docker-compose.yml
84 ↗(On Diff #12414)

It looks like we shouldn't specify the container name D3847. If you agree, the best way forward is to put a new diff before this one in the stack that deletes this property for all the containers.

This revision now requires changes to proceed.May 9 2022, 7:14 AM
ashoat added a subscriber: jim.

Adding @jimpo since it's Docker-related

max added inline comments.
services/docker-compose.yml
84 ↗(On Diff #12414)

It looks like we shouldn't specify the container name D3847. If you agree, the best way forward is to put a new diff before this one in the stack that deletes this property for all the containers.

That makes sense to me. Stacked on top of the D4014 which removes container_name. Thanks @palys-swm !

tomek added 1 blocking reviewer(s): jim.

Adding @jimpo as a blocking reviewer so that we can clarify if specifying default user and pass is a good practice.

services/docker-compose.yml
94–95 ↗(On Diff #12577)

Is it a good practice to specify the credentials here?

84 ↗(On Diff #12414)

Thanks for creating that diff! This diff still specifies the container_name so please delete it before landing

max marked an inline comment as done.

container_name was removed.

max added inline comments.
services/docker-compose.yml
94–95 ↗(On Diff #12577)

Is it a good practice to specify the credentials here?

This image is for the local dev environment only, it's safe to use it here. Also, we can omit this and use the default rabbitMQ user and password which are guest.
Let's wait for @jimpo to comment on this.

84 ↗(On Diff #12414)

Thanks for creating that diff! This diff still specifies the container_name so please delete it before landing

Fixed, thanks for catching this!

I think it's not crazy to specify the credentials like this for now, especially because this Docker Compose is probably only used for the dev environment. Docker Compose is not really set up for orchestration... for that, you'd need K8s or Docker Swarm, or maybe Terraform can help with some of it.

In the production environment, we should avoid specify username / password like this. @geekbrother, can you create a task to track this? Then we can give @jimpo a couple days to give his thoughts before removing him from the review.

In the production environment, we should avoid specify username / password like this.

Of course, we should not use such an approach in the production. That would be crazy.

@geekbrother, can you create a task to track this? Then we can give @jimpo a couple days to give his thoughts before removing him from the review.

Sure, I've created ENG-1153 task to track this.

This revision is now accepted and ready to land.May 17 2022, 8:06 AM

Rebase and merge on master.