Skip to content

Improve initdb logic to properly source *.sql files as well, without the use of --single #75

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

Merged
merged 1 commit into from
Jul 24, 2015

Conversation

tianon
Copy link
Member

@tianon tianon commented Jul 23, 2015

postgres --single already has a lot of quirks, but is also discouraged for use by upstream (although why that fact isn't included prominently in the documentation for single-user mode, I have no idea -- it seems like an important bit to point out).

This mimics the server startup logic in docker-library/mysql#53 to give us "proper" PostgreSQL server access, while also setting listen_addresses to the empty string to prevent remote connections during our initialization process.

@md5
Copy link
Contributor

md5 commented Jul 23, 2015

This looks like a good change, though I'm still disappointed that the Postgres folks don't want to just fix --single...

One thing I'll point out is that this change will break any existing images using postgres --single in their initdb *.sh scripts, unless I'm missing something.

@tianon
Copy link
Member Author

tianon commented Jul 23, 2015 via email

@Starefossen
Copy link

It is so frustrating that Postgres still hasn't fixed --single yet 😡

@md5
Copy link
Contributor

md5 commented Jul 23, 2015

@Starefossen Indeed. Especially since last I heard MySQL was actually planning to implement something like --single in order to support containerized use cases (cf. docker-library/mysql#53 (comment)).

@yosifkit
Copy link
Member

I really do want to merge this change but would like a way to notify users of this breaking change. It might be 100's of users (github search).

@tianon
Copy link
Member Author

tianon commented Jul 24, 2015

On the plus side, it does at least fail cleanly (not just randomly overwrite state and destroy data):

/docker-entrypoint.sh: running /docker-entrypoint-initdb.d/init.sh
FATAL:  lock file "postmaster.pid" already exists
HINT:  Is another postmaster (PID 57) running in data directory "/var/lib/postgresql/data"?

@yosifkit
Copy link
Member

LGTM

yosifkit added a commit that referenced this pull request Jul 24, 2015
Improve initdb logic to properly source *.sql files as well, without the use of --single
@yosifkit yosifkit merged commit a82c28e into docker-library:master Jul 24, 2015
@yosifkit yosifkit deleted the better-initdb branch July 24, 2015 17:58
tianon added a commit to infosiftr/stackbrew that referenced this pull request Jul 24, 2015
md5 added a commit to postgis/docker-postgis that referenced this pull request Jul 24, 2015
@andrew-ang
Copy link

Ow :( This change cost us half a day of debugging. We were using https://registry.hub.docker.com/u/aidanlister/postgres-hstore/ and it just stopped working -- no useful error message to suggest why.

db_1       | LOG:  database system was shut down at 2015-07-27 04:32:36 UTC
db_1       | LOG:  MultiXact member wraparound protections are now enabled
db_1       | LOG:  autovacuum launcher started
db_1       | LOG:  database system is ready to accept connections

@md5
Copy link
Contributor

md5 commented Jul 27, 2015

@andrew-ang Looks like @aidanlister has updated that image, FWIW.

@marclennox
Copy link

@tianon With this initialization update, how would you suggest we perform initialization steps that require the database to be shutdown? Concrete example, I have an init script that recovers a base database backup and then places a recovery.conf file in the data directory prior to start.

@yosifkit
Copy link
Member

If you need to do things before the postgres server is running, you could make a new entrypoint script that does your restore and then execs to the regular entrypoint script. You would probably lose the ability to make a user by the environment vars, but you probably overwrite that in you database backup anyhow.

@mattgiles
Copy link

love postgres. love docker. love this repo. but lol @ "it might be 100s of users".

is there a reason not to use docker tags for version control (instead of meaning something like "postgres version") and enable users to pin the base image without forking the entire git repo?

@Starefossen
Copy link

I'm with @mattgiles on this one. I think this highlights a serious problem with how the (official) images are tagged.

@tianon
Copy link
Member Author

tianon commented Aug 7, 2015 via email

@mattgiles
Copy link

Any mapping of the ultimate image id to a unique tag would do the trick. I don't understand how changes to master in this repo get built / pushed to docker hub. Which makes it harder to offer concrete proposals. And this repository does seem to be consistent with the tagging behavior of other official images. Which would suggest the root issue is rather with broader docker conventions, as @Starefossen points out.

My suspicion is that it only seems cleaner to conflate software version with docker image version, where really it elides an important difference. I would personally prefer the additional mental burden of FROM postgres9.4:0.9 or FROM postgres9.4:snarky_berkus or FROM postgres:9.4_v1.1 to the burden of a production system suddenly not being able to use Dockerfiles that have not changed in months (and that derive from official images).

If this were to be addressed by docker, it seems like being able to substitute an image id for a tag would solve the problem nicely. Something like FROM postgres:730d1d72bda2 seems totally reasonable to me.

@md5
Copy link
Contributor

md5 commented Aug 8, 2015

@mattgiles You can already do FROM 730d1d72bda2 if you have "730d1d72bda2" pulled locally. However, docker pull 730d1d72bda2 doesn't work (it tries to pull library/730d1d72bda2).

$ { echo 'FROM 730d1d72bda2'; echo 'MAINTAINER foo'; } | docker build -
Sending build context to Docker daemon 2.048 kB
Sending build context to Docker daemon 
Step 0 : FROM 730d1d72bda2
 ---> 730d1d72bda2
Step 1 : MAINTAINER foo
 ---> Running in 85b5a7ab2b66
 ---> cade641f5c74
Removing intermediate container 85b5a7ab2b66
Successfully built cade641f5c74

@mattgiles
Copy link

So it turns out a viable solution already exists, using digests.

However, currently if I do:

$ docker pull postgres:9.4
$ docker images --digests | grep postgres
postgres            9.4            <none>            730d1d72bda2            2 weeks ago            265.3 MB

... the appropriate digest is <none>.

The only way I know of to acquire a digest is to push the image again to a v2 registry and then read the digest from stdout. Which isn't a global solution as this digest is now specific to my registry.

Is it a reasonable proposal for the postgres repo to publish the digest for new image versions when changes to master are pushed to docker hub? For instance in a readme or as a comment in the relevant Dockerfile?

@md5
Copy link
Contributor

md5 commented Aug 9, 2015

@mattgiles It appears that digests are only stored locally if the image is pulled by the digest. See the thread on docker-user@: https://groups.google.com/d/msg/docker-user/PvAcxDrvP30/XVrUe9hujdAJ

@md5
Copy link
Contributor

md5 commented Aug 9, 2015

BTW, as I understand it, the official images have been going into a V2 registry behind the scenes for months, so most if not all of them should already have a digest. It's just a bit of a pain to figure out what it is.

Perhaps the official images docs process could be updated to start including the digest in the README.md?

@mattgiles
Copy link

@md5 i think that makes a lot of sense. where does one make such a proposal?

@md5
Copy link
Contributor

md5 commented Aug 9, 2015

I think that https://github.com/docker-library/docs would be the appropriate place to make that proposal.

I'm not sure what would be a good format for including that information in the README.md, but it would be good to have some idea of what it would look like before making such a proposal.

justinlittman pushed a commit to gwu-libraries/social-feed-manager that referenced this pull request Aug 13, 2015
mbehrle added a commit to mbehrle/docker-gnuhealth-demo that referenced this pull request Mar 25, 2016
- The new initdb logic of the postgres container broke the --single
  setup of our container. As the --single switch is deprecated by the
  postgres folks, this is the cleaner way anyway.
- Simplifying the check for an existing gnuhealth database.

Refs:
- #2
- docker-library/postgres#75
- docker-library/postgres#78
RichardScothern pushed a commit to RichardScothern/official-images that referenced this pull request Jun 14, 2016
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.

7 participants