From 732af63051fac55b57b7720b295fc91aca4f99aa Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Fri, 2 Feb 2018 19:27:57 +0000 Subject: [PATCH] Add unit tests for main Armada handler This PS adds unit tests for the armada handler in armada.handlers.armada. These unit tests are among the most important as the Armada handler itself interacts with most every other part of the application. Note that the current unit tests are not only incorrect (they don't pass) but they skip unconditionally as well. So this PS is needed to correctly implement the intent behind the original unit tests herein. Change-Id: Iecb3e540e1d52eb5a25d9f6825b3d0f5339ede2a --- armada/handlers/armada.py | 5 +- armada/tests/unit/api/base.py | 7 +- armada/tests/unit/base.py | 3 + armada/tests/unit/handlers/test_armada.py | 316 +++++++++++++--------- armada/tests/unit/utils/test_source.py | 7 +- 5 files changed, 191 insertions(+), 147 deletions(-) diff --git a/armada/handlers/armada.py b/armada/handlers/armada.py index afc83c02..d7b828b8 100644 --- a/armada/handlers/armada.py +++ b/armada/handlers/armada.py @@ -91,8 +91,8 @@ class Armada(object): tiller_namespace=tiller_namespace) self.values = values self.documents = file - self.config = None self.target_manifest = target_manifest + self.config = self.get_armada_manifest() def get_armada_manifest(self): return Manifest( @@ -126,9 +126,6 @@ class Armada(object): self.documents, overrides=self.overrides, values=self.values).update_manifests() - # Get config and validate - self.config = self.get_armada_manifest() - if not lint.validate_armada_object(self.config): raise lint_exceptions.InvalidArmadaObjectException() diff --git a/armada/tests/unit/api/base.py b/armada/tests/unit/api/base.py index 5610f60a..df4b9356 100644 --- a/armada/tests/unit/api/base.py +++ b/armada/tests/unit/api/base.py @@ -12,12 +12,12 @@ # See the License for the specific language governing permissions and # limitations under the License. -import importlib import os from falcon import testing as falcon_testing import mock +from armada.api import server import armada.conf from armada.tests.unit import base as test_base from armada.tests.unit import fixtures @@ -40,11 +40,6 @@ class BaseControllerTest(test_base.ArmadaTestCase): mock_get_config_files.return_value = [ os.path.join(sample_conf_dir, x) for x in sample_conf_files ] - # FIXME(fmontei): Workaround for the fact that `armada.api` always - # calls `create()` via `api = create()` at the bottom of the module - # which invokes oslo.conf functionality that has yet to be set up - # properly in this module. - server = importlib.import_module('armada.api.server') self.app = falcon_testing.TestClient( server.create(enable_middleware=False)) self.policy = self.useFixture(fixtures.RealPolicyFixture()) diff --git a/armada/tests/unit/base.py b/armada/tests/unit/base.py index bc553301..41a15013 100644 --- a/armada/tests/unit/base.py +++ b/armada/tests/unit/base.py @@ -20,6 +20,8 @@ import mock from oslo_config import cfg import testtools +from armada.conf import default + CONF = cfg.CONF @@ -28,6 +30,7 @@ class ArmadaTestCase(testtools.TestCase): def setUp(self): super(ArmadaTestCase, self).setUp() self.useFixture(fixtures.FakeLogger('armada')) + default.register_opts(CONF) def override_config(self, name, override, group=None): CONF.set_override(name, override, group) diff --git a/armada/tests/unit/handlers/test_armada.py b/armada/tests/unit/handlers/test_armada.py index 272e0b5a..54863b1a 100644 --- a/armada/tests/unit/handlers/test_armada.py +++ b/armada/tests/unit/handlers/test_armada.py @@ -1,179 +1,227 @@ +# Copyright 2017 AT&T Intellectual Property. All other rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + import mock -import unittest import yaml -from armada.handlers.armada import Armada -from armada.handlers.manifest import Manifest +from armada.handlers import armada +from armada.tests.unit import base -class ArmadaTestCase(unittest.TestCase): - test_yaml = """ - --- - schema: armada/Manifest/v1 - metadata: - schema: metadata/Document/v1 - name: example-manifest - data: - release_prefix: armada - chart_groups: - - example-group - --- - schema: armada/ChartGroup/v1 - metadata: - schema: metadata/Document/v1 - name: example-group - data: - description: this is a test - sequenced: False - chart_group: - - example-chart-1 - - example-chart-2 - --- - schema: armada/Chart/v1 - metadata: - schema: metadata/Document/v1 - name: example-chart-2 - data: - name: test_chart_2 - release_name: test_chart_2 - namespace: test - values: {} - source: - type: local - location: /tmp/dummy/armada - subpath: chart_2 - reference: null - dependencies: [] - timeout: 5 - --- - schema: armada/Chart/v1 - metadata: - schema: metadata/Document/v1 - name: example-chart-1 - data: - name: test_chart_1 - release_name: test_chart_1 - namespace: test - values: {} - source: - type: git - location: git://github.com/dummy/armada - subpath: chart_1 - reference: master - dependencies: [] - timeout: 50 - """ +TEST_YAML = """ +--- +schema: armada/Manifest/v1 +metadata: + schema: metadata/Document/v1 + name: example-manifest +data: + release_prefix: armada + chart_groups: + - example-group +--- +schema: armada/ChartGroup/v1 +metadata: + schema: metadata/Document/v1 + name: example-group +data: + description: this is a test + sequenced: False + chart_group: + - example-chart-1 + - example-chart-2 +--- +schema: armada/Chart/v1 +metadata: + schema: metadata/Document/v1 + name: example-chart-2 +data: + chart_name: test_chart_2 + release: test_chart_2 + namespace: test + values: {} + source: + type: local + location: /tmp/dummy/armada + subpath: chart_2 + reference: null + dependencies: [] + timeout: 5 +--- +schema: armada/Chart/v1 +metadata: + schema: metadata/Document/v1 + name: example-chart-1 +data: + chart_name: test_chart_1 + release: test_chart_1 + namespace: test + values: {} + source: + type: git + location: git://github.com/dummy/armada + subpath: chart_1 + reference: master + dependencies: [] + timeout: 50 +""" - @unittest.skip('temp') - @mock.patch('armada.handlers.armada.git') - @mock.patch('armada.handlers.armada.lint') +CHART_SOURCES = [('git://github.com/dummy/armada', 'chart_1'), + ('/tmp/dummy/armada', 'chart_2')] + + +class ArmadaHandlerTestCase(base.ArmadaTestCase): + + def _test_pre_flight_ops(self, armada_obj): + armada_obj.pre_flight_ops() + + expected_config = { + 'armada': { + 'release_prefix': 'armada', + 'chart_groups': [ + { + 'chart_group': [ + { + 'chart': { + 'dependencies': [], + 'chart_name': 'test_chart_1', + 'namespace': 'test', + 'release': 'test_chart_1', + 'source': { + 'location': ( + 'git://github.com/dummy/armada'), + 'reference': 'master', + 'subpath': 'chart_1', + 'type': 'git' + }, + 'source_dir': CHART_SOURCES[0], + 'timeout': 50, + 'values': {} + } + }, + { + 'chart': { + 'dependencies': [], + 'chart_name': 'test_chart_2', + 'namespace': 'test', + 'release': 'test_chart_2', + 'source': { + 'location': '/tmp/dummy/armada', + 'reference': None, + 'subpath': 'chart_2', + 'type': 'local' + }, + 'source_dir': CHART_SOURCES[1], + 'timeout': 5, + 'values': {} + } + } + ], + 'description': 'this is a test', + 'name': 'example-group', + 'sequenced': False + } + ] + } + } + + self.assertTrue(hasattr(armada_obj, 'config')) + self.assertIsInstance(armada_obj.config, dict) + self.assertIn('armada', armada_obj.config) + self.assertEqual(expected_config, armada_obj.config) + + @mock.patch.object(armada, 'source') @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.documents = yaml.safe_load_all(self.test_yaml) - armada.config = Manifest(armada.documents).get_manifest() + def test_pre_flight_ops(self, mock_tiller, mock_source): + """Test pre-flight checks and operations.""" + yaml_documents = list(yaml.safe_load_all(TEST_YAML)) + armada_obj = armada.Armada(yaml_documents) - CHART_SOURCES = [('git://github.com/dummy/armada', 'chart_1'), - ('/tmp/dummy/armada', 'chart_2')] - - # mock methods called by pre_flight_ops() + # 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] + mock_source.git_clone.return_value = CHART_SOURCES[0][0] - armada.pre_flight_ops() + self._test_pre_flight_ops(armada_obj) - 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_tiller.assert_called_once_with(tiller_host=None, + tiller_namespace='kube-system', + tiller_port=44134) + mock_source.git_clone.assert_called_once_with( + 'git://github.com/dummy/armada', 'master', None) - @unittest.skip('temp') - @mock.patch('armada.handlers.armada.git') - @mock.patch('armada.handlers.armada.lint') + @mock.patch.object(armada, 'source') @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 - tmp_doc = yaml.safe_load_all(self.test_yaml) - armada.config = Manifest(tmp_doc).get_manifest() + def test_post_flight_ops(self, mock_tiller, mock_source): + """Test post-flight operations.""" + yaml_documents = list(yaml.safe_load_all(TEST_YAML)) + armada_obj = armada.Armada(yaml_documents) - CHART_SOURCES = [('git://github.com/dummy/armada', 'chart_1'), - ('/tmp/dummy/armada', 'chart_2')] - - # mock methods called by pre_flight_ops() + # 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_source.git_clone.return_value = CHART_SOURCES[0][0] - armada.post_flight_ops() + self._test_pre_flight_ops(armada_obj) - for group in yaml.load(self.test_yaml).get('armada').get('charts'): + armada_obj.post_flight_ops() + + for group in armada_obj.config['armada']['chart_groups']: 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_source.source_cleanup.assert_called_with( + CHART_SOURCES[counter][0]) - @unittest.skip('temp') - @mock.patch.object(Armada, 'post_flight_ops') - @mock.patch.object(Armada, 'pre_flight_ops') + @mock.patch.object(armada.Armada, 'post_flight_ops') + @mock.patch.object(armada.Armada, 'pre_flight_ops') @mock.patch('armada.handlers.armada.ChartBuilder') @mock.patch('armada.handlers.armada.Tiller') 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('', wait=True, timeout=1000) - armada.tiller = mock_tiller - tmp_doc = yaml.safe_load_all(self.test_yaml) - armada.config = Manifest(tmp_doc).get_manifest() + # Instantiate Armada object. + yaml_documents = list(yaml.safe_load_all(TEST_YAML)) + armada_obj = armada.Armada(yaml_documents) - charts = armada.config['armada']['charts'][0]['chart_group'] + charts = armada_obj.config['armada']['chart_groups'][0]['chart_group'] chart_1 = charts[0]['chart'] chart_2 = charts[1]['chart'] - # mock irrelevant methods called by armada.sync() + # Mock irrelevant methods called by `armada.sync()`. mock_tiller.list_charts.return_value = [] mock_chartbuilder.get_source_path.return_value = None mock_chartbuilder.get_helm_chart.return_value = None - armada.sync() + armada_obj.sync() - # check params that should be passed to tiller.install_release() + # Check params that should be passed to `tiller.install_release()`. method_calls = [ mock.call( mock_chartbuilder().get_helm_chart(), - "{}-{}".format(armada.config['armada']['release_prefix'], - chart_1['release_name']), + "{}-{}".format(armada_obj.config['armada']['release_prefix'], + chart_1['release']), chart_1['namespace'], - dry_run=armada.dry_run, + dry_run=armada_obj.dry_run, values=yaml.safe_dump(chart_1['values']), - wait=armada.wait, - timeout=1000), + wait=armada_obj.wait, + timeout=armada_obj.timeout), mock.call( mock_chartbuilder().get_helm_chart(), - "{}-{}".format(armada.config['armada']['release_prefix'], - chart_2['release_name']), + "{}-{}".format(armada_obj.config['armada']['release_prefix'], + chart_2['release']), chart_2['namespace'], - dry_run=armada.dry_run, + dry_run=armada_obj.dry_run, values=yaml.safe_dump(chart_2['values']), - wait=armada.wait, - timeout=1000) + wait=armada_obj.wait, + timeout=armada_obj.timeout) ] - mock_tiller.install_release.assert_has_calls(method_calls) - - @unittest.skip('skipping update') - def test_upgrade(self): - '''Test upgrade functionality from the sync() method''' - # TODO + mock_tiller.return_value.install_release.assert_has_calls(method_calls) diff --git a/armada/tests/unit/utils/test_source.py b/armada/tests/unit/utils/test_source.py index dd8461dd..9e7fb59d 100644 --- a/armada/tests/unit/utils/test_source.py +++ b/armada/tests/unit/utils/test_source.py @@ -103,9 +103,10 @@ class GitTestCase(testtools.TestCase): def test_git_clone_fake_proxy(self): url = 'http://github.com/att-comdev/armada' - self.assertRaises(source_exceptions.GitProxyException, - source.git_clone, url, - proxy_server='http://not.a.proxy:8080') + self.assertRaises( + source_exceptions.GitProxyException, + source.git_clone, url, + proxy_server='http://not.a.proxy.that.works.and.never.will:8080') @mock.patch('armada.utils.source.tempfile') @mock.patch('armada.utils.source.requests')