Merge "Check that the networks are defined on update"

tripleoclient/tests/test_utils.py

@@ -382,6 +382,71 @@ class TestWaitForStackUtil(TestCase):
             utils.wait_for_provision_state(baremetal_client, 'UUID',
                                            "available", loops=1, sleep=0.01)
 
+    def test_check_stack_network_matches_env_files(self):
+        stack_reg = {
+            'OS::TripleO::Hosts::SoftwareConfig': 'val',
+            'OS::TripleO::Network': 'val',
+            'OS::TripleO::Network::External': 'val',
+            'OS::TripleO::Network::ExtraConfig': 'OS::Heat::None',
+            'OS::TripleO::Network::InternalApi': 'val',
+            'OS::TripleO::Network::Port::InternalApi': 'val',
+            'OS::TripleO::Network::Management': 'val',
+            'OS::TripleO::Network::Storage': 'val',
+            'OS::TripleO::Network::StorageMgmt': 'val',
+            'OS::TripleO::Network::Tenant': 'val'
+        }
+        env_reg = {
+            'OS::TripleO::Hosts::SoftwareConfig': 'newval',
+            'OS::TripleO::Network': 'newval',
+            'OS::TripleO::Network::External': 'newval',
+            'OS::TripleO::Network::ExtraConfig': 'OS::Heat::None',
+            'OS::TripleO::Network::InternalApi': 'newval',
+            'OS::TripleO::Network::Management': 'newval',
+            'OS::TripleO::Network::Storage': 'val',
+            'OS::TripleO::Network::StorageMgmt': 'val',
+            'OS::TripleO::Network::Tenant': 'val'
+        }
+        mock_stack = mock.MagicMock()
+        mock_stack.environment = mock.MagicMock()
+        mock_stack.environment.return_value = {
+            'resource_registry':  stack_reg
+        }
+        env = {
+            'resource_registry':  env_reg
+        }
+        utils.check_stack_network_matches_env_files(mock_stack, env)
+
+    def test_check_stack_network_matches_env_files_fail(self):
+        stack_reg = {
+            'OS::TripleO::Hosts::SoftwareConfig': 'val',
+            'OS::TripleO::LoggingConfiguration': 'val',
+            'OS::TripleO::Network': 'val',
+            'OS::TripleO::Network::External': 'val',
+            'OS::TripleO::Network::ExtraConfig': 'OS::Heat::None',
+            'OS::TripleO::Network::InternalApi': 'val',
+            'OS::TripleO::Network::Port::InternalApi': 'val',
+            'OS::TripleO::Network::Management': 'val',
+            'OS::TripleO::Network::Storage': 'val',
+            'OS::TripleO::Network::StorageMgmt': 'val',
+            'OS::TripleO::Network::Tenant': 'val'
+        }
+        env_reg = {
+            'OS::TripleO::Hosts::SoftwareConfig': 'newval',
+            'OS::TripleO::LoggingConfiguration': 'newval',
+            'OS::TripleO::Network': 'newval',
+            'OS::TripleO::Network::InternalApi': 'newval'
+        }
+        mock_stack = mock.MagicMock()
+        mock_stack.environment = mock.MagicMock()
+        mock_stack.environment.return_value = {
+            'resource_registry':  stack_reg
+        }
+        env = {
+            'resource_registry':  env_reg
+        }
+        with self.assertRaises(exceptions.InvalidConfiguration):
+            utils.check_stack_network_matches_env_files(mock_stack, env)
+
     @mock.patch('subprocess.check_call')
     @mock.patch('os.path.exists')
     def test_remove_known_hosts(self, mock_exists, mock_check_call):

tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py

@@ -93,6 +93,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
         self.cmd._download_missing_files_from_plan = self.real_download_missing
         shutil.rmtree = self.real_shutil
 
+    @mock.patch('tripleoclient.utils.check_stack_network_matches_env_files')
     @mock.patch("heatclient.common.event_utils.get_events")
     @mock.patch('tripleo_common.update.add_breakpoints_cleanup_into_env',
                 autospec=True)
@@ -106,7 +107,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
                        mock_create_tempest_deployer_input,
                        mock_create_parameters_env,
                        mock_breakpoints_cleanup,
-                       mock_events):
+                       mock_events, mock_stack_network_check):
         fixture = deployment.DeploymentWorkflowFixture()
         self.useFixture(fixture)
         clients = self.app.client_manager
@@ -304,6 +305,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
         self.assertEqual(env_map.get('parameter_defaults'),
                          parameters_env.get('parameter_defaults'))
 
+    @mock.patch('tripleoclient.utils.check_stack_network_matches_env_files')
     @mock.patch('shutil.rmtree', autospec=True)
     @mock.patch('tripleoclient.utils.get_overcloud_endpoint', autospec=True)
     @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
@@ -328,7 +330,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
             mock_validate_args,
             mock_breakpoints_cleanup,
             mock_postconfig, mock_shutil_rmtree,
-            mock_invoke_plan_env_wf):
+            mock_invoke_plan_env_wf,
+            mock_stack_network_check):
         fixture = deployment.DeploymentWorkflowFixture()
         self.useFixture(fixture)
         plane_management_fixture = deployment.PlanManagementFixture()
@@ -432,6 +435,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
         clients.tripleoclient.object_store.put_object.assert_called()
         self.assertTrue(mock_invoke_plan_env_wf.called)
 
+    @mock.patch('tripleoclient.utils.check_stack_network_matches_env_files')
     @mock.patch('tripleoclient.workflows.parameters.'
                 'check_deprecated_parameters', autospec=True)
     @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
@@ -451,7 +455,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
             mock_get_template_contents,
             mock_create_parameters_env, mock_validate_args,
             mock_breakpoints_cleanup,
-            mock_postconfig, mock_deprecated_params):
+            mock_postconfig, mock_deprecated_params, mock_stack_network_check):
         fixture = deployment.DeploymentWorkflowFixture()
         self.useFixture(fixture)
         plane_management_fixture = deployment.PlanManagementFixture()
@@ -515,6 +519,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
         deploy_plan_call_input = deploy_plan_call[1]['workflow_input']
         self.assertTrue(deploy_plan_call_input['skip_deploy_identifier'])
 
+    @mock.patch('tripleoclient.utils.check_stack_network_matches_env_files')
     @mock.patch("heatclient.common.event_utils.get_events", autospec=True)
     @mock.patch('tripleo_common.update.add_breakpoints_cleanup_into_env',
                 autospec=True)
@@ -528,7 +533,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
                                      mock_create_tempest_deployer_input,
                                      mock_deploy_postconfig,
                                      mock_breakpoints_cleanup,
-                                     mock_events):
+                                     mock_events, mock_stack_network_check):
         fixture = deployment.DeploymentWorkflowFixture()
         self.useFixture(fixture)
         plane_management_fixture = deployment.PlanManagementFixture()
@@ -621,6 +626,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
             self.cmd.take_action, parsed_args)
         self.assertFalse(mock_deploy_tht.called)
 
+    @mock.patch('tripleoclient.utils.check_stack_network_matches_env_files')
     @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
                 '_deploy_postconfig', autospec=True)
     @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
@@ -628,7 +634,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
     @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
                 '_heat_deploy', autospec=True)
     def test_environment_dirs(self, mock_deploy_heat,
-                              mock_update_parameters, mock_post_config):
+                              mock_update_parameters, mock_post_config,
+                              mock_stack_network_check):
         fixture = deployment.DeploymentWorkflowFixture()
         self.useFixture(fixture)
         plane_management_fixture = deployment.PlanManagementFixture()
@@ -683,6 +690,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
         parsed_args = self.check_parser(self.cmd, arglist, verifylist)
         self.cmd.take_action(parsed_args)
 
+    @mock.patch('tripleoclient.utils.check_stack_network_matches_env_files')
     @mock.patch('tripleoclient.utils.get_stack', autospec=True)
     @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
                 '_deploy_postconfig', autospec=True)
@@ -692,7 +700,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
                 '_heat_deploy', autospec=True)
     def test_environment_dirs_env(self, mock_deploy_heat,
                                   mock_update_parameters, mock_post_config,
-                                  mock_utils_get_stack):
+                                  mock_utils_get_stack,
+                                  mock_stack_network_check):
 
         plane_management_fixture = deployment.PlanManagementFixture()
         self.useFixture(plane_management_fixture)
@@ -861,6 +870,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
 
         utils_fixture.mock_deploy_tht.assert_called_with()
 
+    @mock.patch('tripleoclient.utils.check_stack_network_matches_env_files')
     @mock.patch("heatclient.common.event_utils.get_events", autospec=True)
     @mock.patch('tripleo_common.update.add_breakpoints_cleanup_into_env',
                 autospec=True)
@@ -878,7 +888,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
                              mock_create_tempest_deployer_input,
                              mock_deploy_postconfig,
                              mock_breakpoints_cleanup,
-                             mock_events):
+                             mock_events, mock_stack_network_check):
         clients = self.app.client_manager
         orchestration_client = clients.orchestration
         mock_stack = fakes.create_tht_stack()
@@ -1094,12 +1104,13 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
         self.assertFalse(utils_fixture.mock_create_ocrc.called)
         self.assertFalse(mock_create_tempest_deployer_input.called)
 
+    @mock.patch('tripleoclient.utils.check_stack_network_matches_env_files')
     @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
                 '_heat_deploy', autospec=True)
     @mock.patch('tempfile.mkdtemp', autospec=True)
     @mock.patch('shutil.rmtree', autospec=True)
     def test_answers_file(self, mock_rmtree, mock_tmpdir,
-                          mock_heat_deploy):
+                          mock_heat_deploy, mock_stack_network_check):
         fixture = deployment.DeploymentWorkflowFixture()
         self.useFixture(fixture)
         clients = self.app.client_manager
@@ -1213,6 +1224,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
             role_counts,
             self.cmd._get_default_role_counts(mock.Mock()))
 
+    @mock.patch('tripleoclient.utils.check_stack_network_matches_env_files')
     @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
                 '_create_parameters_env', autospec=True)
     @mock.patch('tripleoclient.utils.write_overcloudrc')
@@ -1223,7 +1235,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
     def test_ntp_server_mandatory(self, mock_get_template_contents,
                                   mock_process_env,
                                   mock_write_overcloudrc,
-                                  mock_create_parameters_env):
+                                  mock_create_parameters_env,
+                                  mock_stack_network_check):
         plane_management_fixture = deployment.PlanManagementFixture()
         self.useFixture(plane_management_fixture)
         clients = self.app.client_manager
@@ -1264,6 +1277,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
                           self.cmd.take_action,
                           parsed_args)
 
+    @mock.patch('tripleoclient.utils.check_stack_network_matches_env_files')
     @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.'
                 '_deploy_postconfig', autospec=True)
     @mock.patch('tripleo_common.update.add_breakpoints_cleanup_into_env')
@@ -1283,7 +1297,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud):
                                  mock_create_parameters_env,
                                  mock_validate_args,
                                  mock_breakpoints_cleanup,
-                                 mock_deploy_post_config):
+                                 mock_deploy_post_config,
+                                 mock_stack_network_check):
         fixture = deployment.DeploymentWorkflowFixture()
         self.useFixture(fixture)
         plane_management_fixture = deployment.PlanManagementFixture()

tripleoclient/utils.py

@@ -468,6 +468,43 @@ def get_stack(orchestration_client, stack_name):
         pass
 
 
+def check_stack_network_matches_env_files(stack, environment):
+    """Check stack against proposed env files to ensure non-breaking change
+
+    Historically we have have had issues with folks forgetting the network
+    isolation templates in subsequent overcloud actions which have completely
+    broken the stack. We need to check that the networks continue to be
+    provided on updates and if they aren't, it's likely that the user has
+    failed to provide the network-isolation templates. This is a light check
+    to only ensure they are defined. A user can still change settings in these
+    networks that may break things but this will catch folks who forget
+    network-isolation in a subsequent update.
+    """
+    def _get_networks(registry):
+        nets = set()
+        for k, v in six.iteritems(registry):
+            if (k.startswith('OS::TripleO::Network::')
+                and not k.startswith('OS::TripleO::Network::Port')
+                    and v != 'OS::Heat::None'):
+                nets.add(k)
+        return nets
+
+    stack_registry = stack.environment().get('resource_registry', {})
+    env_registry = environment.get('resource_registry', {})
+
+    stack_nets = _get_networks(stack_registry)
+    env_nets = _get_networks(env_registry)
+
+    env_diff = set(stack_nets) - set(env_nets)
+    if env_diff:
+        raise exceptions.InvalidConfiguration('Missing networks from '
+                                              'environment configuration. '
+                                              'Ensure the following networks '
+                                              'are properly configured in '
+                                              'the provided environment files '
+                                              '[{}]'.format(env_diff))
+
+
 def remove_known_hosts(overcloud_ip):
     """For a given IP address remove SSH keys from the known_hosts file"""
 

tripleoclient/v1/overcloud_deploy.py

@@ -442,6 +442,8 @@ class DeployOvercloud(command.Command):
         template_utils.deep_update(env, localenv)
 
         if stack:
+            # note(aschultz): network validation goes here before we deploy
+            utils.check_stack_network_matches_env_files(stack, env)
             bp_cleanup = self._create_breakpoint_cleanup_env(
                 tht_root, parsed_args.stack)
             template_utils.deep_update(env, bp_cleanup)