new build and deployment process

[joefutrelle]

• fixed image build
• use of Docker Hub images, eliminating need for deployer to do a docker build
• attempt to configure everything with .env requiring few if any changes to docker-compose file

[joefutrelle]

TODO: pin dependencies and remove :latest from postgis and other images where new releases might break existing functionality.

[srstsavage]

This isn't a change introduced by this branch, but I think it's cleaner to remove the container_name configs and allow Docker Compose to manage the container names. With the current config only one instance of the stack can be run on a single computer since the container names will conflict...

[srstsavage]

Related, the init commands should use docker compose instead of docker for portability:

docker compose exec ifcbdb python manage.py migrate
docker compose exec ifcbdb python manage.py collectstatic

docker compose exec ifcbdb python manage.py createsuperuser

[joefutrelle]

Thanks @srstsavage these suggestions LGTM

[srstsavage]

On master, settings.py imports from local_settings at the end of the file.

https://github.com/WHOIGit/ifcbdb/blob/master/ifcbdb/ifcbdb/settings.py#L155-L158

try:
    from .local_settings import *
except ImportError as e:
    raise ImportError('local settings not found') from e

This new branch doesn't seem to have that import, or reference local_settings anywhere else.

https://github.com/WHOIGit/ifcbdb/blob/new_build/ifcbdb/ifcbdb/settings.py#L153

Does that need to be fixed, or am I missing some settings conventions?

[joefutrelle]

For a typical deployment it doesn't, because every relevant parameter is configured via environment variables, but for atypical deployments I agree that we should restore this.

[srstsavage]

Yep, that makes sense. A few other things:

• README mentions that SSL is required, but I see that the included http only nginx.conf.template is selectable via NGINX_TEMPLATE and is useful for those doing TLS termination at the web proxy layer. Might be worth giving that a short mention in the README.
• It would be good to be able to set an alternate postgres password via env var. Should be straightforward to use the same env var for POSTGRES_PASSWORD in the postgis container and access in ifcbdb settings.py. Granted the postgis container has no externally exposed ports and is isolated to postgres_network, but I still think password should be configurable.
• Usage of latest images in docker-compose.yml I think eventually will cause issues as upstream images release new major versions with breaking changes. Ideally the compose file specifies known working versions so that upgrades are deliberate, or at a minimum all of the images should be configurable via env vars as IFCBDB_IMAGE and POSTGIS_IMAGE are now.

Let me know if you'd like me to put together PRs against new_build for any of these. Thanks, overall this updated deployment approach looks great!

[joefutrelle]

Agreed on all points, I would love a PR against new_build with all the changes you suggested but I'm happy to implement it myself if you don't have time.

[srstsavage]

Implemented all suggested changes in #382, let me know if anything needs to be changed!

[joefutrelle]

Great that you've also reduced the size of the image @srstsavage, any other things you want to do before I review #382?

[srstsavage]

@joefutrelle Nope, all ready for review!

[joefutrelle]

Note that when we merge this we should not delete the new_build branch until I notify the IFCB Users Group since I currently am instructing alpha testers to use it.

[joefutrelle]

@srstsavage I pinned all the dependencies and created a corresponding release tag for pyifcb and so I think this is buttoned up. Can you give this a spin?

[srstsavage]

@joefutrelle Seems to be working well!

[joefutrelle]

There's the matter of #383 which is pending against this branch, and I want to get that in before the release. @mike-kaimika do you want to retarget #383 against master after doing this merge, or do you think it's better to get those changes into new_build before we merge this PR?

[mike-kaimika]

I think it probably makes more sense to get it merged into this branch, assuming neither of you had any issues with it. That way there's just one final branch for all the revisions.

[joefutrelle]

• Not deleting this branch
• Next steps are cutting a release and pushing a freshly-built multiplatform image to Docker Hub.

[srstsavage]

Deployment using the updated whoi/ifcb-dashboard Docker Hub image (sha256:4f13deefec11da3076ba48d5d1f479029ea6f8bea0a05fd07aa1385f6c74fde4) is working well for me.

[joefutrelle]

Thanks for verifying that @srstsavage !