Generate docker images automatically

[mschoema]

No description provided.

[mschoema]

I believe that the PR is complete now.
I also updated the README to reflect the changes, such as password declaration.

[ImreSamu]

@mschoema :
I'll need a few days to thoroughly review the PR.

Among the suggestions, there will likely be some that are not urgent or can wait,
or are important for future users (such as potential future development opportunities).

I'll let you know once I've completed the PR review.

[ImreSamu]

Since there are no GitHub CI runs for this pull request, it's challenging to confirm whether the proposal is valid.
Meanwhile, it would be helpful to include a local test script that contributors can use if they want to modify the current code.

I would suggest a script similar to this:

#!/usr/bin/env bash
set -Eeo pipefail
# shellcheck disable=SC2043

# mobilitydb local build matrix
for mobilitydb_ver in v1.1.0-beta1 ; do
  for pg_major in 16 15 14 13 12 ; do
    for postgis_ver in 3.4 ; do
        docker build --pull --progress=plain  \
            --build-arg POSTGRES_VERSION=$pg_major \
            --build-arg POSTGIS_VERSION=$postgis_ver \
            --build-arg MOBILITYDB_TAG=$mobilitydb_ver \
            -t testmobilitydb:${pg_major}-${postgis_ver}-${mobilitydb_ver} \
            -f docker/Dockerfile\
            .
    done
  done
done

Currently, the size of the completed MobilityDB Docker images ( ~1.44GB ) might be reduced by running the installation, compilation, mobilityDb installation, and removal of programs required for building all within a single RUN statement.

Even if size is not the most urgent issue right now, it will need to be addressed in the future.

$ docker images testmobilitydb*
REPOSITORY       TAG                   IMAGE ID       CREATED             SIZE
testmobilitydb   12-3.4-v1.1.0-beta1   4e457c04f51c   About an hour ago   1.43GB
testmobilitydb   13-3.4-v1.1.0-beta1   196c5b13904a   About an hour ago   1.43GB
testmobilitydb   14-3.4-v1.1.0-beta1   c20011180165   About an hour ago   1.44GB
testmobilitydb   15-3.4-v1.1.0-beta1   4345aed73dc8   About an hour ago   1.44GB
testmobilitydb   16-3.4-v1.1.0-beta1   6f070a6c3a3c   About an hour ago   1.45GB

[ImreSamu]

I would also note that the CI script could perhaps be made more contributor-friendly.

Docker build with Pull request

The docker-postgis uses this approach: The Docker build always runs, as seen at https://github.com/postgis/docker-postgis/blob/master/.github/workflows/main.yml, but the "docker push" does not execute for pull requests, as indicated by if: ${{ (github.ref == 'refs/heads/master') && (github.event_name != 'pull_request') }}. Perhaps the MobilityDB script could be modified to ensure minimal building occurs during PRs.

Testing MobilityDB images:

Later on, adding MobilityDB Docker image testing could be beneficial.
The docker-postgis runs standard Docker Postgres tests:

• https://github.com/docker-library/official-images/blob/master/test/tests/postgres-basics/run.sh
• https://github.com/docker-library/official-images/blob/master/test/tests/postgres-initdb/run.sh

and a minimal PostGIS test:

• https://github.com/postgis/docker-postgis/blob/master/test/tests/postgis-basics/run.sh

I can assist with this if needed.

[ImreSamu]

@mschoema

I have completed the testing.

I have minimally tested the new MobilityDB images and made some comments and suggestions.
If you have any questions about these, I am happy to provide answers.

[mschoema]

@ImreSamu
Thank you very much for your comments.
I have indeed managed to reduce the image size by running everything in a single RUN command and removing the build packages at the end.

REPOSITORY        TAG             IMAGE ID       CREATED          SIZE
testmobilitydb    12-3.4-docker   6b48d60d46b1   3 minutes ago    635MB
testmobilitydb    13-3.4-docker   04bdd51d30e0   4 minutes ago    635MB
testmobilitydb    14-3.4-docker   8ea108b7a053   5 minutes ago    638MB
testmobilitydb    15-3.4-docker   77433061cda0   5 minutes ago    642MB
testmobilitydb    16-3.4-docker   e596e898ff78   6 minutes ago    648MB

As you suggested, I also added a test script to build to the images locally.

Concerning your second comment, I think I will leave this for a future improvement.
To build mobilitydb, the Dockerfile is currently fetching an archive using:
wget -O MobilityDB.tar.gz "https://github.com/MobilityDB/MobilityDB/archive/$MOBILITYDB_TAG.tar.gz"
In a PR, we would want to fetch and build from the fork that started the PR, so this will have to change.
Since we have the files already we actually shouldn't need to fetch, but I wasn't able to get this working.
The next improvement could thus be to build the docker image using the local files, but I still have to figure out how.
If you have suggestions on how to do this I'd gladly take them.

Note that the issue is also present in the test script.
For local testing, the dockerfile should thus be modified to fetch from the right fork.
Similarly, the mobilitydb_ver variable in test_docker.sh should be set to the correct branch name.

For the moment I would thus conclude this PR and leave the CI build and test in PR's for a later improvement.

What are your thoughts on this?

[ImreSamu]

@mschoema :

The changes in the code look good! Thank you!

I have just one important request, which personally matters to me.
The other aspects can wait until the next iteration.
If this suggestion is acceptable, then I am already in favor of accepting the PR. ✅

For compatibility reasons, I would suggest renaming
/docker-entrypoint-initdb.d/mobilitydb.sh to /docker-entrypoint-initdb.d/11_mobilitydb.sh.
This change will help maintain the correct setup order in Docker when users add their own initialization files,
like the end user adding : 99_setup_my_testdata.sh.

my proposal - in this section:

-  && cp ../docker/initdb-mobilitydb.sh /docker-entrypoint-initdb.d/mobilitydb.sh \
+  && cp ../docker/initdb-mobilitydb.sh /docker-entrypoint-initdb.d/11_mobilitydb.sh \

This naming convention follows the guidelines set in the upstream Docker-PostgreSQL documentation,
widely used by many Docker-Postgres users. ( myself included )
Since the Docker-PostGIS initialization script is 10_postgis.sh,
choosing a name close to it makes the transition easier for existing users.

You can read more about this in the Docker-Postgres Documentation:
"These initialization files will be executed in sorted name order as defined by the current locale, which defaults to en_US.utf8." See more at https://github.com/docker-library/docs/tree/master/postgres#initialization-scripts

Thank you in advance.

[mschoema]

Sounds good, I'll update the file name to 11_mobilitydb.sh

[ImreSamu]

Thank You!

[ImreSamu]

Approved!

[mschoema]

Ok, I managed to get both entrypoint files to run correctly.
I think the PR is complete now.