Dockerize pre-commit

[fyliu]

Overview

We need to dockerize pre-commit so that it's easily deployable for development.

Action Items

• [x] dockerize pre-commit
  • [ ] make sure the existing hooks will work
• [ ] check and release #189 from icebox

Resources/Instructions

• our pre-commit config
• docker-pre-commit has a pre-commit hook script that starts and runs a docker container

[fyliu]

I dockerized pre-commit and most of the tasks are running fine except running the project inside docker containers. This is running docker inside docker. There's a permission denied error when mounting the host directory to the container.

[fyliu]

Looks like my mistake was trying to take the shortcut and mount the host's docker socket into the container. When the container's docker-compose tries to do a bind mount of the container's path into the container's docker container, it tries to use the root user. But the docker socket belongs to the host and it throws an error when it tries to do the mount.

Anyway, the solution is to use an actual docker in docker (dind) setup.

[fyliu]

It feels like it's possible to do this with just mounting the host docker socket. It seems to work if I feed it with the host's path rather than the path inside the container.

Progress was slow due to many image rebuilds. I attempted to use cache mounts but that seemed to have created problems. I'm now passing --no-cache to docker-compose build so it will do clean builds and show when a change breaks something.

[fyliu]

Status

I got to the point where pre-commit works from within the docker container. Link to branch in my fork.

Here are some useful commands.

Note: APP_PATH should probably use the more generic

APP_PATH=`git rev-parse --show-toplevel`/app

but I'm just pasting the commands that work for me.

from host

# build the pre-commit tools image
APP_PATH=/home/fang/src/peopledepot/app docker-compose build tools

# run an interactive shell in the container. Useful for working on small steps inside the container.
docker-compose run tools sh

# these next lines run pre-commit from inside the container

# run a single pre-commit hook
docker-compose run tools sh -c "pre-commit run --all-files check-django-migrations"

# run all the hooks
docker-compose run tools sh -c "pre-commit run --all-files"

from container

# build the app and run in the background
# pass in the host's app path even though we're in the container
APP_PATH=/home/fang/src/peopledepot/app ./scripts/buildrun.sh

# run the app in the foreground. useful for making sure it runs without errors
APP_PATH=/home/fang/src/peopledepot/app docker-compose up

# run pre-commit specifying a single hook. This hook requires the app to be running
APP_PATH=/home/fang/src/peopledepot/app pre-commit run --all-files check-django-migrations

Next step

The next step would be to create a pre-commit hook that calls the pre-commit in the container. Either it also has to set up the container or there needs to be a setup script to build the pre-commit image. Another option is to have the image built somewhere else and upload it to docker hub. But The first step should be to build it as a separate step and call it from the pre-commit hook script.

[fyliu]

Another option instead of dockerizing pre-commit is to ask developers to install it locally via pipx. pipx installs the package in an isolated environment rather than globally. Installing packages globally is a bad idea because of dependency conflict hell. pipx can be used to install other things like poetry.

[fyliu]

I think that the pipx option is a better one at this point. Even though I would like to dockerize everything, this one is a much greater effort than I anticipated and I think it will take more time to get right. I feel the strategic thing to do is to leave this for later and do #199 to solve the current issue.

[ExperimentsInHonesty]

• [ ] Create a decision record in the wiki for why we did not implement https://github.com/hackforla/peopledepot/wiki/Decision-Records-on-Solutions-Not-Implemented