Subordinate port open/close support

Subordinate port open/close support

Subordinates will open ports in their container. They will report open ports on the container and in the subordinate.

https://code.launchpad.net/~bcsaller/juju/subordinate-ports/+merge/111534

(do not edit description out of merge proposal)

[bcsaller, 2012-06-22 04:26:41]

Please take a look.

[hazmat, 2012-07-01 02:32:21]

while the implementation LGTM, the tests need fixing.

https://codereview.appspot.com/6324045/diff/1/juju/hooks/tests/test_invoker.py
File juju/hooks/tests/test_invoker.py (right):

https://codereview.appspot.com/6324045/diff/1/juju/hooks/tests/test_invoker.p...
juju/hooks/tests/test_invoker.py:37: self._invokers = {} # client_id -> Invoker
pep8 need an extra space before comment

https://codereview.appspot.com/6324045/diff/1/juju/hooks/tests/test_invoker.p...
juju/hooks/tests/test_invoker.py:1519: (yield unit_state.get_open_ports()),
This test never verifies the implementation, namely that the container ports are
open.

https://codereview.appspot.com/6324045/diff/1/juju/hooks/tests/test_invoker.p...
juju/hooks/tests/test_invoker.py:1560: "closed 80/tcp"])
pep8 need an extra blank line after this test

https://codereview.appspot.com/6324045/diff/1/juju/state/tests/test_firewall.py
File juju/state/tests/test_firewall.py (right):

https://codereview.appspot.com/6324045/diff/1/juju/state/tests/test_firewall....
juju/state/tests/test_firewall.py:192: """
this test also seems superfluous. the only impl file being modified by the
branch is the invoker, yet we have tests in numerous state test files verifying
what has already been verified by other tests in those files

https://codereview.appspot.com/6324045/diff/1/juju/state/tests/test_service.py
File juju/state/tests/test_service.py (right):

https://codereview.appspot.com/6324045/diff/1/juju/state/tests/test_service.p...
juju/state/tests/test_service.py:176: m1 = yield
self.machine_state_manager.add_machine_state(
machine creation and assignment is superflous

https://codereview.appspot.com/6324045/diff/1/juju/state/tests/test_service.p...
juju/state/tests/test_service.py:2771: def test_get_open_ports_subordiate(self):
typo in test name

https://codereview.appspot.com/6324045/diff/1/juju/state/tests/test_service.p...
juju/state/tests/test_service.py:2810: self.assertEqual(
how is this entire test not entirely superfluous?

[bcsaller, 2012-07-02 22:29:14]

https://codereview.appspot.com/6324045/diff/1/juju/hooks/tests/test_invoker.py
File juju/hooks/tests/test_invoker.py (right):

https://codereview.appspot.com/6324045/diff/1/juju/hooks/tests/test_invoker.p...
juju/hooks/tests/test_invoker.py:1519: (yield unit_state.get_open_ports()),
On 2012/07/01 02:32:22, hazmat wrote:
> This test never verifies the implementation, namely that the container ports
are
> open.

Right you are, thanks

https://codereview.appspot.com/6324045/diff/1/juju/state/tests/test_firewall.py
File juju/state/tests/test_firewall.py (right):

https://codereview.appspot.com/6324045/diff/1/juju/state/tests/test_firewall....
juju/state/tests/test_firewall.py:192: """
fair enough, removed 

On 2012/07/01 02:32:22, hazmat wrote:
> this test also seems superfluous. the only impl file being modified by the
> branch is the invoker, yet we have tests in numerous state test files
verifying
> what has already been verified by other tests in those files

https://codereview.appspot.com/6324045/diff/1/juju/state/tests/test_service.py
File juju/state/tests/test_service.py (right):

https://codereview.appspot.com/6324045/diff/1/juju/state/tests/test_service.p...
juju/state/tests/test_service.py:2810: self.assertEqual(
On 2012/07/01 02:32:22, hazmat wrote:
> how is this entire test not entirely superfluous?

removed