From c78ab588125116fb336f095e4514f354d9b8867c Mon Sep 17 00:00:00 2001 From: Francesco Pantano Date: Tue, 27 Apr 2021 09:41:53 +0200 Subject: [PATCH] Check if both swift and rgw are enabled during Upgrade Prepare As per [1] RGW is now deployed by default when the cephadm envs are included. However, during the upgrade procedure we should warn users if both services are enabled, failing early during the update or upgrade prepare step and suggesting that the cephadm-rbd-only env should be added to make sure Swift is kept and RGW is not automatically included in the heat stack. This change introduces this kind of check, and an exception is raised if the stack already exists, Ceph is deployed by TripleO and swift is found in the previosly created stack. [1] https://review.opendev.org/786827 Change-Id: I32387f19346697655355e15b3cf4ff41bd303b08 --- tripleoclient/tests/test_utils.py | 36 ++++++++++++++++++ .../overcloud_deploy/test_overcloud_deploy.py | 26 +++++++++---- tripleoclient/utils.py | 37 +++++++++++++++++++ tripleoclient/v1/overcloud_deploy.py | 3 ++ 4 files changed, 94 insertions(+), 8 deletions(-) diff --git a/tripleoclient/tests/test_utils.py b/tripleoclient/tests/test_utils.py index d53f4358e..659d7b9b6 100644 --- a/tripleoclient/tests/test_utils.py +++ b/tripleoclient/tests/test_utils.py @@ -655,6 +655,42 @@ class TestWaitForStackUtil(TestCase): with self.assertRaises(exceptions.InvalidConfiguration): utils.check_ceph_fsid_matches_env_files(mock_stack, provided_env) + def test_check_swift_and_rgw(self): + stack_reg = { + 'OS::TripleO::Services::SwiftProxy': 'OS::Heat::None', + } + env_reg = { + 'OS::TripleO::Services::CephRgw': '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_swift_and_rgw(mock_stack, env, 'UpgradePrepare') + + def test_check_swift_and_rgw_fail(self): + stack_reg = { + 'OS::TripleO::Services::SwiftProxy': 'val', + } + env_reg = { + 'OS::TripleO::Services::CephRgw': 'val', + } + 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_swift_and_rgw(mock_stack, env, 'UpgradePrepare') + @mock.patch('subprocess.check_call') @mock.patch('os.path.exists') def test_remove_known_hosts(self, mock_exists, mock_check_call): diff --git a/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py b/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py index 8df6a8b6c..14a6f212f 100644 --- a/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py +++ b/tripleoclient/tests/v1/overcloud_deploy/test_overcloud_deploy.py @@ -135,6 +135,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): return_value='192.168.0.1 uc.ctlplane.localhost uc.ctlplane') @mock.patch('tripleoclient.utils.check_stack_network_matches_env_files') @mock.patch('tripleoclient.utils.check_ceph_fsid_matches_env_files') + @mock.patch('tripleoclient.utils.check_swift_and_rgw') @mock.patch("heatclient.common.event_utils.get_events") @mock.patch('tripleo_common.update.add_breakpoints_cleanup_into_env', autospec=True) @@ -148,7 +149,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): mock_create_parameters_env, mock_breakpoints_cleanup, mock_events, mock_stack_network_check, - mock_ceph_fsid, + mock_ceph_fsid, mock_swift_rgw, mock_get_undercloud_host_entry, mock_copy, mock_get_ctlplane_attrs, mock_nic_ansiblei, mock_process_env, mock_roles_data, @@ -352,6 +353,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): return_value='192.168.0.1 uc.ctlplane.localhost uc.ctlplane') @mock.patch('tripleoclient.utils.check_stack_network_matches_env_files') @mock.patch('tripleoclient.utils.check_ceph_fsid_matches_env_files') + @mock.patch('tripleoclient.utils.check_swift_and_rgw') @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' '_deploy_postconfig', autospec=True) @mock.patch('tripleo_common.update.add_breakpoints_cleanup_into_env', @@ -369,7 +371,8 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): mock_create_parameters_env, mock_validate_args, mock_breakpoints_cleanup, mock_postconfig, mock_stack_network_check, - mock_ceph_fsid, mock_get_undercloud_host_entry, mock_copy, + mock_ceph_fsid, mock_swift_rgw, + mock_get_undercloud_host_entry, mock_copy, mock_chdir, mock_process_env, mock_roles_data, mock_image_prepare, mock_generate_password, @@ -435,6 +438,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): return_value='192.168.0.1 uc.ctlplane.localhost uc.ctlplane') @mock.patch('tripleoclient.utils.check_stack_network_matches_env_files') @mock.patch('tripleoclient.utils.check_ceph_fsid_matches_env_files') + @mock.patch('tripleoclient.utils.check_swift_and_rgw') @mock.patch("heatclient.common.event_utils.get_events", autospec=True) @mock.patch('tripleo_common.update.add_breakpoints_cleanup_into_env', autospec=True) @@ -449,7 +453,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): mock_deploy_postconfig, mock_breakpoints_cleanup, mock_events, mock_stack_network_check, - mock_ceph_fsid, + mock_ceph_fsid, mock_swift_rgw, mock_get_undercloud_host_entry, mock_copy, mock_nic_ansible, mock_process_env, @@ -532,6 +536,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): @mock.patch('tripleoclient.utils.copy_clouds_yaml') @mock.patch('tripleoclient.utils.check_stack_network_matches_env_files') @mock.patch('tripleoclient.utils.check_ceph_fsid_matches_env_files') + @mock.patch('tripleoclient.utils.check_swift_and_rgw') @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' '_deploy_postconfig', autospec=True) @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' @@ -543,7 +548,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): def test_environment_dirs(self, mock_deploy_heat, mock_create_env, mock_update_parameters, mock_post_config, mock_stack_network_check, mock_ceph_fsid, - mock_copy, mock_nic_ansible, + mock_swift_rgw, mock_copy, mock_nic_ansible, mock_process_env, mock_rc_params, mock_check_service_vip_migr): fixture = deployment.DeploymentWorkflowFixture() @@ -745,13 +750,15 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): return_value='192.168.0.1 uc.ctlplane.localhost uc.ctlplane') @mock.patch('tripleoclient.utils.check_stack_network_matches_env_files') @mock.patch('tripleoclient.utils.check_ceph_fsid_matches_env_files') + @mock.patch('tripleoclient.utils.check_swift_and_rgw') @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_stack_network_check, - mock_ceph_fsid, mock_get_undercloud_host_entry, + mock_ceph_fsid, mock_swift_rgw, + mock_get_undercloud_host_entry, mock_copy, mock_nic_ansible, mock_roles_data, mock_image_prepare, mock_generate_password, mock_rc_params, @@ -845,6 +852,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): return_value='192.168.0.1 uc.ctlplane.localhost uc.ctlplane') @mock.patch('tripleoclient.utils.check_stack_network_matches_env_files') @mock.patch('tripleoclient.utils.check_ceph_fsid_matches_env_files') + @mock.patch('tripleoclient.utils.check_swift_and_rgw') @mock.patch('tripleoclient.utils.create_parameters_env', autospec=True) @mock.patch('heatclient.common.template_utils.' 'process_environment_and_files', autospec=True) @@ -854,7 +862,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): mock_process_env, mock_create_parameters_env, mock_stack_network_check, - mock_ceph_fsid, + mock_ceph_fsid, mock_swift_rgw, mock_get_undercloud_host_entry, mock_nic_ansible, mock_roles_data, @@ -911,6 +919,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): return_value='192.168.0.1 uc.ctlplane.localhost uc.ctlplane') @mock.patch('tripleoclient.utils.check_stack_network_matches_env_files') @mock.patch('tripleoclient.utils.check_ceph_fsid_matches_env_files') + @mock.patch('tripleoclient.utils.check_swift_and_rgw') @mock.patch('tripleoclient.v1.overcloud_deploy.DeployOvercloud.' '_deploy_postconfig', autospec=True) @mock.patch('tripleo_common.update.add_breakpoints_cleanup_into_env') @@ -931,7 +940,7 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): mock_breakpoints_cleanup, mock_deploy_post_config, mock_stack_network_check, - mock_ceph_fsid, + mock_ceph_fsid, mock_swift_rgw, mock_get_undercloud_host_entry, mock_copy, mock_get_ctlplane_attrs, mock_roles_data, @@ -1294,10 +1303,11 @@ class TestDeployOvercloud(fakes.TestDeployOvercloud): '_heat_deploy', autospec=True) @mock.patch('tripleoclient.utils.check_stack_network_matches_env_files') @mock.patch('tripleoclient.utils.check_ceph_fsid_matches_env_files') + @mock.patch('tripleoclient.utils.check_swift_and_rgw') @mock.patch('heatclient.common.template_utils.deep_update', autospec=True) def test_config_download_timeout( self, mock_hc, mock_stack_network_check, - mock_ceph_fsid, mock_hd, + mock_ceph_fsid, mock_swift_rgw, mock_hd, mock_get_undercloud_host_entry, mock_copy, mock_get_ctlplane_attrs, mock_nic_ansible, mock_process_env, mock_roles_data, diff --git a/tripleoclient/utils.py b/tripleoclient/utils.py index 3bcb7e80b..dbc1626a6 100644 --- a/tripleoclient/utils.py +++ b/tripleoclient/utils.py @@ -1094,6 +1094,43 @@ def check_ceph_fsid_matches_env_files(stack, environment): stack_ceph_fsid)) +def check_swift_and_rgw(stack, env, stage): + """Check that Swift and RGW aren't both enabled in the overcloud + + When Ceph is deployed by TripleO using the default cephadm environment + file, the RGW component is included by default and deployed on both + greenfield and brownfield deployments. + However, if an overcloud upgrade is run and Swift was already deployed, + the RGW resource shouldn't replace Swift and -e cephadm-rbd-only.yaml + should be passed. + For this reason we need to check if Swift was previously enabled, and + fail if the RGW resource is passed. + """ + rgw_env = env.get('resource_registry', + {}).get('OS::TripleO::Services::CephRgw', + 'OS::Heat::None') + + allowed_stage = re.compile("(Upgrade|Update)Prepare", re.I) + # if the RGW resource isn't passed or we're not in the upgrade context + # there's no need to run this check + if not re.match(allowed_stage, stage) or rgw_env == 'OS::Heat::None': + return + + sw = stack.environment().get('resource_registry', + {}).get('OS::TripleO::Services::SwiftProxy', + 'OS::Heat::None') + + # RGW is present in the env list and swift was previously deployed + if sw != "OS::Heat::None": + raise exceptions.InvalidConfiguration('Both Swift and RGW resources ' + 'detected. ' + 'Ensure you have only one of ' + 'them enabled (or provide the ' + 'cephadm-rbd-only.yaml ' + 'environment file to exclude ' + 'RGW)') + + def check_nic_config_with_ansible(stack, environment): registry = environment.get('resource_registry', {}) stack_registry = {} diff --git a/tripleoclient/v1/overcloud_deploy.py b/tripleoclient/v1/overcloud_deploy.py index 4618388a7..46f3d00c1 100644 --- a/tripleoclient/v1/overcloud_deploy.py +++ b/tripleoclient/v1/overcloud_deploy.py @@ -360,6 +360,9 @@ class DeployOvercloud(command.Command): if (ceph_deployed != "OS::Heat::None" or ceph_external != "OS::Heat::None"): utils.check_ceph_fsid_matches_env_files(stack, env) + # upgrades: check if swift is deployed + utils.check_swift_and_rgw(stack, env, + self.__class__.__name__) # check migration to new nic config with ansible utils.check_nic_config_with_ansible(stack, env) # check migration to service vips managed by servce