Review queue: apache2

[David Ames]

Please review lp:~thedac/charms/precise/apache2/trunk for inclusion in the charm store

[James Page (james-page)]

Hi David

Do you have an example deployment for this charm? I think I'm missing the bigger picture of how this relates to other services.

[David Ames (thedac)]

James, we are using this extensively in our infrastrucutre. This apache charm provides a front end to app services. It can use mod_proxy_balancer to load balance between app services . We use it like this: apache2 -> squid -> haproxy -> app servers.

[Tom Haddon (mthaddon)]

Another use is for an internal subordinate charm here at Canonical that connects to launchpad, and generates flat files that are then served statically by the apache2 charm. It could also be used with django, for instance, by specifying mod_wsgi in the modules to enable, and then having a subordinate charm create the django application on disk to be served by mod_wsgi.

[Robert Ayres (robert-ayres)]

Thanks for the submission.

Apache is awkward to charm but I like your template style here, and your Juju ASCII logo :)

Please see bugs/comments below.

Bugs:
*Updating mpm variables or changing the mpm_type doesn't reflect on the server unless you manually restart it. I'm guessing this is because you're calling 'reload' on the service.

*README refers to non-existant variable 'schema', presumably this is meant to be 'vhost_http_template'.

Comments:
*You could change the method for passing in a SSL key/certficate to using base64 config variables. This has the advantage of not needing to upgrade the charm to change either and copying to the appropriate directories could happen automatically. mod_ssl could also be directly enabled.

*For the charm's default config, you might enable the 'Deny All' directives for '/' under /etc/apache2/conf.d/security.

Please fix the bugs and consider the comments, then reopen for further review.

[Tom Haddon (mthaddon)]

In terms of the issue around changes to mpm variables not being applied because we're reloading not restarting, I think we need to be very careful around doing service restarts as part of charms like this, as it will mean (admittedly short) service outages. In PostgreSQL we got around this by having whether we "reload" or "restart" be a config variable, so admins can chose as appropriate for their environment. If you've made a change that requires a service restart then it's up to you to actually restart the service to apply it (if you set it at reload). This is a trade off, but not an easily fixable one...

[David Ames (thedac)]

> Bugs:
> *Updating mpm variables or changing the mpm_type doesn't reflect on the server unless you manually restart it. I'm guessing this is because you're calling 'reload' on the service.

Please see Tom's post #5. We do not want to subtlety introduce an apache restart into a production environment

> *README refers to non-existant variable 'schema', presumably this is meant to be 'vhost_http_template'.

Fixed

Comments:
*You could change the method for passing in a SSL key/certficate to using base64 config variables. This has the advantage of not needing to upgrade the charm to change either and copying to the appropriate directories could happen automatically. mod_ssl could also be directly enabled.

We have security concerns around this method. The certificate being in memory so to speak. We prefer the file method of the SSL cert.

*For the charm's default config, you might enable the 'Deny All' directives for '/' under /etc/apache2/conf.d/security.

Done

[Robert Ayres (robert-ayres)]

> Please see Tom's post #5. We do not want to subtlety introduce an apache restart into a production environment

Presumably this is where you have multiple load-balanced Apaches and they'd all restart at once :)

I think Juju definitely needs assistance from some kind of automated load balanced service restarter, that could do a cluster wide restart whilst keeping at least some of the services up for no downtime. This type of thing would benefit all the charms (after all, what's the point of scaling if it doesn't protect uptime!).

I'm thinking you should probably add a config variable to the charm, like your PostgreSQL solution, to specify 'reload' or 'restart'. I'm concered the average user will be thrown by the need to ssh in and manually restart.

> We have security concerns around this method. The certificate being in memory so to speak. We prefer the file method > of the SSL cert.

Ah, presumably this is more about the Python implementation of Juju and the lack of ACLs on Zookeeper, understandble.

[David Ames (thedac)]

Agreed. That is our concern that a config change runs on *all* the apache hosts at the same time. If it restarts them all in production at the same time that is bad.

Nevertheless, I have implemented the config variable setting for reload or restart.

Please review

[Juan L. Negron (negronjl)]

Reviewing this now.

-Juan

[Juan L. Negron (negronjl)]

Hi:

This is working as advertised. Everything that was pointed out was addressed.

Approved.

Merging.

-Juan

[Juan L. Negron (negronjl)]

Promulgated.

-Juan

[Robert Ayres (robert-ayres)]

Have just merged ~thedac/charms/precise/apache2/trunk/+merge/146539. Without this, the charm fails to even deploy with the missing security template. This should really have been tested at review.