Check that the networks are defined on update

Check defined networks in the resource registry of the stack against the
networks defined in the environment files. If the environment provided
doesn't have the networks defined, it's likely they were improperly
dropped which can lead to deployment issues. This is a light check that
only checks for the existance of the networks but not the contents of
those networks. This handles the case where a user forgets to include
the network-isolation configuration on a subsequent update to the cloud.
This does not prevent a user from changing the contents of the networks
to something that breaks their deployment.

Partial-Bug: #1817631
Change-Id: Ia97a2367770e37bf8c55f2fd04c9a9efde914a67

tripleoclient/tests/test_utils.py

@@ -380,6 +380,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

@@ -467,6 +467,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)