From 66e720d2551a74dad1b1511a2f9d7f40c4950928 Mon Sep 17 00:00:00 2001 From: sslypushenko Date: Fri, 4 Dec 2015 18:03:00 +0200 Subject: [PATCH] Argument '--file' should be required Argument '--file' should be required for command 'openstack-config --upload' and 'openstack-config --download'. Both v1 and v2 clients fixed. Change-Id: Iaa1c214b3535857c7d90ab575638a030cebba9c9 Closes-Bug: #1521914 --- fuelclient/cli/actions/openstack_config.py | 4 +-- fuelclient/commands/openstack_config.py | 2 +- .../tests/unit/v1/test_openstack_config.py | 36 ++++++++++++++++++- .../unit/v2/cli/test_openstack_config.py | 24 +++++++++++++ 4 files changed, 62 insertions(+), 4 deletions(-) diff --git a/fuelclient/cli/actions/openstack_config.py b/fuelclient/cli/actions/openstack_config.py index aa83d59..406b1ee 100644 --- a/fuelclient/cli/actions/openstack_config.py +++ b/fuelclient/cli/actions/openstack_config.py @@ -85,7 +85,7 @@ class OpenstackConfigAction(Action): ) ) - @check_all('config-id') + @check_all('config-id', 'file') def download(self, params): """Download an existing configuration to file: fuel openstack-config --download --config-id 1 --file config.yaml @@ -96,7 +96,7 @@ class OpenstackConfigAction(Action): OpenstackConfig.write_file(params.file, { 'configuration': data['configuration']}) - @check_all('env') + @check_all('env', 'file') def upload(self, params): """Upload new configuration from file: fuel openstack-config --upload --env 1 --file config.yaml diff --git a/fuelclient/commands/openstack_config.py b/fuelclient/commands/openstack_config.py index 22cdad7..ecbbe9c 100644 --- a/fuelclient/commands/openstack_config.py +++ b/fuelclient/commands/openstack_config.py @@ -30,7 +30,7 @@ class OpenstackConfigMixin(object): @staticmethod def add_file_arg(parser): parser.add_argument( - '-f', '--file', + '-f', '--file', required=True, type=str, help='YAML file that contains openstack configuration.') @staticmethod diff --git a/fuelclient/tests/unit/v1/test_openstack_config.py b/fuelclient/tests/unit/v1/test_openstack_config.py index 9f328fb..b2e59f5 100644 --- a/fuelclient/tests/unit/v1/test_openstack_config.py +++ b/fuelclient/tests/unit/v1/test_openstack_config.py @@ -42,6 +42,23 @@ class TestOpenstackConfigActions(base.UnitTestCase): self.assertEqual(self.config['configuration'], content['configuration']) + @mock.patch('sys.stderr') + def test_config_download_fail(self, mocked_stderr): + self.assertRaises( + SystemExit, + self.execute, ['fuel', 'openstack-config', '--download', + '--config-id', '1']) + mocked_stderr.write.assert_called_once_with( + '"--config-id" and "--file" required!\n') + mocked_stderr.reset_mock() + + self.assertRaises( + SystemExit, + self.execute, ['fuel', 'openstack-config', '--download', + '--file', 'config.yaml']) + mocked_stderr.write.assert_called_once_with( + '"--config-id" and "--file" required!\n') + def test_config_upload(self): m_post = self.m_request.post( '/api/v1/openstack-config/', json=self.config) @@ -52,7 +69,24 @@ class TestOpenstackConfigActions(base.UnitTestCase): with mock.patch('fuelclient.objects.openstack_config.os'): self.execute(['fuel', 'openstack-config', '--env', '1', '--upload', '--file', 'config.yaml']) - self.assertTrue(m_post.called) + self.assertTrue(m_post.called) + + @mock.patch('sys.stderr') + def test_config_upload_fail(self, mocked_stderr): + self.assertRaises( + SystemExit, + self.execute, ['fuel', 'openstack-config', '--env', '1', + '--upload']) + mocked_stderr.write.assert_called_once_with( + '"--env" and "--file" required!\n') + mocked_stderr.reset_mock() + + self.assertRaises( + SystemExit, + self.execute, ['fuel', 'openstack-config', '--upload', + '--file', 'config.yaml']) + mocked_stderr.write.assert_called_once_with( + '"--env" and "--file" required!\n') def test_config_list(self): m_get = self.m_request.get( diff --git a/fuelclient/tests/unit/v2/cli/test_openstack_config.py b/fuelclient/tests/unit/v2/cli/test_openstack_config.py index f2b5607..7e723a6 100644 --- a/fuelclient/tests/unit/v2/cli/test_openstack_config.py +++ b/fuelclient/tests/unit/v2/cli/test_openstack_config.py @@ -66,6 +66,21 @@ class TestOpenstackConfig(test_engine.BaseCLITest): path='config.yaml', cluster_id=self.CLUSTER_ID, node_id=self.NODE_ID, node_role=None) + @mock.patch('sys.stderr') + def test_config_upload_fail(self, mocked_stderr): + cmd = 'openstack-config upload --env {0} ' \ + '--node {1}'.format(self.CLUSTER_ID, self.NODE_ID) + self.assertRaises(SystemExit, self.exec_command, cmd) + self.assertIn('-f/--file', + mocked_stderr.write.call_args_list[-1][0][0]) + mocked_stderr.reset_mock() + + cmd = 'openstack-config upload --node {1} ' \ + '--file config.yaml'.format(self.CLUSTER_ID, self.NODE_ID) + self.assertRaises(SystemExit, self.exec_command, cmd) + self.assertIn('-e/--env', + mocked_stderr.write.call_args_list[-1][0][0]) + def test_config_download(self): self.m_client.download.return_value = 'config.yaml' @@ -75,6 +90,15 @@ class TestOpenstackConfig(test_engine.BaseCLITest): self.m_get_client.assert_called_once_with('openstack-config', mock.ANY) self.m_client.download.assert_called_once_with(1, 'config.yaml') + @mock.patch('sys.stderr') + def test_config_download_fail(self, mocked_stderr): + cmd = 'openstack-config download 1' + self.assertRaises(SystemExit, self.exec_command, cmd) + + self.assertRaises(SystemExit, self.exec_command, cmd) + self.assertIn('-f/--file', + mocked_stderr.write.call_args_list[-1][0][0]) + def test_config_execute(self): cmd = 'openstack-config execute --env {0} --node {1}' \ ''.format(self.CLUSTER_ID, self.NODE_ID)