Charm Needed: Varnish Cache - Web Application Accelerator

Bug #993532 reported by Nathan Williams
8
This bug affects 1 person
Affects Status Importance Assigned to Milestone
Juju Charms Collection
Fix Released
Undecided
Unassigned

Bug Description

  Varnish Cache is a state of the art web accelerator written with
  performance and flexibility in mind. Varnish Cache stores web pages
  in memory so web servers don't have to create the same web page over
  and over again. Varnish serves pages much faster than any application
  server; giving the website a significant speed up. Some of the
  features include: * A modern design * VCL - a very flexible
  configuration language * Load balancing with health checking of
  backends * Partial support for ESI - Edge Side Includes * URL
  rewriting * Graceful handling of "dead" backends

  The varnish charm is written to automatically add units providing http
  interface to a round-robin balancer, and remove units from the balancer
  when removed in juju

Changed in charms:
status: New → In Progress
Changed in charms:
assignee: nobody → Nathan Williams (nathwill)
Revision history for this message
Clint Byrum (clint-fewbar) wrote :

Nathan, I see you linked a branch, but your status is still In Progress. Is this is ready for review yet?

Revision history for this message
Nathan Williams (nathwill-deactivatedaccount-deactivatedaccount) wrote :

Hi Clint,

the charm is fully functional in its current state, but I was hoping to add a piece to the relation-joined/config-changed hooks to specify the port from a `relation-get port`.

Currently the charm configures varnish to assume that each backend is using the default http port, and i thought that might be best to have before requesting a review.

The progress on this got held up due to my SSD going bad, but I'm getting my work env set up again, and should be able to kick this over to "New" within the next day or two.

Thanks.

Changed in charms:
status: In Progress → New
assignee: Nathan Williams (nathwill) → nobody
Changed in charms:
status: New → Fix Committed
Revision history for this message
Nathan Williams (nathwill-deactivatedaccount-deactivatedaccount) wrote :

we're ready to go on this :) i made some additional changes that were recommended in #juju (add maintainer and move data file extra.vcl to /data)

Jorge Castro (jorge)
tags: removed: new-charm
Revision history for this message
Clint Byrum (clint-fewbar) wrote :

Hi Nathan! Thanks for the awesome work on Varnish. I think we're pretty close, these blockers are all minor!

Blockers:

* website-relation-joined does not set the 'hostname' argument. While most charms will set this to 'private-address', we do not use this in the relation because its possible that some charms may want to send the load balancer elsewhere.

* Likewise your reverseproxy hooks are using private-address instead of hostname, which is not how the 'http' interface is supposed to work.

* reverseproxy-relation-joined is just an empty hook with a log entry. Please remove that.

* reverseproxy-relation-broken will not work. $JUJU_REMOTE_UNIT is not set during broken, because all the other side's units are effectively gone. The only thing you can use is $JUJU_RELATION_ID. Because of this fact, its usually best to use $JUJU_RELATION_ID in the joined/changed hooks as well, so you can easily clean up after things.

* hooks/upgrade-charm says vsftpd. ;)

Non Blockers:

* Using config-get during install is almost never the right way to use config-get. Move the configuration section to the 'config-changed' hook so that users can change the port after deploy. In fact I'd just move all except the install of varnish to config-changed.

* reverseproxy-relation-departed - I think I'd just symlink this to changed. It will do the same relation-list, but the departed member will not be in it anymore, so the config will get generated. Otherwise you have to do the 'sed' stuff, which may be unreliable.

***** Next step *****

Please fix the blocking issues, and then subscribe the 'charmers' team again so we will see your entry in the queue again.

THANKS!

Changed in charms:
status: Fix Committed → Incomplete
Changed in charms:
status: Incomplete → In Progress
Revision history for this message
Nathan Williams (nathwill-deactivatedaccount-deactivatedaccount) wrote :

Hi Clint,

Thanks so much for the helpful review and feedback.

Blockers:
* website-relation-joined: fixed, now setting hostname
* reverseproxy hooks: [1]
* reverseproxy-relation-joined: fixed, removed the hook
* reverseproxy-relation-broken: fixed, hook removed. also, thanks for the explanation on $JUJU_REMOTE_UNIT, i was having a helluva time figuring out why this was not working as expected.
* upgrade-charm: derp. fixed.

Non-Blockers:
* moved config-get business to config-changed
* reverseproxy-relation-departed: symlinked. i only recently realized the convenience of re-definition by iteration in a config-changed vs. addition/removal of individual service units in relation-joined and relation-departed hooks. would be awesome to see this documented somewhere as best practice, as all the popular kids are doing it.

[1] one of the problems i have with using hostname instead of private/public address is that in local environment deployments, even `hostname -f` == "localhost", which is obviously correct, but totally useless for referring to a service unit from another service unit. i'm not sure what the solution for this would be, except juju setting a fqdn for local deployments... if i'm overlooking something obvious, please let me know.

last thing, do you have a good documentation reference for using JUJU_RELATION_ID ? i couldn't find anything...

https://www.google.com/search?q=site:juju.ubuntu.com+JUJU_RELATION_ID

cheers!

Revision history for this message
Nathan Williams (nathwill-deactivatedaccount-deactivatedaccount) wrote :

that should be 'hostname -f', not 'even hostname -f'...

Changed in charms:
status: In Progress → Fix Committed
Revision history for this message
Juan L. Negron (negronjl) wrote :

Nice work Nathan.

This looks good to me.

-Juan

Revision history for this message
Juan L. Negron (negronjl) wrote :

Promulgated :)

-Juan

Changed in charms:
status: Fix Committed → Fix Released
Revision history for this message
Nathan Williams (nathwill-deactivatedaccount-deactivatedaccount) wrote :

thanks Clint & Juan!

To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.