[PATCH 3/4] Support testing with PostgreSQL

Tue Sep 26 21:34:52 AEST 2017

Stephen Finucane <stephen at that.guru> writes:

> On Mon, 2017-09-04 at 01:14 +1000, Daniel Axtens wrote:
>> This allows us to easily test against PostgreSQL using the same
>> tooling we normally use. This is helpful in (for example) shaking
>> out the test failures that were observed on ozlabs.org
>> 
>> To use it:
>>   docker-compose -f docker-compose-pg.yml <usual arguments>
>> 
>> (You may find it necessary to do a 'docker-compose down' first,
>> depending on what state the system is in and what command you're
>> running.)
>> 
>> Signed-off-by: Daniel Axtens <dja at axtens.net>
>
> Some comments below.
>
>> ---
>>  docker-compose-pg.yml      | 30 ++++++++++++++++++++++++++
>>  requirements-test.txt      |  3 ++-
>>  tools/docker/Dockerfile    |  2 +-
>>  tools/docker/entrypoint.sh | 53 +++++++++++++++++++++++++++++++++++++-------
>> --
>>  4 files changed, 76 insertions(+), 12 deletions(-)
>>  create mode 100644 docker-compose-pg.yml
>> 
>> diff --git a/docker-compose-pg.yml b/docker-compose-pg.yml
>> new file mode 100644
>> index 000000000000..34900418375e
>> --- /dev/null
>> +++ b/docker-compose-pg.yml
>> @@ -0,0 +1,30 @@
>> +# the version of docker-compose shipped in ubuntu 16.04 is
>> +# 1.5.2, which doesn't support version 2 syntax. Yay!
>> +# also, v1 doesn't support explicit build args, so if you're not
>> +# uid 1000, you will either need to manually hack the Dockerfile
>> +# or upgrade to v2 and use the build-arg to override it.
>
> Only 7 months to 18.04, guys!
>
>> +
>> +db:
>> +  image: postgres
>
> Any particular version needed?

The default is pretty good, but I'll stick it to 9.6 for now.
>
>> +  environment:
>> +    - POSTGRES_PASSWORD=super_sekrit_postgres_passwd
>> +  volumes:
>> +    - ./tools/docker/db/postdata:/var/lib/postgresql/data
>> +
>> +web:
>> +  build: .
>> +  dockerfile: ./tools/docker/Dockerfile
>> +  command: python3 manage.py runserver 0.0.0.0:8000
>> +  volumes:
>> +    - .:/home/patchwork/patchwork/
>> +  ports:
>> +    - "8000:8000"
>> +  links:
>> +    - db
>> +  environment:
>> +    - PGPASSWORD=super_sekrit_postgres_passwd
>> +    - PW_TEST_DB_HOST=db
>> +    - PW_TEST_DB_PORT=5432
>> +    - PW_TEST_DB_TYPE=postgres
>> +    - PW_TEST_DB_USER=postgres
>> +    - PW_TEST_DB_PASS=super_sekrit_postgres_passwd
>
> Any harm in just using 'password', like we do for mysql. It is for dev
> purposes, after all.

Will do in v2.

>
>> diff --git a/requirements-test.txt b/requirements-test.txt
>> index cead336c7c77..141cf661663d 100644
>> --- a/requirements-test.txt
>> +++ b/requirements-test.txt
>> @@ -1,4 +1,5 @@
>> -mysqlclient>=1.3,<1.4  # replace this with psycopg2 for a PostgreSQL backend
>> +mysqlclient>=1.3,<1.4
>> +psycopg2>=2.7,<2.8
>
> Are you sure this works? Last I checked, you needed PostgreSQL installed for
> psycopg2 to install correctly (think I spotted it when adding Vagrant support).
> Could you try rebuilding the MySQL-based container to validate this?
>
Have checked locally and on Travis.

>>  django-debug-toolbar==1.8
>>  python-dateutil>2.0,<3.0
>>  selenium>=3.0,<4.0
>> diff --git a/tools/docker/Dockerfile b/tools/docker/Dockerfile
>> index ff05707a6049..946c646188e5 100644
>> --- a/tools/docker/Dockerfile
>> +++ b/tools/docker/Dockerfile
>> @@ -21,7 +21,7 @@ RUN apt-get update -qq && \
>>      python3.5-dev python3-pip python3-setuptools python3-wheel \
>>      python3.4-dev findutils=4.4.2-7 \
>>      libmysqlclient-dev mysql-client curl unzip xvfb chromium-chromedriver \
>> -    chromium-browser build-essential git && \
>> +    chromium-browser build-essential git postgresql-client && \
>
> ...although maybe this resolves the issue. I'll wait to see if you tested this
> :)
>
It does indeed, and I have indeed :)

>>      ln -s /usr/lib/chromium-browser/chromedriver /usr/bin/
>>  
>>  # User
>> diff --git a/tools/docker/entrypoint.sh b/tools/docker/entrypoint.sh
>> index 2f413b02e83e..4b656adaeb35 100755
>> --- a/tools/docker/entrypoint.sh
>> +++ b/tools/docker/entrypoint.sh
>> @@ -1,13 +1,31 @@
>>  #!/bin/bash
>> -set -euo pipefail
>> +set -eo pipefail
>> +
>> +if [ x${PW_TEST_DB_TYPE} = x ]; then
>> +    PW_TEST_DB_TYPE=mysql
>> +fi
>> +
>> +set -u
>
> Why do we need to move the '-u' (undefined variable?) down here? Does Bash not
> provide an equivalent of Python's 'getattr(ATTR, DEFAULT)'?
>

-u causes the script to exit if an undefined variable is referenced,
rather than just returning the empty string. So, if PW_TEST_DB_TYPE is
unset the script will error out.

There is a bashism that might allow us to get around this, I'll check.
(I use http://redsymbol.net/articles/unofficial-bash-strict-mode/ and I
think it has a section on this.)

Regards,
Daniel

>>  
>>  # functions
>>  
>>  test_db_connection() {
>> -    mysqladmin -h $PW_TEST_DB_HOST -u patchwork --password=password ping >
>> /dev/null 2> /dev/null
>> +    if [ ${PW_TEST_DB_TYPE} = "postgres" ]; then
>> +	echo ';' | psql -h $PW_TEST_DB_HOST -U postgres 2> /dev/null >
>> /dev/null
>> +    else
>> +	mysqladmin -h $PW_TEST_DB_HOST -u patchwork --password=password ping
>> > /dev/null 2> /dev/null
>> +    fi
>>  }
>
> Looks good.
>
>> -reset_data() {
>> +test_database() {
>> +    if [ ${PW_TEST_DB_TYPE} = "postgres" ]; then
>> +	echo ';' | psql -h $PW_TEST_DB_HOST -U postgres patchwork 2>
>> /dev/null
>> +    else
>> +	echo ';' | mysql -h $PW_TEST_DB_HOST -u patchwork -ppassword
>> patchwork 2> /dev/null
>> +    fi
>> +}
>> +
>
> Looks good.
>
>> +reset_data_mysql() {
>>      mysql -u$db_user -p$db_pass -h $PW_TEST_DB_HOST << EOF
>>  DROP DATABASE IF EXISTS patchwork;
>>  CREATE DATABASE patchwork CHARACTER SET utf8;
>> @@ -15,6 +33,21 @@ GRANT ALL ON patchwork.* TO 'patchwork' IDENTIFIED BY
>> 'password';
>>  GRANT ALL PRIVILEGES ON test_patchwork.* TO 'patchwork'@'%';
>>  FLUSH PRIVILEGES;
>>  EOF
>> +}
>> +
>> +reset_data_postgres() {
>> +    psql -h $PW_TEST_DB_HOST -U postgres <<EOF
>> +DROP DATABASE IF EXISTS patchwork;
>> +CREATE DATABASE patchwork WITH ENCODING = 'UTF8';
>> +EOF
>> +}
>> +
>> +reset_data() {
>> +    if [ x${PW_TEST_DB_TYPE} = x"postgres" ]; then
>> +	reset_data_postgres
>> +    else
>> +	reset_data_mysql
>> +    fi
>>  
>>      # load initial data
>>      python3 $PROJECT_HOME/manage.py migrate #> /dev/null
>> @@ -46,13 +79,13 @@ for x in /tmp/requirements-*.txt; do
>>      fi
>>  done
>>  
>> -# check if mysql is connected
>> +# check if db is connected
>>  if ! test_db_connection; then
>> -    echo "MySQL seems not to be connected, or the patchwork user is broken"
>> -    echo "MySQL may still be starting. Waiting 5 seconds."
>> +    echo "The database seems not to be connected, or the patchwork user is
>> broken"
>> +    echo "MySQL/Postgres may still be starting. Waiting 5 seconds."
>>      sleep 5
>>      if ! test_db_connection; then
>> -        echo "Still cannot connect to MySQL."
>> +        echo "Still cannot connect to database."
>>          echo "Maybe you are starting the db for the first time. Waiting up
>> to 60 seconds."
>>          for i in {0..9}; do
>>              sleep 5
>> @@ -61,19 +94,19 @@ if ! test_db_connection; then
>>              fi
>>          done
>>          if ! test_db_connection; then
>> -            echo "Still cannot connect to MySQL. Giving up."
>> +            echo "Still cannot connect to database. Giving up."
>>              echo "Are you using docker-compose? If not, have you set up the
>> link correctly?"
>>              exit 1
>>          fi
>>      fi
>>  fi
>>  
>> -# rebuild mysql db
>> +# rebuild db
>>  # do this on --reset or if the db doesn't exist
>>  if [[ "$1" == "--reset" ]]; then
>>      shift
>>      reset_data
>> -elif ! ( echo ';' | mysql -h db -u patchwork -ppassword patchwork 2>
>> /dev/null ); then
>> +elif ! test_database; then
>>      reset_data
>>  fi
>>  
>
> Yup, all this is fine by me.
>
> Stephen