Fix docker migration test workflow

docker/Dockerfile

@@ -19,7 +19,7 @@ COPY --chmod=644 docker/services/logrotate /etc/
 # Copy our poetry files into the image and install our dependencies.
 COPY --chown=simplified:simplified poetry.lock pyproject.toml /var/www/circulation/
 RUN . env/bin/activate && \
-    poetry install --only main,pg --sync
+    poetry install --only main,pg --sync --no-root
 
 COPY --chown=simplified:simplified . /var/www/circulation
 

docker/Dockerfile.baseimage

@@ -18,7 +18,7 @@ RUN apt-get update && \
     apt-get upgrade -y --no-install-recommends -o Dpkg::Options::="--force-confold" && \
     /bd_build/cleanup.sh
 
-ARG POETRY_VERSION=1.6.1
+ARG POETRY_VERSION=1.7.1
 
 # Install required packages including python, pip, compiliers and libraries needed
 # to build the python wheels we need and poetry.
@@ -65,7 +65,7 @@ RUN python3 -m venv env && \
     echo "if [ -f $SIMPLIFIED_ENVIRONMENT ]; then source $SIMPLIFIED_ENVIRONMENT; fi" >> env/bin/activate && \
     . env/bin/activate && \
     pip install --upgrade pip && \
-    poetry install --only main,pg --sync && \
+    poetry install --only main,pg --sync --no-root && \
     python3 -m textblob.download_corpora lite && \
     mv /root/nltk_data /usr/lib/ && \
     find /usr/lib/nltk_data -name *.zip -delete && \

docker/ci/test_migrations.sh

@@ -3,139 +3,214 @@
 # This script makes sure that our database migrations bring the database up to date
 # so that the resulting database is the same as if we had initialized a new instance.
 #
-# This is done by (1) checking out an older version of our codebase at the commit on
-# which the first migration was added and then (2) initializing a new instance. Then
-# we check out the current version of our codebase and run our migrations.
+# This is done by:
+# (1) Finding the id of the first DB migration.
+# (2) Initializing the database with an old version of the app. This old version is
+#     the version of the app that was current when the first migration was created.
+#     This version is started in a separate container called `webapp-old`. This
+#     container is defined in the `test_migrations.yml` file.
+# (3) Then the current version of the app is started in a container called `webapp`.
+# (4) We run the migrations in the `webapp` container to bring the database up to date.
+#     and then check that the database schema matches the model.
+# (5) We then run the downgrade migrations in the `webapp` container to bring the database
+#     back to the state it was in when the first migration was created.
+# (6) We then check that the database schema matches the model in the `webapp-old` container.
+# (7) Finally, we repeat step (4) to make sure that the up migrations stay in sync.
 #
-# After the migrations are complete we use `alembic check` [1] to make sure that the
-# database model matches the migrated database. If the model matches, then the database
-# database is in sync and the migrations are up to date. If the database doesn't match
-# then a new migration is required. We then repeat this process with our down
-# migrations to make sure that the down migrations stay in sync as well.
+# After the migrations are complete in step (4) and (6) we use `alembic check` [1] to
+# make sure that the database model matches the migrated database. If the model matches,
+# then the database is in sync and the migrations are up to date. If the database doesn't
+# match then there is a problem with the migrations and the script will fail.
 #
 # Note: This test cannot be added to the normal migration test suite since it requires
-# manipulating the git history and checking out older versions of the codebase. All of
-# the commands in this script are run inside a docker-compose environment.
+# us to have access to an older version of our code base. To facilitate this we use the
+# `test_migrations.yml` file to define a container that runs an older version of the app.
+# And run all the commands in this script in a docker-compose environment.
 #
 # [1] https://alembic.sqlalchemy.org/en/latest/autogenerate.html#running-alembic-check-to-test-for-new-upgrade-operations
 
+# Text colors
+RESET='\033[0m'       # Text Reset
+GREEN='\033[1;32m'       # Green
 
+# Keeps track of whether we are in a group or not
+IN_GROUP=0
+
+# Allow a command to run without echoing its output
+DEBUG_ECHO_ENABLED=1
+
+# Functions to interact with GitHub Actions
+# https://docs.github.com/en/actions/reference/workflow-commands-for-github-actions
+gh_command() {
+  local COMMAND=$1
+  local MESSAGE=${2:-""}
+  echo "::${COMMAND}::${MESSAGE}"
+}
+
+# Create a group of log lines
+# https://docs.github.com/en/actions/using-workflows/workflow-commands-for-github-actions#grouping-log-lines
+gh_group() {
+  local MESSAGE=$1
+  gh_command group "$MESSAGE"
+  IN_GROUP=1
+}
+
+# End a group of log lines
+gh_endgroup() {
+  if [[ $IN_GROUP -eq 1 ]]; then
+    gh_command endgroup
+    IN_GROUP=0
+  fi
+}
+
+# Log an error message
+# Note: if this is called in a group, the group will be closed before the error message is logged.
+gh_error() {
+  gh_endgroup
+  local MESSAGE=$1
+  gh_command error "$MESSAGE"
+}
+
+# Log a success message
+# Note: if this is called in a group, the group will be closed before the success message is logged.
+success() {
+  gh_endgroup
+  local MESSAGE=$1
+  echo -e "${GREEN}Success:${RESET} ${MESSAGE}"
+}
+
+debug_echo() {
+  if [[ $DEBUG_ECHO_ENABLED -eq 1 ]]; then
+    printf "%q " "$@"
+    printf "\n"
+  fi
+}
+
+# Run a docker-compose command
 compose_cmd() {
-  docker --log-level ERROR compose --progress quiet "$@"
+  args=(docker compose -f docker-compose.yml -f docker/ci/test_migrations.yml --progress quiet)
+  debug_echo "++" "${args[@]}" "$@"
+  "${args[@]}" "$@"
 }
 
+# Run a command in a particular container using docker-compose
+# The command is run in a bash shell with the palace virtualenv activated
 run_in_container()
 {
-  compose_cmd run --build --rm --no-deps webapp /bin/bash -c "source env/bin/activate && $*"
+  local CONTAINER_NAME=$1
+  shift 1
+  debug_echo "+" "$@"
+  compose_cmd run --build --rm --no-deps "${CONTAINER_NAME}" /bin/bash -c "source env/bin/activate && $*"
 }
 
+# Cleanup any running containers
 cleanup() {
   compose_cmd down
-  git checkout -q "${current_branch}"
 }
 
+# Cleanup any running containers and exit with an error message
+error_and_cleanup() {
+  local MESSAGE=$1
+  local EXIT_CODE=$2
+  cleanup
+  gh_error "$MESSAGE"
+  exit "$EXIT_CODE"
+}
+
+# Run an alembic migration command in a container
 run_migrations() {
-  ALEMBIC_CMD=$1
-  run_in_container "alembic ${ALEMBIC_CMD}"
+  local CONTAINER_NAME=$1
+  shift 1
+  run_in_container "${CONTAINER_NAME}" "alembic" "$@"
   exit_code=$?
   if [[ $exit_code -ne 0 ]]; then
-    echo "ERROR: Running database migrations failed."
-    cleanup
-    exit $exit_code
+    error_and_cleanup "Running database migrations failed." $exit_code
   fi
-  echo ""
 }
 
+# Check if the database is in sync with the model
 check_db() {
-  DETAILED_ERROR=$1
-  run_in_container "alembic check"
-  exit_code=$?
-  if [[ $exit_code -eq 0 ]]; then
-    echo "SUCCESS: Database is in sync."
-    echo ""
-  else
-    echo "ERROR: Database is out of sync! ${DETAILED_ERROR}"
-    cleanup
-    exit $exit_code
+  local CONTAINER_NAME=$1
+  local DETAILED_ERROR=$2
+  run_in_container "${CONTAINER_NAME}" alembic check
+  local exit_code=$?
+  if [[ $exit_code -ne 0 ]]; then
+    error_and_cleanup "Database is out of sync! ${DETAILED_ERROR}" $exit_code
   fi
+  success "Database is in sync."
 }
 
-if ! git diff --quiet; then
-  echo "ERROR: You have uncommitted changes. These changes will be lost if you run this script."
-  echo "  Please commit or stash your changes and try again."
-  exit 1
-fi
-
-# Find the currently checked out branch
-current_branch=$(git symbolic-ref -q --short HEAD)
-current_branch_exit_code=$?
-
-# If we are not on a branch, then we are in a detached HEAD state, so
-# we use the commit hash instead. This happens in CI when being run
-# against a PR instead of a branch.
-# See: https://stackoverflow.com/questions/69935511/how-do-i-save-the-current-head-so-i-can-check-it-back-out-in-the-same-way-later
-if [[ $current_branch_exit_code -ne 0 ]]; then
-  current_branch=$(git rev-parse HEAD)
-  echo "WARNING: You are in a detached HEAD state. This is normal when running in CI."
-  echo "  The current commit hash will be used instead of a branch name."
-fi
-
-echo "Current branch: ${current_branch}"
-
-# Find the first migration file
-first_migration_id=$(run_in_container alembic history -r'base:base+1' -v | head -n 1 | cut -d ' ' -f2)
+# Find all the info we need about the first migration in the git history.
+gh_group "Finding first migration"
+run_in_container "webapp" alembic history -r'base:base+1' -v
+# Debug echo is disabled since we are capturing the output of the command
+DEBUG_ECHO_ENABLED=0
+first_migration_id=$(run_in_container "webapp" alembic history -r'base:base+1' -v | head -n 1 | cut -d ' ' -f2)
+DEBUG_ECHO_ENABLED=1
 if [[ -z $first_migration_id ]]; then
-  echo "ERROR: Could not find first migration id."
-  exit 1
+  error_and_cleanup "Could not find first migration id." 1
 fi
 
 first_migration_file=$(find alembic/versions -name "*${first_migration_id}*.py")
 if [[ -z $first_migration_file ]]; then
-  echo "ERROR: Could not find first migration file."
-  exit 1
+  error_and_cleanup "Could not find first migration file." 1
 fi
 
-echo "First migration file: ${first_migration_file}"
-echo ""
-
-# Find the git commit where the first migration file was added
 first_migration_commit=$(git log --follow --format=%H --reverse "${first_migration_file}" | head -n 1)
 if [[ -z $first_migration_commit ]]; then
-  echo "ERROR: Could not find first migration commit hash."
-  exit 1
+  error_and_cleanup "Could not find first migration commit hash." 1
+fi
+first_migration_container="ghcr.io/thepalaceproject/circ-webapp:sha-${first_migration_commit:0:7}"
+echo "First migration info:"
+echo "  id: ${first_migration_id}"
+echo "  file: ${first_migration_file}"
+echo "  commit: ${first_migration_commit}"
+echo "  container: ${first_migration_container}"
+
+container_image=$(sed -n 's/^ *image: "\(.*\)"/\1/p' docker/ci/test_migrations.yml)
+if [[ -z $container_image ]]; then
+  error_and_cleanup "Could not find container image in test_migrations.yml" 1
+fi
+
+if [[ "$container_image" != "$first_migration_container" ]]; then
+  error_and_cleanup "Incorrect container image in test_migrations.yml. Please update." 1
 fi
+gh_endgroup
 
-echo "Starting containers"
+gh_group "Starting service containers"
 compose_cmd down
-compose_cmd up -d pg
+compose_cmd up -d pg os
+gh_endgroup
 
-echo "Initializing database at commit ${first_migration_commit}"
-git checkout -q "${first_migration_commit}"
-run_in_container "./bin/util/initialize_instance"
+gh_group "Initializing database"
+run_in_container "webapp-old" "./bin/util/initialize_instance"
 initialize_exit_code=$?
 if [[ $initialize_exit_code -ne 0 ]]; then
-  echo "ERROR: Failed to initialize instance."
-  cleanup
-  exit $initialize_exit_code
+  error_and_cleanup "Failed to initialize instance." $initialize_exit_code
 fi
-echo ""
+gh_endgroup
 
 # Migrate up to the current commit and check if the database is in sync
-echo "Testing upgrade migrations on branch ${current_branch}"
-git checkout -q "${current_branch}"
-run_migrations "upgrade head"
-check_db "A new migration is required or a up migration is broken."
+gh_group "Testing upgrade migrations"
+run_migrations "webapp" upgrade head
+check_db "webapp" "A new migration is required or an up migration is broken."
+gh_endgroup
 
 # Migrate down to the first migration and check if the database is in sync
-echo "Testing downgrade migrations"
-run_migrations "downgrade ${first_migration_id}"
-git checkout -q "${first_migration_commit}"
-check_db "A down migration is broken."
+gh_group "Testing downgrade migrations"
+run_migrations "webapp" downgrade "${first_migration_id}"
+check_db "webapp-old" "A down migration is broken."
+gh_endgroup
 
 # Migrate back up once more to make sure that the database is still in sync
-echo "Testing upgrade migrations a second time"
-git checkout -q "${current_branch}"
-run_migrations "upgrade head"
-check_db "A up migration is likely broken."
+gh_group "Testing upgrade migrations a second time"
+run_migrations "webapp" upgrade head
+check_db "webapp" "An up migration is likely broken."
+gh_endgroup
+
+echo ""
+success "All migrations are up to date 🎉"
 
+gh_group "Shutting down service containers"
 cleanup
+gh_endgroup

docker/ci/test_migrations.yml

@@ -0,0 +1,18 @@
+version: "3.9"
+
+# This docker-compose file is used to run the old webapp for testing purposes
+# see test_migrations.sh for more information.
+
+services:
+  webapp-old:
+    image: "ghcr.io/thepalaceproject/circ-webapp:sha-54fa9ef"
+
+    environment:
+      SIMPLIFIED_PRODUCTION_DATABASE: "postgresql://palace:test@pg:5432/circ"
+      PALACE_SEARCH_URL: "http://os:9200"
+
+    depends_on:
+      pg:
+        condition: service_healthy
+      os:
+        condition: service_healthy