From 84055814f9303a3fea6a252a24c2d9d123535bea Mon Sep 17 00:00:00 2001 From: John Fulton Date: Wed, 22 Dec 2021 15:48:27 +0000 Subject: [PATCH] Make deployed ceph baremetal file optional If the --ceph-spec option is passed to 'openstack overcloud ceph deploy', then it is not necessary for the path to the environment file output from 'openstack overcloud node provision' to be passed. Add conditional to ensure that either --ceph-spec or are used and add two unit tests to cover all three scenarios: a. metal is passed, b. spec is passed and c. neither are passed so a failure is asserted. Depends-On: I9a6bd27f825d226b508053e4b7d2670a14f6f773 Change-Id: Ibd350956f401fa48aaeae89fdd5e0bd9c28d0685 --- .../v2/overcloud_ceph/test_overcloud_ceph.py | 53 +++++++++++++++++++ tripleoclient/v2/overcloud_ceph.py | 38 ++++++++----- 2 files changed, 78 insertions(+), 13 deletions(-) diff --git a/tripleoclient/tests/v2/overcloud_ceph/test_overcloud_ceph.py b/tripleoclient/tests/v2/overcloud_ceph/test_overcloud_ceph.py index 51e614979..73184dbf5 100644 --- a/tripleoclient/tests/v2/overcloud_ceph/test_overcloud_ceph.py +++ b/tripleoclient/tests/v2/overcloud_ceph/test_overcloud_ceph.py @@ -74,6 +74,49 @@ class TestOvercloudCephDeploy(fakes.FakePlaybookExecution): } ) + @mock.patch('tripleoclient.utils.get_ceph_networks', autospect=True) + @mock.patch('tripleoclient.utils.TempDirs', autospect=True) + @mock.patch('os.path.abspath', autospect=True) + @mock.patch('os.path.exists', autospect=True) + @mock.patch('tripleoclient.utils.run_ansible_playbook', autospec=True) + def test_deploy_ceph_spec(self, mock_playbook, mock_abspath, + mock_path_exists, mock_tempdirs, + mock_get_ceph_networks): + arglist = ['--yes', + '--stack', 'overcloud', + '--skip-user-create', + '--mon-ip', '127.0.0.1', + '--ceph-spec', 'ceph_spec.yaml', + '--cephadm-ssh-user', 'jimmy', + '--output', 'deployed-ceph.yaml', + '--container-namespace', 'quay.io/ceph', + '--container-image', 'ceph', + '--container-tag', 'latest'] + parsed_args = self.check_parser(self.cmd, arglist, []) + self.cmd.take_action(parsed_args) + mock_playbook.assert_called_once_with( + playbook='cli-deployed-ceph.yaml', + inventory=mock.ANY, + workdir=mock.ANY, + playbook_dir=mock.ANY, + verbosity=3, + skip_tags='cephadm_ssh_user', + reproduce_command=False, + extra_vars={ + "deployed_ceph_tht_path": mock.ANY, + "working_dir": mock.ANY, + "stack_name": 'overcloud', + 'tripleo_roles_path': mock.ANY, + 'tripleo_cephadm_first_mon_ip': '127.0.0.1', + 'dynamic_ceph_spec': False, + 'ceph_spec_path': mock.ANY, + 'tripleo_cephadm_container_ns': 'quay.io/ceph', + 'tripleo_cephadm_container_image': 'ceph', + 'tripleo_cephadm_container_tag': 'latest', + 'tripleo_cephadm_ssh_user': 'jimmy', + } + ) + @mock.patch('os.path.abspath', autospect=True) @mock.patch('os.path.exists', autospect=True) def test_overcloud_deploy_ceph_no_overwrite(self, mock_abspath, @@ -85,6 +128,16 @@ class TestOvercloudCephDeploy(fakes.FakePlaybookExecution): self.assertRaises(osc_lib_exc.CommandError, self.cmd.take_action, parsed_args) + @mock.patch('os.path.abspath', autospect=True) + @mock.patch('os.path.exists', autospect=True) + def test_overcloud_deploy_ceph_no_metal(self, mock_abspath, + mock_path_exists): + arglist = ['--stack', 'overcloud', + '--output', 'deployed-ceph.yaml'] + parsed_args = self.check_parser(self.cmd, arglist, []) + self.assertRaises(osc_lib_exc.CommandError, + self.cmd.take_action, parsed_args) + class TestOvercloudCephUserDisable(fakes.FakePlaybookExecution): def setUp(self): diff --git a/tripleoclient/v2/overcloud_ceph.py b/tripleoclient/v2/overcloud_ceph.py index 365b0bce8..4e49879a0 100644 --- a/tripleoclient/v2/overcloud_ceph.py +++ b/tripleoclient/v2/overcloud_ceph.py @@ -75,11 +75,13 @@ class OvercloudCephDeploy(command.Command): def get_parser(self, prog_name): parser = super(OvercloudCephDeploy, self).get_parser(prog_name) - parser.add_argument('baremetal_env', + parser.add_argument('baremetal_env', nargs='?', metavar='', help=_('Path to the environment file ' 'output from "openstack ' - 'overcloud node provision".')) + 'overcloud node provision". ' + 'This argument may be excluded ' + 'only if --ceph-spec is used.')) parser.add_argument('-o', '--output', required=True, metavar='', help=_('The path to the output environment ' @@ -162,10 +164,12 @@ class OvercloudCephDeploy(command.Command): spec_group = parser.add_mutually_exclusive_group() spec_group.add_argument('--ceph-spec', help=_( - "Path to an existing Ceph spec file. " - "If not provided a spec will be generated " + "Path to an existing Ceph spec file. If " + "not provided a spec will be generated " "automatically based on --roles-data and " - ""), + ". The " + " parameter is " + "optional only if --ceph-spec is used."), default=None) spec_group.add_argument('--osd-spec', help=_( @@ -235,14 +239,7 @@ class OvercloudCephDeploy(command.Command): def take_action(self, parsed_args): self.log.debug("take_action(%s)" % parsed_args) - baremetal_env_path = os.path.abspath(parsed_args.baremetal_env) output_path = os.path.abspath(parsed_args.output) - - if not os.path.exists(baremetal_env_path): - raise oscexc.CommandError( - "Baremetal environment file does not exist:" - " %s" % parsed_args.baremetal_env) - overwrite = parsed_args.yes if (os.path.exists(output_path) and not overwrite and not oooutils.prompt_user_for_confirmation( @@ -273,12 +270,27 @@ class OvercloudCephDeploy(command.Command): # mandatory extra_vars are now set, add others conditionally extra_vars = { - "baremetal_deployed_path": baremetal_env_path, "deployed_ceph_tht_path": output_path, "working_dir": working_dir, "stack_name": parsed_args.stack, } # optional paths to pass to playbook + if parsed_args.ceph_spec is None and \ + parsed_args.baremetal_env is None: + raise oscexc.CommandError( + "Either " + "or --ceph-spec must be used.") + + if parsed_args.baremetal_env: + baremetal_env_path = os.path.abspath(parsed_args.baremetal_env) + if not os.path.exists(baremetal_env_path): + raise oscexc.CommandError( + "Baremetal environment file does not exist:" + " %s" % parsed_args.baremetal_env) + else: + extra_vars['baremetal_deployed_path'] = \ + os.path.abspath(parsed_args.baremetal_env) + if parsed_args.roles_data: if not os.path.exists(parsed_args.roles_data): raise oscexc.CommandError(