[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
This commit is contained in:
Felipe Monteiro 2018-04-02 19:40:14 +01:00
parent 99ced61d3e
commit f27ab29db7
4 changed files with 191 additions and 26 deletions

View File

@ -15,6 +15,8 @@
from __future__ import absolute_import from __future__ import absolute_import
import socket
import fixtures import fixtures
import mock import mock
from oslo_config import cfg from oslo_config import cfg
@ -25,6 +27,20 @@ from armada.conf import default
CONF = cfg.CONF 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): class ArmadaTestCase(testtools.TestCase):
def setUp(self): def setUp(self):

View File

@ -15,8 +15,10 @@
import mock import mock
import yaml import yaml
from armada import const
from armada.handlers import armada from armada.handlers import armada
from armada.tests.unit import base from armada.tests.unit import base
from armada.utils.release import release_prefix
TEST_YAML = """ TEST_YAML = """
@ -152,7 +154,8 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
armada_obj = armada.Armada(yaml_documents) armada_obj = armada.Armada(yaml_documents)
# Mock methods called by `pre_flight_ops()`. # 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] mock_source.git_clone.return_value = CHART_SOURCES[0][0]
self._test_pre_flight_ops(armada_obj) self._test_pre_flight_ops(armada_obj)
@ -165,6 +168,43 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
'git://github.com/dummy/armada', 'master', auth_method=None, 'git://github.com/dummy/armada', 'master', auth_method=None,
proxy_server=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.object(armada, 'source')
@mock.patch('armada.handlers.armada.Tiller') @mock.patch('armada.handlers.armada.Tiller')
def test_post_flight_ops(self, mock_tiller, mock_source): def test_post_flight_ops(self, mock_tiller, mock_source):
@ -173,7 +213,8 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
armada_obj = armada.Armada(yaml_documents) armada_obj = armada.Armada(yaml_documents)
# Mock methods called by `pre_flight_ops()`. # 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] mock_source.git_clone.return_value = CHART_SOURCES[0][0]
self._test_pre_flight_ops(armada_obj) self._test_pre_flight_ops(armada_obj)
@ -186,6 +227,124 @@ class ArmadaHandlerTestCase(base.ArmadaTestCase):
mock_source.source_cleanup.assert_called_with( mock_source.source_cleanup.assert_called_with(
CHART_SOURCES[counter][0]) 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, 'post_flight_ops')
@mock.patch.object(armada.Armada, 'pre_flight_ops') @mock.patch.object(armada.Armada, 'pre_flight_ops')
@mock.patch('armada.handlers.armada.ChartBuilder') @mock.patch('armada.handlers.armada.ChartBuilder')

View File

@ -13,7 +13,6 @@
# limitations under the License. # limitations under the License.
import os import os
import socket
import shutil import shutil
import fixtures import fixtures
@ -26,20 +25,6 @@ from armada.tests import test_utils
from armada.utils import source 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): class GitTestCase(base.ArmadaTestCase):
def _validate_git_clone(self, repo_dir, expected_ref=None): 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()) self.assertIn(expected_ref, git_file.read())
@testtools.skipUnless( @testtools.skipUnless(
is_connected(), 'git clone requires network connectivity.') base.is_connected(), 'git clone requires network connectivity.')
def test_git_clone_good_url(self): def test_git_clone_good_url(self):
url = 'https://github.com/openstack/airship-armada' url = 'https://github.com/openstack/airship-armada'
git_dir = source.git_clone(url) git_dir = source.git_clone(url)
self._validate_git_clone(git_dir) self._validate_git_clone(git_dir)
@testtools.skipUnless( @testtools.skipUnless(
is_connected(), 'git clone requires network connectivity.') base.is_connected(), 'git clone requires network connectivity.')
def test_git_clone_commit(self): def test_git_clone_commit(self):
url = 'https://github.com/openstack/airship-armada' url = 'https://github.com/openstack/airship-armada'
commit = 'cba78d1d03e4910f6ab1691bae633c5bddce893d' commit = 'cba78d1d03e4910f6ab1691bae633c5bddce893d'
@ -70,7 +55,7 @@ class GitTestCase(base.ArmadaTestCase):
self._validate_git_clone(git_dir) self._validate_git_clone(git_dir)
@testtools.skipUnless( @testtools.skipUnless(
is_connected(), 'git clone requires network connectivity.') base.is_connected(), 'git clone requires network connectivity.')
def test_git_clone_ref(self): def test_git_clone_ref(self):
ref = 'refs/changes/54/457754/73' ref = 'refs/changes/54/457754/73'
git_dir = source.git_clone( git_dir = source.git_clone(
@ -79,7 +64,7 @@ class GitTestCase(base.ArmadaTestCase):
@test_utils.attr(type=['negative']) @test_utils.attr(type=['negative'])
@testtools.skipUnless( @testtools.skipUnless(
is_connected(), 'git clone requires network connectivity.') base.is_connected(), 'git clone requires network connectivity.')
def test_git_clone_empty_url(self): def test_git_clone_empty_url(self):
url = '' url = ''
# error_re = '%s is not a valid git repository.' % url # error_re = '%s is not a valid git repository.' % url
@ -89,7 +74,7 @@ class GitTestCase(base.ArmadaTestCase):
@test_utils.attr(type=['negative']) @test_utils.attr(type=['negative'])
@testtools.skipUnless( @testtools.skipUnless(
is_connected(), 'git clone requires network connectivity.') base.is_connected(), 'git clone requires network connectivity.')
def test_git_clone_bad_url(self): def test_git_clone_bad_url(self):
url = 'https://github.com/dummy/armada' url = 'https://github.com/dummy/armada'
@ -100,7 +85,7 @@ class GitTestCase(base.ArmadaTestCase):
# difficult to achieve behind a corporate proxy # difficult to achieve behind a corporate proxy
@test_utils.attr(type=['negative']) @test_utils.attr(type=['negative'])
@testtools.skipUnless( @testtools.skipUnless(
is_connected(), 'git clone requires network connectivity.') base.is_connected(), 'git clone requires network connectivity.')
def test_git_clone_fake_proxy(self): def test_git_clone_fake_proxy(self):
url = 'https://github.com/openstack/airship-armada' url = 'https://github.com/openstack/airship-armada'
proxy_url = test_utils.rand_name( proxy_url = test_utils.rand_name(
@ -162,7 +147,7 @@ class GitTestCase(base.ArmadaTestCase):
mock_tarfile.extractall.assert_not_called() mock_tarfile.extractall.assert_not_called()
@testtools.skipUnless( @testtools.skipUnless(
is_connected(), 'git clone requires network connectivity.') base.is_connected(), 'git clone requires network connectivity.')
@mock.patch.object(source, 'LOG') @mock.patch.object(source, 'LOG')
def test_source_cleanup(self, mock_log): def test_source_cleanup(self, mock_log):
url = 'https://github.com/openstack/airship-armada' url = 'https://github.com/openstack/airship-armada'
@ -206,7 +191,7 @@ class GitTestCase(base.ArmadaTestCase):
actual_call) actual_call)
@testtools.skipUnless( @testtools.skipUnless(
is_connected(), 'git clone requires network connectivity.') base.is_connected(), 'git clone requires network connectivity.')
@test_utils.attr(type=['negative']) @test_utils.attr(type=['negative'])
@mock.patch.object(source, 'os') @mock.patch.object(source, 'os')
def test_git_clone_ssh_auth_method_fails_auth(self, mock_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') ref='refs/changes/17/388517/5', auth_method='SSH')
@testtools.skipUnless( @testtools.skipUnless(
is_connected(), 'git clone requires network connectivity.') base.is_connected(), 'git clone requires network connectivity.')
@test_utils.attr(type=['negative']) @test_utils.attr(type=['negative'])
@mock.patch.object(source, 'os') @mock.patch.object(source, 'os')
def test_git_clone_ssh_auth_method_missing_ssh_key(self, mock_os): def test_git_clone_ssh_auth_method_missing_ssh_key(self, mock_os):

View File

@ -15,6 +15,8 @@
import os import os
import yaml import yaml
import testtools
from armada.tests.unit import base from armada.tests.unit import base
from armada.utils import validate from armada.utils import validate
@ -211,6 +213,9 @@ data:
self.assertTrue(is_valid) self.assertTrue(is_valid)
@testtools.skipUnless(
base.is_connected(),
'validate_manifest_url requires network connectivity.')
def test_validate_manifest_url(self): def test_validate_manifest_url(self):
value = 'url' value = 'url'
self.assertFalse(validate.validate_manifest_url(value)) self.assertFalse(validate.validate_manifest_url(value))