From f27ab29db7f96958f983c27dedfce591edf85994 Mon Sep 17 00:00:00 2001 From: Felipe Monteiro Date: Mon, 2 Apr 2018 19:40:14 +0100 Subject: [PATCH] [test] Increase armada.handlers.armada test coverage This is a multi-part PS because these patches may include small fix-ups to the code base itself, so the intention is to keep the patches small and easily reversible. This patchset introduces the following: * html coverage report (execute tox -e cover then open index.html under htmlcov folder which is created by py.test) * adds additional unit tests for pre_flight_ops * adds more robust assertions for those tests Change-Id: Ib29d7d8d0c3b686a36c5a87fc46d4594bb1838a6 --- armada/tests/unit/base.py | 16 +++ armada/tests/unit/handlers/test_armada.py | 163 +++++++++++++++++++++- armada/tests/unit/utils/test_source.py | 33 ++--- armada/tests/unit/utils/test_validate.py | 5 + 4 files changed, 191 insertions(+), 26 deletions(-) diff --git a/armada/tests/unit/base.py b/armada/tests/unit/base.py index 41a15013..3a4c8d3e 100644 --- a/armada/tests/unit/base.py +++ b/armada/tests/unit/base.py @@ -15,6 +15,8 @@ from __future__ import absolute_import +import socket + import fixtures import mock from oslo_config import cfg @@ -25,6 +27,20 @@ from armada.conf import default CONF = cfg.CONF +def is_connected(): + """Verifies whether network connectivity is up. + + :returns: True if connected else False. + """ + try: + host = socket.gethostbyname("www.github.com") + socket.create_connection((host, 80), 2) + return True + except (socket.error, socket.herror, socket.timeout): + pass + return False + + class ArmadaTestCase(testtools.TestCase): def setUp(self): diff --git a/armada/tests/unit/handlers/test_armada.py b/armada/tests/unit/handlers/test_armada.py index e2117d70..8b82797a 100644 --- a/armada/tests/unit/handlers/test_armada.py +++ b/armada/tests/unit/handlers/test_armada.py @@ -15,8 +15,10 @@ import mock import yaml +from armada import const from armada.handlers import armada from armada.tests.unit import base +from armada.utils.release import release_prefix TEST_YAML = """ @@ -152,7 +154,8 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): armada_obj = armada.Armada(yaml_documents) # Mock methods called by `pre_flight_ops()`. - mock_tiller.tiller_status.return_value = True + m_tiller = mock_tiller.return_value + m_tiller.tiller_status.return_value = True mock_source.git_clone.return_value = CHART_SOURCES[0][0] self._test_pre_flight_ops(armada_obj) @@ -165,6 +168,43 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): 'git://github.com/dummy/armada', 'master', auth_method=None, proxy_server=None) + @mock.patch.object(armada, 'source') + @mock.patch('armada.handlers.armada.Tiller') + def test_pre_flight_ops_with_failed_releases(self, mock_tiller, + mock_source): + """Test pre-flight functions uninstalls failed Tiller releases.""" + yaml_documents = list(yaml.safe_load_all(TEST_YAML)) + armada_obj = armada.Armada(yaml_documents) + + # Mock methods called by `pre_flight_ops()`. + m_tiller = mock_tiller.return_value + m_tiller.tiller_status.return_value = True + mock_source.git_clone.return_value = CHART_SOURCES[0][0] + + # Only the first two releases failed and should be uninstalled. Armada + # looks at index [4] for each release to determine the status. + m_tiller.list_charts.return_value = [ + ['armada-test_chart_1', None, None, None, const.STATUS_FAILED], + ['armada-test_chart_2', None, None, None, const.STATUS_FAILED], + [None, None, None, None, const.STATUS_DEPLOYED] + ] + + self._test_pre_flight_ops(armada_obj) + + # Assert both failed releases were uninstalled. + m_tiller.uninstall_release.assert_has_calls([ + mock.call('armada-test_chart_1'), + mock.call('armada-test_chart_2') + ]) + + mock_tiller.assert_called_once_with(tiller_host=None, + tiller_namespace='kube-system', + tiller_port=44134, + dry_run=False) + mock_source.git_clone.assert_called_once_with( + 'git://github.com/dummy/armada', 'master', auth_method=None, + proxy_server=None) + @mock.patch.object(armada, 'source') @mock.patch('armada.handlers.armada.Tiller') def test_post_flight_ops(self, mock_tiller, mock_source): @@ -173,7 +213,8 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): armada_obj = armada.Armada(yaml_documents) # Mock methods called by `pre_flight_ops()`. - mock_tiller.tiller_status.return_value = True + m_tiller = mock_tiller.return_value + m_tiller.tiller_status.return_value = True mock_source.git_clone.return_value = CHART_SOURCES[0][0] self._test_pre_flight_ops(armada_obj) @@ -186,6 +227,124 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase): mock_source.source_cleanup.assert_called_with( CHART_SOURCES[counter][0]) + def _test_sync(self, known_releases): + """Test install functionality from the sync() method.""" + + @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 _do_test(mock_tiller, mock_chartbuilder, mock_pre_flight, + mock_post_flight): + # Instantiate Armada object. + yaml_documents = list(yaml.safe_load_all(TEST_YAML)) + armada_obj = armada.Armada(yaml_documents) + armada_obj.show_diff = mock.Mock() + + charts = armada_obj.manifest['armada']['chart_groups'][0][ + 'chart_group'] + + m_tiller = mock_tiller.return_value + m_tiller.list_charts.return_value = known_releases + + # Stub out irrelevant methods called by `armada.sync()`. + mock_chartbuilder.get_source_path.return_value = None + mock_chartbuilder.get_helm_chart.return_value = None + + armada_obj.sync() + + expected_install_release_calls = [] + expected_update_release_calls = [] + + for c in charts: + chart = c['chart'] + chart_name = chart['chart_name'] + prefix = armada_obj.manifest['armada']['release_prefix'] + release = release_prefix(prefix, chart_name) + # Simplified check because the actual code uses logical-or's + # multiple conditions, so this is enough. + this_chart_should_wait = chart['wait']['timeout'] > 0 + + if release not in [x[0] for x in known_releases]: + expected_install_release_calls.append( + mock.call( + mock_chartbuilder().get_helm_chart(), + "{}-{}".format(armada_obj.manifest['armada'][ + 'release_prefix'], + chart['release']), + chart['namespace'], + values=yaml.safe_dump(chart['values']), + wait=this_chart_should_wait, + timeout=chart['wait']['timeout'] + ) + ) + else: + expected_update_release_calls.append( + mock.call( + mock_chartbuilder().get_helm_chart(), + "{}-{}".format(armada_obj.manifest['armada'][ + 'release_prefix'], + chart['release']), + chart['namespace'], + pre_actions={}, + post_actions={}, + disable_hooks=False, + values=yaml.safe_dump(chart['values']), + wait=this_chart_should_wait, + timeout=chart['wait']['timeout'] + ) + ) + + # Verify that at least 1 release is either installed or updated. + self.assertTrue( + len(expected_install_release_calls) >= 1 or + len(expected_update_release_calls) >= 1) + # Verify that the expected number of non-deployed releases are + # installed with expected arguments. + self.assertEqual(len(expected_install_release_calls), + m_tiller.install_release.call_count) + m_tiller.install_release.assert_has_calls( + expected_install_release_calls) + # Verify that the expected number of deployed releases are + # updated with expected arguments. + self.assertEqual(len(expected_update_release_calls), + m_tiller.update_release.call_count) + m_tiller.update_release.assert_has_calls( + expected_update_release_calls) + + _do_test() + + def _get_chart_by_name(self, name): + name = name.split('armada-')[-1] + yaml_documents = list(yaml.safe_load_all(TEST_YAML)) + return [c for c in yaml_documents + if c['data'].get('chart_name') == name][0] + + def test_armada_sync_with_no_deployed_releases(self): + known_releases = [] + self._test_sync(known_releases) + + def test_armada_sync_with_one_deployed_release(self): + c1 = 'armada-test_chart_1' + + known_releases = [ + [c1, None, self._get_chart_by_name(c1), None, + const.STATUS_DEPLOYED] + ] + self._test_sync(known_releases) + + def test_armada_sync_with_both_deployed_releases(self): + c1 = 'armada-test_chart_1' + c2 = 'armada-test_chart_2' + + known_releases = [ + [c1, None, self._get_chart_by_name(c1), None, + const.STATUS_DEPLOYED], + [c2, None, self._get_chart_by_name(c2), None, + const.STATUS_DEPLOYED] + ] + self._test_sync(known_releases) + @mock.patch.object(armada.Armada, 'post_flight_ops') @mock.patch.object(armada.Armada, 'pre_flight_ops') @mock.patch('armada.handlers.armada.ChartBuilder') diff --git a/armada/tests/unit/utils/test_source.py b/armada/tests/unit/utils/test_source.py index 7c7601e9..3813dcb0 100644 --- a/armada/tests/unit/utils/test_source.py +++ b/armada/tests/unit/utils/test_source.py @@ -13,7 +13,6 @@ # limitations under the License. import os -import socket import shutil import fixtures @@ -26,20 +25,6 @@ from armada.tests import test_utils from armada.utils import source -def is_connected(): - """Verifies whether network connectivity is up. - - :returns: True if connected else False. - """ - try: - host = socket.gethostbyname("www.github.com") - socket.create_connection((host, 80), 2) - return True - except (socket.error, socket.herror, socket.timeout): - pass - return False - - class GitTestCase(base.ArmadaTestCase): def _validate_git_clone(self, repo_dir, expected_ref=None): @@ -55,14 +40,14 @@ class GitTestCase(base.ArmadaTestCase): self.assertIn(expected_ref, git_file.read()) @testtools.skipUnless( - is_connected(), 'git clone requires network connectivity.') + base.is_connected(), 'git clone requires network connectivity.') def test_git_clone_good_url(self): url = 'https://github.com/openstack/airship-armada' git_dir = source.git_clone(url) self._validate_git_clone(git_dir) @testtools.skipUnless( - is_connected(), 'git clone requires network connectivity.') + base.is_connected(), 'git clone requires network connectivity.') def test_git_clone_commit(self): url = 'https://github.com/openstack/airship-armada' commit = 'cba78d1d03e4910f6ab1691bae633c5bddce893d' @@ -70,7 +55,7 @@ class GitTestCase(base.ArmadaTestCase): self._validate_git_clone(git_dir) @testtools.skipUnless( - is_connected(), 'git clone requires network connectivity.') + base.is_connected(), 'git clone requires network connectivity.') def test_git_clone_ref(self): ref = 'refs/changes/54/457754/73' git_dir = source.git_clone( @@ -79,7 +64,7 @@ class GitTestCase(base.ArmadaTestCase): @test_utils.attr(type=['negative']) @testtools.skipUnless( - is_connected(), 'git clone requires network connectivity.') + base.is_connected(), 'git clone requires network connectivity.') def test_git_clone_empty_url(self): url = '' # error_re = '%s is not a valid git repository.' % url @@ -89,7 +74,7 @@ class GitTestCase(base.ArmadaTestCase): @test_utils.attr(type=['negative']) @testtools.skipUnless( - is_connected(), 'git clone requires network connectivity.') + base.is_connected(), 'git clone requires network connectivity.') def test_git_clone_bad_url(self): url = 'https://github.com/dummy/armada' @@ -100,7 +85,7 @@ class GitTestCase(base.ArmadaTestCase): # difficult to achieve behind a corporate proxy @test_utils.attr(type=['negative']) @testtools.skipUnless( - is_connected(), 'git clone requires network connectivity.') + base.is_connected(), 'git clone requires network connectivity.') def test_git_clone_fake_proxy(self): url = 'https://github.com/openstack/airship-armada' proxy_url = test_utils.rand_name( @@ -162,7 +147,7 @@ class GitTestCase(base.ArmadaTestCase): mock_tarfile.extractall.assert_not_called() @testtools.skipUnless( - is_connected(), 'git clone requires network connectivity.') + base.is_connected(), 'git clone requires network connectivity.') @mock.patch.object(source, 'LOG') def test_source_cleanup(self, mock_log): url = 'https://github.com/openstack/airship-armada' @@ -206,7 +191,7 @@ class GitTestCase(base.ArmadaTestCase): actual_call) @testtools.skipUnless( - is_connected(), 'git clone requires network connectivity.') + base.is_connected(), 'git clone requires network connectivity.') @test_utils.attr(type=['negative']) @mock.patch.object(source, 'os') def test_git_clone_ssh_auth_method_fails_auth(self, mock_os): @@ -219,7 +204,7 @@ class GitTestCase(base.ArmadaTestCase): ref='refs/changes/17/388517/5', auth_method='SSH') @testtools.skipUnless( - is_connected(), 'git clone requires network connectivity.') + base.is_connected(), 'git clone requires network connectivity.') @test_utils.attr(type=['negative']) @mock.patch.object(source, 'os') def test_git_clone_ssh_auth_method_missing_ssh_key(self, mock_os): diff --git a/armada/tests/unit/utils/test_validate.py b/armada/tests/unit/utils/test_validate.py index 3267adc2..5ecdb408 100644 --- a/armada/tests/unit/utils/test_validate.py +++ b/armada/tests/unit/utils/test_validate.py @@ -15,6 +15,8 @@ import os import yaml +import testtools + from armada.tests.unit import base from armada.utils import validate @@ -211,6 +213,9 @@ data: self.assertTrue(is_valid) + @testtools.skipUnless( + base.is_connected(), + 'validate_manifest_url requires network connectivity.') def test_validate_manifest_url(self): value = 'url' self.assertFalse(validate.validate_manifest_url(value))