usnistgov / cdcs-docker

Other
5 stars 9 forks source link

README documentation about helper service ports is unclear #4

Open jat255 opened 3 years ago

jat255 commented 3 years ago

The README.md indicates you can change values such as POSTGRES_PORT, MONGO_PORT, etc. in your .env file. While this partially works due to the environment part of the cdcs service definition in docker-compose.yml:

    environment:
      - DJANGO_SETTINGS_MODULE=${PROJECT_NAME}.${SETTINGS}
      - DJANGO_SECRET_KEY=${DJANGO_SECRET_KEY}
      - SERVER_URI=${SERVER_URI}
      - ALLOWED_HOSTS=${ALLOWED_HOSTS}
      - SERVER_NAME=${SERVER_NAME:-}
      - MONGO_HOST=${PROJECT_NAME}_cdcs_mongo
      - MONGO_PORT=${MONGO_PORT:-27017}
      - MONGO_DB=${MONGO_DB}
      - MONGO_USER=${MONGO_USER}
      - MONGO_PASS=${MONGO_PASS}
      - POSTGRES_HOST=${PROJECT_NAME}_cdcs_postgres
      - POSTGRES_PORT=${POSTGRES_PORT:-5432}
      - POSTGRES_DB=${POSTGRES_DB}
      - POSTGRES_USER=${POSTGRES_USER}
      - POSTGRES_PASS=${POSTGRES_PASS}
      - REDIS_HOST=${PROJECT_NAME}_cdcs_redis
      - REDIS_PORT=${REDIS_PORT:-6379}
      - REDIS_PASS=${REDIS_PASS}
      - MONITORING_SERVER_URI=${MONITORING_SERVER_URI:-}
      - ELASTICSEARCH_HOST=${ELASTICSEARCH_HOST:-}
      - ELASTICSEARCH_PORT=${ELASTICSEARCH_PORT:-}

It doesn't actually work when you try to bring the deployment up, because the individual services (PostgreSQL, Mongo, and Redis) haven't changed what port they're running on internally (and they don't expose ports onto the host). Since docker-compose uses internal networking for all of these services, changing the ports isn't actually necessary if you want to run multiple CDCS instances on the same machine, since the ports are local to the individual services, and won't conflict on the host (since they're not being exposed). The only ones that need to be changed are the nginx ones.

I would suggest adding a note to the description field for these values in the README.md field that changing them will break things (and it's not necessary, as described above).

pdessauw commented 3 years ago

@jat255, thanks for letting us know. We'll update the README to clarify this part!