Compacted to improve readability / maintainability master

Multiple clean up.

* There were about eight methods that did almost exactly the same thing. Most of them actually _did_ do exactly the same thing. But since a person would actually have to _read_ all of them to find that out, they actually made the thing harder to maintain. So I squished them into one method. Doc strings are also improved.

1 files changed, 122 insertions, 142 deletions

diff --git a/theforeman.py b/theforeman.py
index 398c3ea..efcb79e 100755
--- a/theforeman.py
+++ b/theforeman.py
@@ -16,6 +16,8 @@
 # TORTUOUS ACTION, ARISING OUT OF OR IN CONNECTION WITH THE USE OR PERFORMANCE OF
 # THIS SOFTWARE.
 
+# TODO -- proper exception handling and meaningful error codes
+
 
 '''
 Foreman external inventory script
@@ -53,8 +55,9 @@ Examples:
   Execute uname on all instances in the dev group
   $ ansible -i theforeman.py dev -m shell -a \"/bin/uname -a\"
 
-Author: Franck Cuny <franckcuny@gmail.com>
-Version: 0.0.1
+Authors:  Franck Cuny <franckcuny@gmail.com>
+          Andrew Deck <andrew.deck@outlook.com>
+Version: 0.0.2
 '''
 
 import sys
@@ -77,6 +80,55 @@ except ImportError, e:
     print e
     sys.exit(1)
 
+class CLIMain(object):
+  def __init__(self):
+    """Program entry point"""
+    settings = self.read_settings()
+    self.parse_cli_args()
+    foreman = ForemanInventory(settings['username']
+                              ,settings['password']
+                              ,settings['base_url'])
+    if self.args.all:
+      data_to_print = foreman.get_all()
+    elif self.args.host:
+      data_to_print = foreman.get_host_info(self.args.host)
+    elif self.args.list:
+      data_to_print = foreman.get_inventory()
+    else:
+      data_to_print = {}
+    print(json.dumps(data_to_print, sort_keys=True, indent=4))
+    
+  def read_settings(self):
+    """Read the settings from the foreman.ini file"""
+    config = ConfigParser.SafeConfigParser()
+    scriptdir = os.path.dirname(os.path.realpath(__file__))
+    foreman_default_ini_path = os.path.join(scriptdir, 'foreman.ini')
+    foreman_ini_path = os.environ.get('FOREMAN_INI_PATH', foreman_default_ini_path)
+    config.read(foreman_ini_path)
+    settings = {}
+    required = ['base_url', 'username', 'password']
+    missing = []
+    for setting in required:
+      val = config.get('foreman', setting)
+      if val is None:
+        missing.append(setting)
+      settings[setting] = val
+    if missing != []:
+      raise Exception("Could not find values for Foreman " + ', '.join(missing)
+                      + ". They must be specified via ini file.")
+    return settings
+
+  def parse_cli_args(self):
+    """Command line argument processing"""
+    description = 'Produce an Ansible Inventory file based on Foreman'
+    parser = optparse.OptionParser(description=description)
+    parser.add_option('--list', action='store_true', default=True,
+          help='List instances (default: True)')
+    parser.add_option('--host', action='store',
+          help='Get all the variables about a specific instance')
+    parser.add_option('--all', action='store_true',
+          help= 'Get all the variables for all the instances')
+    (self.args, self.options) = parser.parse_args()
 
 class ForemanInventory(object):
     """Foreman Inventory"""
@@ -87,46 +139,26 @@ class ForemanInventory(object):
 
     def _empty_cache(self):
         """Empty cache"""
-        keys = ['operatingsystem', 'hostgroup', 'environment', 'model', 'compute_resource', 'domain', 'subnet', 'architecture', 'host']
+        keys = ['operatingsystem', 'hostgroup', 'environment', 'model',
+                'compute_resource', 'domain', 'subnet', 'architecture',
+                'host']
         keys_d = {}
         for i in keys:
           keys_d[i] = {}
         return keys_d
 
-    def __init__(self):
-        """Main execution path"""
-
+    def __init__(self, username, password, foreman_url):
+        # initializing vars
+        self.base_url = foreman_url
+        self.username = username
+        self.password = password
         self.inventory = self._empty_inventory()
         self._cache = self._empty_cache()
-
-        self.base_url = None
-        self.username = None
-        self.password = None
-
-        # Read settings and parse CLI arguments
-        self.read_settings()
-        self.parse_cli_args()
-
-        if self.base_url is None or self.username is None or self.password is None:
-            print '''Could not find values for Foreman base_url, username or password.
-They must be specified via ini file.'''
-            sys.exit(1)
-
         try:
-            self.client = Foreman(self.base_url, (self.username, self.password))
+          self.client = Foreman(self.base_url, (self.username, self.password))
         except ConnectionError, e:
-            print '''It looks like Foreman's API is unreachable.'''
-            print e
-            sys.exit(1)
-
-        if self.args.host:
-            data_to_print = self.get_host_info(self.args.host)
-        elif self.args.list:
-            data_to_print = self.get_inventory()
-        else:
-            data_to_print = {}
-
-        print(json.dumps(data_to_print, sort_keys=True, indent=4))
+          raise Exception("It looks like Foreman's API is unreachable. Error "
+                          "was: " + str(e))
 
     def get_host_info(self, host_id):
         """Get information about an host"""
@@ -135,33 +167,27 @@ They must be specified via ini file.'''
         meta = self._get_object_from_id('host', host_id)
         if meta is None:
             return host_desc
-
-        host_desc = {
-            'id': meta.get('id'),
-            'ip': meta.get('ip'),
-            'name': meta.get('name'),
-            'environment': meta.get('environment').get('environment').get('name').lower(),
-            'os': self._get_os_from_id(meta.get('operatingsystem_id')),
-            'model': self._get_model_from_id(meta.get('model_id')),
-            'compute_resource': self._get_compute_resource_from_id(meta.get('compute_resource_id')),
-            'domain': self._get_domain_from_id(meta.get('domain_id')),
-            'subnet': self._get_subnet_from_id(meta.get('subnet_id')),
-            'architecture': self._get_architecture_from_id(meta.get('architecture_id')),
-            'created': meta.get('created_at'),
-            'updated': meta.get('updated_at'),
-            'status': meta.get('status'),
-            'hostgroup': self._get_hostgroup_from_id(meta.get('hostgroup_id')),
-            # to ssh from ansible
-            'ansible_ssh_host': meta.get('ip'),
-        }
-
+        for k in ['id', 'ip', 'name', 'status']:
+          host_desc[k] = meta.get(k)
+        for k in ['model', 'compute_resource', 'domain', 'subnet'
+                  ,'architecture', 'hostgroup']:
+          host_desc[k] = self._get_from_type(k, meta)
+        for k in ['created', 'updated']:
+          host_desc[k] = meta.get(k + '_at')
+        host_desc['os'] = self._get_from_type('operatingsystem', meta)
+        try:
+          k = 'environment'
+          host_desc[k] = meta.get(k).get(k).get('name').lower()
+        except Exception:
+          pass # do nothing
+        # to ssh from ansible
+        host_desc['ansible_ssh_host'] = host_desc['ip']
         return host_desc
 
     def get_inventory(self):
-        """Get all the host from the inventory"""
+        """Get all the hosts from the inventory"""
         groups = collections.defaultdict(list)
         hosts  = []
-
         page = 1
         while True:
             resp = self.client.index_hosts(page=page)
@@ -169,112 +195,66 @@ They must be specified via ini file.'''
                 break
             page  += 1
             hosts += resp
-
         if len(hosts) < 1:
             return groups
-
         for host in hosts:
-            host_group = self._get_hostgroup_from_id(host.get('host').get('hostgroup_id'))
+            host_group = self._get_from_id('hostgroup', host.get('host').get('hostgroup_id'))
             server_name = host.get('host').get('name')
             groups[host_group].append(server_name)
-
         return groups
 
-    def read_settings(self):
-        """Read the settings from the foreman.ini file"""
-        config = ConfigParser.SafeConfigParser()
-        foreman_default_ini_path = os.path.join(os.path.dirname(os.path.realpath(__file__)), 'foreman.ini')
-        foreman_ini_path = os.environ.get('FOREMAN_INI_PATH', foreman_default_ini_path)
-        config.read(foreman_ini_path)
-        self.base_url = config.get('foreman', 'base_url')
-        self.username = config.get('foreman', 'username')
-        self.password = config.get('foreman', 'password')
-
-    def parse_cli_args(self):
-        """Command line argument processing"""
-        parser = optparse.OptionParser(description='Produce an Ansible Inventory file based on Foreman')
-        parser.add_option('--list', action='store_true', default=True, help='List instances (default: True)')
-        parser.add_option('--host', action='store', help='Get all the variables about a specific instance')
-        (self.args, self.options) = parser.parse_args()
-
-    def _get_os_from_id(self, os_id):
-        """Get operating system name"""
-        os_obj = self._get_object_from_id('operatingsystem', os_id)
-        if os_obj is None:
-            return os_obj
-
-        os_name = "{0}-{1}".format(os_obj.get('name'), os_obj.get('major'))
-        return os_name
-
-    def _get_hostgroup_from_id(self, host_id):
-        """Get hostgroup name"""
-        group = self._get_object_from_id('hostgroup', host_id)
-        if group is None:
-            return group
-
-        return group.get('label')
-
-    def _get_environment_from_id(self, env_id):
-        """Get environment name"""
-        environment = self._get_object_from_id('environment', env_id)
-        if environment is None:
-            return environment
-
-        return environment.get('name').lower()
-
-    def _get_model_from_id(self, model_id):
-        """Get model from an ID"""
-        model = self._get_object_from_id('model', model_id)
-        if model is None:
-            return model
-
-        return model.get('name')
-
-    def _get_compute_resource_from_id(self, resource_id):
-        """Get compute resource from id"""
-        compute_resource =  self._get_object_from_id('compute_resource', resource_id)
-        if compute_resource is None:
-            return compute_resource
-
-        return compute_resource.get('name')
-
-    def _get_domain_from_id(self, domain_id):
-        """Get domain from id"""
-        domain = self._get_object_from_id('domain', domain_id)
-        if domain is None:
-            return domain
-        return domain.get('name')
-
-    def _get_subnet_from_id(self, subnet_id):
-        """Get subnet from id"""
-        subnet = self._get_object_from_id('subnet', subnet_id)
-        if subnet is None:
-            return subnet
-
-        return subnet.get('name')
-
-    def _get_architecture_from_id(self, arch_id):
-        """Get architecture from id"""
-        arch = self._get_object_from_id('architecture', arch_id)
-        if arch is None:
+    def get_all(self):
+      """Get all the machines and all the variables for all the machines"""
+      groups = self.get_inventory()
+      hosts = {}
+      for group in groups:
+        for host in groups[group]:
+          hosts[host] = True
+      for host in hosts:
+        hosts[host] = self.get_host_info(host)
+      groups['_meta'] = {'hostvars': hosts}
+      return groups
+
+    def _get_from_type(self, param_type, host):
+      return self._get_from_id(param_type, host.get(param_type + '_id'))
+
+    def _get_from_id(self, param_type, param_id):
+        """Get the object of type param_type associated with the ID param_id
+        The following values for param_type are explicitly accounted for:
+        - architecture
+        - subnet
+        - domain
+        - compute_resource
+        - model
+        - environment
+        - label
+        - hostgroup
+        - operatingsystem
+        """
+        param = self._get_object_from_id(param_type, param_id)
+        if param is None:
             return None
-
-        return arch.get('name')
+        if param_type == "hostgroup":
+            return param.get('label')
+        elif param_type == 'operatingsystem':
+            return "{0}-{1}".format(param.get('name'), param.get('major'))
+        elif param_type == 'environment':
+            return param.get('name').lower()
+        else:
+            return param.get('name')
 
     def _get_object_from_id(self, obj_type, obj_id):
         """Get an object from it's ID"""
         if obj_id is None:
             return None
-
         obj = self._cache.get(obj_type).get(obj_id, None)
-
         if obj is None:
             method_name = "show_{0}s".format(obj_type)
             func = getattr(self.client, method_name)
             obj = func(obj_id)
             self._cache[obj_type][obj_id] = obj
-
         return obj.get(obj_type)
 
+if __name__ == '__main__':
+  CLIMain()
 
-ForemanInventory()