diff --git a/armada/cli/apply.py b/armada/cli/apply.py index 348eadb9..97c49a8a 100644 --- a/armada/cli/apply.py +++ b/armada/cli/apply.py @@ -22,7 +22,6 @@ def applyCharts(args): args.disable_update_pre, args.disable_update_post, args.enable_chart_cleanup, - args.skip_pre_flight, args.dry_run, args.wait, args.timeout, @@ -36,8 +35,6 @@ class ApplyChartsCommand(cmd.Command): help='Armada yaml file') parser.add_argument('--dry-run', action='store_true', default=False, help='Run charts with dry run') - parser.add_argument('--skip-pre-flight', action='store_true', - default=False, help='Skip Pre Flight') parser.add_argument('--debug-logging', action='store_true', default=False, help='Show debug logs') parser.add_argument('--disable-update-pre', action='store_true', diff --git a/armada/handlers/armada.py b/armada/handlers/armada.py index 8d2c8dcc..b9100de9 100644 --- a/armada/handlers/armada.py +++ b/armada/handlers/armada.py @@ -43,7 +43,6 @@ class Armada(object): disable_update_pre=False, disable_update_post=False, enable_chart_cleanup=False, - skip_pre_flight=False, dry_run=False, wait=False, timeout=DEFAULT_TIMEOUT, @@ -55,7 +54,6 @@ class Armada(object): self.disable_update_pre = disable_update_pre self.disable_update_post = disable_update_post self.enable_chart_cleanup = enable_chart_cleanup - self.skip_pre_flight = skip_pre_flight self.dry_run = dry_run self.wait = wait self.timeout = timeout @@ -75,20 +73,55 @@ class Armada(object): if chart_name == name: return chart, values - def pre_flight_checks(self): + def pre_flight_ops(self): + ''' + Perform a series of checks and operations to ensure proper deployment + ''' + # Ensure tiller is available and yaml is valid + if not self.tiller.tiller_status(): + raise Exception("Tiller Services is not Available") + if not lint.valid_manifest(self.config): + raise Exception("Invalid Armada Manifest") + + # Clone the chart sources + # + # We only support a git source type right now, which can also + # handle git:// local paths as well + repos = {} for group in self.config.get('armada').get('charts'): for ch in group.get('chart_group'): location = ch.get('chart').get('source').get('location') ct_type = ch.get('chart').get('source').get('type') + reference = ch.get('chart').get('source').get('reference') + subpath = ch.get('chart').get('source').get('subpath') - if ct_type == 'git' and not git.check_available_repo(location): - raise ValueError(str("Invalid Url Path: " + location)) + if ct_type == 'local': + ch.get('chart')['source_dir'] = (location, subpath) + elif ct_type == 'git': + if location not in repos.keys(): + try: + LOG.info('Cloning repo: %s', location) + repo_dir = git.git_clone(location, reference) + except Exception as e: + raise ValueError(e) + repos[location] = repo_dir + ch.get('chart')['source_dir'] = (repo_dir, subpath) + else: + ch.get('chart')['source_dir'] = (repos.get(location), + subpath) + else: + raise Exception("Unknown source type %s for chart %s", + ct_type, ch.get('chart').get('name')) - if not self.tiller.tiller_status(): - raise Exception("Tiller Services is not Available") - - if not lint.valid_manifest(self.config): - raise Exception("Invalid Armada Manifest") + def post_flight_ops(self): + ''' + Operations to run after deployment process has terminated + ''' + # Delete git repos cloned for deployment + for group in self.config.get('armada').get('charts'): + for ch in group.get('chart_group'): + if ch.get('chart').get('source').get('type') == 'git': + git.source_cleanup(ch.get('chart').get('source_dir')[0]) def sync(self): ''' @@ -98,11 +131,8 @@ class Armada(object): # TODO: (gardlt) we need to break up this func into # a more cleaner format # extract known charts on tiller right now - if not self.skip_pre_flight: - LOG.info("Performing Pre-Flight Checks") - self.pre_flight_checks() - else: - LOG.info("Skipping Pre-Flight Checks") + LOG.info("Performing Pre-Flight Operations") + self.pre_flight_ops() known_releases = self.tiller.list_charts() prefix = self.config.get('armada').get('release_prefix') @@ -209,7 +239,8 @@ class Armada(object): LOG.debug("Cleaning up chart source in %s", chartbuilder.source_directory) - chartbuilder.source_cleanup() + LOG.info("Performing Post-Flight Operations") + self.post_flight_ops() if self.enable_chart_cleanup: self.tiller.chart_cleanup(prefix, self.config['armada']['charts']) diff --git a/armada/handlers/chartbuilder.py b/armada/handlers/chartbuilder.py index 817afed5..af1d1e5a 100644 --- a/armada/handlers/chartbuilder.py +++ b/armada/handlers/chartbuilder.py @@ -21,8 +21,6 @@ from hapi.chart.metadata_pb2 import Metadata from hapi.chart.config_pb2 import Config from supermutes.dot import dotify -from ..utils.git import git_clone, source_cleanup - from oslo_config import cfg from oslo_log import log as logging @@ -62,48 +60,14 @@ class ChartBuilder(object): self.chart = chart # extract, pull, whatever the chart from its source - self.source_directory = self.source_clone() + self.source_directory = self.get_source_path() - def source_clone(self): + def get_source_path(self): ''' - Clone the charts source - - We only support a git source type right now, which can also - handle git:// local paths as well + Return the joined path of the source directory and subpath ''' - - if self.chart.source.type == 'git': - if self.parent: - LOG.info("Cloning %s/%s as dependency for %s", - self.chart.source.location, - self.chart.source.subpath, - self.parent) - else: - LOG.info("Cloning %s/%s for release %s", - self.chart.source.location, - self.chart.source.subpath, - self.chart.release_name) - - self._source_tmp_dir = git_clone(self.chart.source.location, - self.chart.source.reference) - - return os.path.join(self._source_tmp_dir, - self.chart.source.subpath) - if self.chart.source.type == 'local': - return os.path.join(self.chart.source.location, - self.chart.source.subpath) - - else: - LOG.exception("Unknown source type %s for chart %s", - self.chart.name, - self.chart.source.type) - - def source_cleanup(self): - ''' - Cleanup source - ''' - if self.chart.source.type == 'git': - source_cleanup(self._source_tmp_dir) + return os.path.join(self.chart.source_dir[0], + self.chart.source_dir[1]) def get_metadata(self): ''' diff --git a/armada/tests/unit/handlers/test_armada.py b/armada/tests/unit/handlers/test_armada.py index 215a62c8..e9d4732d 100644 --- a/armada/tests/unit/handlers/test_armada.py +++ b/armada/tests/unit/handlers/test_armada.py @@ -22,10 +22,10 @@ class ArmadaTestCase(unittest.TestCase): namespace: test values: {} source: - type: null - location: null - subpath: null - reference: null + type: git + location: git://github.com/dummy/armada + subpath: chart_1 + reference: master dependencies: [] timeout: 50 @@ -35,22 +35,78 @@ class ArmadaTestCase(unittest.TestCase): namespace: test values: {} source: - type: null - location: null - subpath: null + type: local + location: /tmp/dummy/armada + subpath: chart_2 reference: null dependencies: [] timeout: 5 """ + @mock.patch('armada.handlers.armada.git') + @mock.patch('armada.handlers.armada.lint') + @mock.patch('armada.handlers.armada.Tiller') + def test_pre_flight_ops(self, mock_tiller, mock_lint, mock_git): + '''Test pre-flight checks and operations''' + armada = Armada('') + armada.tiller = mock_tiller + armada.config = yaml.load(self.test_yaml) + + CHART_SOURCES = [('git://github.com/dummy/armada', 'chart_1'), + ('/tmp/dummy/armada', 'chart_2')] + + # mock methods called by pre_flight_ops() + mock_tiller.tiller_status.return_value = True + mock_lint.valid_manifest.return_value = True + mock_git.git_clone.return_value = CHART_SOURCES[0][0] + + armada.pre_flight_ops() + + mock_git.git_clone.assert_called_once_with(CHART_SOURCES[0][0], + 'master') + for group in armada.config.get('armada').get('charts'): + for counter, chart in enumerate(group.get('chart_group')): + self.assertEqual(chart.get('chart').get('source_dir')[0], + CHART_SOURCES[counter][0]) + self.assertEqual(chart.get('chart').get('source_dir')[1], + CHART_SOURCES[counter][1]) + + @mock.patch('armada.handlers.armada.git') + @mock.patch('armada.handlers.armada.lint') + @mock.patch('armada.handlers.armada.Tiller') + def test_post_flight_ops(self, mock_tiller, mock_lint, mock_git): + '''Test post-flight operations''' + armada = Armada('') + armada.tiller = mock_tiller + armada.config = yaml.load(self.test_yaml) + + CHART_SOURCES = [('git://github.com/dummy/armada', 'chart_1'), + ('/tmp/dummy/armada', 'chart_2')] + + # mock methods called by pre_flight_ops() + mock_tiller.tiller_status.return_value = True + mock_lint.valid_manifest.return_value = True + mock_git.git_clone.return_value = CHART_SOURCES[0][0] + armada.pre_flight_ops() + + armada.post_flight_ops() + + for group in yaml.load(self.test_yaml).get('armada').get('charts'): + for counter, chart in enumerate(group.get('chart_group')): + if chart.get('chart').get('source').get('type') == 'git': + mock_git.source_cleanup \ + .assert_called_with(CHART_SOURCES[counter][0]) + + @mock.patch.object(Armada, 'post_flight_ops') + @mock.patch.object(Armada, 'pre_flight_ops') @mock.patch('armada.handlers.armada.ChartBuilder') @mock.patch('armada.handlers.armada.Tiller') - def test_install(self, mock_tiller, mock_chartbuilder): + def test_install(self, mock_tiller, mock_chartbuilder, + mock_pre_flight, mock_post_flight): '''Test install functionality from the sync() method''' # instantiate Armada and Tiller objects armada = Armada('', - skip_pre_flight=True, wait=True, timeout=1000) armada.tiller = mock_tiller @@ -62,9 +118,8 @@ class ArmadaTestCase(unittest.TestCase): # mock irrelevant methods called by armada.sync() mock_tiller.list_charts.return_value = [] - mock_chartbuilder.source_clone.return_value = None + mock_chartbuilder.get_source_path.return_value = None mock_chartbuilder.get_helm_chart.return_value = None - mock_chartbuilder.source_cleanup.return_value = None armada.sync() diff --git a/armada/tests/unit/handlers/test_chartbuilder.py b/armada/tests/unit/handlers/test_chartbuilder.py index 964ab5b6..5df837f9 100644 --- a/armada/tests/unit/handlers/test_chartbuilder.py +++ b/armada/tests/unit/handlers/test_chartbuilder.py @@ -14,8 +14,6 @@ import unittest import mock -from supermutes.dot import dotify - # Required Oslo configuration setup from armada.conf import default default.register_opts() @@ -74,16 +72,6 @@ class ChartBuilderTestCase(unittest.TestCase): memory: 128Mi """ - def test_chartbuilder_source_clone(self): - - chart = dotify(self.chart_stream) - ChartBuilder.source_clone = mock.Mock(return_value='path') - chartbuilder = ChartBuilder(chart) - resp = getattr(chartbuilder, 'source_directory', None) - - self.assertIsNotNone(resp) - self.assertIsInstance(resp, basestring) - @unittest.skip("we are having wierd scenario") @mock.patch('armada.handlers.chartbuilder.dotify') @mock.patch('armada.handlers.chartbuilder.os') diff --git a/armada/tests/unit/utils/test_git.py b/armada/tests/unit/utils/test_git.py index b55744cd..717b2312 100644 --- a/armada/tests/unit/utils/test_git.py +++ b/armada/tests/unit/utils/test_git.py @@ -21,7 +21,7 @@ class GitTestCase(unittest.TestCase): @mock.patch('armada.utils.git.tempfile') @mock.patch('armada.utils.git.pygit2') - def test_git_clone_repo_pass(self, mock_pygit, mock_temp): + def test_git_clone_good_url(self, mock_pygit, mock_temp): mock_temp.mkdtemp.return_value = '/tmp/armada' mock_pygit.clone_repository.return_value = "Repository" url = 'http://github.com/att-comdev/armada' @@ -37,17 +37,24 @@ class GitTestCase(unittest.TestCase): def test_git_clone_bad_url(self): url = 'http://github.com/dummy/armada' - dir = git.git_clone(url) - self.assertFalse(dir) + with self.assertRaises(Exception): + git.git_clone(url) - def test_check_available_repo_pass(self): - url = 'http://github.com/att-comdev/armada' - resp = git.check_available_repo(url) + @mock.patch('armada.utils.git.shutil') + @mock.patch('armada.utils.git.path') + def test_source_cleanup(self, mock_path, mock_shutil): + mock_path.exists.return_value = True + path = 'armada' - self.assertTrue(resp) + git.source_cleanup(path) + mock_shutil.rmtree.assert_called_with(path) - def test_check_available_repo_dummy_url(self): - url = 'http://github.com/dummy/armada' - resp = git.check_available_repo(url) - self.assertFalse(resp) + @mock.patch('armada.utils.git.shutil') + @mock.patch('armada.utils.git.path') + def test_source_cleanup_bad_path(self, mock_path, mock_shutil): + mock_path.exists.return_value = False + path = 'armada' + + git.source_cleanup(path) + mock_shutil.rmtree.assert_not_called() diff --git a/armada/utils/git.py b/armada/utils/git.py index a7e66a6c..4ba83392 100644 --- a/armada/utils/git.py +++ b/armada/utils/git.py @@ -1,16 +1,14 @@ import pygit2 import tempfile import shutil -import requests - -HTTP_OK = 200 +from os import path def git_clone(repo_url, branch='master'): ''' clones repo to a /tmp/ dir ''' - if repo_url == '' or not check_available_repo(repo_url): + if repo_url == '': return False _tmp_dir = tempfile.mkdtemp(prefix='armada', dir='/tmp') @@ -22,15 +20,5 @@ def source_cleanup(target_dir): ''' Clean up source ''' - shutil.rmtree(target_dir) - - -def check_available_repo(repo_url): - try: - resp = requests.get(repo_url.replace('git:', 'http:')) - if resp.status_code == HTTP_OK: - return True - - return False - except Exception: - return False + if path.exists(target_dir): + shutil.rmtree(target_dir)