Resolve port conflicts for cf-logrouter

Resolve port conflicts for cf-logrouter



https://code.launchpad.net/~johnsca/charms/trusty/cf-logrouter/port-conflicts/+merge/222676

(do not edit description out of merge proposal)

[johnsca, 2014-06-10 16:00:26]

Please take a look.

[johnsca, 2014-06-11 22:18:10]

Please take a look.

[benjamin.saller, 2014-06-11 22:31:19]

LGTM

wondering if we want to formalize the relation-set but all good

https://codereview.appspot.com/105880043/diff/20001/hooks/config.py
File hooks/config.py (right):

https://codereview.appspot.com/105880043/diff/20001/hooks/config.py#newcode21
hooks/config.py:21: {'shared_secret': host.pwgen(20)}
nice

https://codereview.appspot.com/105880043/diff/20001/hooks/logrouter-relation-...
File hooks/logrouter-relation-changed (right):

https://codereview.appspot.com/105880043/diff/20001/hooks/logrouter-relation-...
hooks/logrouter-relation-changed:5: config.send_relation_settings()
interesting, I'd have expected this to be a callback in the service decl (but
usually not a data-ready, something else).

[johnsca, 2014-06-12 14:09:17]

On 2014/06/11 22:31:19, benjamin.saller wrote:
> hooks/logrouter-relation-changed:5: config.send_relation_settings()
> interesting, I'd have expected this to be a callback in the service decl (but
> usually not a data-ready, something else).

I played around with having that as a callback (in data-ready) but it introduced
deadlocks since it wasn't sending out the relation settings that other services
were waiting on.  As for formalizing sending relation data to a callback (or
something) other than data-ready, it seems like a good idea, but I'm not sure
what form it should take.  The conditions on which a charm would want to send
relation data seem to be limited to: 1) a relation or config hook is called
(e.g., it's static or config data that doesn't depend on another service), or 2)
data-ready (e.g., it requires the entirety of the config to be known or the
service to be up to be at all meaningful, yet it won't block other services that
we depend on, creating a deadlock).

We can always add to the spec later, once we have more data points on exactly
how it will be used and can concretely identify the repetitive bits.

[johnsca, 2014-06-12 14:33:50]

Please take a look.

[benjamin.saller, 2014-06-12 16:34:29]

LGTM with minors 

I think there are still conflict here on some of the port numbers. If my read of
this is wrong feel free to merge, otherwise change the defaults and you can land
it.

Thanks

https://codereview.appspot.com/105880043/diff/40001/hooks/config.py
File hooks/config.py (right):

https://codereview.appspot.com/105880043/diff/40001/hooks/config.py#newcode47
hooks/config.py:47: 'varz_port': 8888,
Won't these ports still conflict with loggregator/logrouter in the single
machine case? don't we need to change the defaults here? I think varz is the
same for both. 

Additionally I'd defined these up top so they don't have to be changed in the
relation set at the same time.

[johnsca, 2014-06-12 16:49:53]

Please take a look.

[johnsca, 2014-06-12 18:33:11]

*** Submitted:

Resolve port conflicts for cf-logrouter

R=benjamin.saller
CC=
https://codereview.appspot.com/105880043