Fixes as per PR review.

Veebers · Veebers · commit 276795916f78 · 2018-02-27T15:48:40.000+13:00
diff --git a/acceptancetests/assess_log_forward.py b/acceptancetests/assess_log_forward.py
@@ -23,7 +23,7 @@
 from utility import (
     add_basic_testing_arguments,
     configure_logging,
-    get_unit_ipaddress,
+    get_unit_public_ip,
     JujuAssertionError,
     temp_dir,
 )
@@ -190,7 +190,7 @@ def deploy_rsyslog(client):
 def setup_tls_rsyslog(client, app_name):
     unit_machine = '{}/0'.format(app_name)
 
-    ip_address = get_unit_ipaddress(client, unit_machine)
+    ip_address = get_unit_public_ip(client, unit_machine)
 
     client.juju(
         'ssh',
diff --git a/acceptancetests/assess_upgrade.py b/acceptancetests/assess_upgrade.py
@@ -15,10 +15,6 @@
 import argparse
 import logging
 import sys
-from subprocess import (
-    CalledProcessError,
-)
-
 
 from collections import (
     namedtuple,
@@ -40,18 +36,14 @@
     agent_tgz_from_juju_binary,
     )
 from jujupy.wait_condition import (
-    ModelCheckFailed,
     WaitModelVersion,
-    wait_for_model_check,
+    wait_until_model_upgrades,
 )
 from jujupy.workloads import (
     deploy_mediawiki_with_db,
-    assert_media_wiki_is_responding
+    assert_mediawiki_is_responding
     )
 
-from utility import (
-    JujuAssertionError
-)
 
 __metaclass__ = type
 
@@ -67,10 +59,10 @@ def assess_upgrade_from_stable_to_develop(args, stable_bsm, devel_client):
     with temp_dir() as base_dir:
         stream_server = StreamServer(base_dir)
         setup_agent_metadata(
-            stream_server, args.stable_agent,
+            stream_server, args.stable_juju_agent,
             stable_client, base_dir, 'proposed')
         setup_agent_metadata(
-            stream_server, args.devel_agent,
+            stream_server, args.devel_juju_agent,
             devel_client, base_dir, 'proposed')
         with stream_server.server() as url:
             stable_client.env.update_config({
@@ -81,9 +73,9 @@ def assess_upgrade_from_stable_to_develop(args, stable_bsm, devel_client):
                 assert_stable_model_is_correct(stable_client)
 
                 deploy_mediawiki_with_db(stable_client)
-                assert_media_wiki_is_responding(stable_client)
+                assert_mediawiki_is_responding(stable_client)
                 upgrade_stable_to_devel_version(devel_client)
-                assert_media_wiki_is_responding(devel_client)
+                assert_mediawiki_is_responding(devel_client)
 
 
 def upgrade_stable_to_devel_version(client):
@@ -97,26 +89,6 @@ def upgrade_stable_to_devel_version(client):
     wait_until_model_upgrades(client)
 
 
-def wait_until_model_upgrades(client, timeout=300):
-    # Poll using a command that will fail until the upgrade is complete.
-    def model_upgrade_status_check(client):
-        try:
-            log.info('Attempting API connection, failure is not fatal.')
-            client.list_models()
-            client.juju('list-users', (), include_e=False)
-            return True
-        except CalledProcessError:
-            # Upgrade will still be in progress and thus refuse the api call.
-            return False
-    try:
-        wait_for_model_check(client, model_upgrade_status_check, timeout)
-    except ModelCheckFailed:
-        raise JujuAssertionError(
-            'Upgrade for model {} failed to complete within the alloted '
-            'timeout ({} seconds)'.format(
-                client.model_name, timeout))
-
-
 def assert_stable_model_is_correct(stable_client):
     assert_model_is_version(
         stable_client.get_controller_client(),
@@ -170,11 +142,11 @@ def parse_args(argv):
         '--stable-juju-bin',
         help='Path to juju binary to be used as the stable version of juju.')
     parser.add_argument(
-        '--stable-agent',
+        '--stable-juju-agent',
         help='Path to agent to use when bootstrapping with stable binary'
     )
     parser.add_argument(
-        '--devel-agent',
+        '--devel-juju-agent',
         help='Path to agent to use when bootstrapping with stable binary'
     )
     return parser.parse_args(argv)
diff --git a/acceptancetests/jujupy/binaries.py b/acceptancetests/jujupy/binaries.py
@@ -61,9 +61,9 @@ def get_stable_juju(args, stable_juju_bin=None):
             raise RuntimeError(
                 'Provided stable juju path is not valid: {}'.format(e))
     known_juju_paths = (
+        '{}/bin/juju'.format(os.environ.get('GOPATH')),
         '/snap/bin/juju',
-        '/usr/bin/juju',
-        '{}/bin/juju'.format(os.environ.get('GOPATH')))
+        '/usr/bin/juju')
 
     for path in known_juju_paths:
         try:
@@ -77,4 +77,4 @@ def get_stable_juju(args, stable_juju_bin=None):
             log.debug('Attempt at using {} failed.'.format(path))
             pass
 
-    raise RuntimeError('Unable to get a stable system juju binary.')
+    raise RuntimeError('Unable to get a stable system juju binary.')
diff --git a/acceptancetests/jujupy/utility.py b/acceptancetests/jujupy/utility.py
@@ -128,7 +128,7 @@ def split_address_port(address_port):
     return address, port
 
 
-def get_unit_ipaddress(client, unit_name):
+def get_unit_public_ip(client, unit_name):
     status = client.get_status()
     return status.get_unit(unit_name)['public-address']
 
diff --git a/acceptancetests/jujupy/wait_condition.py b/acceptancetests/jujupy/wait_condition.py
@@ -13,8 +13,10 @@
 # You should have received a copy of the Lesser GNU General Public License
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 
+import logging
 from datetime import datetime
 from time import sleep
+from subprocess import CalledProcessError
 
 from jujupy.exceptions import (
     VersionsNotUpdated,
@@ -28,6 +30,8 @@
 )
 
 
+log = logging.getLogger(__name__)
+
 __metaclass__ = type
 
 
@@ -53,6 +57,25 @@ def wait_for_model_check(client, model_check, timeout):
     raise ModelCheckFailed()
 
 
+def wait_until_model_upgrades(client, timeout=300):
+    # Poll using a command that will fail until the upgrade is complete.
+    def model_upgrade_status_check(client):
+        try:
+            log.info('Attempting API connection, failure is not fatal.')
+            client.juju('list-users', (), include_e=False)
+            return True
+        except CalledProcessError:
+            # Upgrade will still be in progress and thus refuse the api call.
+            return False
+    try:
+        wait_for_model_check(client, model_upgrade_status_check, timeout)
+    except ModelCheckFailed:
+        raise AssertionError(
+            'Upgrade for model {} failed to complete within the alloted '
+            'timeout ({} seconds)'.format(
+                client.model_name, timeout))
+
+
 class BaseCondition:
     """Base class for conditions that support client.wait_for."""
 
diff --git a/acceptancetests/jujupy/workloads.py b/acceptancetests/jujupy/workloads.py
@@ -27,7 +27,7 @@
     AllApplicationWorkloads,
     )
 from jujupy.utility import (
-    get_unit_ipaddress,
+    get_unit_public_ip,
     temp_dir,
 )
 
@@ -54,13 +54,13 @@ def deploy_mediawiki_with_db(client):
     client.wait_for_started()
 
 
-def assert_media_wiki_is_responding(client):
-    log.debug('Assert media wiki is responding.')
+def assert_mediawiki_is_responding(client):
+    log.debug('Assert mediawiki is responding.')
     status = client.get_status()
     [wiki_unit_name] = [
         k for k, v in status.get_applications()['mediawiki']['units'].items()
         if v.get('leader', False)]
-    wiki_ip = get_unit_ipaddress(client, wiki_unit_name)
+    wiki_ip = get_unit_public_ip(client, wiki_unit_name)
     resp = requests.get('http://{}'.format(wiki_ip))
     if not resp.ok:
         raise AssertionError('Mediawiki not responding; {}: {}'.format(
@@ -132,7 +132,7 @@ def assert_deployed_charm_is_responding(client, expected_output=None):
     # Set default value if needed.
     if expected_output is None:
         expected_output = 'simple-server.'
-    ipaddress = get_unit_ipaddress(client, 'simple-resource-http/0')
+    ipaddress = get_unit_public_ip(client, 'simple-resource-http/0')
     if expected_output != get_server_response(ipaddress):
         raise AssertionError('Server charm is not responding as expected.')
 
diff --git a/acceptancetests/pprof_collector.py b/acceptancetests/pprof_collector.py
@@ -7,7 +7,7 @@
 import os
 import requests
 
-from utility import get_unit_ipaddress
+from utility import get_unit_public_ip
 
 __metaclass__ = type
 
@@ -105,7 +105,7 @@ def install_introspection_charm(client, machine_id):
     client.wait_for_workloads()
     client.juju('expose', 'juju-introspection')
 
-    return get_unit_ipaddress(client, 'juju-introspection/0')
+    return get_unit_public_ip(client, 'juju-introspection/0')
 
 
 def _get_introspection_charm_url():
diff --git a/acceptancetests/utility.py b/acceptancetests/utility.py
@@ -20,7 +20,7 @@
     ensure_deleted,
     ensure_dir,
     get_timeout_path,
-    get_unit_ipaddress,
+    get_unit_public_ip,
     is_ipv6_address,
     print_now,
     qualified_model_name,
@@ -37,7 +37,7 @@
     'ensure_deleted',
     'ensure_dir',
     'get_timeout_path',
-    'get_unit_ipaddress',
+    'get_unit_public_ip',
     'qualified_model_name',
     'quote',
     'scoped_environ',
