summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorZuul <zuul@review.openstack.org>2018-11-05 19:08:25 +0000
committerGerrit Code Review <review@openstack.org>2018-11-05 19:08:25 +0000
commitd35896537b998fe9771f115c6489a81a8ae77d14 (patch)
tree65061b366e2a3092a36a4193473a1b3ce5638cba
parent1986a935f6dcfac70e348a874ddf90011c064c60 (diff)
parentcbb8ed33e13f2d9a2e3742b994949999c5379297 (diff)
Merge "Add caching and cleanup of chart tarballs"
-rw-r--r--armada/handlers/armada.py54
-rw-r--r--armada/tests/unit/handlers/test_armada.py1
-rw-r--r--armada/tests/unit/utils/test_source.py23
-rw-r--r--armada/utils/source.py28
4 files changed, 41 insertions, 65 deletions
diff --git a/armada/handlers/armada.py b/armada/handlers/armada.py
index fa6afcd..a95401e 100644
--- a/armada/handlers/armada.py
+++ b/armada/handlers/armada.py
@@ -103,7 +103,7 @@ class Armada(object):
103 raise 103 raise
104 self.manifest = Manifest( 104 self.manifest = Manifest(
105 self.documents, target_manifest=target_manifest).get_manifest() 105 self.documents, target_manifest=target_manifest).get_manifest()
106 self.cloned_dirs = set() 106 self.chart_cache = {}
107 self.chart_deploy = ChartDeploy( 107 self.chart_deploy = ChartDeploy(
108 disable_update_pre, disable_update_post, self.dry_run, 108 disable_update_pre, disable_update_post, self.dry_run,
109 k8s_wait_attempts, k8s_wait_attempt_sleep, timeout, self.tiller) 109 k8s_wait_attempts, k8s_wait_attempt_sleep, timeout, self.tiller)
@@ -119,16 +119,12 @@ class Armada(object):
119 raise tiller_exceptions.TillerServicesUnavailableException() 119 raise tiller_exceptions.TillerServicesUnavailableException()
120 120
121 # Clone the chart sources 121 # Clone the chart sources
122 repos = {}
123 manifest_data = self.manifest.get(const.KEYWORD_ARMADA, {}) 122 manifest_data = self.manifest.get(const.KEYWORD_ARMADA, {})
124 for group in manifest_data.get(const.KEYWORD_GROUPS, []): 123 for group in manifest_data.get(const.KEYWORD_GROUPS, []):
125 for ch in group.get(const.KEYWORD_CHARTS, []): 124 for ch in group.get(const.KEYWORD_CHARTS, []):
126 self.tag_cloned_repo(ch, repos) 125 self.get_chart(ch)
127 126
128 for dep in ch.get('chart', {}).get('dependencies', []): 127 def get_chart(self, ch):
129 self.tag_cloned_repo(dep, repos)
130
131 def tag_cloned_repo(self, ch, repos):
132 chart = ch.get('chart', {}) 128 chart = ch.get('chart', {})
133 chart_source = chart.get('source', {}) 129 chart_source = chart.get('source', {})
134 location = chart_source.get('location') 130 location = chart_source.get('location')
@@ -138,25 +134,30 @@ class Armada(object):
138 if ct_type == 'local': 134 if ct_type == 'local':
139 chart['source_dir'] = (location, subpath) 135 chart['source_dir'] = (location, subpath)
140 elif ct_type == 'tar': 136 elif ct_type == 'tar':
141 LOG.info('Downloading tarball from: %s', location) 137 source_key = (ct_type, location)
142 138
143 if not CONF.certs: 139 if source_key not in self.chart_cache:
144 LOG.warn('Disabling server validation certs to extract charts') 140 LOG.info('Downloading tarball from: %s', location)
145 tarball_dir = source.get_tarball(location, verify=False)
146 else:
147 tarball_dir = source.get_tarball(location, verify=CONF.cert)
148 141
149 chart['source_dir'] = (tarball_dir, subpath) 142 if not CONF.certs:
143 LOG.warn(
144 'Disabling server validation certs to extract charts')
145 tarball_dir = source.get_tarball(location, verify=False)
146 else:
147 tarball_dir = source.get_tarball(
148 location, verify=CONF.cert)
149 self.chart_cache[source_key] = tarball_dir
150 chart['source_dir'] = (self.chart_cache.get(source_key), subpath)
150 elif ct_type == 'git': 151 elif ct_type == 'git':
151 reference = chart_source.get('reference', 'master') 152 reference = chart_source.get('reference', 'master')
152 repo_branch = (location, reference) 153 source_key = (ct_type, location, reference)
153 154
154 if repo_branch not in repos: 155 if source_key not in self.chart_cache:
155 auth_method = chart_source.get('auth_method') 156 auth_method = chart_source.get('auth_method')
156 proxy_server = chart_source.get('proxy_server') 157 proxy_server = chart_source.get('proxy_server')
157 158
158 logstr = 'Cloning repo: {} from branch: {}'.format( 159 logstr = 'Cloning repo: {} from branch: {}'.format(
159 *repo_branch) 160 location, reference)
160 if proxy_server: 161 if proxy_server:
161 logstr += ' proxy: {}'.format(proxy_server) 162 logstr += ' proxy: {}'.format(proxy_server)
162 if auth_method: 163 if auth_method:
@@ -164,19 +165,20 @@ class Armada(object):
164 LOG.info(logstr) 165 LOG.info(logstr)
165 166
166 repo_dir = source.git_clone( 167 repo_dir = source.git_clone(
167 *repo_branch, 168 location,
169 reference,
168 proxy_server=proxy_server, 170 proxy_server=proxy_server,
169 auth_method=auth_method) 171 auth_method=auth_method)
170 self.cloned_dirs.add(repo_dir)
171 172
172 repos[repo_branch] = repo_dir 173 self.chart_cache[source_key] = repo_dir
173 chart['source_dir'] = (repo_dir, subpath) 174 chart['source_dir'] = (self.chart_cache.get(source_key), subpath)
174 else:
175 chart['source_dir'] = (repos.get(repo_branch), subpath)
176 else: 175 else:
177 chart_name = chart.get('chart_name') 176 chart_name = chart.get('chart_name')
178 raise source_exceptions.ChartSourceException(ct_type, chart_name) 177 raise source_exceptions.ChartSourceException(ct_type, chart_name)
179 178
179 for dep in ch.get('chart', {}).get('dependencies', []):
180 self.get_chart(dep)
181
180 def sync(self): 182 def sync(self):
181 ''' 183 '''
182 Synchronize Helm with the Armada Config(s) 184 Synchronize Helm with the Armada Config(s)
@@ -293,9 +295,9 @@ class Armada(object):
293 LOG.info("Performing post-flight operations.") 295 LOG.info("Performing post-flight operations.")
294 296
295 # Delete temp dirs used for deployment 297 # Delete temp dirs used for deployment
296 for cloned_dir in self.cloned_dirs: 298 for chart_dir in self.chart_cache.values():
297 LOG.debug('Removing cloned temp directory: %s', cloned_dir) 299 LOG.debug('Removing temp chart directory: %s', chart_dir)
298 source.source_cleanup(cloned_dir) 300 source.source_cleanup(chart_dir)
299 301
300 def _chart_cleanup(self, prefix, charts, msg): 302 def _chart_cleanup(self, prefix, charts, msg):
301 LOG.info('Processing chart cleanup to remove unspecified releases.') 303 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 92eecdf..90bca08 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'),
152 ('/tmp/dummy/armada', 'chart_4')] 152 ('/tmp/dummy/armada', 'chart_4')]
153 153
154 154
155# TODO(seaneagan): Add unit tests with dependencies, including transitive.
155class ArmadaHandlerTestCase(base.ArmadaTestCase): 156class ArmadaHandlerTestCase(base.ArmadaTestCase):
156 157
157 def _test_pre_flight_ops(self, armada_obj): 158 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 21fd630..78682d7 100644
--- a/armada/tests/unit/utils/test_source.py
+++ b/armada/tests/unit/utils/test_source.py
@@ -15,7 +15,6 @@
15import os 15import os
16import shutil 16import shutil
17 17
18import fixtures
19import mock 18import mock
20import testtools 19import testtools
21 20
@@ -159,23 +158,6 @@ class GitTestCase(base.ArmadaTestCase):
159 @test_utils.attr(type=['negative']) 158 @test_utils.attr(type=['negative'])
160 @mock.patch.object(source, 'LOG') 159 @mock.patch.object(source, 'LOG')
161 @mock.patch('armada.utils.source.shutil') 160 @mock.patch('armada.utils.source.shutil')
162 def test_source_cleanup_fake_git_path(self, mock_shutil, mock_log):
163 # Verify that passing in a fake path does nothing but log a warning.
164 # Don't want anyone using the function to delete random directories.
165 fake_path = self.useFixture(fixtures.TempDir()).path
166 self.addCleanup(shutil.rmtree, fake_path)
167 source.source_cleanup(fake_path)
168
169 mock_shutil.rmtree.assert_not_called()
170 self.assertTrue(mock_log.warning.called)
171 actual_call = mock_log.warning.mock_calls[0][1][:2]
172 self.assertEqual(
173 ('%s is not a valid git repository. Details: %s', fake_path),
174 actual_call)
175
176 @test_utils.attr(type=['negative'])
177 @mock.patch.object(source, 'LOG')
178 @mock.patch('armada.utils.source.shutil')
179 @mock.patch('armada.utils.source.os.path') 161 @mock.patch('armada.utils.source.os.path')
180 def test_source_cleanup_missing_git_path(self, mock_path, mock_shutil, 162 def test_source_cleanup_missing_git_path(self, mock_path, mock_shutil,
181 mock_log): 163 mock_log):
@@ -187,9 +169,8 @@ class GitTestCase(base.ArmadaTestCase):
187 mock_shutil.rmtree.assert_not_called() 169 mock_shutil.rmtree.assert_not_called()
188 self.assertTrue(mock_log.warning.called) 170 self.assertTrue(mock_log.warning.called)
189 actual_call = mock_log.warning.mock_calls[0][1] 171 actual_call = mock_log.warning.mock_calls[0][1]
190 self.assertEqual( 172 self.assertEqual(('Could not find the chart path %s to delete.', path),
191 ('Could not delete the path %s. Is it a git repository?', path), 173 actual_call)
192 actual_call)
193 174
194 @testtools.skipUnless(base.is_connected(), 175 @testtools.skipUnless(base.is_connected(),
195 'git clone requires network connectivity.') 176 'git clone requires network connectivity.')
diff --git a/armada/utils/source.py b/armada/utils/source.py
index 727884c..510c3d3 100644
--- a/armada/utils/source.py
+++ b/armada/utils/source.py
@@ -153,27 +153,19 @@ def extract_tarball(tarball_path):
153 return temp_dir 153 return temp_dir
154 154
155 155
156def source_cleanup(git_path): 156def source_cleanup(chart_path):
157 '''Clean up the git repository that was created by ``git_clone`` above. 157 '''Clean up the chart path that was created above.
158 158
159 Removes the ``git_path`` repository and all associated files if they 159 Removes the ``chart_path`` and all associated files if they
160 exist. 160 exist.
161 161
162 :param str git_path: The git repository to delete. 162 :param str chart_path: The chart path to delete.
163 ''' 163 '''
164 if os.path.exists(git_path): 164 if os.path.exists(chart_path):
165 try: 165 try:
166 # Internally validates whether the path points to an actual repo. 166 shutil.rmtree(chart_path)
167 Repo(git_path) 167 except OSError as e:
168 except git_exc.InvalidGitRepositoryError as e: 168 LOG.warning('Could not delete the path %s. Details: %s',
169 LOG.warning('%s is not a valid git repository. Details: %s', 169 chart_path, e)
170 git_path, e)
171 else:
172 try:
173 shutil.rmtree(git_path)
174 except OSError as e:
175 LOG.warning('Could not delete the path %s. Details: %s',
176 git_path, e)
177 else: 170 else:
178 LOG.warning('Could not delete the path %s. Is it a git repository?', 171 LOG.warning('Could not find the chart path %s to delete.', chart_path)
179 git_path)