Skip to content

Add CouchDB #1288

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
Feb 25, 2016
Merged

Add CouchDB #1288

merged 1 commit into from
Feb 25, 2016

Conversation

klaemo
Copy link
Contributor

@klaemo klaemo commented Dec 17, 2015

Hey there,

I'm the CouchDB Docker person and I'd like to add an official CouchDB image to the registry.
It is based on the popular klaemo/couchdb image.
Let me know, if you need me to change anything.

Thanks for reviewing.

@psftw
Copy link
Contributor

psftw commented Jan 20, 2016

Greetings! Thank you for submitting this. Apologies for taking a month to get you some feedback. It's clear you've done your homework, which is appreciated. I successfully built the image and put it through some basic smoke testing.

I saw that there is an official PPA that only includes the latest stable release, and that your Dockerfile builds from source. I wonder if you've tried the PPA route or know if CouchDB upstream has an opinion on the matter.

The 2.0* images which perform git clone and build the latest revision won't work for an official image. The Dockerfile needs to be reproducible, and while bleeding edge betas/RCs/etc that have a unique git hash reference can be supported, building from master should be done in a non-official image.

One of the features of CouchDB is around supporting distributed setups for performance/redundancy. I wonder what would be needed to make that work in a Docker context or if you've explored that topic.

Some nitpicks on the Dockerfile: The permissions RUN command should be merged into the larger build/install RUN command above, otherwise it will create an unnecessary 1.5MB layer. The bind_address update RUN command could also be merged. Can logging to disk be disabled somehow, and hence avoid an extra volume? Is there a reason to include /usr/local/etc/couchdb as a volume, which appears to just contain configuration files? Does CouchDB respond to signals? The image failed my docker stop test, so at least how it is setup now it isn't responding to SIGTERM.

@klaemo
Copy link
Contributor Author

klaemo commented Jan 21, 2016

Thanks for the feedback, Peter :)

Where do I find the "official" PPA you're referring to? I'm not aware of one.

Concerning 2.0 images: I can use git hashes. That shouldn't be a problem and totally makes sense.
Since 2.0 will introduce a totally new clustering approach, we're going to have to think about the best way of spinning up containers. I haven't figured that out yet. For 1.6 we just need to start a single node, so nothing special there.

I'll address your points about the Dockerfile. Thanks :)
Logging to disk can be disabled. So, in containers it is preferable to only log to stdout?
/usr/local/etc/couchdb was included as a volume, because some users requested it. That's not the recommended behavior, I take it?

I'll look into the signal behavior.

Do you think we should allow configuration of admin user and password via environment vars? I've seen other database images do that.

@psftw
Copy link
Contributor

psftw commented Jan 21, 2016

The PPA is at https://launchpad.net/~couchdb/+archive/ubuntu/stable

What I meant about reproducible builds and git hashes regarding 2.x is that we can't support checking out master as part of the build, but we would support checking out something like a 2.0-RC1 tag (via git hash, which is actually unique/permanent). Unless there are tags like this to refer to, then we are left with a CI-style build that just grabs the latest code, which I don't think is appropriate. While something like daily/weekly builds could work, I would prefer to avoid that and just publish supported releases and maybe beta/RC that have specific version tags from upstream. CI-style builds are better suited to advanced users and testers who are running on the bleeding edge for a specific purpose -- not usually what gets published as an official repository. Hope this helps clarify.

I would recommend disabling logging except to stdout and removing both the ETC and LOG related volumes. It should be a quick blurb in the docs to inform users on how to pass in their own configuration via a volume, or by extending the image. Every VOLUME you put into the Dockerfile now forces it upon all downstream users, so having just one for the actual data would be ideal.

Yes, I believe being able to specify user/password via environment vars would be useful. I haven't dug in to the image much yet other than play with the /_util/ endpoint manually, so I'd recommend thinking about the common use cases and what would be helpful to specify via environment variables.

@tianon
Copy link
Member

tianon commented Jan 21, 2016 via email

@klaemo
Copy link
Contributor Author

klaemo commented Jan 21, 2016

So should I base the image on ubuntu and use the PPA?

@tianon
Copy link
Member

tianon commented Jan 21, 2016 via email

@klaemo klaemo force-pushed the couchdb branch 2 times, most recently from f081dc9 to 2cb61c1 Compare January 25, 2016 19:50
@klaemo
Copy link
Contributor Author

klaemo commented Jan 25, 2016

So I took care of some of the things:

  • removed 2.0 as there aren't any git tags or official pre-releases yet. I'll get back to that once we have those, but that shouldn't block this submission for now.
  • decided to keep using debian and build from source, because there is some confusion in the couchdb project as to wether the PPA is official or not 😄
  • disabled logging to file and removed log and etc volumes. Amended Readme accordingly
  • merged RUN instructions

Signal handling is still puzzling to me. CouchDB responds to SIGTERM in general, but doesn't when containerized. I'm not doing anything special in entrypoint.sh that would prevent signal handling, I think.

Anyways, thanks for your feedback so far :)

@yosifkit
Copy link
Member

I made a PR with some recommendations in klaemo/docker-couchdb#44.

Other comments:

We expect to see some docs with a PR to https://github.com/docker-library/docs/; which is how the content on the Docker Hub gets filed in (mostly just need to make a couchdb/content.md, but feel free to ask questions if it is not clear).

As far as the responding to SIGTERM I would say that couchdb is not explicitly handling it, so it gets ignored. Normally the kernel would then kill you if you don't handle the signal, but since it is in a container as pid 1 it will not be killed by the kernel. You might look into tini as recommend on the readme to handle signals for you.

$ docker run -d --name couch couchdb
$ docker ps -l
CONTAINER ID        IMAGE               COMMAND                  CREATED             STATUS              PORTS               NAMES
1dc512e8e4d9        couchdb             "/docker-entrypoint.s"   8 minutes ago       Up 8 minutes        5984/tcp            couch
$ docker kill -s INT couch
$ docker ps -lq
CONTAINER ID        IMAGE               COMMAND                  CREATED             STATUS              PORTS               NAMES
1dc512e8e4d9        couchdb             "/docker-entrypoint.s"   15 minutes ago      Up 15 minutes       5984/tcp            couch
$ # still running!

@klaemo
Copy link
Contributor Author

klaemo commented Jan 27, 2016

Thanks for your feedback @yosifkit :)

Would you mind taking a look at klaemo/docker-couchdb#45? It adds tini for signal handling and works great in my tests.

@dch
Copy link

dch commented Feb 1, 2016

wrt to this becoming an "official" image, perhaps we should have it under apache/couchdb ? Not sure if its possible to have both.

@klaemo
Copy link
Contributor Author

klaemo commented Feb 1, 2016

@dch there already is https://github.com/apache/couchdb-docker for that purpose which will become the single source of truth for the couchdb docker stuff. For details see https://github.com/klaemo/docker-couchdb/issues/22#issuecomment-159381133

@yosifkit
Copy link
Member

yosifkit commented Feb 1, 2016

ping @klaemo, could we get an update of the commit hashes here? The latest Dockerfile looks good to me. 👍

One new thought, would this work better to be FROM erlang:17 or erlang:17-slim?

@klaemo
Copy link
Contributor Author

klaemo commented Feb 1, 2016

@yosifkit I had the same thought about erlang yesterday, but the images are incredibly huge (erlang:17 740mb, erlang:17-slim 280mb). It would be convenient, though, as I'd love to pin the erlang version.

Is it okay to include https://github.com/klaemo/docker-couchdb/blob/master/1.6.1-couchperuser/Dockerfile with the official image? It's the same CouchDB but with a useful plugin pre-installed.

@klaemo
Copy link
Contributor Author

klaemo commented Feb 2, 2016

btw I force pushed the latest commit hashes

@yosifkit
Copy link
Member

yosifkit commented Feb 2, 2016

The couchperuser image seems like a fine variant to add; if that is what you guys want to support.

You'll need to update its FROM line and we recommend adding a sha256 or similar to verify the downloaded file. We have an example of this in the readme on the "better" list item. If you can convince the releaser of it to do a gpg signature that would be even better.

@klaemo
Copy link
Contributor Author

klaemo commented Feb 3, 2016

Done. Added sha256 verification and changed FROM.

I'll prepare a PR for the docs tomorrow.

@yosifkit
Copy link
Member

yosifkit commented Feb 3, 2016

Nice! 🎉 It all looks good to me. I would document how users would save their own log file since it logs to /dev/null (I wasn't able to get it to work with /dev/stdout). I look forward to the docs!

cc @tianon for final review as well.

Build test of #1288; 53419b9 (couchdb):

$ bashbrew build "couchdb"
Cloning couchdb (git://github.com/klaemo/docker-couchdb) ...
Processing couchdb:latest ...
Processing couchdb:1.6.1 ...
Processing couchdb:1.6 ...
Processing couchdb:1 ...
Processing couchdb:1.6.1-couchperuser ...
Processing couchdb:1.6-couchperuser ...
Processing couchdb:1-couchperuser ...
$ bashbrew list --uniq "$url" | xargs test/run.sh
testing couchdb:latest
    'utc' [1/4]...passed
    'cve-2014--shellshock' [2/4]...passed
    'no-hard-coded-passwords' [3/4]...passed
    'override-cmd' [4/4]...passed
testing couchdb:1.6.1-couchperuser
    'utc' [1/4]...passed
    'cve-2014--shellshock' [2/4]...passed
    'no-hard-coded-passwords' [3/4]...passed
    'override-cmd' [4/4]...passed

@tianon
Copy link
Member

tianon commented Feb 11, 2016

VOLUME ["/usr/local/var/lib/couchdb"]
WORKDIR /var/lib/couchdb

Shouldn't these two paths be the same? Which one is correct? 😄

@tianon
Copy link
Member

tianon commented Feb 11, 2016

(the entrypoint then also references the path in /usr/local)

@tianon
Copy link
Member

tianon commented Feb 11, 2016

Other than that and the point about /dev/null, this is looking pretty good to me, too. 👍

@klaemo
Copy link
Contributor Author

klaemo commented Feb 16, 2016

@tianon the paths are correct (as per https://cwiki.apache.org/confluence/display/COUCHDB/Debian) . /var/lib/couchdb is the couchdb user's HOME.

/usr/local/var/lib/couchdb is where the actual data lives in *.couch files.

@klaemo
Copy link
Contributor Author

klaemo commented Feb 18, 2016

Just opened a PR for the documentation.

@yosifkit
Copy link
Member

LGTM, Build test of #1288; 53419b9 (couchdb):

$ bashbrew build "couchdb"
Cloning couchdb (git://github.com/klaemo/docker-couchdb) ...
Processing couchdb:latest ...
Processing couchdb:1.6.1 ...
Processing couchdb:1.6 ...
Processing couchdb:1 ...
Processing couchdb:1.6.1-couchperuser ...
Processing couchdb:1.6-couchperuser ...
Processing couchdb:1-couchperuser ...
$ bashbrew list --uniq "$url" | xargs test/run.sh
testing couchdb:latest
    'utc' [1/4]...passed
    'cve-2014--shellshock' [2/4]...passed
    'no-hard-coded-passwords' [3/4]...passed
    'override-cmd' [4/4]...passed
testing couchdb:1.6.1-couchperuser
    'utc' [1/4]...passed
    'cve-2014--shellshock' [2/4]...passed
    'no-hard-coded-passwords' [3/4]...passed
    'override-cmd' [4/4]...passed

@tianon
Copy link
Member

tianon commented Feb 25, 2016

LET'S FINISH THIS

tianon added a commit that referenced this pull request Feb 25, 2016
@tianon tianon merged commit f133246 into docker-library:master Feb 25, 2016
@klaemo
Copy link
Contributor Author

klaemo commented Feb 25, 2016

🎉🎉 Thank you :)

Am 25.02.2016 um 01:38 schrieb yosifkit [email protected]:

LGTM, Build test of #1288; 53419b9 (couchdb):

$ bashbrew build "couchdb"
Cloning couchdb (git://github.com/klaemo/docker-couchdb) ...
Processing couchdb:latest ...
Processing couchdb:1.6.1 ...
Processing couchdb:1.6 ...
Processing couchdb:1 ...
Processing couchdb:1.6.1-couchperuser ...
Processing couchdb:1.6-couchperuser ...
Processing couchdb:1-couchperuser ...
$ bashbrew list --uniq "$url" | xargs test/run.sh
testing couchdb:latest
'utc' [1/4]...passed
'cve-2014--shellshock' [2/4]...passed
'no-hard-coded-passwords' [3/4]...passed
'override-cmd' [4/4]...passed
testing couchdb:1.6.1-couchperuser
'utc' [1/4]...passed
'cve-2014--shellshock' [2/4]...passed
'no-hard-coded-passwords' [3/4]...passed
'override-cmd' [4/4]...passed

Reply to this email directly or view it on GitHub.

@almereyda
Copy link

Congratulations.

On 25 February 2016 at 10:17, Clemens Stolle [email protected]
wrote:

🎉🎉 Thank you :)

Am 25.02.2016 um 01:38 schrieb yosifkit [email protected]:

LGTM, Build test of #1288; 53419b9 (couchdb):

$ bashbrew build "couchdb"
Cloning couchdb (git://github.com/klaemo/docker-couchdb) ...
Processing couchdb:latest ...
Processing couchdb:1.6.1 ...
Processing couchdb:1.6 ...
Processing couchdb:1 ...
Processing couchdb:1.6.1-couchperuser ...
Processing couchdb:1.6-couchperuser ...
Processing couchdb:1-couchperuser ...
$ bashbrew list --uniq "$url" | xargs test/run.sh
testing couchdb:latest
'utc' [1/4]...passed
'cve-2014--shellshock' [2/4]...passed
'no-hard-coded-passwords' [3/4]...passed
'override-cmd' [4/4]...passed
testing couchdb:1.6.1-couchperuser
'utc' [1/4]...passed
'cve-2014--shellshock' [2/4]...passed
'no-hard-coded-passwords' [3/4]...passed
'override-cmd' [4/4]...passed

Reply to this email directly or view it on GitHub.


Reply to this email directly or view it on GitHub
#1288 (comment)
.

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

Successfully merging this pull request may close these issues.

7 participants