Add eggdrop to official-images

[vanosg]

Checklist for Review

• associated with or contacted upstream?
• does it fit into one of the common categories? ("service", "language stack", "base distribution")
• is it reasonably popular, or does it solve a particular use case well?
• does a documentation PR exist? (should be reviewed and merged at roughly the same time so that we don't have an empty image page on the Hub for very long)
  • Init docs repo for Eggdrop docs#632
• dockerization review for best practices and cache gotchas/improvements (ala the official review guidelines)?
• 2+ dockerization review?
• existing official images have been considered as a base? (ie, if foobar needs Node.js, has FROM node:... instead of grabbing node via other means been considered?)
• if FROM scratch, tarballs only exist in a single commit within the associated history?
• passes current tests? any simple new tests that might be appropriate to add? (https://github.com/docker-library/official-images/tree/master/test)

[tianon]

Hey, sorry for the delay!

A few comments so far: (and I'm happy to turn any of this into a PR instead if you'd rather or if that'd make it more clear what I mean!)

• the final built image size can probably be reduced significantly by installing and removing build dependencies in the same layer (including the eggdrop source itself); see redis for a reasonably simple example of this (many official images follow this pattern, but redis is an especially short one to show off the pattern itself instead of the details of building some particular bit of software 😄)
• gpg --batch --verify eggdrop.tar.gz.asc should be gpg --batch --verify eggdrop.tar.gz.asc eggdrop.tar.gz (see Fix suggested "gpg" usage to stop relying on deprecated and insecure behavior #1420 (comment) for details)
• grabbing the initial tarball should probably be more specific than simply hitting geteggdrop.com and hoping it gives back 1.6.21 (probably want to grab ftp://ftp.eggheads.org/pub/eggdrop/source/stable/eggdrop1.6.21.tar.gz explicitly)
• E7C0E7F7 should be replaced with the full fingerprint, which should be B0B3D92ABE1D20233A2ECB01DB909F5EE7C0E7F7
• pgpkeys.mit.edu isn't always as reliable as we'd like, so it'd be a good idea to swap that over to ha.pool.sks-keyservers.net (which still has issues, but less often overall than MIT's server individually does)
• in entrypoint.sh, you're prompting for user input, but my understanding of eggdrop is that it's normally a background/server process (and thus not likely to be run interactively) -- do you think it'd be appropriate to instead print the warning with a short delay to give the astute operator a chance to rectify the situation but still give the "brave" a simple way to run without? 😄

I think this is a really great start, and again, happy to turn any of this into a PR if that'd be helpful. 😄 👍

[vanosg]

Thanks for the great ideas. Here's what I've done:

• the final built image size can probably be reduced significantly by...

Addressed in eggheads/eggdrop-docker@90ebb77 . Attempted to consolidate build/remove to one step. Didn't trim much space, but was successful in converting to alpine in eggheads/eggdrop-docker@c0cadfa! Any suggestions on that to further trim down the space?

• gpg --batch --verify eggdrop.tar.gz.asc should be gpg --batch --verify ...
• E7C0E7F7 should be replaced with the full fingerprint...
• pgpkeys.mit.edu isn't always as reliable as we'd like...

Addressed in eggheads/eggdrop-docker@3309626

• grabbing the initial tarball should probably be more specific ...

Addressed in eggheads/eggdrop-docker@46272ab

• in entrypoint.sh, you're prompting for user input, but my understanding of eggdrop

You're right. And I don't have a good answer. Can you expound a little more on your proposed solution? Alternatively, is there a way I can still write to stdout even if the process is started with -d ?

Thank you for the helpful tips! I am updating the tags for 1.6 (which you looked at) now. I'll follow-up with the 1.8 branch tomorrow. And then probably bug you again on status over the weekend :)

[TimWolla]

@vanosg TimWolla here. We talked in IRC about 18 hours ago :-)

I just added some comments to your commits that should explain how to reduce the image size properly!

[yosifkit]

Ok, I am making a few minor fixes and suggestions to the Dockerfile (soon to be a PR). When I run the container, it is unable to connect to an irc network with DNS lookup failed. Higher in the log I see "No nameservers found" which I have traced it to coredns.c. After further digging into res_init, I found that it is likely impossible to use it with musl libc (the libc in Alpine) since their definition is lacking.

I think their are three possible solutions:

• switch to Debian or Ubuntu to get glibc, but gain larger image size
• use sed to uncomment #set dns-servers line
  • would need to warm users to set dns in a custom config file
• use sed to copy nameservers from resolve.conf to #set dns-servers line
  • still would need to warn users to set dns-servers in their config
  • is there a limit to the number of servers in this line?

I could add any of these in the PR I am working on, just let me know what you think is best for your image.

[vanosg]

yosifkit, that’s some impressive detective work! Let me go over this with my team- coredns.c is part of an ‘optional’ (but really, semi-important) module that allows async DNS requests to go out, so that the code isn’t blocking while waiting for a DNS reply with standard DNS lookups. The “easy” fix would be to not load the DNS module by default, but then the user would have a less-than-ideal experience. Your suggestions are all great as well- let us chat on this a bit and I’ll get back to you shortly. Thanks for the feedback! If there is any work I can do on this side to save you effort on your PR, don’t hesitate to let us know.

Thanks!

[yosifkit]

The Dockerfiles look pretty good to me. There is a minor issue of the rm on line 23 needing the $EGGDROP_COMMIT which makes the image bigger, but it is still only ~51MB, so that could be fixed after we merge this today (hopefully).

(left a comment over on the docs too)

I give it my LGTM, Build test of #1916; 10d9602 (eggdrop):

$ bashbrew build eggdrop:1.6.21
Using bashbrew/cache:551e74f64bd7d147b187408e85a67c7da6a39c851bd9ee8a086e6c94149fc4a2 (eggdrop:1.6.21)
Tagging eggdrop:1.6.21
Tagging eggdrop:1.6
Tagging eggdrop:stable
Tagging eggdrop:latest

$ test/run.sh eggdrop:1.6.21
testing eggdrop:1.6.21
    'utc' [1/4]...passed
    'cve-2014--shellshock' [2/4]...passed
    'no-hard-coded-passwords' [3/4]...passed
    'override-cmd' [4/4]...passed


$ bashbrew build eggdrop:1.8.0
Using bashbrew/cache:c14e958e2a61105b208817451b6b5b439309c210fcc9c95ba8bd91132856957c (eggdrop:1.8.0)
Tagging eggdrop:1.8.0
Tagging eggdrop:1.8
Tagging eggdrop:develop

$ test/run.sh eggdrop:1.8.0
testing eggdrop:1.8.0
    'utc' [1/4]...passed
    'cve-2014--shellshock' [2/4]...passed
    'no-hard-coded-passwords' [3/4]...passed
    'override-cmd' [4/4]...passed

cc @tianon for second "dockerization review"

[tianon]

LGTM

[vanosg]

Fixed the rm issue- this is why making changes after midnight after a 12 hour day is a bad idea... sigh. You guys are the best, thanks so much for catching all these silly errors!

[yosifkit]

Thanks @vanosg! Can I get a bump to the commit id here for 1.8?

[yosifkit]

There is an extra GitRepo: here. 😄 Since they are the same git repo, you could just put it once at the top of the file with Maintainers and it will apply to both tag lists.

[yosifkit]

$ docker images eggdrop
REPOSITORY          TAG                 IMAGE ID            CREATED             SIZE
eggdrop             1.8                 47cb412364d9        13 minutes ago      21.13 MB
eggdrop             1.8.0               47cb412364d9        13 minutes ago      21.13 MB
eggdrop             develop             47cb412364d9        13 minutes ago      21.13 MB
eggdrop             1.6                 6e31a5a53ea9        4 days ago          16.84 MB
eggdrop             1.6.21              6e31a5a53ea9        4 days ago          16.84 MB
eggdrop             latest              6e31a5a53ea9        4 days ago          16.84 MB
eggdrop             stable              6e31a5a53ea9        4 days ago          16.84 MB

Everything else is in order, but Docker has asked us to not push any images today, so this will have to wait until tomorrow morning to be merged and built for the hub (I can just fix the extra GitRepo on merge if you don't get to it before tomorrow morning).

[yosifkit]

LGTM, Build test of #1916; ed10194 (eggdrop):

$ bashbrew build eggdrop:1.6.21
Using bashbrew/cache:3b933f6e2c6179a48276fff7f1ff37e00ba3aba7408df4485ebd9975839065d7 (eggdrop:1.6.21)
Tagging eggdrop:1.6.21
Tagging eggdrop:1.6
Tagging eggdrop:stable
Tagging eggdrop:latest

$ test/run.sh eggdrop:1.6.21
testing eggdrop:1.6.21
    'utc' [1/4]...passed
    'cve-2014--shellshock' [2/4]...passed
    'no-hard-coded-passwords' [3/4]...passed
    'override-cmd' [4/4]...passed


$ bashbrew build eggdrop:1.8.0
Using bashbrew/cache:9b042c3a4a78da537b4f88f4e0f10f54c761babe136cd388f3f57aa066a81831 (eggdrop:1.8.0)
Tagging eggdrop:1.8.0
Tagging eggdrop:1.8
Tagging eggdrop:develop

$ test/run.sh eggdrop:1.8.0
testing eggdrop:1.8.0
    'utc' [1/4]...passed
    'cve-2014--shellshock' [2/4]...passed
    'no-hard-coded-passwords' [3/4]...passed
    'override-cmd' [4/4]...passed