Merge pull request #11 from secynic/dev

CHANGES.rst

@@ -12,6 +12,7 @@ Changelog
 - Fixed splitlines list[bytes] decode on Python 3
 - Logging output tweaks
 - Fixed redundant TCPDump.check_packet_print() in nfsinkhole-setup.py
+- Simplified utils.set_system_timezone(), removing unnecessary system calls.
 
 0.1.0 (2016-08-29)
 ------------------

nfsinkhole/tests/test_utils.py

@@ -2,7 +2,8 @@
 from nfsinkhole.exceptions import (IPTablesError, IPTablesExists,
                                    IPTablesNotExists, SubprocessError)
 from nfsinkhole.tests import TestCommon
-from nfsinkhole.utils import (popen_wrapper, set_system_timezone)
+from nfsinkhole.utils import (popen_wrapper, get_default_interface,
+                              get_interface_addr, set_system_timezone)
 
 LOG_FORMAT = ('[%(asctime)s] [%(levelname)s] [%(filename)s:%(lineno)s] '
               '[%(funcName)s()] %(message)s')
@@ -17,49 +18,21 @@ def test_popen_wrapper(self):
         # Argument checks
         self.assertRaises(ValueError, popen_wrapper)
         self.assertRaises(TypeError, popen_wrapper, 'notalist')
+        self.assertNotEqual(popen_wrapper(['ls'], log_stdout_line=False), None)
+        self.assertNotEqual(get_interface_addr('eth0'), None)
+        self.assertEqual(get_interface_addr('asdasd'), None)
 
         # raise_err test
         self.assertRaises(SubprocessError, popen_wrapper, **dict(
             cmd_arr=['asdasd'],
             raise_err=True
         ))
 
-    def test_timezone(self):
+    def test_get_default_interface(self):
 
-        set_system_timezone('UTC')
-
-        # Remove /etc/localtime
-        out, err = popen_wrapper(['rm', '/usr/bin/timedatectl'],
-                                 raise_err=True, sudo=True)
-
-        # stdout is not expected on success.
-        if (out or err) and (len(out) > 0 or len(err) > 0):
-            raise SubprocessError('{0}{1}'.format(
-                '{0}\n'.format(out) if out else '',
-                '{0}\n'.format(err) if err else ''
-            ))
-
-        # Create bad symbolic link
-        cmd = ['ln', '-s', '/usr/bin/sudo', '/usr/bin/timedatectl']
-        out, err = popen_wrapper(cmd, raise_err=True, sudo=True)
+        self.assertNotEqual(get_default_interface(), None)
 
-        # stdout is not expected on success.
-        if (out or err) and (len(out) > 0 or len(err) > 0):
-            raise SubprocessError('{0}{1}'.format(
-                '{0}\n'.format(out) if out else '',
-                '{0}\n'.format(err) if err else ''
-            ))
+    def test_set_system_timezone(self):
 
-        # Remove /etc/localtime
-        out, err = popen_wrapper(['rm', '/etc/localtime'],
-                                 raise_err=True, sudo=True)
-
-        # stdout is not expected on success.
-        if (out or err) and (len(out) > 0 or len(err) > 0):
-            raise SubprocessError('{0}{1}'.format(
-                '{0}\n'.format(out) if out else '',
-                '{0}\n'.format(err) if err else ''
-            ))
-
-        # localtime is removed, set_system_timezone should fail on copy attempt
-        self.assertRaises(SubprocessError, set_system_timezone, 'UTC')
+        set_system_timezone('UTC')
+        set_system_timezone('UTC', skip_timedatectl=True)

nfsinkhole/utils.py

@@ -176,7 +176,7 @@ def get_default_interface():
         log.info('Default network interface found: {0}'
                  ''.format(default_interface))
 
-    else:
+    else:  # pragma: no cover
 
         log.info('Default network interface not found')
 
@@ -204,12 +204,18 @@ def get_interface_addr(interface=None):
     log.debug('Attempting IP extract/conversion... '.format(interface))
     try:
 
+        # py3, except py2
+        try:
+            iface = bytes(interface[:15], encoding='utf-8')
+        except TypeError:  # pragma: no cover
+            iface = interface[:15]
+
         # TODO: is this faster than ifconfig/ip route parsing?
         addr = socket.inet_ntoa(
             fcntl.ioctl(
                 s.fileno(),
                 0x8915,
-                struct.pack('256s', interface[:15])
+                struct.pack('256s', iface)
             )[20:24]
         )
 
@@ -224,12 +230,14 @@ def get_interface_addr(interface=None):
         return None
 
 
-def set_system_timezone(timezone='UTC'):
+def set_system_timezone(timezone='UTC', skip_timedatectl=False):
     """
     The function for setting the system timezone.
 
     Args:
         timezone: The timezone to set, see /usr/share/zoneinfo/* for options.
+        skip_timedatectl: Skip attempting to set the timezone via timedatectl,
+            mainly used for testing.
 
     Raises:
         SubprocessError: One of the processes associated with manual timezone
@@ -238,49 +246,25 @@ def set_system_timezone(timezone='UTC'):
 
     log.info('Setting system timzone to {0}.'.format(timezone))
 
-    # Try setting the timzone with timedatectl
-    cmd = ['timedatectl', 'set-timezone', timezone]
-    out, err = popen_wrapper(cmd, sudo=True)
+    out = None
+    err = None
+    if not skip_timedatectl:
 
-    if out or err:
+        # Try setting the timzone with timedatectl
+        cmd = ['timedatectl', 'set-timezone', timezone]
+        out, err = popen_wrapper(cmd, sudo=True)
+
+    if skip_timedatectl or out or err:
 
         # timedatectl failed or missing, set /etc/localtime manually
         log.info('Reverting to manual timezone config (no timedatectl, or '
                  'errors).'.format(timezone))
 
-        # Backup localtime to /root/localtime.old
-        cmd = ['cp', '/etc/localtime', '/root/localtime.old']
-        out, err = popen_wrapper(cmd, raise_err=True, sudo=True)
-
-        # stdout is not expected on success.
-        if (out or err) and (len(out) > 0 or len(err) > 0):
-            raise SubprocessError('{0}{1}'.format(
-                '{0}\n'.format(out) if out else '',
-                '{0}\n'.format(err) if err else ''
-            ))
-
-        # Remove /etc/localtime
-        cmd = ['rm', '/etc/localtime']
-        out, err = popen_wrapper(cmd, raise_err=True, sudo=True)
-
-        # stdout is not expected on success.
-        if (out or err) and (len(out) > 0 or len(err) > 0):
-            raise SubprocessError('{0}{1}'.format(
-                '{0}\n'.format(out) if out else '',
-                '{0}\n'.format(err) if err else ''
-            ))
-
+        # TODO: this won't fail even if timezone is invalid
         # Create symbolic link to /usr/share/zoneinfo/{timezone} for
         # /etc/localtime
         cmd = [
-            'ln', '-s', '/usr/share/zoneinfo/{0}'.format(timezone),
+            'ln', '-fs', '/usr/share/zoneinfo/{0}'.format(timezone),
             '/etc/localtime'
         ]
-        out, err = popen_wrapper(cmd, raise_err=True, sudo=True)
-
-        # stdout is not expected on success.
-        if (out or err) and (len(out) > 0 or len(err) > 0):
-            raise SubprocessError('{0}{1}'.format(
-                '{0}\n'.format(out) if out else '',
-                '{0}\n'.format(err) if err else ''
-            ))
+        popen_wrapper(cmd, raise_err=True, sudo=True)