Skip to content
This repository was archived by the owner on Mar 19, 2025. It is now read-only.

Cleanup our Docker dev env. #41

Merged
merged 2 commits into from
Sep 12, 2019
Merged

Conversation

sadleb
Copy link

@sadleb sadleb commented Sep 12, 2019

Move the services onto a common network shared by all other apps.
Don't remove volumes on restart and rebuild. The dev can do that if
they need to b/c they want to nuke everything. We should rely on a
generally working env and not require nuking the volumes in the normal
dev flow. Use COPY instead of ADD b/c ADD wasn't doing what I thought it
was. COPY is recommended and volumes are mounted through the
docker-compose.yml file so that changes to files inside and outside the
container are reflected in both places.

TESTING

  • brought up canvas, canvas-js-css, rubycas (aka SSO), join (aka beyondz-platform), nginx locally and was able to login to both join and canvas using this setup (when the other repos had their containers moved to this same docker network).

Move the services onto a common network shared by all other apps.
Don't remove volumes on restart and rebuild. The dev can do that if
they need to b/c they want to nuke everything. We should rely on a
generally working env and not require nuking the volumes in the normal
dev flow. Use COPY instead of ADD b/c ADD wasn't doing what I thought it
was. COPY is recommended and volumes are mounted through the
docker-compose.yml file so that changes to files inside and outside the
container are reflected in both places.
Copy link

@StetsenTech StetsenTech left a comment

Choose a reason for hiding this comment

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

I'm fine with the changes!

@@ -8,7 +8,7 @@ RUN echo "deb [check-valid-until=no] http://archive.debian.org/debian jessie-bac
RUN sed -i '/deb http:\/\/deb.debian.org\/debian jessie-updates main/d' /etc/apt/sources.list

# Note: gettext is installed to be able to use envsubst to inject config values
RUN apt-get -o Acquire::Check-Valid-Until=false update -qq && apt-get install -y build-essential libpq-dev gettext
RUN apt-get -o Acquire::Check-Valid-Until=false update -qq && apt-get install -y build-essential libpq-dev gettext vim

Choose a reason for hiding this comment

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

😭 Thank you for this!

@sadleb sadleb removed the request for review from rshipp September 12, 2019 16:58
@sadleb
Copy link
Author

sadleb commented Sep 12, 2019

Unrelated to this change, i tried to rebuild this container to get teh volume mounted and there are now bundle install issues with it. I think it's related to dependencies of dependencies getting updated and no longer supporting our version of ruby. Once I fix that I'll push to this branch for another review @StetsenTech

…reflected inside.

This is done by mounting the entire codebase as a volume when started
using docker-compose.

Note: a dependency of webmock (dev builds only) got updated to the point
of only supporting ruby 2.3 and above. So I locked it down to the last
version that supported ruby 2.1. The dependency tree was:

  webmock was resolved to 1.24.6, which depends on
    addressable was resolved to 2.7.0, which depends on
      public_suffix was resolved to 4.0.1

Also, stop supporting a local dev. All dev should be done with the
container. Removed Gemfile.lock for the docker build so that everything
inside is the same as outside and it's like you're just developing
locally with the Docker container acting as the web-server.
@sadleb
Copy link
Author

sadleb commented Sep 12, 2019

Fixed up that last bit. It was just specifying a specific version of a gem that was only used in the dev build (not prod) so based on my testing I'm just going to merge this instead of asking for another review @StetsenTech

@sadleb sadleb merged commit e19d216 into bebraven:master Sep 12, 2019
@sadleb sadleb deleted the docker_dev_env branch September 12, 2019 18:05
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants