Skip to content
This repository has been archived by the owner. It is now read-only.

Feature/configuration volumes #13

Conversation

yankeeinlondon
Copy link

This combines what was discussed in PR #11 but adds two levels of configuration:

  • Automatic Config
    • Just run it ... sudo docker run -d nginx:1.x
    • You get standard config as it runs today
  • New Service Config
    • Added in a volume share /nginx/conf.d which allows new server sections to be added on a shared volume
    • The existing server config (aka, port 80) is still in play as are all the default settings in the "http" section
  • Complete Control
    • Added another share under /nginx/conf which gives the host complete control
    • This allows setting anything wanted in "http" section, allows dynamic template generation of core config
    • Can be done in conjunction or independently of above share

Note: By choosing /nginx as a base for configuration I have opted for a non-unix directory but in effect all I'm aiming to do is provide a compact and explanatory CLI for the host. Not sure if you'll agree with my thinking but I hope so ... I tend to fall in love with my own ideas.

Last comment ... I'm sorry I couldn't have just let PR #11 run it's course but I needed something up and running immediately that met my needs. I'd love it if this could be brought back into the fold but if not then thank you for an excellent starting point.

&& rm -r /usr/src/nginx \
&& chown -R www-data:www-data /usr/local/nginx \
&& rm -r /usr/src/nginx
# Modify nginx.conf file for Docker use
Copy link
Author

Choose a reason for hiding this comment

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

Note: I broken up this long && chain of commands partly because I felt there was a semantic separation but also because there is a performance benefit from keeping these apart as any change to the configuration files no longer forces a rebuild.

Choose a reason for hiding this comment

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

By separating the following line && apt-get purge -y --auto-remove $buildDeps from the above you effectively break the main reason (as I understand it) of doing it that way. Removing files or packages in the next layer will not make the image any smaller.

I see your point though. If it could be made without splitting up the build process from cleanup I would like it.

Copy link
Author

Choose a reason for hiding this comment

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

Ok, sounds like we push the --auto-remove into the && chain but drop out the configuration changes as it's own item. I'll do that right now.

Choose a reason for hiding this comment

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

Now it will work. No matter how its done I badly need the config dirs for
volumes.
Den 19 sep 2014 00:10 skrev "Ken Snyder" [email protected]:

In 1.6/Dockerfile:

@@ -135,19 +135,30 @@ RUN buildDeps="
&& make -j"$(nproc)"
&& make install
&& cd / \

  • && rm -r /usr/src/nginx \
  • && chown -R www-data:www-data /usr/local/nginx \
  • && rm -r /usr/src/nginx
    +# Modify nginx.conf file for Docker use

Ok, sounds like we push the --auto-remove into the && chain but drop out
the configuration changes as it's own item. I'll do that right now.


Reply to this email directly or view it on GitHub
https://github.com/docker-library/nginx/pull/13/files#r17758447.

@yankeeinlondon
Copy link
Author

@vovimayhem looping you in here as I know this is near and dear to your heart too ... let me know if you'd be ok with my rendition or if you can offer any improvements you'd want to see.

EXPOSE 80
CMD ["nginx"]
ENTRYPOINT ["nginx"]
Copy link
Author

Choose a reason for hiding this comment

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

Oh, also didn't mean to sneak this in there ... I just forgot to mention this change too. If this is a real bone of contention this is the least important part for me and I'd happily take it out. However, I may very well add a bootstrapper script to the entrypoint so that:

ENTRYPOINT ["bootstrapper"]
CMD ["start"]

This would allow the start param or no param to have the same effect of just running the nginx server but allows for other commands to be hung off of it too like possibly: sudo docker run nginx version which would report back a version number or sudo docker run nginx ps which would show the running processes. Do know of any examples of this style of setup? Seems intuitive but it has a few gotcha's in it too ... not least of which it would put Nginx back into daemon mode and the bootstrapper would need to preserve the running state.

Choose a reason for hiding this comment

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

Correct me if I am wrong, but adding an ENTRYPOINT to a base image of this kind would not work that well. For example, when I package web applications based on this image ENTRYPOINT would be overridden by my own setup scripts.

I suppose I could make them work together but that should not be something forced upon me.

Copy link
Author

Choose a reason for hiding this comment

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

I may be missing something here. Can you give me an example of where this would interfere?

Choose a reason for hiding this comment

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

It would seem I was tired and thought you actually put bootstrapper in

`````` ENTRYPOINT```.

In that case its just the question of CMD vs ENTRYPOINT, which
I won't get much into. Only one thing. When it comes to run once scripts,
would they not run every time with CMD? I currently use
ENTRYPOINT for setup at runtime.
Den 19 sep 2014 00:13 skrev "Ken Snyder" [email protected]:

In 1.7/Dockerfile:

EXPOSE 80
-CMD ["nginx"]
+ENTRYPOINT ["nginx"]

I may be missing something here. Can you give me an example of where this
would interfere?


Reply to this email directly or view it on GitHub
https://github.com/docker-library/nginx/pull/13/files#r17758637.

Copy link
Author

Choose a reason for hiding this comment

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

Yeah sorry I probably confused matters by bringing it up. Let's keep the idea of a proxy out of this PR.

@yankeeinlondon
Copy link
Author

One other thought I'd been having recently is that instead of the shares:

  • /nginx/conf
  • /nginx/conf.d

Maybe better nomenclature would be:

  • /app/conf
  • /app/conf.d

This would allow for a consistent signature to develop for volume sharing across containers. Initially I had thought this might represent lower specificity but really having /nginx in the prefix is redundant and in its redundancy I think less clear than the more generic naming convention. Anyway, interested in your thoughts.

@AsavarTzeth
Copy link

Nice in theory. I see a few problems though.

From a practical view I see no huge issue with it.

The only sure thing that comes to mind is if we use packages instead.
Depending on how the package was built we might not even be able to do this.

Also as I understand it the long term goal with the official images is to
get upstream involved. That means adjusting to the preferences of each
project. I cannot say for sure if this goes against that, but it is
something to keep in mind.
Den 19 sep 2014 00:30 skrev "Ken Snyder" [email protected]:

One other thought I'd been having recently is that instead of the shares:

  • /nginx/conf
  • /nginx/conf.d

Maybe better nomenclature would be:

  • /app/conf
  • /app/conf.d

This would allow for a consistent signature to develop for volume sharing
across containers. Initially I had thought this might represent lower
specificity but really having /nginx in the prefix is redundant and in
its redundancy I think less clear than the more generic naming convention.
Anyway, interested in your thoughts.


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

@yankeeinlondon
Copy link
Author

@AsavarTzeth there should be no difference in limitations between /nginx/* and /app/* so I have made this change.

With regards to moving away from building from source and using a package manager instead -- I think this is what you were referring to -- I don't expect this to pose any problems you can always use a symlink strategy to provide the docker command line in it's compact form. This is desirable as implementation details like where the package manager has installed packages should not need to be known. Make sense?

BTW, I am a bit surprised we're even considering moving toward package managers ... the current approach is quite simple and provides ultimate flexibility. I think ideally we could/should provide a -e flag variable for users to add or remove configuration options, something like:

sudo docker run -d -e WITHOUT-MAIL nginx:1.x

For now, however, I see no downside to maintaining the current configure/make/make install approach but leaving the -e switches off.

@yankeeinlondon
Copy link
Author

Also as I understand it the long term goal with the official images is to
get upstream involved.

@AsavarTzeth this sounds interesting but I'm not sure what it means. Can you explain a bit more or point me at an resources that describe it?

@AsavarTzeth
Copy link

I actually agree with you on most of this; I too prefer building it from source. But at least they do maintain their own yum and apt repositories with the latest builds, in case those will be used.

I think your idea makes perfect sense, I just personally prefer to avoid symlinks if possible and have had some issues with that solution if I am using volumes in certain ways. I suppose I could have phrased things better.

Regarding my assumptions, I assume one goal (more specifically this time) is to get people to build applications in docker, instead of packaging these "legacy apps" as an afterthought. Things would go a long way towards that if the developers of the different projects out there recognize docker as a useful tool and start building with it. From there it would be a lot easier for them to maintain their own official base images.

You can see how preferences of the developers play a role in this? nginx upstream happens to recommend using their packages (for package managers, yes). Its all about adoption and scaling as I see it.

In the end it is mostly assumptions but I still think it makes a lot of sense. This is an understanding I have gradually picked up from issues and various parts of the docs, rather than an official announcement or anything.

@yankeeinlondon
Copy link
Author

@AsavarTzeth thanks for sharing, i'm just getting my hands around Docker so I'm prone to making some naive assumptions from time to time. Your thoughts/ideas seem very reasonable and sound. :)

BTW, I too would always prefer to keep symlinks to a minimum too but I do think they have a role to play at times including often serving as a tool to present an appropriate directory structure to nginx (a.g., foo --> ../src/foo/dist, bar --> ../src/bar/public, etc.). Not meant to be a Docker per se, just an example of symlinks having a role to play in the filesystem architecture.

@AsavarTzeth
Copy link

Well to be truthful I am relatively new to Docker myself. In fact considering the age of Docker I think most people still are. That being said, I am happy you find my ideas are reasonable. It must mean I am on the right path :)

@yankeeinlondon
Copy link
Author

I will stop here and start working off a different feature branch. I hope this can be accepted soon and then we can talk about future PR's. If not then I will fork going forward. Either way thanks for giving this a great starting point.

@yosifkit
Copy link
Member

Nginx upstream will now be providing and maintaining the images nginxinc/docker-nginx.

@yosifkit yosifkit closed this Sep 25, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants