Add caching and cleanup of chart tarballs

Caching and cleanup of git repository chart sources was previously
implemented. This adds these features for tarball sources as well.

This also implements transitive chart dependency sourcing. Previously
only a single level of dependencies were being downloaded, which
would lead to an error when multiple dependency levels exist.

Change-Id: I988e473a6ea29331e036d26c3ec7269374e0188f

armada/handlers/armada.py

@@ -103,7 +103,7 @@ class Armada(object):
             raise
         self.manifest = Manifest(
             self.documents, target_manifest=target_manifest).get_manifest()
-        self.cloned_dirs = set()
+        self.chart_cache = {}
         self.chart_deploy = ChartDeploy(
             disable_update_pre, disable_update_post, self.dry_run,
             k8s_wait_attempts, k8s_wait_attempt_sleep, timeout, self.tiller)
@@ -119,16 +119,12 @@ class Armada(object):
             raise tiller_exceptions.TillerServicesUnavailableException()
 
         # Clone the chart sources
-        repos = {}
         manifest_data = self.manifest.get(const.KEYWORD_ARMADA, {})
         for group in manifest_data.get(const.KEYWORD_GROUPS, []):
             for ch in group.get(const.KEYWORD_CHARTS, []):
-                self.tag_cloned_repo(ch, repos)
+                self.get_chart(ch)
 
-                for dep in ch.get('chart', {}).get('dependencies', []):
+    def get_chart(self, ch):
-                    self.tag_cloned_repo(dep, repos)
-
-    def tag_cloned_repo(self, ch, repos):
         chart = ch.get('chart', {})
         chart_source = chart.get('source', {})
         location = chart_source.get('location')
@@ -138,25 +134,30 @@ class Armada(object):
         if ct_type == 'local':
             chart['source_dir'] = (location, subpath)
         elif ct_type == 'tar':
-            LOG.info('Downloading tarball from: %s', location)
+            source_key = (ct_type, location)
 
-            if not CONF.certs:
+            if source_key not in self.chart_cache:
-                LOG.warn('Disabling server validation certs to extract charts')
+                LOG.info('Downloading tarball from: %s', location)
-                tarball_dir = source.get_tarball(location, verify=False)
-            else:
-                tarball_dir = source.get_tarball(location, verify=CONF.cert)
 
-            chart['source_dir'] = (tarball_dir, subpath)
+                if not CONF.certs:
+                    LOG.warn(
+                        'Disabling server validation certs to extract charts')
+                    tarball_dir = source.get_tarball(location, verify=False)
+                else:
+                    tarball_dir = source.get_tarball(
+                        location, verify=CONF.cert)
+                self.chart_cache[source_key] = tarball_dir
+            chart['source_dir'] = (self.chart_cache.get(source_key), subpath)
         elif ct_type == 'git':
             reference = chart_source.get('reference', 'master')
-            repo_branch = (location, reference)
+            source_key = (ct_type, location, reference)
 
-            if repo_branch not in repos:
+            if source_key not in self.chart_cache:
                 auth_method = chart_source.get('auth_method')
                 proxy_server = chart_source.get('proxy_server')
 
                 logstr = 'Cloning repo: {} from branch: {}'.format(
-                    *repo_branch)
+                    location, reference)
                 if proxy_server:
                     logstr += ' proxy: {}'.format(proxy_server)
                 if auth_method:
@@ -164,19 +165,20 @@ class Armada(object):
                 LOG.info(logstr)
 
                 repo_dir = source.git_clone(
-                    *repo_branch,
+                    location,
+                    reference,
                     proxy_server=proxy_server,
                     auth_method=auth_method)
-                self.cloned_dirs.add(repo_dir)
 
-                repos[repo_branch] = repo_dir
+                self.chart_cache[source_key] = repo_dir
-                chart['source_dir'] = (repo_dir, subpath)
+            chart['source_dir'] = (self.chart_cache.get(source_key), subpath)
-            else:
-                chart['source_dir'] = (repos.get(repo_branch), subpath)
         else:
             chart_name = chart.get('chart_name')
             raise source_exceptions.ChartSourceException(ct_type, chart_name)
 
+        for dep in ch.get('chart', {}).get('dependencies', []):
+            self.get_chart(dep)
+
     def sync(self):
         '''
         Synchronize Helm with the Armada Config(s)
@@ -293,9 +295,9 @@ class Armada(object):
         LOG.info("Performing post-flight operations.")
 
         # Delete temp dirs used for deployment
-        for cloned_dir in self.cloned_dirs:
+        for chart_dir in self.chart_cache.values():
-            LOG.debug('Removing cloned temp directory: %s', cloned_dir)
+            LOG.debug('Removing temp chart directory: %s', chart_dir)
-            source.source_cleanup(cloned_dir)
+            source.source_cleanup(chart_dir)
 
     def _chart_cleanup(self, prefix, charts, msg):
         LOG.info('Processing chart cleanup to remove unspecified releases.')

armada/tests/unit/handlers/test_armada.py

@@ -151,6 +151,7 @@ CHART_SOURCES = [('git://github.com/dummy/armada', 'chart_1'),
                  ('/tmp/dummy/armada', 'chart_4')]
 
 
+# TODO(seaneagan): Add unit tests with dependencies, including transitive.
 class ArmadaHandlerTestCase(base.ArmadaTestCase):
 
     def _test_pre_flight_ops(self, armada_obj):

armada/tests/unit/utils/test_source.py

@@ -15,7 +15,6 @@
 import os
 import shutil
 
-import fixtures
 import mock
 import testtools
 
@@ -156,23 +155,6 @@ class GitTestCase(base.ArmadaTestCase):
         source.source_cleanup(git_path)
         mock_log.warning.assert_not_called()
 
-    @test_utils.attr(type=['negative'])
-    @mock.patch.object(source, 'LOG')
-    @mock.patch('armada.utils.source.shutil')
-    def test_source_cleanup_fake_git_path(self, mock_shutil, mock_log):
-        # Verify that passing in a fake path does nothing but log a warning.
-        # Don't want anyone using the function to delete random directories.
-        fake_path = self.useFixture(fixtures.TempDir()).path
-        self.addCleanup(shutil.rmtree, fake_path)
-        source.source_cleanup(fake_path)
-
-        mock_shutil.rmtree.assert_not_called()
-        self.assertTrue(mock_log.warning.called)
-        actual_call = mock_log.warning.mock_calls[0][1][:2]
-        self.assertEqual(
-            ('%s is not a valid git repository. Details: %s', fake_path),
-            actual_call)
-
     @test_utils.attr(type=['negative'])
     @mock.patch.object(source, 'LOG')
     @mock.patch('armada.utils.source.shutil')
@@ -187,9 +169,8 @@ class GitTestCase(base.ArmadaTestCase):
         mock_shutil.rmtree.assert_not_called()
         self.assertTrue(mock_log.warning.called)
         actual_call = mock_log.warning.mock_calls[0][1]
-        self.assertEqual(
+        self.assertEqual(('Could not find the chart path %s to delete.', path),
-            ('Could not delete the path %s. Is it a git repository?', path),
+                         actual_call)
-            actual_call)
 
     @testtools.skipUnless(base.is_connected(),
                           'git clone requires network connectivity.')

armada/utils/source.py

@@ -153,27 +153,19 @@ def extract_tarball(tarball_path):
     return temp_dir
 
 
-def source_cleanup(git_path):
+def source_cleanup(chart_path):
-    '''Clean up the git repository that was created by ``git_clone`` above.
+    '''Clean up the chart path that was created above.
 
-    Removes the ``git_path`` repository and all associated files if they
+    Removes the ``chart_path`` and all associated files if they
     exist.
 
-    :param str git_path: The git repository to delete.
+    :param str chart_path: The chart path to delete.
     '''
-    if os.path.exists(git_path):
+    if os.path.exists(chart_path):
         try:
-            # Internally validates whether the path points to an actual repo.
+            shutil.rmtree(chart_path)
-            Repo(git_path)
+        except OSError as e:
-        except git_exc.InvalidGitRepositoryError as e:
+            LOG.warning('Could not delete the path %s. Details: %s',
-            LOG.warning('%s is not a valid git repository. Details: %s',
+                        chart_path, e)
-                        git_path, e)
-        else:
-            try:
-                shutil.rmtree(git_path)
-            except OSError as e:
-                LOG.warning('Could not delete the path %s. Details: %s',
-                            git_path, e)
     else:
-        LOG.warning('Could not delete the path %s. Is it a git repository?',
+        LOG.warning('Could not find the chart path %s to delete.', chart_path)
-                    git_path)