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

@@ -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()
 

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())

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)

armada/tests/unit/handlers/test_armada.py

@@ -1,41 +1,53 @@
+# 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 import armada
-from armada.handlers.manifest import Manifest
+from armada.tests.unit import base
 
 
-class ArmadaTestCase(unittest.TestCase):
+TEST_YAML = """
-    test_yaml = """
+---
-    ---
+schema: armada/Manifest/v1
-    schema: armada/Manifest/v1
+metadata:
-    metadata:
   schema: metadata/Document/v1
   name: example-manifest
-    data:
+data:
   release_prefix: armada
   chart_groups:
     - example-group
-    ---
+---
-    schema: armada/ChartGroup/v1
+schema: armada/ChartGroup/v1
-    metadata:
+metadata:
   schema: metadata/Document/v1
   name: example-group
-    data:
+data:
   description: this is a test
   sequenced: False
   chart_group:
     - example-chart-1
     - example-chart-2
-    ---
+---
-    schema: armada/Chart/v1
+schema: armada/Chart/v1
-    metadata:
+metadata:
   schema: metadata/Document/v1
   name: example-chart-2
-    data:
+data:
-        name: test_chart_2
+    chart_name: test_chart_2
-        release_name: test_chart_2
+    release: test_chart_2
     namespace: test
     values: {}
     source:
@@ -45,14 +57,14 @@ class ArmadaTestCase(unittest.TestCase):
       reference: null
     dependencies: []
     timeout: 5
-    ---
+---
-    schema: armada/Chart/v1
+schema: armada/Chart/v1
-    metadata:
+metadata:
   schema: metadata/Document/v1
   name: example-chart-1
-    data:
+data:
-        name: test_chart_1
+    chart_name: test_chart_1
-        release_name: test_chart_1
+    release: test_chart_1
     namespace: test
     values: {}
     source:
@@ -62,118 +74,154 @@ class ArmadaTestCase(unittest.TestCase):
       reference: master
     dependencies: []
     timeout: 50
-    """
+"""
 
-    @unittest.skip('temp')
+CHART_SOURCES = [('git://github.com/dummy/armada', 'chart_1'),
-    @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.documents = yaml.safe_load_all(self.test_yaml)
-        armada.config = Manifest(armada.documents).get_manifest()
-
-        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()
+class ArmadaHandlerTestCase(base.ArmadaTestCase):
 
-        mock_git.git_clone.assert_called_once_with(CHART_SOURCES[0][0],
+    def _test_pre_flight_ops(self, armada_obj):
-                                                   'master')
+        armada_obj.pre_flight_ops()
-        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])
 
-    @unittest.skip('temp')
+        expected_config = {
-    @mock.patch('armada.handlers.armada.git')
+            'armada': {
-    @mock.patch('armada.handlers.armada.lint')
+                '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_post_flight_ops(self, mock_tiller, mock_lint, mock_git):
+    def test_pre_flight_ops(self, mock_tiller, mock_source):
-        '''Test post-flight operations'''
+        """Test pre-flight checks and operations."""
-        armada = Armada('')
+        yaml_documents = list(yaml.safe_load_all(TEST_YAML))
-        armada.tiller = mock_tiller
+        armada_obj = armada.Armada(yaml_documents)
-        tmp_doc = yaml.safe_load_all(self.test_yaml)
-        armada.config = Manifest(tmp_doc).get_manifest()
 
-        CHART_SOURCES = [('git://github.com/dummy/armada', 'chart_1'),
+        # Mock methods called by `pre_flight_ops()`.
-                         ('/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_source.git_clone.return_value = CHART_SOURCES[0][0]
-        mock_git.git_clone.return_value = CHART_SOURCES[0][0]
-        armada.pre_flight_ops()
 
-        armada.post_flight_ops()
+        self._test_pre_flight_ops(armada_obj)
 
-        for group in yaml.load(self.test_yaml).get('armada').get('charts'):
+        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)
+
+    @mock.patch.object(armada, 'source')
+    @mock.patch('armada.handlers.armada.Tiller')
+    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)
+
+        # Mock methods called by `pre_flight_ops()`.
+        mock_tiller.tiller_status.return_value = True
+        mock_source.git_clone.return_value = CHART_SOURCES[0][0]
+
+        self._test_pre_flight_ops(armada_obj)
+
+        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 \
+                    mock_source.source_cleanup.assert_called_with(
-                            .assert_called_with(CHART_SOURCES[counter][0])
+                        CHART_SOURCES[counter][0])
 
-    @unittest.skip('temp')
+    @mock.patch.object(armada.Armada, 'post_flight_ops')
-    @mock.patch.object(Armada, 'post_flight_ops')
+    @mock.patch.object(armada.Armada, 'pre_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, mock_pre_flight,
                      mock_post_flight):
         '''Test install functionality from the sync() method'''
 
-        # instantiate Armada and Tiller objects
+        # Instantiate Armada object.
-        armada = Armada('', wait=True, timeout=1000)
+        yaml_documents = list(yaml.safe_load_all(TEST_YAML))
-        armada.tiller = mock_tiller
+        armada_obj = armada.Armada(yaml_documents)
-        tmp_doc = yaml.safe_load_all(self.test_yaml)
-        armada.config = Manifest(tmp_doc).get_manifest()
 
-        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'],
+                "{}-{}".format(armada_obj.config['armada']['release_prefix'],
-                               chart_1['release_name']),
+                               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,
+                wait=armada_obj.wait,
-                timeout=1000),
+                timeout=armada_obj.timeout),
             mock.call(
                 mock_chartbuilder().get_helm_chart(),
-                "{}-{}".format(armada.config['armada']['release_prefix'],
+                "{}-{}".format(armada_obj.config['armada']['release_prefix'],
-                               chart_2['release_name']),
+                               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,
+                wait=armada_obj.wait,
-                timeout=1000)
+                timeout=armada_obj.timeout)
         ]
-        mock_tiller.install_release.assert_has_calls(method_calls)
+        mock_tiller.return_value.install_release.assert_has_calls(method_calls)
-
-    @unittest.skip('skipping update')
-    def test_upgrade(self):
-        '''Test upgrade functionality from the sync() method'''
-        # TODO

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,
+        self.assertRaises(
+            source_exceptions.GitProxyException,
             source.git_clone, url,
-                          proxy_server='http://not.a.proxy:8080')
+            proxy_server='http://not.a.proxy.that.works.and.never.will:8080')
 
     @mock.patch('armada.utils.source.tempfile')
     @mock.patch('armada.utils.source.requests')