Add fallback for unit.get_public_address()

Due to bug [1], it seems that there is a issue with libjuju
communicating with the Juju controller which causes a permanent
model disconnect that libjuju doesn't resolve.  Thus, for zaza, this
patch wraps unit.get_public.address() with a wrapper that can choose,
based on the environment variable `ZAZA_FEATURE_BUG472` to use a
subprocess shell call to the `juju status` command to get the public
address. This should always succeed.

The feature environment variable `ZAZA_FEATURE_BUG472` was added so that
the library can switch between the native libjuju function and the
fallback wrapper to enable testing of the issue as libjuju continues to
evolve.

By default, the wrapper function is used, to enable zaza to interoperate
with libjuju with Juju 2.9 on OpenStack providers.

The implementation is slightly complicated because an async version of
the wrapper `get_unit_public_address()` is needed as it is called from
async code.

[1]: juju/python-libjuju#615

scripts/README.md

@@ -0,0 +1,5 @@
+# Scripts sub-directory
+
+The scripts in this directory are for regression/manual testing of the
+libjuju unit.get_public_address() function.  The tox target 'third' can
+also be used for regression testing.

scripts/bash_tester.sh

@@ -0,0 +1,15 @@
+#!/bin/bash
+
+_dir="$( cd "$(dirname "${BASH_SOURCE[0]}" )" && pwd)"
+runner="${_dir}/fetch1.py"
+
+# loop 10 times and fetch an instance address
+i=0
+while [ $i -ne 10 ];
+do
+    printf "\n\n\n!!!!!!"
+    printf "\n\n\nDoing number $i"
+    printf "\n\n"
+    $runner $i
+    i=$(($i+1))
+done

scripts/fetch1.py

@@ -0,0 +1,98 @@
+#!/usr/bin/env python3
+
+from juju.model import Model
+import asyncio
+import sys
+
+# set to use something other than the current model
+MODEL=None
+
+
+async def get_units():
+    model = Model()
+    if MODEL is None:
+        await model.connect()
+    else:
+        await model.connect_model(MODEL)
+    units = sorted(model.applications['ubuntu'].units, key=lambda u: u.name)
+    await model.disconnect()
+    return units
+
+
+async def get_address(unit):
+    model = Model()
+    await model.connect_model(MODEL)
+    print("{} Address: .public_address {}".format(unit.name, unit.public_address))
+    while True:
+        try:
+            print("{} Address: get_public_address() {}".format(unit.name, await unit.get_public_address()))
+            break
+        except Exception as e:
+            print("Exception was: %s", e)
+            await asyncio.sleep(.25)
+            await ensure_model_connected(model)
+    print("{} Address: .public_address {}".format(unit.name, unit.public_address))
+    print("\n")
+    await model.disconnect()
+
+
+def run_it(step):
+    loop = asyncio.get_event_loop()
+    task = loop.create_task(step)
+    loop.run_until_complete(asyncio.wait([task], loop=loop))
+    result = task.result()
+    return result
+
+
+async def get_unit(n):
+    units = await get_units()
+    print("units", units)
+    await get_address(units[n])
+
+
+def is_model_disconnected(model):
+    """Return True if the model is disconnected.
+
+    :param model: the model to check
+    :type model: :class:'juju.Model'
+    :returns: True if disconnected
+    :rtype: bool
+    """
+    print("is_model_disconnected?: %s, %s", model.is_connected(), model.connection().is_open)
+    return not (model.is_connected() and model.connection().is_open)
+
+
+async def ensure_model_connected(model):
+    """Ensure that the model is connected.
+
+    If model is disconnected then reconnect it.
+
+    :param model: the model to check
+    :type model: :class:'juju.Model'
+    """
+    if is_model_disconnected(model):
+        model_name = model.info.name
+        print(
+            "model: %s has disconnected, forcing full disconnection "
+            "and then reconnecting ...", model_name)
+        try:
+            await model.disconnect()
+        except Exception:
+            # We don't care if disconnect fails; we're much more
+            # interested in re-connecting, and this is just to clean up
+            # anything that might be left over (i.e.
+            # model.is_connected() might be true, but
+            # model.connection().is_open may be false
+            pass
+        print("Attempting to reconnect model %s", model_name)
+        await model.connect_model(model_name)
+
+
+if __name__ == '__main__':
+    unit_num = 0
+    if len(sys.argv) > 1:
+        unit_num = int(sys.argv[1])
+
+    run_it(get_unit(unit_num))
+    asyncio.get_event_loop().close()
+

scripts/tester.py

@@ -0,0 +1,47 @@
+#!/usr/bin/env python3
+
+from juju.model import Model
+import asyncio
+
+# set to use something other than the current model
+MODEL=None
+
+
+async def get_units():
+    model = Model()
+    if MODEL is None:
+        await model.connect()
+    else:
+        await model.connect_model(MODEL)
+    units = sorted(model.applications['ubuntu'].units, key=lambda u: u.name)
+    await model.disconnect()
+    return units
+
+
+async def get_address(unit):
+    model = Model()
+    await model.connect_model(MODEL)
+    print("{} Address: .public_address {}".format(unit.name, unit.public_address))
+    print("{} Address: get_public_address() {}".format(unit.name, await unit.get_public_address()))
+    print("{} Address: .public_address {}".format(unit.name, unit.public_address))
+    print("\n")
+    await model.disconnect()
+
+
+def run_it(step):
+    loop = asyncio.get_event_loop()
+    task = loop.create_task(step)
+    loop.run_until_complete(asyncio.wait([task], loop=loop))
+    result = task.result()
+    return result
+
+
+def get_all_units():
+    units = run_it(get_units())
+    print("units", units)
+    for unit in units:
+        run_it(get_address(unit))
+
+
+get_all_units()
+asyncio.get_event_loop().close()

tests/tests.yaml

@@ -5,6 +5,7 @@ gate_bundles:
 # destruction
 - first
 - second
+- third
 target_deploy_status:
   magpie-xenial:
     workload-status: active

unit_tests/test_zaza_model.py

@@ -31,6 +31,7 @@
 import copy
 import concurrent
 import mock
+import yaml
 
 import unit_tests.utils as ut_utils
 from juju import loop
@@ -536,14 +537,105 @@ def test_get_lead_unit_name(self):
             'app/4')
 
     def test_get_unit_public_address(self):
-        self.assertEqual(model.get_unit_public_address(self.unit1), 'ip1')
+        async def mock_fallback(*args, **kwargs):
+            return 'fip'
+
+        async def mock_libjuju(*args, **kwargs):
+            return 'ljip'
+
+        self.patch_object(model,
+                          'async_get_unit_public_address__fallback',
+                          name='mock_fallback')
+        self.patch_object(model,
+                          'async_get_unit_public_address__libjuju',
+                          name='mock_libjuju')
+        self.mock_fallback.side_effect = mock_fallback
+        self.mock_libjuju.side_effect = mock_libjuju
+
+        _env = {"ZAZA_FEATURE_BUG472": ""}
+        with mock.patch.dict(model.os.environ, _env):
+            self.assertEqual(model.get_unit_public_address(
+                self.unit1, model_name='a-model'), 'fip')
+            self.mock_fallback.assert_called_once_with(
+                self.unit1, model_name='a-model')
+            self.mock_libjuju.assert_not_called()
+        _env = {"ZAZA_FEATURE_BUG472": "1"}
+        self.mock_fallback.reset_mock()
+        with mock.patch.dict(model.os.environ, _env):
+            self.assertEqual(model.get_unit_public_address(self.unit1), 'ljip')
+            self.mock_libjuju.assert_called_once_with(
+                self.unit1, model_name=None)
+            self.mock_fallback.assert_not_called()
+
+    def test_get_unit_public_address__libjuju(self):
+        sync_get_unit_public_address__libjuju = model.sync_wrapper(
+            model.async_get_unit_public_address__libjuju)
+        self.assertEqual(
+            sync_get_unit_public_address__libjuju(
+                self.unit1, model_name='a-model'), 'ip1')
+
+    def test_get_unit_public_address__fallback(self):
+        self.patch_object(model, 'logging', name='mock_logging')
+        sync_get_unit_public_address__fallback = model.sync_wrapper(
+            model.async_get_unit_public_address__fallback)
+
+        async def mock_async_get_juju_model():
+            return 'a-model'
+
+        self.patch_object(model, 'async_get_juju_model')
+        self.async_get_juju_model.side_effect = mock_async_get_juju_model
+
+        status = yaml.safe_dump({
+            'applications': {
+                'an-app': {
+                    'units': {
+                        'an-app/0': {
+                            'public-address': '2.3.4.5'
+                        }
+                    }
+                }
+            }
+        })
+
+        async def mock_check_output(*args, **kwargs):
+            return {'Stdout': status}
+
+        self.patch_object(
+            model.generic_utils, 'check_output', name='async_check_output')
+        self.async_check_output.side_effect = mock_check_output
+
+        mock_unit = mock.Mock()
+        mock_p_name = mock.PropertyMock(return_value='an-app/0')
+        type(mock_unit).name = mock_p_name
+        self.assertEqual(
+            sync_get_unit_public_address__fallback(
+                mock_unit, model_name='b-model'), '2.3.4.5')
+        self.async_check_output.assert_called_once_with(
+            "juju status --format=yaml -m b-model".split(), log_stderr=False,
+            log_stdout=False)
+        mock_p_name = mock.PropertyMock(return_value='an-app/1')
+        type(mock_unit).name = mock_p_name
+
+        self.async_check_output.reset_mock()
+        self.assertEqual(
+            sync_get_unit_public_address__fallback(mock_unit), None)
+        self.async_check_output.assert_called_once_with(
+            "juju status --format=yaml -m a-model".split(), log_stderr=False,
+            log_stdout=False)
 
     def test_get_lead_unit_ip(self):
-        self.patch_object(model, 'get_juju_model', return_value='mname')
-        self.patch_object(model, 'get_units')
-        self.get_units.return_value = self.units
-        self.patch_object(model, 'Model')
-        self.Model.return_value = self.Model_mock
+        async def mock_async_get_lead_unit(*args, **kwargs):
+            return [self.unit2]
+
+        async def mock_async_get_unit_public_address(*args):
+            return 'ip2'
+
+        self.patch_object(
+            model, 'async_get_lead_unit', side_effect=mock_async_get_lead_unit)
+        self.patch_object(
+            model,
+            'async_get_unit_public_address',
+            side_effect=mock_async_get_unit_public_address)
         self.assertEqual(
             model.get_lead_unit_ip('app', 'model'),
             'ip2')
@@ -577,14 +669,31 @@ def test_get_unit_from_name(self):
             model.get_unit_from_name('bad_name', model_name='mname')
 
     def test_get_app_ips(self):
-        self.patch_object(model, 'get_juju_model', return_value='mname')
-        self.patch_object(model, 'async_get_units')
+        # self.patch_object(model, 'get_juju_model', return_value='mname')
 
-        async def mock_async_aget_units(*args, **kwargs):
+        async def mock_async_get_units(*args, **kwargs):
             return self.units
 
-        self.async_get_units.side_effect = mock_async_aget_units
-        self.assertEqual(model.get_app_ips('model', 'app'), ['ip1', 'ip2'])
+        async def mock_async_get_unit_public_address(unit, **kwargs):
+            if unit is self.unit1:
+                return 'ip1'
+            elif unit is self.unit2:
+                return 'ip2'
+            return None
+
+        self.patch_object(model, 'async_get_units',
+                          side_effect=mock_async_get_units)
+        self.patch_object(
+            model,
+            'async_get_unit_public_address',
+            side_effect=mock_async_get_unit_public_address)
+
+        self.assertEqual(model.get_app_ips('app', 'model'), ['ip1', 'ip2'])
+        self.async_get_units.assert_called_once_with(
+            'app', model_name='model')
+        self.async_get_unit_public_address.assert_has_calls([
+            mock.call(self.unit1, model_name='model'),
+            mock.call(self.unit2, model_name='model')])
 
     def test_run_on_unit(self):
         self.patch_object(model, 'get_juju_model', return_value='mname')