Merge bootstack-ops:juju-select-largest-model-improve into bootstack-ops:master

[Giuseppe Petralia]

Accept comments from juju-select-largest-model branch

https://code.launchpad.net/~canonical-bootstack/bootstack-ops/+git/bootstack-ops/+merge/366113

- Unify retry mechanism for juju and maas calls
- Use --format=yaml for any juju call

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Giuseppe Petralia (peppepetra)]

Cloud_report generation has been tested in: Kambi, Docapost and Plus65

[Andrea Ieri (aieri)]

Some cleanups inline.
Also: there's some trailing whitespace that could be removed.

review: Needs Fixing

Unmerged commits

30c266e... by Giuseppe Petralia on 2019-09-20

Accept comments from juju-select-largest-model branch.
- Unify retry mechanism for juju and maas calls
- Use --format=yaml for any juju call

4c31caf... by Andrea Ieri on 2019-09-06

Stage pacemaker to allow crm_attribute calls. Bump version.

Reviewed-on: https://code.launchpad.net/~aieri/bootstack-ops/+git/bootstack-ops/+merge/372419
Reviewed-by: David O Neill <email address hidden>

2b6323c... by Andrea Ieri on 2019-09-06

Stage pacemaker to allow crm_attribute calls. Bump version.

d3bedc2... by Diko Parvanov on 2019-09-03

Got the backup script from the infra-node charm and updated the one in the ops snap.

Reviewed-on: https://code.launchpad.net/~dparv/bootstack-ops/+git/bootstack-ops/+merge/370549
Reviewed-by: David O Neill <email address hidden>

e293df0... by Andrea Ieri on 2019-09-03

A set of expect scripts to automate operations on Cisco BMCs

Reviewed-on: https://code.launchpad.net/~aieri/bootstack-ops/+git/bootstack-ops/+merge/371547
Reviewed-by: David O Neill <email address hidden>

d58317d... by Andrea Ieri on 2019-09-02

Added goliveinfo.sh and portalinfo.sh
Added CDK golive data

7eaaf02... by Andrea Ieri on 2019-08-20

A set of expect scripts to automate operations on Cisco BMCs

917d27a... by Diko Parvanov on 2019-07-24

Updated backup script with latest changes

127965c... by Diko Parvanov on 2019-06-25

Merge branch 'juju-select-largest-model' of https://git.launchpad.net/bootstack-ops

8000536... by James Hebden on 2019-06-24

Moved to using snapcraft bases and updated for snapcraft 3.0

Reviewed-on: https://code.launchpad.net/~canonical-bootstack/bootstack-ops/+git/bootstack-ops/+merge/367557
Reviewed-by: David O Neill <email address hidden>

diff --git a/bootstack-ops/cloud_report.py b/bootstack-ops/cloud_report.py
index c880943..4c4b085 100644
--- a/bootstack-ops/cloud_report.py
+++ b/bootstack-ops/cloud_report.py
@@ -43,8 +43,6 @@ import json
 import click
 import subprocess
 import yaml
-import re
-from collections import OrderedDict
 
 
 from maas.client import login, connect
@@ -53,7 +51,7 @@ from gevent.pywsgi import WSGIServer
 from maas.client.viscera.controllers import RackController, RegionController
 from oauthlib import oauth1
 import requests
-from aiohttp.client_exceptions import ServerDisconnectedError
+from functools import wraps
 
 
 JUJU_DATA = os.environ.get('JUJU_DATA')
@@ -69,8 +67,7 @@ API_PORT = os.environ.get("API_PORT", 5000)
 
 UNKNOWN_VALUE = "unknown"
 
-RETRY = 10
-SLEEP = 2
+NUMBER_OF_RETRIES = 4
 
 
 def get_timestamp():
@@ -94,13 +91,38 @@ def url_join(base, uri):
         return base + uri
 
 
-def retry(call_to_retry, num=RETRY):
-    for i in range(0, num):
-        try:
-           return call_to_retry()
-        except Exception:
-            time.sleep(SLEEP * i)
-            print("Error: failed to retrieve machines from maas [{}/{}]".format(i+1,num))
+def retry(exceptions, tries=NUMBER_OF_RETRIES, delay=3, backoff=2, logger=None):
+    """
+    Retry calling the decorated function using an exponential backoff.
+
+    Args:
+        exceptions: The exception to check. may be a tuple of
+            exceptions to check.
+        tries: Number of times to try (not retry) before giving up.
+        delay: Initial delay between retries in seconds.
+        backoff: Backoff multiplier (e.g. value of 2 will double the delay
+            each retry).
+        logger: Logger to use. If None, print.
+    """
+    def deco_retry(f):
+        @wraps(f)
+        def f_retry(*args, **kwargs):
+            mtries, mdelay = tries, delay
+            while mtries > 1:
+                try:
+                    return f(*args, **kwargs)
+                except exceptions as e:
+                    msg = '{}, Retrying in {} seconds...'.format(e, mdelay)
+                    if logger:
+                        logger.warning(msg)
+                    else:
+                        print(msg)
+                    time.sleep(mdelay)
+                    mtries -= 1
+                    mdelay *= backoff
+            return None
+        return f_retry  # true decorator
+    return deco_retry
 
 
 class MaasV2(object):
@@ -109,6 +131,7 @@ class MaasV2(object):
         self.name = "maasv2"
         self.maas_endpoint = None
 
+    @retry(Exception)
     def connect(self):
         """
         Connect to Maas controller using credentials extracted from the environemnt variables.
@@ -133,56 +156,44 @@ class MaasV2(object):
         else:
             self.maas_endpoint = MAAS_ENDPOINT
 
-        for i in range(0, RETRY):
-            try:
-                if MAAS_PASSWORD:
-                    self.client = login(
-                        self.maas_endpoint,
-                        username=MAAS_USERNAME, password=MAAS_PASSWORD,
-                    )
-                elif MAAS_APIKEY:
-                    self.client = connect(
-                        self.maas_endpoint,
-                        apikey=MAAS_APIKEY
-                    )
-                return self.client is not None
-            except Exception as e:
-                print("Error connecting to MAAS endpoint: {}. {}".format(self.maas_endpoint, e))
-                time.sleep(SLEEP * i)
+        if MAAS_PASSWORD:
+            self.client = login(
+                self.maas_endpoint,
+                username=MAAS_USERNAME, password=MAAS_PASSWORD,
+            )
+        elif MAAS_APIKEY:
+            self.client = connect(
+                self.maas_endpoint,
+                apikey=MAAS_APIKEY
+            )
+        return self.client is not None
 
     def disconnect(self):
         pass
 
+    @retry(Exception)
     def _list_machines(self):
-        try:
-            return retry(self.client.machines.list) or []
-        except Exception as e:
-            print("Error retrieving machines list from MAAS endpoint: {}. {}".format(self.maas_endpoint, e))
-        return []
+        return self.client.machines.list() or []
 
+    @retry(Exception)
     def _list_rack_controllers(self):
-        try:
-            return retry(self.client.rack_controllers.list) or []
-        except Exception as e:
-            print("Error retrieving rack controllers list from MAAS endpoint: {}. {}".format(self.maas_endpoint, e))
-        return []
+        return self.client.rack_controllers.list() or []
 
+    @retry(Exception)
     def _list_region_controllers(self):
-        try:
-            return retry(self.client.region_controllers.list) or []
-        except Exception as e:
-            print("Error retrieving region controllers list from MAAS endpoint: {}. {}".format(self.maas_endpoint, e))
-        return []
+        return self.client.region_controllers.list() or []
 
+    @retry(Exception)
     def _get_maas_version(self):
         version = "None"
-        report_version = retry(self.client.version.get)
+        report_version = self.client.version.get()
         if report_version:
             version = report_version.version
         return version
 
+    @retry(Exception)
     def _get_power_parameters(self, m):
-        return retry(m.get_power_parameters) or {}
+        return m.get_power_parameters() or {}
 
     def _serialize_machine(self, m):
         """
@@ -227,9 +238,19 @@ class MaasV2(object):
 
         maas_version = self._get_maas_version()
 
-        machines = list(map(self._serialize_machine, self._list_machines())) + \
-                   list(map(self._serialize_machine, self._list_rack_controllers())) + \
-                   list(map(self._serialize_machine, self._list_region_controllers()))
+        machines = []
+
+        maas_machine_list = self._list_machines()
+        if maas_machine_list:
+            machines += list(map(self._serialize_machine, maas_machine_list))
+
+        rack_controller_list = self._list_rack_controllers()
+        if rack_controller_list:
+            machines += list(map(self._serialize_machine, rack_controller_list))
+
+        region_controller_list = self._list_region_controllers()
+        if region_controller_list:
+            machines += list(map(self._serialize_machine, region_controller_list))
 
         machine_ids = set()
         maas_machines = list()
@@ -292,22 +313,20 @@ class MaasV1(object):
     def disconnect(self):
         pass
 
+    @retry(Exception)
     def _perform_request(self, uri):
-        try:
-            client = oauth1.Client(self.consumer_key,
-                                   client_secret='',
-                                   resource_owner_key=self.key,
-                                   resource_owner_secret=self.secret,
-                                   signature_method=oauth1.SIGNATURE_PLAINTEXT)
-
-            url = url_join(self.maas_endpoint, uri)
-            uri, headers, body = retry(client.sign(url)) or [None, None, None]
-            response = retry(requests.get(uri, headers=headers)) or ""
-            if response.status_code == 200:
-                return response.json()
-        except Exception as e:
-            print("Error querying maas {}, {}".format(url, e))
-
+        client = oauth1.Client(self.consumer_key,
+                               client_secret='',
+                               resource_owner_key=self.key,
+                               resource_owner_secret=self.secret,
+                               signature_method=oauth1.SIGNATURE_PLAINTEXT)
+
+        url = url_join(self.maas_endpoint, uri)
+        uri, headers, body = client.sign(url) or [None, None, None]
+        response = requests.get(uri, headers=headers) or ""
+
+        if response and response.status_code == 200:
+            return response.json()
         return {}
 
     def _get_power_address(self, system_id):
@@ -382,35 +401,38 @@ class Juju(object):
         self.models = []
         self.loop = asyncio.get_event_loop()
 
-    def disconnect(self):
-        pass
+        if os.path.isfile('/snap/bin/juju'):
+            self.juju_cmd = ['/snap/bin/juju']
+        elif os.path.isfile("/usr/bin/juju"):
+            self.juju_cmd = ['/usr/bin/juju']
+        else:
+            self.juju_cmd = ['snap', 'run', 'juju']
 
-    def _list_machines(self):
         if not JUJU_DATA:
             print("Please export JUJU_DATA env variable before running the script")
             sys.exit(1)
 
+        self.env = os.environ.copy()
+        self.env["JUJU_DATA"] = JUJU_DATA
+
+    def disconnect(self):
+        pass
+
+    @retry(Exception)
+    def run_juju_command(self, *args):
+        return subprocess.check_output(self.juju_cmd + list(args), timeout=60, env=self.env)
+
+    def _list_machines(self, model=None):
         machines = []
-        env = os.environ.copy()
-        env["JUJU_DATA"] = JUJU_DATA
 
-        if os.path.isfile('/snap/bin/juju'):
-            cmd = ['/snap/bin/juju', 'status', '--format=yaml']
-        elif os.path.isfile("/usr/bin/juju"):
-            cmd = ['/usr/bin/juju', 'status', '--format=yaml']
-        else:
-            cmd = ['snap run juju', 'status', '--format=yaml']
-
-        for i in range(0, RETRY):
-            try:
-                output = subprocess.check_output(cmd, timeout=30, env=env)
-                parsed = yaml.safe_load(output)
-                machines = parsed['machines']
-                i = RETRY + 1
-            except Exception as e:
-                print("Error: failed to retrieve machines from juju [{}/{}]\n{}".format(i+1,RETRY,e))
-                time.sleep(SLEEP * i)
-    
+        cmd = ['status', '--format=yaml']
+        if model:
+            cmd.append('-m{}'.format(model))
+
+        output = self.run_juju_command(*cmd)
+        if output:
+            parsed = yaml.safe_load(output)
+            machines = parsed.get('machines', [])
         return machines
 
     def connect(self):
@@ -486,38 +508,18 @@ class Juju(object):
         Switches to a given model
         :param model: the model to switch to      
         """
-
-        if os.path.isfile('/snap/bin/juju'):
-            cmd = ['/snap/bin/juju', 'switch', model]
-        elif os.path.isfile("/usr/bin/juju"):
-            cmd = ['/usr/bin/juju', 'switch', model]
-        else:
-            cmd = ['snap run juju', 'switch', model]
-
-        try:
-            subprocess.check_output(cmd)
-        except subprocess.CalledProcessError as e:
-            print("Exception calling juju status command. {}".format(e))
+        self.run_juju_command('switch', model)
 
     def list_models(self):
         """
         Gets the output of juju list-models
-        :returns dict: the above format       
         """
-        models = None
-
-        if os.path.isfile('/snap/bin/juju'):
-            cmd = ['/snap/bin/juju', 'list-models']
-        elif os.path.isfile("/usr/bin/juju"):
-            cmd = ['/usr/bin/juju', 'list-models']
-        else:
-            cmd = ['snap run juju', 'list-models']
-
-        try:
-            models = subprocess.check_output(cmd).decode("utf-8")
-        except subprocess.CalledProcessError as e:
-            print("Exception calling juju status command. {}".format(e))
+        models = []
+        output = self.run_juju_command('list-models', '--format=yaml')
 
+        if output:
+            parsed = yaml.safe_load(output)
+            models = parsed.get('models', [])
         return models
 
     def get_model_and_sizes(self):
@@ -530,29 +532,12 @@ class Juju(object):
         }
         :returns dict: the above format       
         """
-        models = self.list_models().split('\n')
-        modelsDict = {}
-        machinesColnum = 0
-
-        for line in models:
-            if line.strip() == "":
-                continue
-
-            if line.startswith('Controller') and machinesColnum == 0:
-                continue
-
-            line  = line.replace('\n', ' ').replace('\r', '')
-            cols = re.split('\s{2,100}', line)
-
-            if machinesColnum == 0:
-                for col in cols:
-                    if col == "Machines":
-                        break 
-                    machinesColnum += 1
-            else:
-                modelsDict[cols[0]] = int(cols[machinesColnum])
-
-        return modelsDict
+        result = {}
+        models = self.list_models()
+        for model in models:
+            model_name = model['short-name']
+            result[model_name] = len(self._list_machines(model=model_name))
+        return result
 
     def switch_to_largest_model(self):
         """
@@ -566,22 +551,19 @@ class Juju(object):
         Then calls switch_model        
         """
         largest = -1
-        largestModel = None
+        largest_model = None
         models = self.get_model_and_sizes()
 
         if len(models) == 0:
-            print("No models found ?????")
+            print("Error, no models found")
             return
 
-        for k,v in models.items():
+        for k, v in models.items():
             if v > largest:
                 largest = v
-                largestModel = k
+                largest_model = k
 
-        if "*" not in largestModel:
-            self.switch_model(largestModel)
-        else:
-            print("Largest model already selected: " + largestModel)
+        self.switch_model(largest_model)
 
     def get_report(self, running=False):
         """