ugly -> bad

cdist/config.py

@@ -22,15 +22,14 @@
 
 import logging
 import os
-import shutil
 import sys
 import time
-import pprint
 import itertools
 import tempfile
 import socket
 
 import cdist
+import cdist.hostsource
 
 import cdist.exec.local
 import cdist.exec.remote
@@ -61,55 +60,17 @@ class Config(object):
         self.local.create_files_dirs()
         self.remote.create_files_dirs()
 
-    @staticmethod
-    def hostfile_process_line(line):
-        """Return host from read line or None if no host present."""
-        if not line:
-            return None
-        # remove comment if present
-        comment_index = line.find('#')
-        if comment_index >= 0:
-            host = line[:comment_index]
-        else:
-            host = line
-        # remove leading and trailing whitespaces
-        host = host.strip()
-        # skip empty lines
-        if host:
-            return host
-        else:
-            return None
-
     @staticmethod
     def hosts(source):
-        """Yield hosts from source.
-           Source can be a sequence or filename (stdin if \'-\').
-           In case of filename each line represents one host.
-        """
-        if isinstance(source, str):
-            import fileinput
-            try:
-                for host in fileinput.input(files=(source)):
-                    host = Config.hostfile_process_line(host)
-                    if host:
-                        yield host
-            except (IOError, OSError, UnicodeError) as e:
-                raise cdist.Error(
-                        "Error reading hosts from file \'{}\': {}".format(
-                            source, e))
-        else:
-            if source:
-                for host in source:
-                    yield host
+        try:
+            yield from cdist.hostsource.HostSource(source)()
+        except (IOError, OSError, UnicodeError) as e:
+            raise cdist.Error(
+                    "Error reading hosts from \'{}\': {}".format(
+                        source, e))
 
     @classmethod
-    def commandline(cls, args):
-        """Configure remote system"""
-        import multiprocessing
-
-        # FIXME: Refactor relict - remove later
-        log = logging.getLogger("cdist")
-
+    def _check_and_prepare_args(cls, args):
         if args.manifest == '-' and args.hostfile == '-':
             raise cdist.Error(("Cannot read both, manifest and host file, "
                                "from stdin"))
@@ -134,10 +95,6 @@ class Config(object):
             import atexit
             atexit.register(lambda: os.remove(initial_manifest_temp_path))
 
-        process = {}
-        failed_hosts = []
-        time_start = time.time()
-
         # default remote cmd patterns
         args.remote_exec_pattern = None
         args.remote_copy_pattern = None
@@ -154,10 +111,29 @@ class Config(object):
             if args_dict['remote_copy'] is None:
                 args.remote_copy_pattern = cdist.REMOTE_COPY + mux_opts
 
+    @classmethod
+    def _base_root_path(cls, args):
         if args.out_path:
             base_root_path = args.out_path
         else:
             base_root_path = tempfile.mkdtemp()
+        return base_root_path
+
+    @classmethod
+    def commandline(cls, args):
+        """Configure remote system"""
+        import multiprocessing
+
+        # FIXME: Refactor relict - remove later
+        log = logging.getLogger("cdist")
+
+        cls._check_and_prepare_args(args)
+
+        process = {}
+        failed_hosts = []
+        time_start = time.time()
+
+        base_root_path = cls._base_root_path(args)
 
         hostcnt = 0
         for host in itertools.chain(cls.hosts(args.host),
@@ -199,6 +175,24 @@ class Config(object):
             raise cdist.Error("Failed to configure the following hosts: " +
                               " ".join(failed_hosts))
 
+    @classmethod
+    def _resolve_remote_cmds(cls, args, host_base_path):
+        control_path = os.path.join(host_base_path, "ssh-control-path")
+        # If we constructed patterns for remote commands then there is
+        # placeholder for ssh ControlPath, format it and we have unique
+        # ControlPath for each host.
+        #
+        # If not then use args.remote_exec/copy that user specified.
+        if args.remote_exec_pattern:
+            remote_exec = args.remote_exec_pattern.format(control_path)
+        else:
+            remote_exec = args.remote_exec
+        if args.remote_copy_pattern:
+            remote_copy = args.remote_copy_pattern.format(control_path)
+        else:
+            remote_copy = args.remote_copy
+        return (remote_exec, remote_copy, )
+
     @classmethod
     def onehost(cls, host, host_base_path, host_dir_name, args, parallel):
         """Configure ONE system"""
@@ -206,20 +200,7 @@ class Config(object):
         log = logging.getLogger(host)
 
         try:
-            control_path = os.path.join(host_base_path, "ssh-control-path")
-            # If we constructed patterns for remote commands then there is
-            # placeholder for ssh ControlPath, format it and we have unique
-            # ControlPath for each host.
-            #
-            # If not then use args.remote_exec/copy that user specified.
-            if args.remote_exec_pattern:
-                remote_exec = args.remote_exec_pattern.format(control_path)
-            else:
-                remote_exec = args.remote_exec
-            if args.remote_copy_pattern:
-                remote_copy = args.remote_copy_pattern.format(control_path)
-            else:
-                remote_copy = args.remote_copy
+            remote_exec, remote_copy = cls._resolve_remote_cmds(host_base_path)
             log.debug("remote_exec for host \"{}\": {}".format(
                 host, remote_exec))
             log.debug("remote_copy for host \"{}\": {}".format(

cdist/exec/remote.py

@@ -30,40 +30,13 @@ import multiprocessing
 
 import cdist
 import cdist.exec.util as exec_util
-
-
-# check whether addr is IPv6
-try:
-    # python 3.3+
-    import ipaddress
-
-    def _is_ipv6(addr):
-        try:
-            return ipaddress.ip_address(addr).version == 6
-        except ValueError:
-            return False
-except ImportError:
-    # fallback for older python versions
-    import socket
-
-    def _is_ipv6(addr):
-        try:
-            socket.inet_aton(addr)
-            return False
-        except socket.error:
-            pass
-        try:
-            socket.inet_pton(socket.AF_INET6, addr)
-            return True
-        except socket.error:
-            pass
-        return False
+import cdist.util.ipaddr as ipaddr
 
 
 def _wrap_addr(addr):
     """If addr is IPv6 then return addr wrapped between '[' and ']',
     otherwise return it intact."""
-    if _is_ipv6(addr):
+    if ipaddr.is_ipv6(addr):
         return "".join(("[", addr, "]", ))
     else:
         return addr

cdist/hostsource.py

@@ -0,0 +1,72 @@
+# -*- coding: utf-8 -*-
+#
+# 2016 Darko Poljak (darko.poljak at gmail.com)
+#
+# This file is part of cdist.
+#
+# cdist is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation, either version 3 of the License, or
+# (at your option) any later version.
+#
+# cdist is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with cdist. If not, see <http://www.gnu.org/licenses/>.
+#
+#
+
+import fileinput
+
+
+class HostSource(object):
+    """
+    Host source object.
+    Source can be a sequence or filename (stdin if \'-\').
+    In case of filename each line represents one host.
+    """
+    def __init__(self, source):
+        self.source = source
+
+    def _process_file_line(self, line):
+        """Return host from read line or None if no host present."""
+        if not line:
+            return None
+        # remove comment if present
+        comment_index = line.find('#')
+        if comment_index >= 0:
+            host = line[:comment_index]
+        else:
+            host = line
+        # remove leading and trailing whitespaces
+        host = host.strip()
+        # skip empty lines
+        if host:
+            return host
+        else:
+            return None
+
+    def _hosts_from_sequence(self):
+        for host in self.source:
+            yield host
+
+    def _hosts_from_file(self):
+        for line in fileinput.input(files=(self.source)):
+            host = self._process_file_line(line)
+            if host:
+                yield host
+
+    def hosts(self):
+        if not source:
+            return
+
+        if isinstance(self.source, str):
+            yield from self._hosts_from_file()
+        else:
+            yield from self._hosts_from_sequence()
+
+    def __call__(self):
+        yield from self.hosts()

cdist/util/ipaddr.py

@@ -55,3 +55,29 @@ def resolve_target_addresses(host):
         host_fqdn = ''
 
     return (host, host_name, host_fqdn)
+
+
+# check whether addr is IPv6
+try:
+    # python 3.3+
+    import ipaddress
+
+    def is_ipv6(addr):
+        try:
+            return ipaddress.ip_address(addr).version == 6
+        except ValueError:
+            return False
+except ImportError:
+    # fallback for older python versions
+    def is_ipv6(addr):
+        try:
+            socket.inet_aton(addr)
+            return False
+        except socket.error:
+            pass
+        try:
+            socket.inet_pton(socket.AF_INET6, addr)
+            return True
+        except socket.error:
+            pass
+        return False