Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Functionalize the entrypoint to allow outside sourcing for extreme customizing of startup #496

Merged
merged 7 commits into from
Nov 14, 2019

Conversation

yosifkit
Copy link
Member

@yosifkit yosifkit commented Sep 11, 2018

This will allow a user to write a script to use our script as a base for their own postgres startup.

We need to ensure that that we define the set of functions that users can call. Currently that is any function that doesn't start with an underscore (listed below):

file_env
# create_postgres_dirs
docker_create_db_directories
# init_pgdata
docker_init_database_dir
docker_verify_minimum_env
docker_process_init_files
docker_process_sql
docker_setup_db
docker_setup_env
pg_setup_hba_conf
docker_temp_server_start
docker_temp_server_stop

EDIT: example updated (and tested) with an example docker run.

Here is an example using the current interface to run other init scripts on every start:

#!/usr/bin/env bash
set -Eeo pipefail

# Example using the functions of the postgres entrypoint to customize startup to always run files in /always-initdb.d/

source "$(which docker-entrypoint.sh)"

docker_setup_env
docker_create_db_directories
# assumption: we are already running as the owner of PGDATA

# This is needed if the container is started as `root`
#if [ "$1" = 'postgres' ] && [ "$(id -u)" = '0' ]; then
#	exec gosu postgres "$BASH_SOURCE" "$@"
#fi

if [ -z "$DATABASE_ALREADY_EXISTS" ]; then
	docker_verify_minimum_env
	docker_init_database_dir
	pg_setup_hba_conf
	
	# only required for '--auth[-local]=md5' on POSTGRES_INITDB_ARGS
	export PGPASSWORD="${PGPASSWORD:-$POSTGRES_PASSWORD}"
	
	docker_temp_server_start "$@" -c max_locks_per_transaction=256
	docker_setup_db
	docker_process_init_files /docker-entrypoint-initdb.d/*
	docker_temp_server_stop
else
	docker_temp_server_start "$@"
	docker_process_init_files /always-initdb.d/*
	docker_temp_server_stop
fi

exec postgres "$@"
$ docker run -it --rm \
  -e POSTGRES_PASSWORD=12345 \
  -v "$PWD/custom-entrypoint.sh":/usr/local/bin/custom-entrypoint.sh \
  -v "$PWD/docker-entrypoint-initdb.d/":/docker-entrypoint-initdb.d/ \
  -v "$PWD/always-initdb.d/":/always-initdb.d/ \
  -v "$PWD/data/":/var/lib/postgresql/data/ \
  --name=pg --user=1000 \
  postgres:11-test custom-entrypoint.sh -c 'max_locks_per_transaction=512'

Closes #463, closes #461

EDIT: Threading the needle; related to docker-library/rabbitmq#281
EDIT: applied new function names from discussion and nearing completion of docker-library/mysql#471 (rebased commits incoming)

@tianon
Copy link
Member

tianon commented Sep 12, 2018

I wonder if it'd make sense to namespace our functions to be more considerate of other functions folks might be using -- something like docker_xxx?

FYI @ltangvald (this is probably relevant to your interests for docker-library/mysql#471 -- it'd be great if we could get both using a similar approach ❤️)

@ltangvald
Copy link

Yeah, makes sense. Prefixing everything with docker_ sounds good to me :)

@yajo
Copy link

yajo commented Sep 19, 2018

I really miss support for /docker-entrypoint-always.d/* as a place where to store your own scripts that would be executed always before the postgres server is started.

Many times one needs to ensure some DB configurations state, no matter if the DB existed previously or not.

Do you think this is a good chance to tackle that?

@yosifkit
Copy link
Member Author

@yajo, the example I show for possible user customization includes exactly that and allows for all sorts of other customization.

@yajo
Copy link

yajo commented Sep 20, 2018

Yeah, I see, but your suggestion is to override the entrypoint AFAICS, but I was referring to supporting that kind of exec scripts in the official entrypoint.

Sometimes you need to override the default entrypoint behavior, but sometimes it's just simpler to drop a script and let it be executed before booting postgres.

A simple example that I have currently at hand: autofill of pg_hba.conf to restrict some connections from local network. Since at each docker run the network CIDR is different, I need to expand a template to get its current CIDR. IMHO it would be simpler to drop a /docker-entrypoint-always.d/generate-hba.sh file than having to write my own entrypoint, doing that stuff, and then falling back to the original entrypoint.

@itscaro
Copy link

itscaro commented Dec 3, 2018

@tianon @yosifkit Any news on this PR? I would love to have the chown fix 👍

@harpunius
Copy link

Any update on this? Our use case is very similar to that of @yajo

@yosifkit
Copy link
Member Author

yosifkit commented Jul 1, 2019

Left out applying update.sh until consensus is reached.

docker-library-bot added a commit to docker-library-bot/official-images that referenced this pull request Nov 15, 2019
Changes:

- docker-library/postgres@5beb1d4: Update to 9.4.25-1.pgdg90+1
- docker-library/postgres@a0ec4f5: Update to 9.5.20
- docker-library/postgres@6dfdc0e: Update to 11.6
- docker-library/postgres@2addeda: Update to 11.6-1.pgdg90+1
- docker-library/postgres@f13fbe5: Update to 9.6.16-1.pgdg90+1
- docker-library/postgres@f2596e6: Update to 10.11-1.pgdg90+1
- docker-library/postgres@c1e547b: Update to 10.11
- docker-library/postgres@cac7a60: Update to 9.6.16
- docker-library/postgres@1d43a9d: Update to 12.1
- docker-library/postgres@06a831c: Update to 9.5.20-1.pgdg90+1
- docker-library/postgres@138e959: Update to 9.4.25
- docker-library/postgres@4a82eb9: Update to 12.1-1.pgdg100+1
- docker-library/postgres@0c29c35: Merge pull request docker-library/postgres#496 from infosiftr/functionalization
- docker-library/postgres@8fada98: Fixes from tianon's review
- docker-library/postgres@7c84645: Apply update.sh for new entrypoint
- docker-library/postgres@d1cc089: Improve _is_sourced check
- docker-library/postgres@6e85168: Resync function interfaces with MySQL, improve comments
- docker-library/postgres@2e70e71: Apply function name changes as discussed in docker-library/mysql#471
- docker-library/postgres@49fb876: Namespace functions for less conflict when sourced
- docker-library/postgres@48f2ad1: Functionalize the entrypoint to allow outside sourcing for extreme customizing of startup
hswong3i added a commit to alvistack/docker-postgres that referenced this pull request Jan 14, 2020
hswong3i added a commit to alvistack/docker-postgres that referenced this pull request Jan 14, 2020
@Chanaka
Copy link

Chanaka commented Mar 3, 2020

This will allow a user to write a script to use our script as a base for their own postgres startup.

We need to ensure that that we define the set of functions that users can call. Currently that is any function that doesn't start with an underscore (listed below):

file_env
# create_postgres_dirs
docker_create_db_directories
# init_pgdata
docker_init_database_dir
docker_verify_minimum_env
docker_process_init_files
docker_process_sql
docker_setup_db
docker_setup_env
pg_setup_hba_conf
docker_temp_server_start
docker_temp_server_stop

EDIT: example updated (and tested) with an example docker run.

Here is an example using the current interface to run other init scripts on every start:

#!/usr/bin/env bash
set -Eeo pipefail

# Example using the functions of the postgres entrypoint to customize startup to always run files in /always-initdb.d/

source "$(which docker-entrypoint.sh)"

docker_setup_env
docker_create_db_directories
# assumption: we are already running as the owner of PGDATA

# This is needed if the container is started as `root`
#if [ "$1" = 'postgres' ] && [ "$(id -u)" = '0' ]; then
#	exec gosu postgres "$BASH_SOURCE" "$@"
#fi

if [ -z "$DATABASE_ALREADY_EXISTS" ]; then
	docker_verify_minimum_env
	docker_init_database_dir
	pg_setup_hba_conf
	
	# only required for '--auth[-local]=md5' on POSTGRES_INITDB_ARGS
	export PGPASSWORD="${PGPASSWORD:-$POSTGRES_PASSWORD}"
	
	docker_temp_server_start "$@" -c max_locks_per_transaction=256
	docker_setup_db
	docker_process_init_files /docker-entrypoint-initdb.d/*
	docker_temp_server_stop
else
	docker_temp_server_start "$@"
	docker_process_init_files /always-initdb.d/*
	docker_temp_server_stop
fi

exec postgres "$@"
$ docker run -it --rm \
  -e POSTGRES_PASSWORD=12345 \
  -v "$PWD/custom-entrypoint.sh":/usr/local/bin/custom-entrypoint.sh \
  -v "$PWD/docker-entrypoint-initdb.d/":/docker-entrypoint-initdb.d/ \
  -v "$PWD/always-initdb.d/":/always-initdb.d/ \
  -v "$PWD/data/":/var/lib/postgresql/data/ \
  --name=pg --user=1000 \
  postgres:11-test custom-entrypoint.sh -c 'max_locks_per_transaction=512'

Closes #463, closes #461

EDIT: Threading the needle; related to docker-library/rabbitmq#281
EDIT: applied new function names from discussion and nearing completion of docker-library/mysql#471 (rebased commits incoming)

Hi @yosifkit I'm following this reference for a custom entry point to cache the database I create. I've below command in my Dockerfile

RUN custom-entrypoint.sh postgres || exit 0  
ENTRYPOINT [ "custom-entrypoint.sh" ]

However when it run the RUN custom-entrypoint.sh postgres I get below error always, bcz of exec command in custom-entrypoint.sh (exec postgres "$@"). Are you able to provide any workaround to resolve this error please ?

postgres: invalid argument: "postgres"
Try "postgres --help" for more information.

I've removed that exec command from custom-entrypoint.sh, and moved to Dockerfile as below, And that seems to be working. But not sure that's the ideal way to do this ? Highly appreciate your thoughs

RUN custom-entrypoint.sh postgres || exit 0
CMD ["postgres"]
ENTRYPOINT [ "custom-entrypoint.sh" ]

@yosifkit
Copy link
Member Author

yosifkit commented Mar 3, 2020

@Chanaka, yeah you only need the postgres in one spot; either in the exec postgres "$@" or as an argument to the entrypoint (custom-entrypoint.sh postgres).

It depends on how you expect to use the custom entrypoint script. If you plan on allowing running things that aren't postgres, like bash or echo, then just ending with exec "$@" is what you want. If you just want the custom entrypoint only running postgres, then make that part of the script and not passing it in. In the default entrypoint, we do both and detect the desire to run postgres by it being the default CMD (i.e. "$1" = postgres) or by the first argument being a flag (starts with a dash, -.

For your use case, you'll probably need to set PGDATA to somewhere outside the already defined VOLUME of /var/lib/postgresql/data so that the database gets saved in the resulting image (like #672).

@godomainz
Copy link

@yosifkit I also had the chanaka's issue so I followed your instrauctions.
Changed Dockerfile to ENTRYPOINT [ "custom-entrypoint.sh","postgres"]
Changed cutome-entrypoint.sh to exec "$@"
but I build the image using this command docker image build --tag myDb:latest .

My build step hangs with below logs

2020-03-08 02:54:46.096 UTC [8] LOG: listening on IPv4 address "0.0.0.0", port 5432 2020-03-08 02:54:46.096 UTC [8] LOG: listening on IPv6 address "::", port 5432 2020-03-08 02:54:46.102 UTC [8] LOG: listening on Unix socket "/var/run/postgresql/.s.PGSQL.5432" 2020-03-08 02:54:46.116 UTC [55] LOG: database system was shut down at 2020-03-08 02:54:46 UTC 2020-03-08 02:54:46.126 UTC [8] LOG: database system is ready to accept connections

I don't run the image yet. I only need the images created and tagged so I can run them later.I have other stuff needs to happen after database image creation.

But currently it seems container starts and stays there.
Simply I need this ,
1. Start postgres 2. Load the database and build an image with database 3. Tag the image so I can use that image in later step

How can I resolve this ?

@yosifkit
Copy link
Member Author

yosifkit commented Mar 9, 2020

My build step hangs with below logs

If you are running the entrypoint in a RUN step, then you need your script to initialize and exit (not to start postgres). So the if [ -z "$DATABASE_ALREADY_EXISTS" ]; then block should exit and not fall through to the exec

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants