diff --git a/armada/handlers/armada.py b/armada/handlers/armada.py index fa6afcd2..a95401ea 100644 --- a/armada/handlers/armada.py +++ b/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', []): - self.tag_cloned_repo(dep, repos) - - def tag_cloned_repo(self, ch, repos): + def get_chart(self, ch): 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: - 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) + if source_key not in self.chart_cache: + LOG.info('Downloading tarball from: %s', location) - 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 - chart['source_dir'] = (repo_dir, subpath) - else: - chart['source_dir'] = (repos.get(repo_branch), subpath) + self.chart_cache[source_key] = repo_dir + chart['source_dir'] = (self.chart_cache.get(source_key), 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: - LOG.debug('Removing cloned temp directory: %s', cloned_dir) - source.source_cleanup(cloned_dir) + for chart_dir in self.chart_cache.values(): + LOG.debug('Removing temp chart directory: %s', chart_dir) + source.source_cleanup(chart_dir) def _chart_cleanup(self, prefix, charts, msg): LOG.info('Processing chart cleanup to remove unspecified releases.') diff --git a/armada/tests/unit/handlers/test_armada.py b/armada/tests/unit/handlers/test_armada.py index 92eecdf3..90bca080 100644 --- a/armada/tests/unit/handlers/test_armada.py +++ b/armada/tests/unit/handlers/test_armada.py @@ -152,6 +152,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): diff --git a/armada/tests/unit/utils/test_source.py b/armada/tests/unit/utils/test_source.py index 21fd6308..78682d79 100644 --- a/armada/tests/unit/utils/test_source.py +++ b/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( - ('Could not delete the path %s. Is it a git repository?', path), - actual_call) + self.assertEqual(('Could not find the chart path %s to delete.', path), + actual_call) @testtools.skipUnless(base.is_connected(), 'git clone requires network connectivity.') diff --git a/armada/utils/source.py b/armada/utils/source.py index 727884c6..510c3d38 100644 --- a/armada/utils/source.py +++ b/armada/utils/source.py @@ -153,27 +153,19 @@ def extract_tarball(tarball_path): return temp_dir -def source_cleanup(git_path): - '''Clean up the git repository that was created by ``git_clone`` above. +def source_cleanup(chart_path): + '''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. - Repo(git_path) - except git_exc.InvalidGitRepositoryError as e: - LOG.warning('%s is not a valid git repository. Details: %s', - 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) + shutil.rmtree(chart_path) + except OSError as e: + LOG.warning('Could not delete the path %s. Details: %s', + chart_path, e) else: - LOG.warning('Could not delete the path %s. Is it a git repository?', - git_path) + LOG.warning('Could not find the chart path %s to delete.', chart_path)