add generic param checker for api

- new method to generically check args
- updated log collector to use API instead of cli

Jira-Issue: OSTACKDEV-18

kollacli/api/async.py

@@ -17,6 +17,7 @@ from kollacli.api.exceptions import InvalidArgument
 from kollacli.api.job import Job
 from kollacli.common.ansible import actions
 from kollacli.common.inventory import Inventory
+from kollacli.common.utils import check_arg
 from kollacli.common.utils import safe_decode
 
 
@@ -37,9 +38,13 @@ class AsyncApi(object):
     def async_upgrade(self, verbose_level=1):
         """Upgrade.
 
+        :param verbose_level: the higher the number, the more verbose
+        :type verbose_level: integer
+
         Upgrade containers to new version specified by the property
         "openstack_release."
         """
+        check_arg(verbose_level, u._('Verbose level'), int)
         ansible_job = actions.upgrade(verbose_level)
         return Job(ansible_job)
 
@@ -49,7 +54,20 @@ class AsyncApi(object):
 
         Stops and removes all kolla related docker containers on the
         specified hosts.
+
+        :param hostnames: host names
+        :type hostnames: list
+        :param destroy_type: either 'kill' or 'stop'
+        :type destroy_type: string
+        :param verbose_level: the higher the number, the more verbose
+        :type verbose_level: integer
+        :param include_data: if true, destroy data containers too.
+        :type include_data: boolean
         """
+        check_arg(hostnames, u._('Host names'), list)
+        check_arg(destroy_type, u._('Destroy type'), str)
+        check_arg(verbose_level, u._('Verbose level'), int)
+        check_arg(include_data, u._('Include data'), bool)
         if destroy_type not in ['stop', 'kill']:
             raise InvalidArgument(
                 u._('Invalid destroy type ({type}). Must be either '
@@ -69,7 +87,13 @@ class AsyncApi(object):
         Check if host is ready for a new deployment. This will fail if
         any of the hosts are not configured correctly or if they have
         already been deployed to.
+        :param hostnames: host names
+        :type hostnames: list
+        :param verbose_level: the higher the number, the more verbose
+        :type verbose_level: integer
         """
+        check_arg(hostnames, u._('Host names'), list)
+        check_arg(verbose_level, u._('Verbose level'), int)
         hostnames = safe_decode(hostnames)
         inventory = Inventory.load()
         inventory.validate_hostnames(hostnames)

kollacli/api/deploy.py

@@ -13,7 +13,10 @@
 #    under the License.
 import logging
 
+import kollacli.i18n as u
+
 from kollacli.common.inventory import Inventory
+from kollacli.common.utils import check_arg
 
 LOG = logging.getLogger(__name__)
 
@@ -30,6 +33,7 @@ class DeployApi(object):
         :param remote_mode: if remote mode is True deployment is done via ssh
         :type remote_mode: bool
         """
+        check_arg(remote_mode, u._('Remote mode'), bool)
         inventory = Inventory.load()
         inventory.set_deploy_mode(remote_mode)
         Inventory.save(inventory)

kollacli/api/group.py

@@ -14,9 +14,8 @@
 from copy import copy
 import kollacli.i18n as u
 
-from kollacli.api.exceptions import InvalidArgument
-from kollacli.api.exceptions import MissingArgument
 from kollacli.common.inventory import Inventory
+from kollacli.common.utils import check_arg
 from kollacli.common.utils import safe_decode
 
 
@@ -51,6 +50,7 @@ class GroupApi(object):
             :type servicename: string
 
             """
+            check_arg(servicename, u._('Service name'), str)
             servicename = safe_decode(servicename)
             inventory = Inventory.load()
             inventory.validate_servicenames([servicename])
@@ -69,6 +69,7 @@ class GroupApi(object):
             :type servicename: string
 
             """
+            check_arg(servicename, u._('Service name'), str)
             servicename = safe_decode(servicename)
             inventory = Inventory.load()
             inventory.validate_servicenames([servicename])
@@ -95,6 +96,7 @@ class GroupApi(object):
             :type hostname: string
 
             """
+            check_arg(hostname, u._('Host name'), str)
             hostname = safe_decode(hostname)
             inventory = Inventory.load()
             inventory.validate_hostnames([hostname])
@@ -113,6 +115,7 @@ class GroupApi(object):
             :type hostname: string
 
             """
+            check_arg(hostname, u._('Host name'), str)
             hostname = safe_decode(hostname)
             inventory = Inventory.load()
             inventory.validate_hostnames([hostname])
@@ -131,8 +134,7 @@ class GroupApi(object):
         :type groupname: string
 
         """
-        if not groupname:
+        check_arg(groupname, u._('Group name'), str)
-            raise MissingArgument(u._('Group name'))
         groupname = safe_decode(groupname)
 
         inventory = Inventory.load()
@@ -146,9 +148,7 @@ class GroupApi(object):
         :type groupname: string
 
         """
-        if not groupname:
+        check_arg(groupname, u._('Group name'), str)
-            raise MissingArgument(u._('Group name'))
-
         inventory = Inventory.load()
         groupname = safe_decode(groupname)
         inventory.remove_group(groupname)
@@ -170,11 +170,7 @@ class GroupApi(object):
         :return: groups
         :rtype: list of Group objects
         """
-        if groupnames is None:
+        check_arg(groupnames, u._('Group names'), list)
-            raise MissingArgument(u._('Group names'))
-        if not isinstance(groupnames, list):
-            raise InvalidArgument(u._('Group names ({names}) is not a list')
-                                  .format(names=groupnames))
         groupnames = safe_decode(groupnames)
         return self._get_groups(groupnames)
 

kollacli/api/host.py

@@ -14,9 +14,8 @@
 from copy import copy
 import kollacli.i18n as u
 
-from kollacli.api.exceptions import InvalidArgument
-from kollacli.api.exceptions import MissingArgument
 from kollacli.common.inventory import Inventory
+from kollacli.common.utils import check_arg
 from kollacli.common.utils import safe_decode
 
 
@@ -49,8 +48,7 @@ class HostApi(object):
 
         :param hostnames: list of strings
         """
-        if not hostnames:
+        check_arg(hostnames, u._('Host names'), list)
-            raise MissingArgument(u._('Host names'))
         hostnames = safe_decode(hostnames)
 
         inventory = Inventory.load()
@@ -67,12 +65,10 @@ class HostApi(object):
 
         :param hostnames: list of strings
         """
-        inventory = Inventory.load()
+        check_arg(hostnames, u._('Host names'), list)
-
-        if not hostnames:
-                raise MissingArgument(u._('Host names'))
-
         hostnames = safe_decode(hostnames)
+
+        inventory = Inventory.load()
         any_changed = False
         for hostname in hostnames:
             changed = inventory.remove_host(hostname)
@@ -101,11 +97,7 @@ class HostApi(object):
         :return: hosts
         :rtype: Host
         """
-        if hostnames is None:
+        check_arg(hostnames, u._('Host names'), list)
-            raise MissingArgument(u._('Host names'))
-        if not isinstance(hostnames, list):
-            raise InvalidArgument(u._('Host names ({names}) is not a list')
-                                  .format(names=hostnames))
         hostnames = safe_decode(hostnames)
         inventory = Inventory.load()
         inventory.validate_hostnames(hostnames)
@@ -130,6 +122,7 @@ class HostApi(object):
         :return: check status
         :rtype: dictionary
         """
+        check_arg(hostnames, u._('Host names'), list)
         inventory = Inventory.load()
         hostnames = safe_decode(hostnames)
         inventory.validate_hostnames(hostnames)
@@ -149,6 +142,7 @@ class HostApi(object):
 
         :param hosts_info: dictionary
         """
+        check_arg(hosts_info, u._('Hosts info'), dict)
         inventory = Inventory.load()
         inventory.validate_hostnames(hosts_info.keys())
         inventory.setup_hosts(hosts_info)

kollacli/api/password.py

@@ -13,10 +13,10 @@
 #    under the License.
 import kollacli.i18n as u
 
-from kollacli.api.exceptions import MissingArgument
 from kollacli.common.passwords import clear_password
 from kollacli.common.passwords import get_password_names
 from kollacli.common.passwords import set_password
+from kollacli.common.utils import check_arg
 
 
 class PasswordApi(object):
@@ -29,8 +29,7 @@ class PasswordApi(object):
         :param value: value of the password
         :type value: string
         """
-        if not name:
+        check_arg(name, u._('Password name'), str)
-            raise(MissingArgument(u._('Password name')))
         set_password(name, value)
 
     def password_clear(self, name):
@@ -39,8 +38,7 @@ class PasswordApi(object):
         :param name: name of the password
         :type name: string
         """
-        if not name:
+        check_arg(name, u._('Password name'), str)
-            raise(MissingArgument(u._('Password name')))
         clear_password(name)
 
     def password_get_names(self):

kollacli/api/service.py

@@ -14,9 +14,8 @@
 from copy import copy
 import kollacli.i18n as u
 
-from kollacli.api.exceptions import InvalidArgument
-from kollacli.api.exceptions import MissingArgument
 from kollacli.common.inventory import Inventory
+from kollacli.common.utils import check_arg
 from kollacli.common.utils import safe_decode
 
 
@@ -95,11 +94,7 @@ class ServiceApi(object):
         :return: services
         :rtype: list of Service objects
         """
-        if servicenames is None:
+        check_arg(servicenames, u._('Service names'), list)
-            raise MissingArgument(u._('Service names'))
-        if not isinstance(servicenames, list):
-            raise InvalidArgument(u._('Service names ({names}) is not a list')
-                                  .format(names=servicenames))
         servicenames = safe_decode(servicenames)
         return self._get_services(servicenames)
 

kollacli/common/utils.py

@@ -23,6 +23,7 @@ import sys
 import kollacli.i18n as u
 
 from kollacli.api.exceptions import InvalidArgument
+from kollacli.api.exceptions import MissingArgument
 
 LOG = logging.getLogger(__name__)
 
@@ -302,3 +303,22 @@ def is_string_true(string):
         return True
     else:
         return False
+
+
+def check_arg(param, param_name, expected_type, none_ok=False, empty_ok=False):
+    if param is None:
+        if none_ok:
+            return
+        # None arg
+        raise MissingArgument(param_name)
+
+    if (not isinstance(param, bool) and
+            not param and not empty_ok):
+            # empty arg
+            raise MissingArgument(param_name)
+
+    if not isinstance(param, expected_type):
+        # wrong type
+        raise InvalidArgument(u._('{name} ({param}) is not a {type}')
+                              .format(name=param_name, param=param,
+                                      type=expected_type))

tests/client_upgrade.py

@@ -95,8 +95,8 @@ class TestFunctional(KollaCliTest):
         hostname = 'test_host_upg'
 
         # This host add will cause the inventory to be upgraded
-        CLIENT.host_add(hostname)
+        CLIENT.host_add([hostname])
-        CLIENT.host_remove(hostname)
+        CLIENT.host_remove([hostname])
 
         # run tests for each version:
         if version <= 1:

tools/log_collector.py

@@ -20,6 +20,7 @@ import tarfile
 import tempfile
 import traceback
 
+from kollacli.api.client import ClientApi
 from kollacli.common.inventory import Inventory
 from kollacli.common.inventory import remove_temp_inventory
 from kollacli.common import properties
@@ -29,6 +30,8 @@ from kollacli.common.utils import safe_decode
 
 tar_file_descr = None
 
+CLIENT = ClientApi()
+
 
 def run_ansible_cmd(cmd, host):
     # sudo -u kolla ansible ol7-c4 -i inv_path -a "cmd"
@@ -180,20 +183,15 @@ def main():
     with tarfile.open(tar_path, 'w:gz') as tar_file_descr:
         # gather dump output from kollacli
         print('Getting kollacli logs')
-        # cliff prints log output to stderr
+        dump_path = None
-        (_, err) = subprocess.Popen('kollacli dump'.split(),
+        try:
-                                    stdout=subprocess.PIPE,
+            dump_path = CLIENT.support_dump()
-                                    stderr=subprocess.PIPE).communicate()
+            tar_file_descr.add(dump_path)
-        err = safe_decode(err)
+        except Exception:
-        if '/' in err:
+            print('ERROR: running dump command %s' % traceback.format_exc())
-            dump_path = '/' + err.strip().split('/', 1)[1]
+        finally:
-            if os.path.isfile(dump_path):
+            if dump_path and os.path.exists(dump_path):
-                tar_file_descr.add(dump_path)
                 os.remove(dump_path)
-            else:
-                print('ERROR: No kolla dump output file at %s' % dump_path)
-        else:
-            print('ERROR: No path found in dump command output: %s' % err)
 
         # gather logs from selected hosts
         for host in hosts: