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

Remove unnecessary installs from DOCKERFILE.api.production #63

Open
3 tasks
MikeTheCanuck opened this issue May 5, 2018 · 5 comments
Open
3 tasks

Remove unnecessary installs from DOCKERFILE.api.production #63

MikeTheCanuck opened this issue May 5, 2018 · 5 comments

Comments

@MikeTheCanuck
Copy link
Contributor

There are three separate enhancements in the production DOCKERFILE that I don't believe are necessary for every project:

  • Line 1: is the "stretch" variant of python 3.6.5 necessary, or can we go with a stock 3.6.5?
  • Lines 5-13: are these packages meant for use by Transportation-Systems or by everyone?
  • Line 22: are the geodjango packages necessary for everyone?
@bhgrant8
Copy link
Member

bhgrant8 commented May 5, 2018

1, So the use of the stretch variant comes down to the question of the use of the wait-for-it script to test whether the container is able to make a connection with the postgres client prior to completing the rest of the bootup cycle. The stretch variant seemed to be necessary as per @znmeb for the use of of the required postgres client necessary to connect to the current database.

The root of this usage is from this recommendation from Docker Compose:
https://docs.docker.com/compose/startup-order/

It does improve on the local dev container experience as usually the api container will attempt to startup prior to the database being booted. We may have adequate healthchecks on the database and api container through aws that this may not be necessary?

  1. Lines 5-13 are packages related to 2 concerns:
  • the wait for it script described in 1. - postgresql-client-9.6 \ and i assume qqy?
  • for any project making use of postgis/geodjango stack then they will require the geospatial libraries to be installed in the container.
    binutils \
    gdal-bin \
    libproj-dev \

see ref in the comments of the docker file: https://docs.djangoproject.com/en/2.0/ref/contrib/gis/install/geolibs/

  1. Line 22 Nope, hence why is separate req file, can be left out from projects not needing it, could be possibly made more automated. See: ADD option for Non-GeoDjango API container Config #22

@MikeTheCanuck
Copy link
Contributor Author

Thanks Brian - this is great insight, and thanks as always to the docs links!

  1. I'm having trouble imagining how this will improve things in either Travis build/test or in the actual spin-up of a Docker container image in ECS - in the former case, if it fails to connect in the usual timeout, it's unlikely to come back during the build/test cycle (that'll probably mean the production DB is down, since it's otherwise never falling down - due to the evidence over the last year that they've never required manual intervention, nor have they ever rebooted) and if it fails the solution is to restart the Travis Build; in the latter case, there is so little we can do with a Docker container in ECS once it starts, that if there's a failure there we can do nothing but restart the container - which the ECS services will do for us automatically anyway.
  2. While the wait-for-it script doesn't seem useful here, the postgis/geodjango stack support sounds useful and would be handy to have already laid out here. Would you mind if we added some comments for goons like me who aren't connecting the dots when looking at this Dockerfile?
  3. This too would benefit from a comment for goons like me who might be tempted to delete stuff that doesn't immediately look "normal".

@bhgrant8
Copy link
Member

bhgrant8 commented May 6, 2018

  1. yeah, i was always a bit on the fence on this being in there
  2. sure
  3. sure, "self-documenting" code is only ever documenting to the self that wrote it. sometimes not even then 😜

@znmeb
Copy link
Contributor

znmeb commented May 6, 2018

  1. A bunch of stuff breaks in development if one container is running "stretch" and another running "jessie". I believe they're all on stretch now and should remain that way.
  2. There's no penalty for using "stretch" in both development and production, and for consistency I think it's a good idea - less cognitive load.

@MikeTheCanuck
Copy link
Contributor Author

Addressing some of item (2) with #71

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants