Code review comment for lp:~frankban/juju-quickstart/new-machine-info

Revision history for this message
Francesco Banconi (frankban) wrote :

Reviewers: mp+214317_code.launchpad.net,

Message:
Please take a look.

Description:
Support MachineInfo addresses.

juju-core 1.18 introduces a change in the mega-watcher:
the MachineInfo includes the public/private addresses
for each machine/container in the environment.
This will be the preferred way to retrieve entity
addresses in future versions of juju-core, which
might also discard the public address field in
the UnitInfo.

This branch updates quickstart so that it can work
in both scenarios: for backward compatibility, the address
is retrieved trying to parse both the unit and the machine
info, without assuming the corresponding fields to be
always included.

This required some testing and documentation efforts,
resulting in a diff longer than usual: sorry about that.

Tests: `make check`.

QA:
Use juju 1.16 (current stable).
In the steps below the command to run is:
`.venv/bin/python juju-quickstart -e {ENV_NAME}`.

1) Bootstrap a local environment with quickstart, ensure
    the quickstart process completes correctly, the juju-gui
    address is retrieved, the GUI is opened. Also ensure
    the user messages showed on stdout make sense.
2) Execute quickstart again, with the local environment already
    bootstrapped. Ensure the process completes correctly,
    and user messages are sane.
3) Destroy the local environment.
4) Bootstrap an ec2 environment with quickstart, ensure
    the quickstart process completes correctly, the juju-gui
    address is retrieved, the GUI is opened. Also ensure
    the user messages showed on stdout make sense.
5) Execute quickstart again, with the ec2 environment already
    bootstrapped. Ensure the process completes correctly,
    and user messages are sane.
6) Destroy the ec2 environment.

Use juju 1.18. This must be compiled from the juju-core 1.18 branch,
which can be found in `lp:juju-core/1.18`.

7) Edit the quickstart/settings.py file included in this branch:
    set `JUJU_CMD` to point to the juju 1.18 path.
8) Follow steps 1) to 6) again, in order to check that
    quickstart works well also with Juju 1.18.

Done, thank you!

https://code.launchpad.net/~frankban/juju-quickstart/new-machine-info/+merge/214317

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/84630043/

Affected files (+500, -139 lines):
   A [revision details]
   M quickstart/app.py
   M quickstart/models/envs.py
   M quickstart/tests/test_app.py
   M quickstart/tests/test_watchers.py
   M quickstart/watchers.py

« Back to merge proposal