Resolve port conflicts in cf-go-router

Resolve port conflicts in cf-go-router



https://code.launchpad.net/~johnsca/charms/trusty/cf-go-router/port-conflicts/+merge/222671

(do not edit description out of merge proposal)

[johnsca, 2014-06-10 15:55:19]

Please take a look.

[johnsca, 2014-06-11 22:12:44]

Please take a look.

[benjamin.saller, 2014-06-11 22:23:56]

Thanks, LGTM with changes

At min we should use the config['port'] the action for the rest might be a card
unless you disagree with the approach.

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

https://codereview.appspot.com/103270043/diff/20001/hooks/config.py#newcode39
hooks/config.py:39: 'ports': [80],
if this is a config option (and it can be because we expose this one) then this
can't be hard-coded here. 

config['port']

The harder issue is that we then need to track the previous value if we are to
close the old port.

We (and it doesn't have to be now) could use a config context that called
callbacks here as callback(new_value, old_value=None) so we have a chance to do
something with the old saved context. seem reasonable?

[johnsca, 2014-06-12 14:01:24]

Please take a look.

[johnsca, 2014-06-12 14:11:06]

On 2014/06/11 22:23:56, benjamin.saller wrote:
> hooks/config.py:39: 'ports': [80],
> if this is a config option (and it can be because we expose this one) then
this
> can't be hard-coded here. 
> 
> The harder issue is that we then need to track the previous value if we are to
> close the old port.

Thanks, good catches.  I added the logic to close the old port when switching to
charm-helpers because it should always happen, and I feel like it should be
transparent to the charm author.

[benjamin.saller, 2014-06-12 16:30:58]

LGTM

Nice fix for the port handling

https://codereview.appspot.com/103270043/diff/40001/hooks/charmhelpers/core/s...
File hooks/charmhelpers/core/services.py (right):

https://codereview.appspot.com/103270043/diff/40001/hooks/charmhelpers/core/s...
hooks/charmhelpers/core/services.py:348: for port in new_ports:
Cool, thanks

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

https://codereview.appspot.com/103270043/diff/40001/hooks/config.py#newcode39
hooks/config.py:39: 'ports': [hookenv.config('port')],
funny that we use the hookenv.config()['domain'] above and this here. I didn't
recall that it even did per-key lookup :)

[johnsca, 2014-06-12 18:08:12]

*** Submitted:

Resolve port conflicts in cf-go-router

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