Don't run 'apt install' when list of packages is empty. (#729)

* Don't run 'apt install' when list of packages is empty.

A common pattern in charms is to call `apt_install()` passing a filtered
list of packages to avoid reinstallation of packages, so in normal
circumstances a charm may end up calling apt_install() with an empty
list producing the execution of "apt-get install" which will produce at
the very least a read of the list of packages database and build the
dependency tree, this is something that for IO constrained environments
impacts the performance of the running services. Also for environments
where the package database is broken (e.g. a post install script failed)
this would add more entropy and potentially making things more difficult
for the operator to fix.

$ sudo apt-get install
Reading package lists... Done
Building dependency tree... Done
Reading state information... Done
0 upgraded, 0 newly installed, 0 to remove and 63 not upgraded.

* Pep8 fixes.

charmhelpers/contrib/hahelpers/cluster.py

@@ -324,7 +324,7 @@ def valid_hacluster_config():
     '''
     vip = config_get('vip')
     dns = config_get('dns-ha')
-    if not(bool(vip) ^ bool(dns)):
+    if not (bool(vip) ^ bool(dns)):
         msg = ('HA: Either vip or dns-ha must be set but not both in order to '
                'use high availability')
         status_set('blocked', msg)

charmhelpers/contrib/network/ip.py

@@ -539,7 +539,7 @@ def port_has_listener(address, port):
     """
     cmd = ['nc', '-z', address, str(port)]
     result = subprocess.call(cmd)
-    return not(bool(result))
+    return not (bool(result))
 
 
 def assert_charm_supports_ipv6():

charmhelpers/contrib/network/ovs/__init__.py

@@ -648,7 +648,7 @@ def patch_ports_on_bridge(bridge):
                     uuid_for_port(
                         interface['options']['peer'])),
                     interface['options']['peer'])
-                yield(Patch(this_end, other_end))
+                yield Patch(this_end, other_end)
             # We expect one result and it is ok if it turns out to be a port
             # for a different bridge. However we need a break here to satisfy
             # the for/else check which is in place to detect interface referring

charmhelpers/contrib/network/ovs/ovsdb.py

@@ -205,7 +205,7 @@ def _deserialize_ovsdb(self, data):
                         decoded_set = []
                         for el in data[1]:
                             decoded_set.append(self._deserialize_ovsdb(el))
-                        return(decoded_set)
+                        return decoded_set
                     # fall back to normal processing below
                     break
 

charmhelpers/contrib/openstack/ssh_migrations.py

@@ -310,7 +310,7 @@ def ssh_known_hosts_lines(application_name, user=None):
         for hosts_line in hosts:
             if hosts_line.rstrip():
                 known_hosts_list.append(hosts_line.rstrip())
-    return(known_hosts_list)
+    return known_hosts_list
 
 
 def ssh_authorized_keys_lines(application_name, user=None):
@@ -327,7 +327,7 @@ def ssh_authorized_keys_lines(application_name, user=None):
         for authkey_line in keys:
             if authkey_line.rstrip():
                 authorized_keys_list.append(authkey_line.rstrip())
-    return(authorized_keys_list)
+    return authorized_keys_list
 
 
 def ssh_compute_remove(public_key, application_name, user=None):

charmhelpers/contrib/openstack/utils.py

@@ -1327,7 +1327,7 @@ def _check_listening_on_services_ports(services, test=False):
     @param test: default=False, if False, test for closed, otherwise open.
     @returns OrderedDict(service: [port-not-open, ...]...), [boolean]
     """
-    test = not(not(test))  # ensure test is True or False
+    test = not (not (test))  # ensure test is True or False
     all_ports = list(itertools.chain(*services.values()))
     ports_states = [port_has_listener('0.0.0.0', p) for p in all_ports]
     map_ports = OrderedDict()
@@ -1583,7 +1583,7 @@ def is_unit_paused_set():
         with unitdata.HookData()() as t:
             kv = t[0]
             # transform something truth-y into a Boolean.
-            return not(not(kv.get('unit-paused')))
+            return not (not (kv.get('unit-paused')))
     except Exception:
         return False
 
@@ -2181,7 +2181,7 @@ def is_unit_upgrading_set():
         with unitdata.HookData()() as t:
             kv = t[0]
             # transform something truth-y into a Boolean.
-            return not(not(kv.get('unit-upgrading')))
+            return not (not (kv.get('unit-upgrading')))
     except Exception:
         return False
 

charmhelpers/core/host.py

@@ -954,7 +954,7 @@ def pwgen(length=None):
     random_generator = random.SystemRandom()
     random_chars = [
         random_generator.choice(alphanumeric_chars) for _ in range(length)]
-    return(''.join(random_chars))
+    return ''.join(random_chars)
 
 
 def is_phy_iface(interface):

charmhelpers/fetch/ubuntu.py

@@ -363,6 +363,9 @@ def apt_install(packages, options=None, fatal=False, quiet=False):
     :type quiet: bool
     :raises: subprocess.CalledProcessError
     """
+    if not packages:
+        log("Nothing to install", level=DEBUG)
+        return
     if options is None:
         options = ['--option=Dpkg::Options::=--force-confold']
 

tests/fetch/test_fetch_ubuntu.py

@@ -897,6 +897,14 @@ def test_installs_apt_packages_with_possible_errors(self, log,
              '--foo', '--bar', 'install', 'foo', 'bar'],
             env={})
 
+    @patch('subprocess.check_call')
+    @patch('charmhelpers.fetch.ubuntu.log')
+    def test_installs_apt_empty_packages(self, log, check_call):
+        packages = []
+        fetch.apt_install(packages, fatal=True)
+
+        check_call.assert_not_called()
+
     @patch('subprocess.check_call')
     @patch('charmhelpers.fetch.ubuntu.log')
     def test_purges_apt_packages_as_string_fatal(self, log, mock_call):