From 7cda9f185345ca6561e902ba94bbb71c628de17c Mon Sep 17 00:00:00 2001 From: Juan Badia Payno Date: Wed, 12 May 2021 16:40:39 +0200 Subject: [PATCH] BnR added error msg in case there is no inventory Currently both the undercloud backup and the overcloud backup dont show any error msg when the inventory file does not exit. This patch adds an error msg in case the inventory file does not exit Change-Id: I57b43a550d8baf23add7794150e1f8591f756275 (cherry picked from commit f737dfcc025536c87c7b344f813150fad877f4e6) --- .../tests/v1/overcloud_backup/test_backup.py | 106 ++++++++++++++-- .../tests/v1/undercloud/test_backup.py | 114 +++++++++++++++--- tripleoclient/v1/overcloud_backup.py | 6 + tripleoclient/v1/undercloud_backup.py | 6 + 4 files changed, 204 insertions(+), 28 deletions(-) diff --git a/tripleoclient/tests/v1/overcloud_backup/test_backup.py b/tripleoclient/tests/v1/overcloud_backup/test_backup.py index ef0b55e84..fe2f8ee9b 100644 --- a/tripleoclient/tests/v1/overcloud_backup/test_backup.py +++ b/tripleoclient/tests/v1/overcloud_backup/test_backup.py @@ -35,12 +35,21 @@ class TestOvercloudBackup(utils.TestCommand): self.cmd = overcloud_backup.BackupOvercloud(self.app, app_args) self.app.client_manager.workflow_engine = mock.Mock() self.workflow = self.app.client_manager.workflow_engine + self.inventory = '/tmp/test_inventory.yaml' + self.file = open(self.inventory, 'w').close() + @mock.patch('os.path.isfile') + @mock.patch('os.access') @mock.patch('tripleoclient.utils.run_ansible_playbook', autospec=True) - def test_overcloud_backup_noargs(self, mock_playbook): + def test_overcloud_backup_noargs(self, + mock_playbook, + mock_access, + mock_isfile): arglist = [] verifylist = [] + mock_isfile.return_value = True + mock_access.return_value = True parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -56,13 +65,20 @@ class TestOvercloudBackup(utils.TestCommand): extra_vars={} ) + @mock.patch('os.path.isfile') + @mock.patch('os.access') @mock.patch('tripleoclient.utils.run_ansible_playbook', autospec=True) - def test_overcloud_backup_init(self, mock_playbook): + def test_overcloud_backup_init(self, + mock_playbook, + mock_access, + mock_isfile): arglist = [ '--init' ] verifylist = [] + mock_isfile.return_value = True + mock_access.return_value = True parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -78,14 +94,21 @@ class TestOvercloudBackup(utils.TestCommand): extra_vars={} ) + @mock.patch('os.path.isfile') + @mock.patch('os.access') @mock.patch('tripleoclient.utils.run_ansible_playbook', autospec=True) - def test_overcloud_backup_init_nfs(self, mock_playbook): + def test_overcloud_backup_init_nfs(self, + mock_playbook, + mock_access, + mock_isfile): arglist = [ '--init', 'nfs' ] verifylist = [] + mock_isfile.return_value = True + mock_access.return_value = True parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -101,13 +124,20 @@ class TestOvercloudBackup(utils.TestCommand): extra_vars={} ) + @mock.patch('os.path.isfile') + @mock.patch('os.access') @mock.patch('tripleoclient.utils.run_ansible_playbook', autospec=True) - def test_overcloud_backup_setup_nfs(self, mock_playbook): + def test_overcloud_backup_setup_nfs(self, + mock_playbook, + mock_access, + mock_isfile): arglist = [ '--setup-nfs' ] verifylist = [] + mock_isfile.return_value = True + mock_access.return_value = True parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -123,13 +153,20 @@ class TestOvercloudBackup(utils.TestCommand): extra_vars={} ) + @mock.patch('os.path.isfile') + @mock.patch('os.access') @mock.patch('tripleoclient.utils.run_ansible_playbook', autospec=True) - def test_overcloud_backup_setup_rear(self, mock_playbook): + def test_overcloud_backup_setup_rear(self, + mock_playbook, + mock_access, + mock_isfile): arglist = [ '--setup-rear', ] verifylist = [] + mock_isfile.return_value = True + mock_access.return_value = True parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -152,14 +189,13 @@ class TestOvercloudBackup(utils.TestCommand): '--setup-nfs', '--setup-rear', '--inventory', - '/tmp/test_inventory.yaml' + self.inventory ] verifylist = [] parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.cmd.take_action(parsed_args) - calls = [call(workdir=mock.ANY, playbook='prepare-nfs-backup.yaml', inventory=parsed_args.inventory, @@ -179,16 +215,22 @@ class TestOvercloudBackup(utils.TestCommand): mock_playbook.assert_has_calls(calls) + @mock.patch('os.path.isfile') + @mock.patch('os.access') @mock.patch('tripleoclient.utils.run_ansible_playbook', autospec=True) def test_overcloud_backup_setup_rear_extra_vars_inline(self, - mock_playbook): + mock_playbook, + mock_access, + mock_isfile): arglist = [ '--setup-rear', '--extra-vars', '{"tripleo_backup_and_restore_nfs_server": "192.168.24.1"}' ] verifylist = [] + mock_isfile.return_value = True + mock_access.return_value = True parsed_args = self.check_parser(self.cmd, arglist, verifylist) extra_vars_dict = { @@ -207,15 +249,22 @@ class TestOvercloudBackup(utils.TestCommand): extra_vars=extra_vars_dict ) + @mock.patch('os.path.isfile') + @mock.patch('os.access') @mock.patch('tripleoclient.utils.run_ansible_playbook', autospec=True) - def test_overcloud_backup_setup_rear_with_extra_vars(self, mock_playbook): + def test_overcloud_backup_setup_rear_with_extra_vars(self, + mock_playbook, + mock_access, + mock_isfile): arglist = [ '--setup-rear', '--extra-vars', '/tmp/test_vars.yaml' ] verifylist = [] + mock_isfile.return_value = True + mock_access.return_value = True parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -236,10 +285,9 @@ class TestOvercloudBackup(utils.TestCommand): def test_overcloud_backup_inventory(self, mock_playbook): arglist = [ '--inventory', - '/tmp/test_inventory.yaml' + self.inventory ] verifylist = [] - parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.cmd.take_action(parsed_args) @@ -253,3 +301,39 @@ class TestOvercloudBackup(utils.TestCommand): verbosity=3, extra_vars={} ) + + @mock.patch('tripleoclient.utils.run_ansible_playbook', + autospec=True) + def test_overcloud_backup_no_inventory(self, mock_playbook): + arglist = [ + '--inventory', + '/tmp/no_inventory.yaml' + ] + verifylist = [] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.assertRaisesRegexp( + RuntimeError, + 'The inventory file', + self.cmd.take_action, + parsed_args) + + @mock.patch('os.access') + @mock.patch('tripleoclient.utils.run_ansible_playbook', + autospec=True) + def test_overcloud_backup_no_readable_inventory(self, + mock_playbook, + mock_access): + arglist = [ + '--inventory', + self.inventory + ] + verifylist = [] + mock_access.return_value = False + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.assertRaisesRegexp( + RuntimeError, + 'The inventory file', + self.cmd.take_action, + parsed_args) diff --git a/tripleoclient/tests/v1/undercloud/test_backup.py b/tripleoclient/tests/v1/undercloud/test_backup.py index 4b262498b..ab58883d8 100644 --- a/tripleoclient/tests/v1/undercloud/test_backup.py +++ b/tripleoclient/tests/v1/undercloud/test_backup.py @@ -35,6 +35,8 @@ class TestUndercloudBackup(utils.TestCommand): self.cmd = undercloud_backup.BackupUndercloud(self.app, app_args) self.app.client_manager.workflow_engine = mock.Mock() self.workflow = self.app.client_manager.workflow_engine + self.inventory = '/tmp/test_inventory.yaml' + self.file = open(self.inventory, 'w').close() @mock.patch('tripleoclient.utils.run_ansible_playbook', autospec=True) @@ -144,11 +146,18 @@ class TestUndercloudBackup(utils.TestCommand): extra_vars={'sources_path': '/home/stack/,/tmp/foo.yaml'}) + @mock.patch('os.path.isfile') + @mock.patch('os.access') @mock.patch('tripleoclient.utils.run_ansible_playbook', autospec=True) - def test_undercloud_backup_noargs(self, mock_playbook): + def test_undercloud_backup_noargs(self, + mock_playbook, + mock_access, + mock_isfile): arglist = [] verifylist = [] + mock_isfile.return_value = True + mock_access.return_value = True parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -164,13 +173,20 @@ class TestUndercloudBackup(utils.TestCommand): extra_vars=None ) + @mock.patch('os.path.isfile') + @mock.patch('os.access') @mock.patch('tripleoclient.utils.run_ansible_playbook', autospec=True) - def test_undercloud_backup_init(self, mock_playbook): + def test_undercloud_backup_init(self, + mock_playbook, + mock_access, + mock_isfile): arglist = [ '--init' ] verifylist = [] + mock_isfile.return_value = True + mock_access.return_value = True parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -186,14 +202,21 @@ class TestUndercloudBackup(utils.TestCommand): extra_vars=None ) + @mock.patch('os.path.isfile') + @mock.patch('os.access') @mock.patch('tripleoclient.utils.run_ansible_playbook', autospec=True) - def test_undercloud_backup_init_nfs(self, mock_playbook): + def test_undercloud_backup_init_nfs(self, + mock_playbook, + mock_access, + mock_isfile): arglist = [ '--init', 'nfs' ] verifylist = [] + mock_isfile.return_value = True + mock_access.return_value = True parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -209,13 +232,20 @@ class TestUndercloudBackup(utils.TestCommand): extra_vars=None ) + @mock.patch('os.path.isfile') + @mock.patch('os.access') @mock.patch('tripleoclient.utils.run_ansible_playbook', autospec=True) - def test_undercloud_backup_setup_nfs(self, mock_playbook): + def test_undercloud_backup_setup_nfs(self, + mock_playbook, + mock_access, + mock_isfile): arglist = [ '--setup-nfs' ] verifylist = [] + mock_isfile.return_value = True + mock_access.return_value = True parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -231,13 +261,20 @@ class TestUndercloudBackup(utils.TestCommand): extra_vars=None ) + @mock.patch('os.path.isfile') + @mock.patch('os.access') @mock.patch('tripleoclient.utils.run_ansible_playbook', autospec=True) - def test_undercloud_backup_setup_rear(self, mock_playbook): + def test_undercloud_backup_setup_rear(self, + mock_playbook, + mock_access, + mock_isfile): arglist = [ '--setup-rear' ] verifylist = [] + mock_isfile.return_value = True + mock_access.return_value = True parsed_args = self.check_parser(self.cmd, arglist, verifylist) @@ -251,24 +288,28 @@ class TestUndercloudBackup(utils.TestCommand): playbook_dir=constants.ANSIBLE_TRIPLEO_PLAYBOOKS, verbosity=3, extra_vars=None - ) + ) + @mock.patch('os.path.isfile') + @mock.patch('os.access') @mock.patch('tripleoclient.utils.run_ansible_playbook', autospec=True) def test_undercloud_backup_setup_rear_extra_vars_inline(self, - mock_playbook): + mock_playbook, + mock_access, + mock_isfile): arglist = [ '--setup-rear', '--extra-vars', '{"tripleo_backup_and_restore_nfs_server": "192.168.24.1"}' ] verifylist = [] - + mock_isfile.return_value = True + mock_access.return_value = True parsed_args = self.check_parser(self.cmd, arglist, verifylist) extra_vars_dict = { 'tripleo_backup_and_restore_nfs_server': '192.168.24.1' } - self.cmd.take_action(parsed_args) mock_playbook.assert_called_once_with( workdir=mock.ANY, @@ -279,7 +320,7 @@ class TestUndercloudBackup(utils.TestCommand): playbook_dir=constants.ANSIBLE_TRIPLEO_PLAYBOOKS, verbosity=3, extra_vars=extra_vars_dict - ) + ) @mock.patch('tripleoclient.utils.run_ansible_playbook', autospec=True) def test_undercloud_backup_setup_nfs_rear_with_inventory(self, @@ -288,14 +329,12 @@ class TestUndercloudBackup(utils.TestCommand): '--setup-nfs', '--setup-rear', '--inventory', - '/tmp/test_inventory.yaml' + self.inventory ] verifylist = [] - parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.cmd.take_action(parsed_args) - calls = [call(workdir=mock.ANY, playbook='prepare-nfs-backup.yaml', inventory=mock.ANY, @@ -315,16 +354,22 @@ class TestUndercloudBackup(utils.TestCommand): mock_playbook.assert_has_calls(calls) + @mock.patch('os.path.isfile') + @mock.patch('os.access') @mock.patch('tripleoclient.utils.run_ansible_playbook', autospec=True) - def test_undercloud_backup_setup_nfs_with_extra_vars(self, mock_playbook): + def test_undercloud_backup_setup_nfs_with_extra_vars(self, + mock_playbook, + mock_access, + mock_isfile): arglist = [ '--setup-nfs', '--extra-vars', '/tmp/test_vars.yaml' ] verifylist = [] - + mock_isfile.return_value = True + mock_access.return_value = True parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.cmd.take_action(parsed_args) @@ -344,10 +389,9 @@ class TestUndercloudBackup(utils.TestCommand): def test_undercloud_backup_inventory(self, mock_playbook): arglist = [ '--inventory', - '/tmp/test_inventory.yaml' + self.inventory ] verifylist = [] - parsed_args = self.check_parser(self.cmd, arglist, verifylist) self.cmd.take_action(parsed_args) @@ -361,3 +405,39 @@ class TestUndercloudBackup(utils.TestCommand): verbosity=3, extra_vars=None ) + + @mock.patch('tripleoclient.utils.run_ansible_playbook', + autospec=True) + def test_undercloud_backup_no_inventory(self, mock_playbook): + arglist = [ + '--inventory', + '/tmp/no_inventory.yaml' + ] + verifylist = [] + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + self.assertRaisesRegexp( + RuntimeError, + 'The inventory file', + self.cmd.take_action, + parsed_args) + + @mock.patch('os.access') + @mock.patch('tripleoclient.utils.run_ansible_playbook', + autospec=True) + def test_undercloud_backup_no_readable_inventory(self, + mock_playbook, + mock_access): + arglist = [ + '--inventory', + self.inventory + ] + verifylist = [] + mock_access.return_value = False + + parsed_args = self.check_parser(self.cmd, arglist, verifylist) + + self.assertRaisesRegexp( + RuntimeError, + 'The inventory file', + self.cmd.take_action, + parsed_args) diff --git a/tripleoclient/v1/overcloud_backup.py b/tripleoclient/v1/overcloud_backup.py index c3dac995c..3e7a3be4b 100644 --- a/tripleoclient/v1/overcloud_backup.py +++ b/tripleoclient/v1/overcloud_backup.py @@ -137,6 +137,12 @@ class BackupOvercloud(command.Command): 'tripleo_backup_and_restore_nfs_server' ] = storage_ip + if not (os.path.isfile(parsed_args.inventory) and + os.access(parsed_args.inventory, os.R_OK)): + raise RuntimeError( + _('The inventory file {} does not exist or is not ' + 'readable'.format(parsed_args.inventory))) + if parsed_args.setup_nfs is True or parsed_args.init == 'nfs': LOG.debug(_('Setting up NFS Backup node')) diff --git a/tripleoclient/v1/undercloud_backup.py b/tripleoclient/v1/undercloud_backup.py index 7b4716189..e166d8a91 100644 --- a/tripleoclient/v1/undercloud_backup.py +++ b/tripleoclient/v1/undercloud_backup.py @@ -155,6 +155,12 @@ class BackupUndercloud(command.Command): extra_vars = self._parse_extra_vars(parsed_args.extra_vars) + if not (os.path.isfile(parsed_args.inventory) and + os.access(parsed_args.inventory, os.R_OK)): + raise RuntimeError( + _('The inventory file {} does not exist or is not ' + 'readable'.format(parsed_args.inventory))) + if parsed_args.setup_nfs is True or parsed_args.init == 'nfs': self._run_ansible_playbook(