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

Added Test::Nginx to alpine-fat #58

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Added Test::Nginx to alpine-fat #58

wants to merge 5 commits into from

Conversation

AlecHFerguson
Copy link

I recently discovered this excellent repo when trying to run Test::Nginx tests for my OpenResty project. Test::Nginx wasn't installed so I've gone ahead and added it.

I built an image from this Dockerfile and am able to run tests successfully (getting 'em passing is another matter!)

The current alpine-fat image is 234MB; this one is 259MB. So definitely a bit bigger. I still think it's in the spirit of fat to include all build utils, including tests.

@neomantra
Copy link
Member

neomantra commented Nov 30, 2017

@AlecHFerguson Thanks for your pull request. I think it does make sense to add this specific testing package, even though the other distros don't include it (e.g. stretch could also load libtest-nginx-perl but it doesn't). I'm also more open to these sorts of changes with the advent of Docker multi-stage builds. Developers can use alpine-fat for all sorts of things and then COPY them into an alpine build without a huge resulting image.

That said, regarding this specific pull request: Can you install this Test::Nginx without "capanminus"? And whatever extra things get installed and aren't needed, can you uninstall them in the same RUN stanza?

@neomantra
Copy link
Member

Following what you have, I don't get a clean build:

! Installing the dependencies failed: Module 'LWP::UserAgent' is not installed, Module 'List::MoreUtils' is not installed
! Bailing out the installation for Test-Nginx-0.26.

I tried installing Test::Nginx with cpan and that failed in some C compilation of nginx?

I also installed cpanm by adding perl-utils to the .build-deps and then running PERL_MM_USE_DEFAULT=1 cpan App::cpanminus. I would prefer to use this method rather than curl+pipe (also note you used the http endpoint rather than https). Installing with cpan worked, but then the installation of Test::Nginx failed the same way as above.

And then perhaps in the end cpanm can uninstall itself?

@AlecHFerguson
Copy link
Author

Thanks for the feedback @neomantra . Glad you support the idea. I'm happy to help add to the fat versions for distros as well.

I'll do some more research into installing without cpanminus. With all the dependencies that Test::Nginx has, I presume a package manager like cpanminus is kinda necessary? I added a line to remove cpanminus after use.

I've switched to using the https url, good call. I generally agree that installing cpanminus via package manager (apk) would be better than curl. Lemme try out straight cpan, see if that works.

As to the failing builds, I'm happy to help debug. The image built cleanly for me so I'm curious what could be different. I'm running Docker for Mac version 17.06, which I don't think should affect it. Is your build environment accessible to me in some way? CI instance I can look at, an AMI I can try?

@neomantra
Copy link
Member

After looking at what it is (and seeing that @agentzh recommends it), I think using cpanminus is OK. I would prefer to install it from cpan than a convenience URL (but maybe it doesn't matter if that's a trusted URL with HTTPS?)

I'm not able to reproduce the errors that I was talking about after fiddling more and building clean. Maybe there was something stale and whacked.

So can you add this to the --virtual section, after perl-dev:
perl-utils \
And this instead of your curl line?
&& PERL_MM_USE_DEFAULT=1 cpan App::cpanminus \

I'll then re-test the build and squash your commits as one. Thanks!

I plan on revamping the Dockerfiles and a build system to use multi-stage builds. That will make managing "fat" builds a lot easier. I'm doing that around Christmas. For OpenResty-packaged variants, I think OpenResty maintains these test packages (e.g. libtest-nginx-perl).

@@ -104,6 +106,7 @@ RUN apk add --no-cache --virtual .build-deps \
--with-lua-include=/usr/local/openresty/luajit/include/luajit-2.1 \
&& make build \
&& make install \
&& PERL_MM_USE_DEFAULT=1 cpan install Test::Nginx \
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neomantra I made these changes. Surprisingly, it bumped the image size up from 255 MB to 291 MB. If we're comfortable with that then I can squash as is. I do think the curl approach is perfectly valid, and does avoid having to install cpan.

Copy link
Member

@neomantra neomantra Nov 30, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm... I think there might be cpan cache's that need to get deleted too -- /root/.cpan is taking 55.6M. So after removing cpanminus, there needs to be this line:
&& rm -rf /root/.cpan /tmp/cpan* \

That was a quick glance using find . -name '*cpan*' within the image.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, i see you aren't using cpanminus... just put that after the install.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gah, sorry again... I think it should just be the build folder? /root/.cpan/build or perhaps there's a way to tell cpan not to save stuff. The build folder is ~18MB.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @neomantra, I'll give this a try. I'm busy all day today but I'll pick this up again in a couple days.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@neomantra You were spot on. Removing the /root/.cpan directory got the image down to 241 MB. Versus the current image that's 234 MB.

@neomantra
Copy link
Member

@AlecHFerguson Does that /root/.cpan directory have anything needed for normal operation of CPAN packages? I later thought maybe just the /root/.cpan/build folder was OK to delete. Could you run test and do development and/or production with these resultant images?

@dorongutman
Copy link

@neomantra are there any updates on the multi-stage builds ? if not, can you share what should I copy from this image to another alpine image in my own multi-stage dockerfile (of my application, not a fork of this)

@AlecHFerguson
Copy link
Author

@dorongutman You want to install Test::Nginx in your image? Line 109, cpan install Test::Nginx should do it no?

My apologies for neglecting to get this PR finished. I’m traveling for the next couple weeks so have no computer available. Should be able to get back to this in a couple weeks when I get home. Thanks for the reminder.

@dorongutman
Copy link

@AlecHFerguson no, I want to build openresty in its own image (with all the building dependencies), then copy whatever needed to a clean alpine image

@dorongutman
Copy link

@AlecHFerguson or for example, copy whatever I need from a openresty/openresty:alpine-fat image into a clean alpine image

@neomantra
Copy link
Member

Here's how to self-service what contents are at that layer:

# clone docker-openresty
git clone https://github.com/openresty/docker-openresty.git
cd docker-openresty

# build without removing intermediate layers
docker build -f alpine/Dockerfile --rm=false .

# find the container for the "make" step that that build just did
MYIMAGE=$(docker ps -a --no-trunc | grep make | head -1 | awk '{print $1}')

# list this contents of that layer
# note that this doesn't include some subsequent COPY commands
docker diff $MYIMAGE

@neomantra
Copy link
Member

@dorongutman Otherwise, I've not had a chance to use multi-stage builds yet. But the idea is more to have alpine-fat derive from the alpine image. Being able to copy-out OpenResty is pretty handy, but if you really want to do that, you might consider using the official OpenResty packages for Debian or CentOS.

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.

3 participants